> -----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?
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-08 18:45:37 CET