> -----Original Message-----
> From: Paul Burba [mailto:pburba_at_collab.net]
> Sent: Tuesday, January 08, 2008 12:28 PM
> To: David Glasser
> Cc: dev
> Subject: RE: [PATCH] Fix cause of merge_tests.py 61 failure
>
> > -----Original Message-----
> > From: Paul Burba [mailto:pburba_at_collab.net]
> > Sent: Monday, January 07, 2008 11:49 AM
> > To: David Glasser
> > Cc: dev
> > Subject: RE: [PATCH] Fix cause of merge_tests.py 61 failure
> >
> > Sorry for the delay, for reasons unknown this ended up in my junk
> > e-mail folder...
> >
> > > -----Original Message-----
> > > From: dglasser_at_gmail.com [mailto:dglasser_at_gmail.com] On Behalf Of
> > > David Glasser
> > > Sent: Friday, January 04, 2008 11:34 AM
> > > To: Paul Burba
> > > Cc: dev
> > > Subject: Re: [PATCH] Fix cause of merge_tests.py 61 failure
> > >
> > > On Jan 3, 2008 6:50 PM, Paul Burba <pburba_at_collab.net> wrote:
> > > > On IRC today...
> > > >
> > > > <glasser> pburba: so basically I'm having some angst with
> > > > merge_fails_if_subtree_is_deleted_on_src test <glasser> it
> > > first does
> > > > a merge of some changes into A_copy/D/gamma, so that file
> > > ends up with
> > > > mergeinfo <glasser> and then it does a merge of merge
> > changes into
> > > > A_copy/ which includes a delete of A/D/gamma <glasser> and
> > > deep inside
> > > > calculate_remaining_ranges -> filter_reflected_revisions
> > it ends up
> > > > called svn_ra_get_mergeinfo on A/D/gamma <glasser> with the
> > > revision
> > > > after it is deleted (which is the last revision specified on the
> > > > commandline) as the rev <glasser> so on the reintegrate
> > > branch, this
> > > > is a file-not-found error <glasser> what should the fix be?
> > > this code
> > > > is pretty complex <pburba> glasser: Let me take a look
> > > >
> > > > The test fails because of the call to
> > > > svn_client__get_repos_mergeinfo() in
> filter_reflected_revisions()
> > > > returns an error when we ask for the mergeinfo for a
> non-existant
> > > > path. On trunk no error is returned, instead
> > > *TARGET_MERGEINFO is set
> > > > to an empty hash. I recall you discussing the fact that on
> > > trunk our
> > > > queries of the SQLite db didn't return an error when a
> requested
> > > > path_at_rev didn't exist. I assume the changes you made on
> > > the integrate
> > > > branch resulted in an error now being returned by
> > > svn_client__get_repos_mergeinfo(), does that sound right?
> > > > If so, then the fix looks as simple as clearing the error. See
> > > > attached patch (which passes all merge tests).
> > >
> > > Thanks! Yes, that is the issue I was discussing. Some further
> > > questions:
> > >
> > > >
> > > > [[[
> > > > On the reintegrate branch, fix cause of merge_tests.py
> 61 failure.
> > > >
> > > > * subversion/libsvn_client/merge.c
> > > > (filter_reflected_revisions): Handle the fact that the second
> > > > URL/revision
> > > > pair might not exist causing
> > svn_client__get_repos_mergeinfo() to
> > > > return an
> > > > error.
> > > > ]]]
> > > >
> > > > Paul
> > > >
> > >
> > > > Index: subversion/libsvn_client/merge.c
> > > >
> > ===================================================================
> > > > --- subversion/libsvn_client/merge.c (revision 28740)
> > > > +++ subversion/libsvn_client/merge.c (working copy)
> > > > @@ -1219,6 +1219,7 @@
> > > > const char *min_url = (revision1 < revision2) ? url1 : url2;
> > > > const char *max_url = (revision1 < revision2) ? url2 : url1;
> > > > const char *min_rel_path, *max_rel_path;
> > > > + svn_error_t *err;
> > > >
> > > >
> > SVN_ERR(svn_client__path_relative_to_root(&min_rel_path, min_url,
> > > >
> > > source_root_url, FALSE,
> > > > ra_session, @@ -1233,25 +1234,38 @@
> > > >
> min_rel_path, min_rev,
> > > >
> > > svn_mergeinfo_inherited, TRUE,
> > > > pool));
> > > > - SVN_ERR(svn_client__get_repos_mergeinfo(ra_session,
> > > &end_mergeinfo,
> > > > -
> max_rel_path, max_rev,
> > > > -
> > > svn_mergeinfo_inherited, TRUE,
> > > > - pool));
> > > > -
> > > > - SVN_ERR(svn_mergeinfo_diff(&deleted_mergeinfo,
> > &added_mergeinfo,
> > > > - start_mergeinfo, end_mergeinfo,
> > > > - FALSE, pool));
> > > > -
> > > > - if (added_mergeinfo)
> > > > + err = svn_client__get_repos_mergeinfo(ra_session,
> > &end_mergeinfo,
> > > > + max_rel_path, max_rev,
> > > > +
> > > svn_mergeinfo_inherited, TRUE,
> > > > + pool);
> > > > + /* When operating on subtrees with intersecting
> > > mergeinfo, URL1_at_REVISION1
> > > > + might have been deleted and URL2_at_REVISION2 won't
> > > exist. Don't return an
> > > > + error in this case, just proceed without bothering
> > to look for
> > > > + ADDED_MERGEINFO. */
> > >
> > > Would there be a reason to make a similar change to the
> > > start_mergeinfo query?
> >
> > I think yes, because URL2_at_REVISION2 might have been added and not
> > exist at URL1_at_REVISION1. Something along the lines of the new
> > attached patch (which passes all merge tests on the reintegrate
> > branch). But I'm seeing an unrelated problem while trying to test
> > this out (a problem which exist on trunk). Specifically, if we
> > perform a merge which adds a path, then, without
> committing, perform a
> > second merge with a range that is a superset of the first we get an
> > error, e.g.:
>
> 'I think yes' is now definitely yes, see below.
>
> > >svn merge %url%/A merge_tests-61\A_copy -r4:6
> > --- Merging r5 through r6 into 'merge_tests-61\A_copy':
> > A merge_tests-61\A_copy\D\newfile
> > D merge_tests-61\A_copy\D\gamma
> >
> > >svn merge %url%/A merge_tests-61\A_copy -r1:6
> > ..\..\..\subversion\libsvn_repos\reporter.c:778: (apr_err=160013)
> > svn: Working copy path 'D/newfile' does not exist in repository
> >
> > Looking at this now.
>
> It takes a relatively contrived setup to expose this error.
> Added issue
> #3067 to track it and merge_test.py 77
> 'new_subtrees_should_not_break_merge' on trunk to test for
> it. This test, when applied to the reintegrate branch does
> double duty, exposing the need for the similar change to the
> start_mergeinfo query.
>
> Shall I commit this (the filter_reflected_revisions changes
> that is) on the reintegrate branch or did you have any other
> questions or concerns?
No one has raised any objections to this fix and with the reintegrate
branch merged to trunk I didn't see any reason to delay further.
Committed the fix for merge_tests.py 61 and 77 in r28988. Any problems,
questions, etc., please let me know!
Thanks,
Paul
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-01-22 17:36:14 CET