[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

RE: Problem with merge notifications?

From: Paul Burba <pburba_at_collab.net>
Date: 2007-10-09 15:50:50 CEST

> -----Original Message-----
> From: Daniel L. Rall [mailto:dlr@finemaltcoding.com]
> Sent: Monday, October 08, 2007 3:55 PM
> To: Paul Burba
> Cc: dev@subversion.tigris.org
> Subject: Re: Problem with merge notifications?
>
> On Mon, 08 Oct 2007, Paul Burba wrote:
> ...
> > Let's say we have the following WC, just a greek tree with
> a copy of
> > the 'A' dir at r2 followed by some simple text mods to some of the
> > files under A in subsequent revisions:
> ...
> > # Resultant mergeinfo looks good
> > >svn pl -vR merge_tests-1
> > Properties on 'merge_tests-1\A_COPY':
> > svn:mergeinfo : /A:1,4-8
> > Properties on 'merge_tests-1\A_COPY\D':
> > svn:mergeinfo : /A/D:1,6
> >
> > === EXAMPLE 1 ==============================================
> ...
> > >svn merge -r2:6 %URL%/A merge_tests-1\A_COPY
> > --- Merging r3 into 'merge_tests-1\A_COPY':
> > U merge_tests-1\A_COPY\D\H\psi
> > --- Merging r4 through r5 into 'merge_tests-1\A_COPY':
> > U merge_tests-1\A_COPY\D\G\rho
> >
> > >svn pl -vR merge_tests-1
> > Properties on 'merge_tests-1\A_COPY':
> > svn:mergeinfo : /A:1,3-8
> > Properties on 'merge_tests-1\A_COPY\D':
> > svn:mergeinfo : /A/D:1,3-6
> >
> > === EXAMPLE 2 ==============================================
> >
> > # ...Now revert the previous merge and try another.
> > # This time revision 6 *is* mentioned in the # notification, even
> > though it is *not* being # applied. Shouldn't the notification
> > instead be # "--- Merging r7 through r10 into
> > 'merge_tests-1\A_COPY':"?
> > # ^^
> > >svn merge -r5:10 %URL%/A merge_tests-1\A_COPY
> > --- Merging r6 through r10 into 'merge_tests-1\A_COPY':
> > U merge_tests-1\A_COPY\D\G\pi
> > U merge_tests-1\A_COPY\D\G\tau
> > U merge_tests-1\A_COPY\D\H\chi
>
> Yup, we shouldn't apply r6 here, as all paths under the merge
> target have already had r6 merged into them -- the
> notification's svn_merge_range_t should have a "start" field
> of r6, which should be printed as r7 by the cmdline client.
>
> > >svn pl -vR merge_tests-1
> > Properties on 'merge_tests-1\A_COPY':
> > svn:mergeinfo : /A:1,4-10
> > Properties on 'merge_tests-1\A_COPY\D':
> > svn:mergeinfo : /A/D:1,6-10
> >
> > === EXAMPLE 3 ==============================================
>
> I take it that this example starts from the initial
> mergeinfo, rather than being a continuation of example 2?

Yes, meant to say "...Now revert the previous merge and try another."

> > # Another problematic example where again r6 is mentioned:
> > >svn merge -r2:10 %URL%/A merge_tests-1\A_COPY
> > --- Merging r3 into 'merge_tests-1\A_COPY':
> > U merge_tests-1\A_COPY\D\H\psi
> > --- Merging r4 through r5 into 'merge_tests-1\A_COPY':
> > U merge_tests-1\A_COPY\D\G\rho
>
> Huh, why aren't the above two merges collapsed into a single
> merge and notification ("Merging r3 through r5 into ...")?

This is a result of r26803/Issue 2821 fix. It has to do with the way
discover_and_merge_children() calls
do_merge()/drive_merge_report_editor(). do_merge() creates the
notification, but drive_merge_report_editor() has the smarts to handle
subtrees with differing mergeinfo. So (just stating the obvious here)
in this case '--- Merging r3 into 'merge_tests-1\A_COPY':' is affecting
the whole tree under 'A_COPY' while '--- Merging r4 through r5 into
'merge_tests-1\A_COPY':' is affecting only the subtree under
'A_COPY/D'...which brings us to the real problem...

> Or, since the second merge is into D, why don't we say as
> much as part of the merge notification (especially when
> A_COPY already contains those revs)?

Kamesh and I had discussed this a little here:
http://svn.haxx.se/dev/archive-2007-10/0274.shtml (I was too quick to
wave the white flag on this.)

Anyway, I agree that is misleading, but what if the second merge of
r4-r5 affected multiple subtrees? Say 'A_COPY\B\E' also had differing
mergeinfo from the target, would we list all the notifications first?

--- Merging r4 through r5 into 'merge_tests-1\A_COPY\D':
--- Merging r4 through r5 into 'merge_tests-1\A_COPY\B\E':
U merge_tests-1\A_COPY\D\G\rho
U merge_tests-1\A_COPY\B\E\beta

Nah, that misleading (though I think simpler to do with the current
code), we'd want something like this:

--- Merging r4 through r5 into 'merge_tests-1\A_COPY\D':
U merge_tests-1\A_COPY\D\G\rho
--- Merging r4 through r5 into 'merge_tests-1\A_COPY\B\E':
U merge_tests-1\A_COPY\B\E\beta

But with the given post-26803 implementation I'm not sure how easy this
is, Kamesh, thoughts?

> > # Should be
> > # "--- Merging r7 through r10 into 'merge_tests-1\A_COPY':"?
> > --- Merging r6 through r10 into 'merge_tests-1\A_COPY':
> > U merge_tests-1\A_COPY\D\G\pi
> > U merge_tests-1\A_COPY\D\G\tau
> > U merge_tests-1\A_COPY\D\H\chi
>
> What about r7 and r8, which have already been merged into A_COPY?

Same issue as above yes? The target paths of notifications shouldn't
always be the merge target, but rather the applicable subtree(s).

> > >svn pl -vR merge_tests-1
> > Properties on 'merge_tests-1\A_COPY':
> > svn:mergeinfo : /A:1,3-10
> >
> > ============================================================
> >
> > There are other examples, but they all deal with the start
> revision of
> > the notification.
> >
> > Does anyone else think the behavior in example 2 and 3 is
> incorrect?
> > Or is this acceptable?
>
> Neither.
>
> In both examples 2 and 3, the mergeinfo on a sub-tree path
> trims part of the start of a requested merge from the actual
> merge performed (or at least from its notification).

Sorry Dan, I'm having some trouble parsing that, the behavior in example
2 and 3 is
neither incorrect nor acceptable or both are incorrect?

~~~~~

Attempting to sum up:

Merge notifications should:

A) Only mention the revision ranges that are being merged (i.e. the
notification shouldn't imply that a repeat merge is being performed).

B1) The notification path, i.e. "--- Merging rX through rY into
'NOTIFICATION_PATH':", is the target of the merge unless the the merge
is affecting only a subtree of the target which has differing mergeinfo.
In that case the notification path should the subtree path.

B2) As B1, but *multiple* subtrees of the merge target are
affected...TBD what notifications look like, but all the affected
subtrees should be mentioned.

Paul

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Oct 9 15:55:05 2007

This is an archived mail posted to the Subversion Dev mailing list.