You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Paul Burba <pb...@collab.net> on 2007/10/08 18:19:52 UTC
Problem with merge notifications?
Kamesh and I have been discussing some merge notification minutiae on a
thread following up on r26803.
I see a small problem with the way we currently do merge notifications
which I want to put before a wider audience:
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:
>svn st merge_tests-1 -v
1 1 jrandom merge_tests-1
1 1 jrandom merge_tests-1\A
1 1 jrandom merge_tests-1\A\B
1 1 jrandom merge_tests-1\A\B\lambda
1 1 jrandom merge_tests-1\A\B\E
1 1 jrandom merge_tests-1\A\B\E\alpha
5 5 jrandom merge_tests-1\A\B\E\beta
1 1 jrandom merge_tests-1\A\B\F
1 1 jrandom merge_tests-1\A\mu
1 1 jrandom merge_tests-1\A\C
1 1 jrandom merge_tests-1\A\D
1 1 jrandom merge_tests-1\A\D\gamma
1 1 jrandom merge_tests-1\A\D\G
7 7 pburba merge_tests-1\A\D\G\pi
4 4 jrandom merge_tests-1\A\D\G\rho
8 8 pburba merge_tests-1\A\D\G\tau
1 1 jrandom merge_tests-1\A\D\H
9 9 pburba merge_tests-1\A\D\H\chi
6 6 jrandom merge_tests-1\A\D\H\omega
3 3 jrandom merge_tests-1\A\D\H\psi
2 2 jrandom merge_tests-1\A_COPY
2 2 jrandom merge_tests-1\A_COPY\B
2 2 jrandom merge_tests-1\A_COPY\B\lambda
2 2 jrandom merge_tests-1\A_COPY\B\E
2 2 jrandom merge_tests-1\A_COPY\B\E\alpha
2 2 jrandom merge_tests-1\A_COPY\B\E\beta
2 2 jrandom merge_tests-1\A_COPY\B\F
2 2 jrandom merge_tests-1\A_COPY\mu
2 2 jrandom merge_tests-1\A_COPY\C
2 2 jrandom merge_tests-1\A_COPY\D
2 2 jrandom merge_tests-1\A_COPY\D\gamma
2 2 jrandom merge_tests-1\A_COPY\D\G
2 2 jrandom merge_tests-1\A_COPY\D\G\pi
2 2 jrandom merge_tests-1\A_COPY\D\G\rho
2 2 jrandom merge_tests-1\A_COPY\D\G\tau
2 2 jrandom merge_tests-1\A_COPY\D\H
2 2 jrandom merge_tests-1\A_COPY\D\H\chi
2 2 jrandom merge_tests-1\A_COPY\D\H\omega
2 2 jrandom merge_tests-1\A_COPY\D\H\psi
1 1 jrandom merge_tests-1\iota
# The only initial mergeinfo is from the copy of 'A'.
>svn pl -vR merge_tests-1
Properties on 'merge_tests-1\A_COPY':
svn:mergeinfo : /A:1
# When merging to a target which has no subtrees
# with differing mergeinfo, the merge notifications
# are straightforward. The only unusual thing is
# that the CL -rX:Y range is exlcusive of the
# start revision X (as it's always been) while the
# notifications 'rA through rB' are inclusive of
# both ranges. This has been discussed to death
# elsewhere and is not why we are here :-P
>svn merge %URL%/A merge_tests-1\A_COPY -r3:8
--- Merging r4 through r8 into 'merge_tests-1\A_COPY':
U merge_tests-1\A_COPY\B\E\beta
U merge_tests-1\A_COPY\D\G\pi
U merge_tests-1\A_COPY\D\G\rho
U merge_tests-1\A_COPY\D\G\tau
U merge_tests-1\A_COPY\D\H\omega
>svn pl -vR merge_tests-1
Properties on 'merge_tests-1\A_COPY':
svn:mergeinfo : /A:1,4-8
# To create a subtree with differing mergeinfo
# we'll reverse merge some of the previously
# applied revision ranges. Again, excepting the
# exclusive end range, the notifications agree with
# the requested -rX:Y range.
>svn merge %URL%/A/D merge_tests-1\A_COPY\D -r5:3
--- Reverse-merging r5 through r4 into 'merge_tests-1\A_COPY\D':
G merge_tests-1\A_COPY\D\G\rho
>svn merge %URL%/A/D merge_tests-1\A_COPY\D -r8:6
--- Reverse-merging r8 through r7 into 'merge_tests-1\A_COPY\D':
G merge_tests-1\A_COPY\D\G\pi
G merge_tests-1\A_COPY\D\G\tau
>svn ci -m "" merge_tests-1
Sending merge_tests-1\A_COPY
Sending merge_tests-1\A_COPY\B\E\beta
Sending merge_tests-1\A_COPY\D
Sending merge_tests-1\A_COPY\D\H\omega
Transmitting file data ..
Committed revision 10.
# 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 ==============================================
# Now perform a merge to a target 'A_COPY' with a
# subtree 'A_COPY\D' with differing mergeinfo.
# Now we get two notifications as r3 is applied to
# both the target and the sub-tree and then r4-5
# is applied to the subtree. The notifications
# reflect this. Revision 6 is not mentioned in either
# notification since it is not being applied.
# Ok, everything looks fine so far...
>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
>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 ==============================================
# 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
# 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
>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?
Paul
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Problem with merge notifications?
Posted by Kamesh Jayachandran <ka...@collab.net>.
> >--- 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?
>
> --- Merging r4 through r5 into 'merge_tests-1\A_COPY\B\E': <---This
> line comes prior to finish report
> U merge_tests-1\A_COPY\B\E\beta <--This line comes from
> notification call back.
>
> To produce this kind of output I think we should move code that prints
> '--- Merging r4 through r5 into 'merge_tests-1\A_COPY\B\E':'
> 'notification callback'.
>
> I am more concerned about the simplicity of this new implementation.
> Let me see.
>
>
Paul filed issue 2963 to address the same.
With regards
Kamesh Jayachandran
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: Problem with merge notifications?
Posted by Kamesh Jayachandran <ka...@collab.net>.
>> 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?
--- Merging r4 through r5 into 'merge_tests-1\A_COPY\B\E': <---This line comes prior to finish report
U merge_tests-1\A_COPY\B\E\beta <--This line comes from notification call back.
To produce this kind of output I think we should move code that prints '--- Merging r4 through r5 into 'merge_tests-1\A_COPY\B\E':' 'notification callback'.
I am more concerned about the simplicity of this new implementation. Let me see.
With regards
Kamesh Jayachandran
RE: Problem with merge notifications?
Posted by Paul Burba <pb...@collab.net>.
> -----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
Re: Problem with merge notifications?
Posted by "Daniel L. Rall" <dl...@finemaltcoding.com>.
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?
> # 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 ...")? 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)?
> # 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?
> >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).
--
Daniel Rall