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/08/22 20:10:57 UTC
RFC: Issue 2883 No-op merge (without skip) should not change mergeinfo
I'd appreciate anyone's thoughts on issue 2883
http://subversion.tigris.org/issues/show_bug.cgi?id=2883
"If merge does not do anything (there are no changes in merge source),
the svn:mergeinfo property should not be modified."
The expected behavior if only the merge target has mergeinfo is clear:
if nothing was changed by the merge, then don't update the target's
mergeinfo.
Also, if the target and some of its children both have mergeinfo but
again nothing was changed by the merge, don't update the target's or the
subtrees' mergeinfo. But what if, in this second case, a path within
one of the subtrees was modified. Should the merge target and the
subtree have thier mergeinfo updated, the subtree and it's ancestors
with mergeinfo, or only the subtree?
e.g.
Given a greek tree with the following changes (this is from
merge_tests.py:mergeinfo_inheritance):
r2 - copy A to A_COPY
r3 - text mod to A/D/H/psi
r4 - text mod to A/D/G/rho
r5 - text mod to A/B/E/beta
r6 - text mod to A/D/H/omega
r7 - merge r4 from A into A_COPY/D
merge r5 from A into A_COPY/B
merge r3 from A into A_COPY
This gives us the following mergeinfo:
>svn pl -vR merge_tests-1
Properties on 'merge_tests-1\A_COPY':
svn:mergeinfo : /A:1
Properties on 'merge_tests-1\A_COPY\B':
svn:mergeinfo : /A/B:1,5
Properties on 'merge_tests-1\A_COPY\D':
svn:mergeinfo : /A/D:1,4
Currently trunk behaves this way if we perform a merge that affects only
a subtree:
>svn merge %URL%/A merge_tests-1\A_COPY -c3
--- Merging r3:
--- Merging r3:
U merge_tests-1\A_COPY\D\H\psi
--- Merging r3:
As you can see, all the subtrees with mergeinfo get r3 added, even
though only the subtree rooted at A_COPY/D was affected.
>svn st merge_tests-1
M merge_tests-1\A_COPY
M merge_tests-1\A_COPY\B
M merge_tests-1\A_COPY\D
M merge_tests-1\A_COPY\D\H\psi
>svn pl -vR merge_tests-1
Properties on 'merge_tests-1\A_COPY':
svn:mergeinfo : /A:1,3
Properties on 'merge_tests-1\A_COPY\B':
svn:mergeinfo : /A/B:1,3,5
Properties on 'merge_tests-1\A_COPY\D':
svn:mergeinfo : /A/D:1,3-4
In fixing issue 2883, should we keep the current behavior or do one of
the following?
Only the affected subtree and its ancestors with mergeinfo get updated:
>svn st merge_tests-1
M merge_tests-1\A_COPY
M merge_tests-1\A_COPY\D
M merge_tests-1\A_COPY\D\H\psi
>svn pl -vR merge_tests-1
Properties on 'merge_tests-1\A_COPY':
svn:mergeinfo : /A:1,3
Properties on 'merge_tests-1\A_COPY\B':
svn:mergeinfo : /A/B:1,5
Properties on 'merge_tests-1\A_COPY\D':
svn:mergeinfo : /A/D:1,3-4
Or only the affected subtree gets updated:
>svn st merge_tests-1
M merge_tests-1\A_COPY\D
M merge_tests-1\A_COPY\D\H\psi
>svn pl -vR merge_tests-1
Properties on 'merge_tests-1\A_COPY':
svn:mergeinfo : /A:1,3
Properties on 'merge_tests-1\A_COPY\B':
svn:mergeinfo : /A/B:1,3,5
Properties on 'merge_tests-1\A_COPY\D':
svn:mergeinfo : /A/D:1,3-4
I have a slight preference for the default behavior of trunk today,
because that allows for optimal mergeinfo elision and therefore the
minimal number of paths with mergeinfo (to see why imagine merging -r2:5
from A to A_COPY, all three paths would end up with the same mergeinfo
(1,3-5) so then A_COPY/B and A_COPY/D's mergeinfo would elide, leaving
only A_COPY's).
Paul
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: RFC: Issue 2883 No-op merge (without skip) should not change mergeinfo
Posted by Daniel Rall <dl...@finemaltcoding.com>.
On Aug 22, 2007, at 5:40 PM, Mark Phippard wrote:
> On 8/22/07, Paul Burba <pb...@collab.net> wrote:
>>> We definitely want the current behavior. Perhaps if that
>>> last merge had been --depth=files then we would not want it,
>>> but otherwise we do.
>>> Revision 3 was merged into the entire tree, whether it
>>> needed to update those parts or not. For auditing reasons if
>>> nothing else, it needs to be recorded everywhere (in your example).
>>
>> Hi Mark,
>>
>> The --depth case is a separate matter, it's issue #2827 in fact.
>>
>> Anyhow, I agree with you that the current behavior is fine, but you
>> understand once issue 2883 is addressed, that if -c3 *didn't* affect
>> anything under the merge target that we will *not* update the
>> mergeinfo
>> on A_COPY, A_COPY\B, or A_COPY/C, even though "Revision 3 was merged
>> into the entire tree". I mention this because your argument "For
>> auditing reasons if nothing else" implies to me you may not agree
>> with
>> issue 2883 at all. Am I misunderstanding you?
>
> I think there is a subtle difference. 2883 is saying if there was no
> activity at all in the merge source, then we should not update the
> mergeinfo on the merge target. I realize this is only a subtle
> difference, and it could be argued that we ought to update the
> mergeinfo. In this case, it seems like a reasonable optimization to
> not do this which should be a little more friendly for scripters.
>
> In your example, there were some changes merged, they just did not
> explicitly effect the areas where the mergeinfo was updated. We
> have been fairly clear that we want this to update the mergeinfo.
>
> In fact there is third scenario, that Dan mentions on one of these
> issues. The merge does not actually merge anything, but it does
> "skip" some stuff. In other words, items were changed in the merge
> source that do not exist in the merge target. In this scenario, Dan
> was pretty adamant that we should still update the mergeinfo and I
> agree.
>
> Basically, we decided to treat the one scenario described in 2883 as a
> "special case" based on feedback from several of the early adopters
> that tried the code.
FWIW, Mark has echoed my thoughts on this thread.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: RFC: Issue 2883 No-op merge (without skip) should not change mergeinfo
Posted by Mark Phippard <ma...@gmail.com>.
On 8/22/07, Paul Burba <pb...@collab.net> wrote:
> > We definitely want the current behavior. Perhaps if that
> > last merge had been --depth=files then we would not want it,
> > but otherwise we do.
> > Revision 3 was merged into the entire tree, whether it
> > needed to update those parts or not. For auditing reasons if
> > nothing else, it needs to be recorded everywhere (in your example).
>
> Hi Mark,
>
> The --depth case is a separate matter, it's issue #2827 in fact.
>
> Anyhow, I agree with you that the current behavior is fine, but you
> understand once issue 2883 is addressed, that if -c3 *didn't* affect
> anything under the merge target that we will *not* update the mergeinfo
> on A_COPY, A_COPY\B, or A_COPY/C, even though "Revision 3 was merged
> into the entire tree". I mention this because your argument "For
> auditing reasons if nothing else" implies to me you may not agree with
> issue 2883 at all. Am I misunderstanding you?
I think there is a subtle difference. 2883 is saying if there was no
activity at all in the merge source, then we should not update the
mergeinfo on the merge target. I realize this is only a subtle
difference, and it could be argued that we ought to update the
mergeinfo. In this case, it seems like a reasonable optimization to
not do this which should be a little more friendly for scripters.
In your example, there were some changes merged, they just did not
explicitly effect the areas where the mergeinfo was updated. We
have been fairly clear that we want this to update the mergeinfo.
In fact there is third scenario, that Dan mentions on one of these
issues. The merge does not actually merge anything, but it does
"skip" some stuff. In other words, items were changed in the merge
source that do not exist in the merge target. In this scenario, Dan
was pretty adamant that we should still update the mergeinfo and I
agree.
Basically, we decided to treat the one scenario described in 2883 as a
"special case" based on feedback from several of the early adopters
that tried the code.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: RFC: Issue 2883 No-op merge (without skip) should not change mergeinfo
Posted by Paul Burba <pb...@collab.net>.
> -----Original Message-----
> From: Mark Phippard [mailto:markphip@gmail.com]
> Sent: Wednesday, August 22, 2007 7:15 PM
> To: Paul Burba
> Cc: dev@subversion.tigris.org
> Subject: Re: RFC: Issue 2883 No-op merge (without skip)
> should not change mergeinfo
>
> On 8/22/07, Paul Burba <pb...@collab.net> wrote:
> > I'd appreciate anyone's thoughts on issue 2883
> > http://subversion.tigris.org/issues/show_bug.cgi?id=2883
> >
> > "If merge does not do anything (there are no changes in
> merge source),
> > the svn:mergeinfo property should not be modified."
> >
> > The expected behavior if only the merge target has
> mergeinfo is clear:
> > if nothing was changed by the merge, then don't update the target's
> > mergeinfo.
> >
> > Also, if the target and some of its children both have
> mergeinfo but
> > again nothing was changed by the merge, don't update the
> target's or
> > the subtrees' mergeinfo. But what if, in this second case, a path
> > within one of the subtrees was modified. Should the merge
> target and
> > the subtree have thier mergeinfo updated, the subtree and it's
> > ancestors with mergeinfo, or only the subtree?
> >
> > e.g.
> >
> > Given a greek tree with the following changes (this is from
> > merge_tests.py:mergeinfo_inheritance):
> >
> > r2 - copy A to A_COPY
> > r3 - text mod to A/D/H/psi
> > r4 - text mod to A/D/G/rho
> > r5 - text mod to A/B/E/beta
> > r6 - text mod to A/D/H/omega
> > r7 - merge r4 from A into A_COPY/D
> > merge r5 from A into A_COPY/B
> > merge r3 from A into A_COPY
> >
> > This gives us the following mergeinfo:
> >
> > >svn pl -vR merge_tests-1
> > Properties on 'merge_tests-1\A_COPY':
> > svn:mergeinfo : /A:1
> > Properties on 'merge_tests-1\A_COPY\B':
> > svn:mergeinfo : /A/B:1,5
> > Properties on 'merge_tests-1\A_COPY\D':
> > svn:mergeinfo : /A/D:1,4
> >
> > Currently trunk behaves this way if we perform a merge that affects
> > only a subtree:
> >
> > >svn merge %URL%/A merge_tests-1\A_COPY -c3
> > --- Merging r3:
> > --- Merging r3:
> > U merge_tests-1\A_COPY\D\H\psi
> > --- Merging r3:
> >
> > As you can see, all the subtrees with mergeinfo get r3 added, even
> > though only the subtree rooted at A_COPY/D was affected.
> >
> > >svn st merge_tests-1
> > M merge_tests-1\A_COPY
> > M merge_tests-1\A_COPY\B
> > M merge_tests-1\A_COPY\D
> > M merge_tests-1\A_COPY\D\H\psi
> >
> > >svn pl -vR merge_tests-1
> > Properties on 'merge_tests-1\A_COPY':
> > svn:mergeinfo : /A:1,3
> > Properties on 'merge_tests-1\A_COPY\B':
> > svn:mergeinfo : /A/B:1,3,5
> > Properties on 'merge_tests-1\A_COPY\D':
> > svn:mergeinfo : /A/D:1,3-4
> >
> > In fixing issue 2883, should we keep the current behavior
> or do one of
> > the following?
>
> We definitely want the current behavior. Perhaps if that
> last merge had been --depth=files then we would not want it,
> but otherwise we do.
> Revision 3 was merged into the entire tree, whether it
> needed to update those parts or not. For auditing reasons if
> nothing else, it needs to be recorded everywhere (in your example).
Hi Mark,
The --depth case is a separate matter, it's issue #2827 in fact.
Anyhow, I agree with you that the current behavior is fine, but you
understand once issue 2883 is addressed, that if -c3 *didn't* affect
anything under the merge target that we will *not* update the mergeinfo
on A_COPY, A_COPY\B, or A_COPY/C, even though "Revision 3 was merged
into the entire tree". I mention this because your argument "For
auditing reasons if nothing else" implies to me you may not agree with
issue 2883 at all. Am I misunderstanding you?
Paul
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: RFC: Issue 2883 No-op merge (without skip) should not change mergeinfo
Posted by Mark Phippard <ma...@gmail.com>.
On 8/22/07, Paul Burba <pb...@collab.net> wrote:
> I'd appreciate anyone's thoughts on issue 2883
> http://subversion.tigris.org/issues/show_bug.cgi?id=2883
>
> "If merge does not do anything (there are no changes in merge source),
> the svn:mergeinfo property should not be modified."
>
> The expected behavior if only the merge target has mergeinfo is clear:
> if nothing was changed by the merge, then don't update the target's
> mergeinfo.
>
> Also, if the target and some of its children both have mergeinfo but
> again nothing was changed by the merge, don't update the target's or the
> subtrees' mergeinfo. But what if, in this second case, a path within
> one of the subtrees was modified. Should the merge target and the
> subtree have thier mergeinfo updated, the subtree and it's ancestors
> with mergeinfo, or only the subtree?
>
> e.g.
>
> Given a greek tree with the following changes (this is from
> merge_tests.py:mergeinfo_inheritance):
>
> r2 - copy A to A_COPY
> r3 - text mod to A/D/H/psi
> r4 - text mod to A/D/G/rho
> r5 - text mod to A/B/E/beta
> r6 - text mod to A/D/H/omega
> r7 - merge r4 from A into A_COPY/D
> merge r5 from A into A_COPY/B
> merge r3 from A into A_COPY
>
> This gives us the following mergeinfo:
>
> >svn pl -vR merge_tests-1
> Properties on 'merge_tests-1\A_COPY':
> svn:mergeinfo : /A:1
> Properties on 'merge_tests-1\A_COPY\B':
> svn:mergeinfo : /A/B:1,5
> Properties on 'merge_tests-1\A_COPY\D':
> svn:mergeinfo : /A/D:1,4
>
> Currently trunk behaves this way if we perform a merge that affects only
> a subtree:
>
> >svn merge %URL%/A merge_tests-1\A_COPY -c3
> --- Merging r3:
> --- Merging r3:
> U merge_tests-1\A_COPY\D\H\psi
> --- Merging r3:
>
> As you can see, all the subtrees with mergeinfo get r3 added, even
> though only the subtree rooted at A_COPY/D was affected.
>
> >svn st merge_tests-1
> M merge_tests-1\A_COPY
> M merge_tests-1\A_COPY\B
> M merge_tests-1\A_COPY\D
> M merge_tests-1\A_COPY\D\H\psi
>
> >svn pl -vR merge_tests-1
> Properties on 'merge_tests-1\A_COPY':
> svn:mergeinfo : /A:1,3
> Properties on 'merge_tests-1\A_COPY\B':
> svn:mergeinfo : /A/B:1,3,5
> Properties on 'merge_tests-1\A_COPY\D':
> svn:mergeinfo : /A/D:1,3-4
>
> In fixing issue 2883, should we keep the current behavior or do one of
> the following?
We definitely want the current behavior. Perhaps if that last merge
had been --depth=files then we would not want it, but otherwise we do.
Revision 3 was merged into the entire tree, whether it needed to
update those parts or not. For auditing reasons if nothing else, it
needs to be recorded everywhere (in your example).
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Output of merges to targets with sub-tree mergeinfo
Posted by Daniel Rall <dl...@collab.net>.
On Fri, 24 Aug 2007, David Glasser wrote:
> On 8/24/07, Daniel Rall <dl...@finemaltcoding.com> wrote:
> >
> > On Aug 23, 2007, at 11:18 AM, David Glasser wrote:
> >
> > > On 8/22/07, Paul Burba <pb...@collab.net> wrote:
> > >> Currently trunk behaves this way if we perform a merge that
> > >> affects only
> > >> a subtree:
> > >>
> > >>> svn merge %URL%/A merge_tests-1\A_COPY -c3
> > >> --- Merging r3:
> > >> --- Merging r3:
> > >> U merge_tests-1\A_COPY\D\H\psi
> > >> --- Merging r3:
> > >
> > > I'm not sure I am qualified to speak to the actual underlying
> > > mergeinfo issues here. But as far as UI is concerned, this output is
> > > somewhat strange, with the three "Merging r3:" lines. Maybe it would
> > > be more clear if the "--- Merging" lines mentioned the paths as well?
> >
> > David, in general I tend to agree. For the particular case above,
> > I'm in violent agreement. How about this?
> >
> > $ svn merge -c -26286 https://svn.collab.net/repos/svn/trunk/www www
> > --- Undoing r26286 for 'www':
> >
> > See attached patch.
>
> Sorry, I'm about to drive to Burning Man, so I think I'll just say I
> trust your taste :)
I committed the patch to trunk in r26647.
Re: Output of merges to targets with sub-tree mergeinfo
Posted by David Glasser <gl...@davidglasser.net>.
On 8/24/07, Daniel Rall <dl...@finemaltcoding.com> wrote:
>
> On Aug 23, 2007, at 11:18 AM, David Glasser wrote:
>
> > On 8/22/07, Paul Burba <pb...@collab.net> wrote:
> >> Currently trunk behaves this way if we perform a merge that
> >> affects only
> >> a subtree:
> >>
> >>> svn merge %URL%/A merge_tests-1\A_COPY -c3
> >> --- Merging r3:
> >> --- Merging r3:
> >> U merge_tests-1\A_COPY\D\H\psi
> >> --- Merging r3:
> >
> > I'm not sure I am qualified to speak to the actual underlying
> > mergeinfo issues here. But as far as UI is concerned, this output is
> > somewhat strange, with the three "Merging r3:" lines. Maybe it would
> > be more clear if the "--- Merging" lines mentioned the paths as well?
>
> David, in general I tend to agree. For the particular case above,
> I'm in violent agreement. How about this?
>
> $ svn merge -c -26286 https://svn.collab.net/repos/svn/trunk/www www
> --- Undoing r26286 for 'www':
>
> See attached patch.
Sorry, I'm about to drive to Burning Man, so I think I'll just say I
trust your taste :)
--dave
--
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Output of merges to targets with sub-tree mergeinfo
Posted by Daniel Rall <dl...@finemaltcoding.com>.
On Aug 23, 2007, at 11:18 AM, David Glasser wrote:
> On 8/22/07, Paul Burba <pb...@collab.net> wrote:
>> Currently trunk behaves this way if we perform a merge that
>> affects only
>> a subtree:
>>
>>> svn merge %URL%/A merge_tests-1\A_COPY -c3
>> --- Merging r3:
>> --- Merging r3:
>> U merge_tests-1\A_COPY\D\H\psi
>> --- Merging r3:
>
> I'm not sure I am qualified to speak to the actual underlying
> mergeinfo issues here. But as far as UI is concerned, this output is
> somewhat strange, with the three "Merging r3:" lines. Maybe it would
> be more clear if the "--- Merging" lines mentioned the paths as well?
David, in general I tend to agree. For the particular case above,
I'm in violent agreement. How about this?
$ svn merge -c -26286 https://svn.collab.net/repos/svn/trunk/www www
--- Undoing r26286 for 'www':
See attached patch.
- Dan
Re: RFC: Issue 2883 No-op merge (without skip) should not change mergeinfo
Posted by David Glasser <gl...@davidglasser.net>.
On 8/22/07, Paul Burba <pb...@collab.net> wrote:
> Currently trunk behaves this way if we perform a merge that affects only
> a subtree:
>
> >svn merge %URL%/A merge_tests-1\A_COPY -c3
> --- Merging r3:
> --- Merging r3:
> U merge_tests-1\A_COPY\D\H\psi
> --- Merging r3:
I'm not sure I am qualified to speak to the actual underlying
mergeinfo issues here. But as far as UI is concerned, this output is
somewhat strange, with the three "Merging r3:" lines. Maybe it would
be more clear if the "--- Merging" lines mentioned the paths as well?
--dave
--
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org