You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Julian Foad <ju...@wandisco.com> on 2011/09/08 17:59:58 UTC
Diff shows added svn:mergeinfo prop as lots of new merges
If Subversion creates subtree mergeinfo on a path that previously didn't
have any, then "svn diff" incorrectly shows all the source revs in that
mergeinfo as newly "merged".
In a Subversion trunk WC, using 1.7.0-rc2:
$ svn merge -c 1000000 ^/subversion/branches/1.6.x/INSTALL INSTALL
--- Merging r1000000 into 'INSTALL':
U INSTALL
--- Recording mergeinfo for merge of r1000000 into 'INSTALL':
G INSTALL
[Note that r1000000 is a no-op on trunk, so in fact no content change
was made despite the 'U' marker.]
$ svn diff INSTALL
Index: INSTALL
===================================================================
--- INSTALL (revision 1166027)
+++ INSTALL (working copy)
Property changes on: INSTALL
___________________________________________________________________
Added: svn:mergeinfo
Merged /subversion/branches/atomic-revprop/INSTALL:r965046-1000689
Merged /subversion/branches/diff-callbacks3/INSTALL:r870059-870761
Merged /subversion/branches/bdb-reverse-deltas/INSTALL:r872050-872529
Merged /subversion/branches/double-delete/INSTALL:r870511-872970
[...]
This is wrong. The property certainly was added, but that does not mean
all those revisions were merged. The expected output is something like:
[...]
Added: svn:mergeinfo
Merged /subversion/branches/1.6.x/INSTALL:r1000000
The bug is that "svn diff" shows a mergeinfo diff assuming that the
previous merginfo was an explicit empty set of mergeinfo, but it wasn't,
it was inherited mergeinfo.
- Julian
Re: Diff shows added svn:mergeinfo prop as lots of new merges
Posted by Julian Foad <ju...@wandisco.com>.
On Mon, 2011-09-12 at 15:33 +0100, Julian Foad wrote:
> On Mon, 2011-09-12 at 10:00 -0400, Mark Phippard wrote:
> > On Mon, Sep 12, 2011 at 9:50 AM, Julian Foad <ju...@wandisco.com> wrote:
> > >> If merge brought in legitimate
> > >> > changes to the svn:mergeinfo property. diff is supposed to show the
> > >> > changes, and those are changes.
> > >
> > > I said this is a choice, and that if we want to display raw changes to
> > > the property then that is a valid alternative but then we shouldn't be
> > > using the terminology "Merged: xxx" to describe the change when "xxx"
> > > was not in fact merged.
> > >
> > > Certainly the trivial way to close this "issue" is just to change the
> > > wording of the currently displayed messages.
> >
> > I think this is where we differ.
[...]
> > I think this is correct and worth showing in
> > the diff output.
>
> I agree that it's useful and necessary to see the merge described as
> coming from both branch1 and branch2. Did I write something that seemed
> to say otherwise?
Oh, I think I see. It's the old confusion over what the term "merged"
means.
In this thread I accept "merged" as meaning directly or indirectly
merged.
- Julian
Re: Diff shows added svn:mergeinfo prop as lots of new merges
Posted by Julian Foad <ju...@wandisco.com>.
On Mon, 2011-09-12 at 10:00 -0400, Mark Phippard wrote:
> On Mon, Sep 12, 2011 at 9:50 AM, Julian Foad <ju...@wandisco.com> wrote:
> >> If merge brought in legitimate
> >> > changes to the svn:mergeinfo property. diff is supposed to show the
> >> > changes, and those are changes.
> >
> > I said this is a choice, and that if we want to display raw changes to
> > the property then that is a valid alternative but then we shouldn't be
> > using the terminology "Merged: xxx" to describe the change when "xxx"
> > was not in fact merged.
> >
> > Certainly the trivial way to close this "issue" is just to change the
> > wording of the currently displayed messages.
>
> I think this is where we differ.
>
> Suppose you merge some changes from /branch1 to /branch2 and commit in r100.
Here's how this looks in a simple test with 1.7.0-rc2. (My r5 is your
r100.)
$ svn merge ^/branch1 branch2
--- Merging r2 through r4 into 'branch2':
A branch2/D
--- Recording mergeinfo for merge of r2 through r4 into 'branch2':
U branch2
$ svn ci -m ""
Sending branch2
Adding branch2/D
Committed revision 5.
> Now you merge r100 into /branch3.
$ svn up
Updating '.':
At revision 5.
$ svn merge ^/branch2 branch3
--- Merging r3 through r5 into 'branch3':
A branch3/D
U branch3
--- Recording mergeinfo for merge of r3 through r5 into 'branch3':
G branch3
> The mergeinfo from this merge will indicate that you merged r100 from
> /branch2 but it will also contain the mergeinfo from the merge of
> branch1 into branch2.
$ svn diff
Index: branch3
===================================================================
--- branch3 (revision 5)
+++ branch3 (working copy)
Property changes on: branch3
___________________________________________________________________
Added: svn:mergeinfo
Merged /branch1:r2-4
Merged /branch2:r3-5
> I think this is correct and worth showing in
> the diff output.
I agree that it's useful and necessary to see the merge described as
coming from both branch1 and branch2. Did I write something that seemed
to say otherwise?
> I also do not really know how you could display it
> differently after the fact.
Like this, for example:
$ svn diff
Index: branch3
===================================================================
--- branch3 (revision 5)
+++ branch3 (working copy)
Property changes on: branch3
___________________________________________________________________
Added: svn:mergeinfo
Mergeinfo differences
===================================================================
Changes merged to 'branch3':
Merged /branch1:r2-4
Merged /branch2:r3-5
> Is it possible that this is sometimes confusing? Sure. But that does
> not mean it is wrong and as we have all pointed out in this thread,
> this has not exactly been a problem people seem to be reporting or
> experiencing.
I am interested in this because it represents a significant *part* of a
bigger problem that I am currently getting my teeth into, which is
trying to describe merges to our users in an understandable way. I
repeat: I agree this particular piece of UI output is not one that users
have complained about specifically. The point is, it is important to me
that we have ways to communicate such things effectively. I want to be
able to have a way of describing the merge, and then present that
description in not just here in 'diff' but for example in some revised
form of the 'svn mergeinfo' command or wherever we decide to do it, but
we ought to be able to do it somewhere, and discussing how it would work
is a good way to get there.
- Julian
Re: Diff shows added svn:mergeinfo prop as lots of new merges
Posted by Julian Foad <ju...@wandisco.com>.
On Mon, 2011-09-12 at 10:41 -0400, Mark Phippard wrote:
> On Mon, Sep 12, 2011 at 10:37 AM, Mark Phippard <ma...@gmail.com> wrote:
> > I have a working copy of trunk and I do the merge shown as r14 (which
> > is what will be created when I commit this merge). Here is the
> > command I run and the diff output of the mergeinfo:
> >
> > $ svn merge ^/branches/b .
> > --- Merging r10 through r13 into '.':
> > A products/roadmap.html
> > U products/index.html
> > U about/index.html
> > U index.html
> > U news/index.html
> > U .
> > --- Recording mergeinfo for merge of r10 through r13 into '.':
> > G .
> >
> > $ svn diff
> > Index: .
> > ===================================================================
> > --- . (revision 13)
> > +++ . (working copy)
> >
> > Property changes on: .
> > ___________________________________________________________________
> > Modified: svn:mergeinfo
> > Merged /branches/a:r6,8-11
> > Merged /branches/b:r10-13
> >
> >
> > This gives a simple example to talk about. What changes would you
> > propose making to this output?
>
> Our emails cross paths. I think this is the output you propose:
>
> Property changes on: .
> ___________________________________________________________________
> Modified: svn:mergeinfo
>
> Mergeinfo differences
> ===================================================================
> Changes merged to '.':
> Merged /branches/a:r6,8-11
> Merged /branches/b:r10-13
Correct, because that example doesn't have any subtree mergeinfo. My
thread is all about what happens when a subtree svn:mergeinfo prop is
added (or removed, or, now I think about it, modified in the same way as
the parent's svn:mergeinfo prop).
> If this is the entire scope of your proposal,
"This" being eliminating mention of svn:mergeinfo changes that would be
invisible to the merge algorithm.
> then I have no objections.
Cool. Thanks for taking the time to understand exactly what I mean.
> I thought you were proposing that you wanted to show that
> /b was merged and then either now show /a at all or somehow
> distinguish that it came with the changes merged from /b. I would not
> object to doing that either, I just do not know how you could.
- Julian
Re: Diff shows added svn:mergeinfo prop as lots of new merges
Posted by Mark Phippard <ma...@gmail.com>.
On Mon, Sep 12, 2011 at 10:37 AM, Mark Phippard <ma...@gmail.com> wrote:
> I have a working copy of trunk and I do the merge shown as r14 (which
> is what will be created when I commit this merge). Here is the
> command I run and the diff output of the mergeinfo:
>
> $ svn merge ^/branches/b .
> --- Merging r10 through r13 into '.':
> A products/roadmap.html
> U products/index.html
> U about/index.html
> U index.html
> U news/index.html
> U .
> --- Recording mergeinfo for merge of r10 through r13 into '.':
> G .
>
> $ svn diff
> Index: .
> ===================================================================
> --- . (revision 13)
> +++ . (working copy)
>
> Property changes on: .
> ___________________________________________________________________
> Modified: svn:mergeinfo
> Merged /branches/a:r6,8-11
> Merged /branches/b:r10-13
>
>
> This gives a simple example to talk about. What changes would you
> propose making to this output?
Our emails cross paths. I think this is the output you propose:
Property changes on: .
___________________________________________________________________
Modified: svn:mergeinfo
Mergeinfo differences
===================================================================
Changes merged to '.':
Merged /branches/a:r6,8-11
Merged /branches/b:r10-13
If this is the entire scope of your proposal, then I have no
objections. I thought you were proposing that you wanted to show that
/b was merged and then either now show /a at all or somehow
distinguish that it came with the changes merged from /b. I would not
object to doing that either, I just do not know how you could.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
Re: Diff shows added svn:mergeinfo prop as lots of new merges
Posted by Mark Phippard <ma...@gmail.com>.
On Mon, Sep 12, 2011 at 10:00 AM, Mark Phippard <ma...@gmail.com> wrote:
> On Mon, Sep 12, 2011 at 9:50 AM, Julian Foad <ju...@wandisco.com> wrote:
>>> If merge brought in legitimate
>>> > changes to the svn:mergeinfo property. diff is supposed to show the
>>> > changes, and those are changes.
>>
>> I said this is a choice, and that if we want to display raw changes to
>> the property then that is a valid alternative but then we shouldn't be
>> using the terminology "Merged: xxx" to describe the change when "xxx"
>> was not in fact merged.
>>
>> Certainly the trivial way to close this "issue" is just to change the
>> wording of the currently displayed messages.
>
> I think this is where we differ.
>
> Suppose you merge some changes from /branch1 to /branch2 and commit in r100.
>
> Now you merge r100 into /branch3.
>
> The mergeinfo from this merge will indicate that you merged r100 from
> /branch2 but it will also contain the mergeinfo from the merge of
> branch1 into branch2. I think this is correct and worth showing in
> the diff output. I also do not really know how you could display it
> differently after the fact.
>
> Is it possible that this is sometimes confusing? Sure. But that does
> not mean it is wrong and as we have all pointed out in this thread,
> this has not exactly been a problem people seem to be reporting or
> experiencing.
I thought it would be helpful to try to give you a better example, so
I dusted off the old repository we used to use to test some of the
merge tracking scenarios. Refer to this graphic:
http://www.open.collab.net/branding/images/projects/merge/repo.png
I have a working copy of trunk and I do the merge shown as r14 (which
is what will be created when I commit this merge). Here is the
command I run and the diff output of the mergeinfo:
$ svn merge ^/branches/b .
--- Merging r10 through r13 into '.':
A products/roadmap.html
U products/index.html
U about/index.html
U index.html
U news/index.html
U .
--- Recording mergeinfo for merge of r10 through r13 into '.':
G .
$ svn diff
Index: .
===================================================================
--- . (revision 13)
+++ . (working copy)
Property changes on: .
___________________________________________________________________
Modified: svn:mergeinfo
Merged /branches/a:r6,8-11
Merged /branches/b:r10-13
This gives a simple example to talk about. What changes would you
propose making to this output?
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
Re: Diff shows added svn:mergeinfo prop as lots of new merges
Posted by Mark Phippard <ma...@gmail.com>.
On Mon, Sep 12, 2011 at 9:50 AM, Julian Foad <ju...@wandisco.com> wrote:
>> If merge brought in legitimate
>> > changes to the svn:mergeinfo property. diff is supposed to show the
>> > changes, and those are changes.
>
> I said this is a choice, and that if we want to display raw changes to
> the property then that is a valid alternative but then we shouldn't be
> using the terminology "Merged: xxx" to describe the change when "xxx"
> was not in fact merged.
>
> Certainly the trivial way to close this "issue" is just to change the
> wording of the currently displayed messages.
I think this is where we differ.
Suppose you merge some changes from /branch1 to /branch2 and commit in r100.
Now you merge r100 into /branch3.
The mergeinfo from this merge will indicate that you merged r100 from
/branch2 but it will also contain the mergeinfo from the merge of
branch1 into branch2. I think this is correct and worth showing in
the diff output. I also do not really know how you could display it
differently after the fact.
Is it possible that this is sometimes confusing? Sure. But that does
not mean it is wrong and as we have all pointed out in this thread,
this has not exactly been a problem people seem to be reporting or
experiencing.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
Re: Diff shows added svn:mergeinfo prop as lots of new merges
Posted by Paul Burba <pt...@gmail.com>.
On Mon, Sep 12, 2011 at 9:50 AM, Julian Foad <ju...@wandisco.com> wrote:
> Paul Burba wrote:
>> If diff is going to take
>> inherited mergeinfo into account (which in a nutshell is really what
>> you are proposing...I think) then *every* child path under an
>> (inheritable) explicit mergeinfo change should show a diff right?
>
> No, only if you choose to display that information *per path*. But I
> suggested we shouldn't show it per path but instead in its own section
> at the bottom of the diff output. (That's what I meant; sorry if it
> wasn't clear because my example showed only one path.)
Ok, I understand you now. I hadn't meant that we'd show the diff for
every subtree, only that if the *target* of the diff had no explicit
mergeinfo but had a parent with a change, then we'd account for that
in the diff. Which is what you show...
>> E.g. We merge a change into the 1.6.x backport branch:
>>
>> C:\SVN\src-branch-1.6.x>svn merge ^^/subversion/trunk . -c996383
>> --- Merging r996383 into '.':
>> U subversion\tests\cmdline\resolve_tests.py
>> U subversion\svn\conflict-callbacks.c
>> --- Recording mergeinfo for merge of r996383 into '.':
>> U .
>>
>> [...] what would you expect a non-recursive diff of that subtree to
>> show:
>>
>> C:\SVN\src-branch-1.6.x>svn diff subversion -N
>
> I'd expect something like this:
>
> [[[
> Mergeinfo differences [### on the whole of the diff target]
> =====================
> Merged to '.':
> Merged /subversion/trunk/subversion:r996383
> ]]]
...here
> And with a recursive diff on that subtree I'd expect:
>
> [[[
> Index: tests\cmdline\resolve_tests.py
> [...]
> Index: svn\conflict-callbacks.c
> [...]
> Mergeinfo differences [### on the whole of the diff target]
> =====================
> Merged to '.':
> Merged /subversion/trunk/subversion:r996383
> ]]]
>
>
> Paul Burba wrote:
>> Do we have reason to believe that users are being "mislead and
>> distracted" by diff's current behavior?
>
> I don't have evidence, I just expect it.
Got it and fair enough (I just like to know of any real-world specific
use cases)
>> I'll admit there is plenty to
>> be tripped up by with merge tracking, but I'm not sure that diff makes
>> the top 10.
>
> Sure, I agree. It's just that it's time now for me to start tackling
> some of this stuff. I'll repeat from my last email: To be clear, this
> is just one issue that I noticed while thinking about what the user
> really wants to see form "svn mergeinfo", "svn diff", etc. It's not the
> most important, and I'm not at all expecting you to jump in and "fix" it
> if we agree it needs fixing. It's just one piece of the puzzle that
> happened to jump out at me.
>
> In fact, I'm having a total re-think of what useful information the
> users really want to see, which probably 'svn mergeinfo' should display.
> High-level stuff like "your last catch-up was with branch B up to
> revision Y", or "you have all the changes from branch B except these:
> C1, C2, ...". I'll write separately about those thoughts.
>
>
>> >> That's where this comes in: I do a simple little merge
>> >> and run "svn diff" to check what's happened in the WC and suddenly it
>> >> says loads of stuff has been "merged", not the simple little merge that
>> >> I expected.
>> >
>> > I do not think I agree with you on this.
>> >
>> > As you say, diff should show what has happened in the WC. I do not
>> > see why it should hide changes.
>
> I'm not suggesting it should hide changes, I'm suggesting it should
> display the changes in a high-level interpretation rather than raw.
>
>> If merge brought in legitimate
>> > changes to the svn:mergeinfo property. diff is supposed to show the
>> > changes, and those are changes.
>
> I said this is a choice, and that if we want to display raw changes to
> the property then that is a valid alternative but then we shouldn't be
> using the terminology "Merged: xxx" to describe the change when "xxx"
> was not in fact merged.
>
> Certainly the trivial way to close this "issue" is just to change the
> wording of the currently displayed messages.
>
>> > If we want a WC to be more specifically aware that is has been
>> > modified by a merge, then maybe we should consider that information in
>> > the output of some of our other commands (status, info) ? Maybe we
>> > need a new command or maybe mergeinfo should grow a new option to show
>> > what revisions were merged into the WC?
>
> Sure -- the 'svn mergeinfo' command is the obvious place for hanging
> functionality along the lines of 'give me some information about what's
> been merged'.
>
>> Or maybe diff should get a new option? --consider-mergeinfo?
>
> ... which would switch between raw and interpreted ways of dislaying
> mergeinfo diffs. Sure.
>
>> Assuming we want a way to provide this info, I prefer the idea of
>> leaving diff as-is and using a new subcommand or option to provide it.
>>
>> > It seems like diff is doing what is should.
>
> Do you acknowledge my point about the wording "Merged: xxx"?
>
> - Julian
>
>
>
Re: Diff shows added svn:mergeinfo prop as lots of new merges
Posted by Julian Foad <ju...@wandisco.com>.
Paul Burba wrote:
> If diff is going to take
> inherited mergeinfo into account (which in a nutshell is really what
> you are proposing...I think) then *every* child path under an
> (inheritable) explicit mergeinfo change should show a diff right?
No, only if you choose to display that information *per path*. But I
suggested we shouldn't show it per path but instead in its own section
at the bottom of the diff output. (That's what I meant; sorry if it
wasn't clear because my example showed only one path.)
> E.g. We merge a change into the 1.6.x backport branch:
>
> C:\SVN\src-branch-1.6.x>svn merge ^^/subversion/trunk . -c996383
> --- Merging r996383 into '.':
> U subversion\tests\cmdline\resolve_tests.py
> U subversion\svn\conflict-callbacks.c
> --- Recording mergeinfo for merge of r996383 into '.':
> U .
>
> [...] what would you expect a non-recursive diff of that subtree to
> show:
>
> C:\SVN\src-branch-1.6.x>svn diff subversion -N
I'd expect something like this:
[[[
Mergeinfo differences [### on the whole of the diff target]
=====================
Merged to '.':
Merged /subversion/trunk/subversion:r996383
]]]
And with a recursive diff on that subtree I'd expect:
[[[
Index: tests\cmdline\resolve_tests.py
[...]
Index: svn\conflict-callbacks.c
[...]
Mergeinfo differences [### on the whole of the diff target]
=====================
Merged to '.':
Merged /subversion/trunk/subversion:r996383
]]]
Paul Burba wrote:
> Do we have reason to believe that users are being "mislead and
> distracted" by diff's current behavior?
I don't have evidence, I just expect it.
> I'll admit there is plenty to
> be tripped up by with merge tracking, but I'm not sure that diff makes
> the top 10.
Sure, I agree. It's just that it's time now for me to start tackling
some of this stuff. I'll repeat from my last email: To be clear, this
is just one issue that I noticed while thinking about what the user
really wants to see form "svn mergeinfo", "svn diff", etc. It's not the
most important, and I'm not at all expecting you to jump in and "fix" it
if we agree it needs fixing. It's just one piece of the puzzle that
happened to jump out at me.
In fact, I'm having a total re-think of what useful information the
users really want to see, which probably 'svn mergeinfo' should display.
High-level stuff like "your last catch-up was with branch B up to
revision Y", or "you have all the changes from branch B except these:
C1, C2, ...". I'll write separately about those thoughts.
> >> That's where this comes in: I do a simple little merge
> >> and run "svn diff" to check what's happened in the WC and suddenly it
> >> says loads of stuff has been "merged", not the simple little merge that
> >> I expected.
> >
> > I do not think I agree with you on this.
> >
> > As you say, diff should show what has happened in the WC. I do not
> > see why it should hide changes.
I'm not suggesting it should hide changes, I'm suggesting it should
display the changes in a high-level interpretation rather than raw.
> If merge brought in legitimate
> > changes to the svn:mergeinfo property. diff is supposed to show the
> > changes, and those are changes.
I said this is a choice, and that if we want to display raw changes to
the property then that is a valid alternative but then we shouldn't be
using the terminology "Merged: xxx" to describe the change when "xxx"
was not in fact merged.
Certainly the trivial way to close this "issue" is just to change the
wording of the currently displayed messages.
> > If we want a WC to be more specifically aware that is has been
> > modified by a merge, then maybe we should consider that information in
> > the output of some of our other commands (status, info) ? Maybe we
> > need a new command or maybe mergeinfo should grow a new option to show
> > what revisions were merged into the WC?
Sure -- the 'svn mergeinfo' command is the obvious place for hanging
functionality along the lines of 'give me some information about what's
been merged'.
> Or maybe diff should get a new option? --consider-mergeinfo?
... which would switch between raw and interpreted ways of dislaying
mergeinfo diffs. Sure.
> Assuming we want a way to provide this info, I prefer the idea of
> leaving diff as-is and using a new subcommand or option to provide it.
>
> > It seems like diff is doing what is should.
Do you acknowledge my point about the wording "Merged: xxx"?
- Julian
RE: Diff shows added svn:mergeinfo prop as lots of new merges
Posted by Bob Jenkins <rj...@collab.net>.
I think the key is that what is done to merge for 1.8 have significant
substance not just be a "nice to have". Users aren't indicating
confusion so much as they are indicating performance issues and
mishandled or unhandled use cases. Better information is good, but real
progress on the items giving users true pain would be much, much better.
It may be that 1.8 can't wait for the time it might take to tackle these
items (and I don't want to see it driving that release where we find
ourselves with another 2+ years without new functionality), but progress
on those items has to start to ever bring relief to the users.
Delivering something around merging in 1.8 isn't as important as making
that progress so that relief can be seen to be around the corner.
Bob
Re: Diff shows added svn:mergeinfo prop as lots of new merges
Posted by Hyrum K Wright <hy...@wandisco.com>.
On Mon, Sep 12, 2011 at 10:08 AM, Mark Phippard <ma...@gmail.com> wrote:
> On Mon, Sep 12, 2011 at 10:54 AM, Julian Foad <ju...@wandisco.com> wrote:
>
>> Heh, well, I did present the thread as "the current behaviour is a
>> problem". I guess I did it that way because it was a bit of detail (in
>> Subversion's overall merge behaviour) that was easy to latch on to and
>> easy to say something specific about what it *does* do, whereas the
>> larger topic that I'm really interested in is currently quite hand-wavy
>> open-ended thinking about what we *could* do, and thus harder to email
>> about.
>
> This might be a minority opinion, but I think the merge support in SVN
> is pretty good and that a lot of the problems users have is simply
> that merge is a hard problem. That said, I think we also have a
> pretty good merge UI and API that gives people the power to do a lot
> with it. As of our 1.6 release, I think these are our biggest
> problems with merge.
>
> 1) We handle renames poorly. Still a problem with 1.7.
>
> 2) Reflective/cyclic merges are a common scenario people need.
> Reintegrate largely makes it possible to do these kinds of merges but
> the fact that you need a special option makes it more difficult.
> Also, people do not like needing to delete and recreate a branch that
> has been reintegrated. This issue still exists with 1.7, though there
> have been continuing improvements in reintegrate to allow it to be
> used in more scenarios.
>
> 3) The merge tracking information is visible to users and the updates
> we need to make to it pollute history. This is an area that has been
> changed in 1.7. If it does not introduce new problems, it should
> resolve this issue.
>
> 4) Merge is slow. The benchmarks I created show that 1.7 has made a
> lot of improvements, however we have found scenarios in our own tree
> where it has gotten a lot worse. So clearly there is still work to do
> here. Obviously performance is an issue for all of SVN, not just
> merge.
>
> I believe #1 and 4 are the biggest weaknesses in SVN and not just for
> merge. Renames are not handled well with update either and
> performance is an area we need to continually improve across the
> product.
I think the problem isn't that merge is terrible, but that folks can
compare it to other systems where, either real or perceived, merge is
much less painful. In an idealistic sense we certainly want merge to
be as best as it can be, but in a practical one, it's really a
question of what do we meet the needs of a user base who sometimes
looks quite longingly at greener grass[1] on the other side of the
fence.
-Hyrum
[1] I'll be the first to point out that if the grass really is greener
"over there," it's due to copious amounts of organic fertilizer. :)
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/
Re: Diff shows added svn:mergeinfo prop as lots of new merges
Posted by Mark Phippard <ma...@gmail.com>.
On Mon, Sep 12, 2011 at 10:54 AM, Julian Foad <ju...@wandisco.com> wrote:
> Heh, well, I did present the thread as "the current behaviour is a
> problem". I guess I did it that way because it was a bit of detail (in
> Subversion's overall merge behaviour) that was easy to latch on to and
> easy to say something specific about what it *does* do, whereas the
> larger topic that I'm really interested in is currently quite hand-wavy
> open-ended thinking about what we *could* do, and thus harder to email
> about.
This might be a minority opinion, but I think the merge support in SVN
is pretty good and that a lot of the problems users have is simply
that merge is a hard problem. That said, I think we also have a
pretty good merge UI and API that gives people the power to do a lot
with it. As of our 1.6 release, I think these are our biggest
problems with merge.
1) We handle renames poorly. Still a problem with 1.7.
2) Reflective/cyclic merges are a common scenario people need.
Reintegrate largely makes it possible to do these kinds of merges but
the fact that you need a special option makes it more difficult.
Also, people do not like needing to delete and recreate a branch that
has been reintegrated. This issue still exists with 1.7, though there
have been continuing improvements in reintegrate to allow it to be
used in more scenarios.
3) The merge tracking information is visible to users and the updates
we need to make to it pollute history. This is an area that has been
changed in 1.7. If it does not introduce new problems, it should
resolve this issue.
4) Merge is slow. The benchmarks I created show that 1.7 has made a
lot of improvements, however we have found scenarios in our own tree
where it has gotten a lot worse. So clearly there is still work to do
here. Obviously performance is an issue for all of SVN, not just
merge.
I believe #1 and 4 are the biggest weaknesses in SVN and not just for
merge. Renames are not handled well with update either and
performance is an area we need to continually improve across the
product.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
Re: Diff shows added svn:mergeinfo prop as lots of new merges
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Julian Foad wrote on Mon, Sep 12, 2011 at 15:54:40 +0100:
> whereas the larger topic that I'm really interested in is currently
> quite hand-wavy open-ended thinking about what we *could* do, and thus
> harder to email about.
It seems you just described 'obliterate' :)
Re: Diff shows added svn:mergeinfo prop as lots of new merges
Posted by Julian Foad <ju...@wandisco.com>.
On Mon, 2011-09-12 at 10:47 -0400, C. Michael Pilato wrote:
> On 09/12/2011 09:58 AM, Julian Foad wrote:
> > I take some offence at that. Sure it's a not a problem that's been
> > proven to need solving -- and I agree it quite likely is low on the
> > priority list in pragmatic terms. I DON'T ASK YOU TO FIX IT. But I'm
> > interested more in the theory and the potential for even being able to
> > describe mergeinfo changes in some way that makes sense to the user.
>
> Sorry, Julian. I guess I was sensing more along the lines of "The current
> behavior is a problem" (assertion) than of "Is the current behavior a
> problem?" (question) as I read the thread. Of course I wouldn't discourage
> anyone from stepping back to evaluate Subversion's behavior at a higher
> level. I guess I would just caution against not stepping back *far enough*.
Heh, well, I did present the thread as "the current behaviour is a
problem". I guess I did it that way because it was a bit of detail (in
Subversion's overall merge behaviour) that was easy to latch on to and
easy to say something specific about what it *does* do, whereas the
larger topic that I'm really interested in is currently quite hand-wavy
open-ended thinking about what we *could* do, and thus harder to email
about.
- Julian
Re: Diff shows added svn:mergeinfo prop as lots of new merges
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 09/12/2011 09:58 AM, Julian Foad wrote:
> I take some offence at that. Sure it's a not a problem that's been
> proven to need solving -- and I agree it quite likely is low on the
> priority list in pragmatic terms. I DON'T ASK YOU TO FIX IT. But I'm
> interested more in the theory and the potential for even being able to
> describe mergeinfo changes in some way that makes sense to the user.
Sorry, Julian. I guess I was sensing more along the lines of "The current
behavior is a problem" (assertion) than of "Is the current behavior a
problem?" (question) as I read the thread. Of course I wouldn't discourage
anyone from stepping back to evaluate Subversion's behavior at a higher
level. I guess I would just caution against not stepping back *far enough*.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: Diff shows added svn:mergeinfo prop as lots of new merges
Posted by Julian Foad <ju...@wandisco.com>.
On Mon, 2011-09-12 at 09:48 -0400, C. Michael Pilato wrote:
> On 09/12/2011 09:24 AM, Paul Burba wrote:
> >>> I'm currently looking at merging from a high-level POV, looking at what
> >>> clues and information we give the users about what they're doing, that
> >>> hopefully guide them in doing the Right Thing and don't mislead and
> >>> distract them.
> >
> > Do we have reason to believe that users are being "mislead and
> > distracted" by diff's current behavior? I'll admit there is plenty to
> > be tripped up by with merge tracking, but I'm not sure that diff makes
> > the top 10.
>
> I'm certainly not convinced. I've heard no user feedback to this effect,
> and have been watching this thread with some amusement as it appears on the
> surface to be an academic exercise aimed at solving a problem that doesn't
> actually exist.
I take some offence at that. Sure it's a not a problem that's been
proven to need solving -- and I agree it quite likely is low on the
priority list in pragmatic terms. I DON'T ASK YOU TO FIX IT. But I'm
interested more in the theory and the potential for even being able to
describe mergeinfo changes in some way that makes sense to the user.
I hardly care a jot whether this particular issue is "solved" if we
can't improve other merge-related behaviour get a whole lot better as
well. This is just something that cropped up and I'm trying to raise
the level of discussion from "it's just that way because it was easy to
implement, who really cares?" up to "right, this shows how we need a
good canonical way of describing merges even to ourselves let alone to
our users so let's acknowledge that a much higher level description
would be useful and then mark it as low-priority for actually fixing".
- Julian
Re: Diff shows added svn:mergeinfo prop as lots of new merges
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 09/12/2011 09:24 AM, Paul Burba wrote:
>>> I'm currently looking at merging from a high-level POV, looking at what
>>> clues and information we give the users about what they're doing, that
>>> hopefully guide them in doing the Right Thing and don't mislead and
>>> distract them.
>
> Do we have reason to believe that users are being "mislead and
> distracted" by diff's current behavior? I'll admit there is plenty to
> be tripped up by with merge tracking, but I'm not sure that diff makes
> the top 10.
I'm certainly not convinced. I've heard no user feedback to this effect,
and have been watching this thread with some amusement as it appears on the
surface to be an academic exercise aimed at solving a problem that doesn't
actually exist.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: Diff shows added svn:mergeinfo prop as lots of new merges
Posted by Paul Burba <pt...@gmail.com>.
On Mon, Sep 12, 2011 at 8:52 AM, Mark Phippard <ma...@gmail.com> wrote:
> On Thu, Sep 8, 2011 at 4:39 PM, Julian Foad <ju...@wandisco.com> wrote:
>> On Thu, 2011-09-08 at 12:37 -0400, Paul Burba wrote:
>>> On Thu, Sep 8, 2011 at 12:08 PM, Julian Foad <ju...@wandisco.com> wrote:
>>> > Can I file an issue for this?
>>>
>>> Hi Julian,
>>>
>>> What problem(s) is the current behavior causing? I understand your
>>> point, but I hesitate to add merge tracking awareness to diff unless
>>> there is some benefit.
>>
>> I'm currently looking at merging from a high-level POV, looking at what
>> clues and information we give the users about what they're doing, that
>> hopefully guide them in doing the Right Thing and don't mislead and
>> distract them.
Do we have reason to believe that users are being "mislead and
distracted" by diff's current behavior? I'll admit there is plenty to
be tripped up by with merge tracking, but I'm not sure that diff makes
the top 10.
>> That's where this comes in: I do a simple little merge
>> and run "svn diff" to check what's happened in the WC and suddenly it
>> says loads of stuff has been "merged", not the simple little merge that
>> I expected.
>
> I do not think I agree with you on this.
>
> As you say, diff should show what has happened in the WC. I do not
> see why it should hide changes. If merge brought in legitimate
> changes to the svn:mergeinfo property. diff is supposed to show the
> changes, and those are changes.
>
> If we want a WC to be more specifically aware that is has been
> modified by a merge, then maybe we should consider that information in
> the output of some of our other commands (status, info) ? Maybe we
> need a new command or maybe mergeinfo should grow a new option to show
> what revisions were merged into the WC?
Or maybe diff should get a new option? --consider-mergeinfo?
Assuming we want a way to provide this info, I prefer the idea of
leaving diff as-is and using a new subcommand or option to provide it.
> It seems like diff is doing what is should.
>
> --
> Thanks
>
> Mark Phippard
> http://markphip.blogspot.com/
>
Re: Diff shows added svn:mergeinfo prop as lots of new merges
Posted by Mark Phippard <ma...@gmail.com>.
On Thu, Sep 8, 2011 at 4:39 PM, Julian Foad <ju...@wandisco.com> wrote:
> On Thu, 2011-09-08 at 12:37 -0400, Paul Burba wrote:
>> On Thu, Sep 8, 2011 at 12:08 PM, Julian Foad <ju...@wandisco.com> wrote:
>> > Can I file an issue for this?
>>
>> Hi Julian,
>>
>> What problem(s) is the current behavior causing? I understand your
>> point, but I hesitate to add merge tracking awareness to diff unless
>> there is some benefit.
>
> I'm currently looking at merging from a high-level POV, looking at what
> clues and information we give the users about what they're doing, that
> hopefully guide them in doing the Right Thing and don't mislead and
> distract them. That's where this comes in: I do a simple little merge
> and run "svn diff" to check what's happened in the WC and suddenly it
> says loads of stuff has been "merged", not the simple little merge that
> I expected.
I do not think I agree with you on this.
As you say, diff should show what has happened in the WC. I do not
see why it should hide changes. If merge brought in legitimate
changes to the svn:mergeinfo property. diff is supposed to show the
changes, and those are changes.
If we want a WC to be more specifically aware that is has been
modified by a merge, then maybe we should consider that information in
the output of some of our other commands (status, info) ? Maybe we
need a new command or maybe mergeinfo should grow a new option to show
what revisions were merged into the WC?
It seems like diff is doing what is should.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
Re: Diff shows added svn:mergeinfo prop as lots of new merges
Posted by Paul Burba <pt...@gmail.com>.
On Mon, Sep 12, 2011 at 8:47 AM, Julian Foad <ju...@wandisco.com> wrote:
> Hi Paul.
>
> Hmm, it's not quite as simple a matter as I first thought.
>
> On Thu, 2011-09-08, Julian Foad wrote:
>> So in "diff" we have a choice. Do we tell the user the physical
>> difference of a particular mergeinfo property, or do we interpret and
>> display what it means? It appears from the wording "Merged" that we are
>> attempting to display the meaning. If so, we're doing it wrong in the
>> cases where the property is added or removed.
>
> In general, the set of paths whose interpreted mergeinfo (that is, the
> logical meaning of the mergeinfo) has changed is not the same set of
> paths that have a physical change of the svn:mergeinfo property. So if
> we're intending to interpret and display the meaning of the mergeinfo
> (change), we can't do this entirely within the path-by-path framework of
> the standard "diff" mechanism.
I didn't fully appreciate that either. If diff is going to take
inherited mergeinfo into account (which in a nutshell is really what
you are proposing...I think) then *every* child path under an
(inheritable) explicit mergeinfo change should show a diff right?
E.g. We merge a change into the 1.6.x backport branch:
C:\SVN\src-branch-1.6.x>svn merge ^^/subversion/trunk . -c996383
--- Merging r996383 into '.':
U subversion\tests\cmdline\resolve_tests.py
U subversion\svn\conflict-callbacks.c
--- Recording mergeinfo for merge of r996383 into '.':
U .
The 'subversion' subtree has no explicit mergeinfo:
C:\SVN\src-branch-1.6.x>svn pg svn:mergeinfo subversion
But what would you expect a non-recursive diff of that subtree to show:
C:\SVN\src-branch-1.6.x>svn diff subversion -N
?????
Obviously it's inherited mergeinfo has changed, so would we show that?
Paul
> I can envisage something like the following style of output being
> helpful, where we omit details of 'svn:mergeinfo' changes from the main
> part of the diff, and instead describe all the mergeinfo changes at the
> end of the diff:
>
> [[[
> $ svn diff INSTALL
> Index: INSTALL
> ===================================================================
> --- INSTALL (revision 1166027)
> +++ INSTALL (working copy)
> [### text diff here if applicable]
>
> Property changes on: INSTALL
> ___________________________________________________________________
> Added: svn:mergeinfo
> [### but don't show any mergeinfo details here]
>
> Mergeinfo differences
> =====================
> Merged to 'INSTALL':
> Merged /subversion/branches/1.6.x/INSTALL:r1000000
> ]]]
>
>
> I haven't thought quite what the format of the "mergeinfo differences"
> output would be, but in principle if I could implement (cleanly)
> something like that, would that make sense?
>
> The intention I'm trying to convey here is that we describe all the
> mergeinfo changes that are relevant to the diffed tree, in terms of
> their logical effect.
>
> This may sound awkward in low-level terms of how to gather this info
> from properties, some of which may be in parent directories in the WC,
> and some of which may be not in the WC at all but only available in the
> repo. But from a user's point of view, if we can't answer the question
> "what have we just merged?" in an understandable way, that would seem
> ridiculous.
>
> To be clear, this is just one issue that I noticed while thinking about
> what the user really wants to see form "svn mergeinfo", "svn diff", etc.
> It's not the most important, and I'm not at all expecting you to jump in
> and "fix" it if we agree it needs fixing. It's just one piece of the
> puzzle that happened to jump out at me.
>
> - Julian
>
>
>> If, instead, we simply want to show a diff of the mergeinfo property
>> itself rather than trying to interpret what it means, the current
>> behaviour would not be surprising. (Nor would it be particularly
>> useful; the raw change of mergeinfo in a particular prop is the sort of
>> thing the user generally doesn't want to know about, beyond the fact
>> that there is some change that they need to commit.) But then we should
>> not say "Merged" but rather "mergeinfo diff" or something.
>>
>> I think the interpreted output is useful.
>>
>> I share your hesitation about "add merge tracking awareness to diff" but
>> there definitely *is* a benefit in showing the user what's logically
>> changed.
>
>
>
>> > > On Thu, 2011-09-08 at 16:59 +0100, Julian Foad wrote:
>> > >> If Subversion creates subtree mergeinfo on a path that previously didn't
>> > >> have any, then "svn diff" incorrectly shows all the source revs in that
>> > >> mergeinfo as newly "merged".
> [...]
>> > >> The bug is that "svn diff" shows a mergeinfo diff assuming that the
>> > >> previous merginfo was an explicit empty set of mergeinfo, but it wasn't,
>> > >> it was inherited mergeinfo.
>
>
>
Re: Diff shows added svn:mergeinfo prop as lots of new merges
Posted by Julian Foad <ju...@wandisco.com>.
Hi Paul.
Hmm, it's not quite as simple a matter as I first thought.
On Thu, 2011-09-08, Julian Foad wrote:
> So in "diff" we have a choice. Do we tell the user the physical
> difference of a particular mergeinfo property, or do we interpret and
> display what it means? It appears from the wording "Merged" that we are
> attempting to display the meaning. If so, we're doing it wrong in the
> cases where the property is added or removed.
In general, the set of paths whose interpreted mergeinfo (that is, the
logical meaning of the mergeinfo) has changed is not the same set of
paths that have a physical change of the svn:mergeinfo property. So if
we're intending to interpret and display the meaning of the mergeinfo
(change), we can't do this entirely within the path-by-path framework of
the standard "diff" mechanism.
I can envisage something like the following style of output being
helpful, where we omit details of 'svn:mergeinfo' changes from the main
part of the diff, and instead describe all the mergeinfo changes at the
end of the diff:
[[[
$ svn diff INSTALL
Index: INSTALL
===================================================================
--- INSTALL (revision 1166027)
+++ INSTALL (working copy)
[### text diff here if applicable]
Property changes on: INSTALL
___________________________________________________________________
Added: svn:mergeinfo
[### but don't show any mergeinfo details here]
Mergeinfo differences
=====================
Merged to 'INSTALL':
Merged /subversion/branches/1.6.x/INSTALL:r1000000
]]]
I haven't thought quite what the format of the "mergeinfo differences"
output would be, but in principle if I could implement (cleanly)
something like that, would that make sense?
The intention I'm trying to convey here is that we describe all the
mergeinfo changes that are relevant to the diffed tree, in terms of
their logical effect.
This may sound awkward in low-level terms of how to gather this info
from properties, some of which may be in parent directories in the WC,
and some of which may be not in the WC at all but only available in the
repo. But from a user's point of view, if we can't answer the question
"what have we just merged?" in an understandable way, that would seem
ridiculous.
To be clear, this is just one issue that I noticed while thinking about
what the user really wants to see form "svn mergeinfo", "svn diff", etc.
It's not the most important, and I'm not at all expecting you to jump in
and "fix" it if we agree it needs fixing. It's just one piece of the
puzzle that happened to jump out at me.
- Julian
> If, instead, we simply want to show a diff of the mergeinfo property
> itself rather than trying to interpret what it means, the current
> behaviour would not be surprising. (Nor would it be particularly
> useful; the raw change of mergeinfo in a particular prop is the sort of
> thing the user generally doesn't want to know about, beyond the fact
> that there is some change that they need to commit.) But then we should
> not say "Merged" but rather "mergeinfo diff" or something.
>
> I think the interpreted output is useful.
>
> I share your hesitation about "add merge tracking awareness to diff" but
> there definitely *is* a benefit in showing the user what's logically
> changed.
> > > On Thu, 2011-09-08 at 16:59 +0100, Julian Foad wrote:
> > >> If Subversion creates subtree mergeinfo on a path that previously didn't
> > >> have any, then "svn diff" incorrectly shows all the source revs in that
> > >> mergeinfo as newly "merged".
[...]
> > >> The bug is that "svn diff" shows a mergeinfo diff assuming that the
> > >> previous merginfo was an explicit empty set of mergeinfo, but it wasn't,
> > >> it was inherited mergeinfo.
Re: Diff shows added svn:mergeinfo prop as lots of new merges
Posted by Julian Foad <ju...@wandisco.com>.
On Thu, 2011-09-08 at 12:37 -0400, Paul Burba wrote:
> On Thu, Sep 8, 2011 at 12:08 PM, Julian Foad <ju...@wandisco.com> wrote:
> > Can I file an issue for this?
>
> Hi Julian,
>
> What problem(s) is the current behavior causing? I understand your
> point, but I hesitate to add merge tracking awareness to diff unless
> there is some benefit.
I'm currently looking at merging from a high-level POV, looking at what
clues and information we give the users about what they're doing, that
hopefully guide them in doing the Right Thing and don't mislead and
distract them. That's where this comes in: I do a simple little merge
and run "svn diff" to check what's happened in the WC and suddenly it
says loads of stuff has been "merged", not the simple little merge that
I expected.
> > Philip said the server makes (or used to make?) the same mistake
> > internally when processing mergeinfo - presumably in the guts of the
> > ra_get_mergeinfo2 API?
> >
> > I assume the deletion (elision)
>
> Minor semantic nitpick, elision isn't synonymous with mergeinfo
> deletion. A svn:mergeinfo property might be deleted outside of merge
> tracking (e.g. svn pd).
Ack.
> Which makes me wonder: The current behavior is arguably wrong *if* the
> mergeinfo change in question was made by a merge tracking aware merge.
> But what if we simply delete a svn:mergeinfo property with 'svn pd'?
> Shouldn't the diff show that all the mergeinfo was removed in that
> case? Of course there is currently no fool-proof way to differentiate
> between a "real" mergetracking merge and manual edits of mergeinfo.
> Or do we just assume that all mergeinfo changes originate in merge
> tracking aware merges?
We have to treat mergeinfo as describing merges. Even if it was a
manual edit. There's no mileage in attempting to distinguish "real"
from "fake" merges; rather, the definition of a "merge" in terms of
tracking has to be "what a mergeinfo diff says". If the user's intended
text edits accompany that mergeinfo change, that's well and good, the
correct text change will be associated with the logical "merge" that's
recorded; if no text edits or the wrong ones accompany the logical
"merge" as described by the mergeinfo, then the user has partially lost
track of exactly when the merge happened and may have difficulties
selecting the right revisions to cherry-pick etc. That's just tough on
the user, we have no better way (short of dump+load) to do manual
mergeinfo edits.
So in "diff" we have a choice. Do we tell the user the physical
difference of a particular mergeinfo property, or do we interpret and
display what it means? It appears from the wording "Merged" that we are
attempting to display the meaning. If so, we're doing it wrong in the
cases where the property is added or removed.
If, instead, we simply want to show a diff of the mergeinfo property
itself rather than trying to interpret what it means, the current
behaviour would not be surprising. (Nor would it be particularly
useful; the raw change of mergeinfo in a particular prop is the sort of
thing the user generally doesn't want to know about, beyond the fact
that there is some change that they need to commit.) But then we should
not say "Merged" but rather "mergeinfo diff" or something.
I think the interpreted output is useful.
I share your hesitation about "add merge tracking awareness to diff" but
there definitely *is* a benefit in showing the user what's logically
changed.
- Julian
> > of a mergeinfo prop would generate an
> > all-revs-unmerged diff output, but haven't tested that.
>
> It does.
>
> Paul
>
> > - Julian
> >
> >
> > On Thu, 2011-09-08 at 16:59 +0100, Julian Foad wrote:
> >> If Subversion creates subtree mergeinfo on a path that previously didn't
> >> have any, then "svn diff" incorrectly shows all the source revs in that
> >> mergeinfo as newly "merged".
> >>
> >> In a Subversion trunk WC, using 1.7.0-rc2:
> >>
> >> $ svn merge -c 1000000 ^/subversion/branches/1.6.x/INSTALL INSTALL
> >> --- Merging r1000000 into 'INSTALL':
> >> U INSTALL
> >> --- Recording mergeinfo for merge of r1000000 into 'INSTALL':
> >> G INSTALL
> >>
> >> [Note that r1000000 is a no-op on trunk, so in fact no content change
> >> was made despite the 'U' marker.]
> >>
> >> $ svn diff INSTALL
> >> Index: INSTALL
> >> ===================================================================
> >> --- INSTALL (revision 1166027)
> >> +++ INSTALL (working copy)
> >>
> >> Property changes on: INSTALL
> >> ___________________________________________________________________
> >> Added: svn:mergeinfo
> >> Merged /subversion/branches/atomic-revprop/INSTALL:r965046-1000689
> >> Merged /subversion/branches/diff-callbacks3/INSTALL:r870059-870761
> >> Merged /subversion/branches/bdb-reverse-deltas/INSTALL:r872050-872529
> >> Merged /subversion/branches/double-delete/INSTALL:r870511-872970
> >> [...]
> >>
> >> This is wrong. The property certainly was added, but that does not mean
> >> all those revisions were merged. The expected output is something like:
> >>
> >> [...]
> >> Added: svn:mergeinfo
> >> Merged /subversion/branches/1.6.x/INSTALL:r1000000
> >>
> >>
> >> The bug is that "svn diff" shows a mergeinfo diff assuming that the
> >> previous merginfo was an explicit empty set of mergeinfo, but it wasn't,
> >> it was inherited mergeinfo.
> >>
> >> - Julian
> >>
> >>
> >
> >
> >
Re: Diff shows added svn:mergeinfo prop as lots of new merges
Posted by Paul Burba <pt...@gmail.com>.
On Thu, Sep 8, 2011 at 12:08 PM, Julian Foad <ju...@wandisco.com> wrote:
> Can I file an issue for this?
Hi Julian,
What problem(s) is the current behavior causing? I understand your
point, but I hesitate to add merge tracking awareness to diff unless
there is some benefit.
> Philip said the server makes (or used to make?) the same mistake
> internally when processing mergeinfo - presumably in the guts of the
> ra_get_mergeinfo2 API?
>
> I assume the deletion (elision)
Minor semantic nitpick, elision isn't synonymous with mergeinfo
deletion. A svn:mergeinfo property might be deleted outside of merge
tracking (e.g. svn pd).
Which makes me wonder: The current behavior is arguably wrong *if* the
mergeinfo change in question was made by a merge tracking aware merge.
But what if we simply delete a svn:mergeinfo property with 'svn pd'?
Shouldn't the diff show that all the mergeinfo was removed in that
case? Of course there is currently no fool-proof way to differentiate
between a "real" mergetracking merge and manual edits of mergeinfo.
Or do we just assume that all mergeinfo changes originate in merge
tracking aware merges?
> of a mergeinfo prop would generate an
> all-revs-unmerged diff output, but haven't tested that.
It does.
Paul
> - Julian
>
>
> On Thu, 2011-09-08 at 16:59 +0100, Julian Foad wrote:
>> If Subversion creates subtree mergeinfo on a path that previously didn't
>> have any, then "svn diff" incorrectly shows all the source revs in that
>> mergeinfo as newly "merged".
>>
>> In a Subversion trunk WC, using 1.7.0-rc2:
>>
>> $ svn merge -c 1000000 ^/subversion/branches/1.6.x/INSTALL INSTALL
>> --- Merging r1000000 into 'INSTALL':
>> U INSTALL
>> --- Recording mergeinfo for merge of r1000000 into 'INSTALL':
>> G INSTALL
>>
>> [Note that r1000000 is a no-op on trunk, so in fact no content change
>> was made despite the 'U' marker.]
>>
>> $ svn diff INSTALL
>> Index: INSTALL
>> ===================================================================
>> --- INSTALL (revision 1166027)
>> +++ INSTALL (working copy)
>>
>> Property changes on: INSTALL
>> ___________________________________________________________________
>> Added: svn:mergeinfo
>> Merged /subversion/branches/atomic-revprop/INSTALL:r965046-1000689
>> Merged /subversion/branches/diff-callbacks3/INSTALL:r870059-870761
>> Merged /subversion/branches/bdb-reverse-deltas/INSTALL:r872050-872529
>> Merged /subversion/branches/double-delete/INSTALL:r870511-872970
>> [...]
>>
>> This is wrong. The property certainly was added, but that does not mean
>> all those revisions were merged. The expected output is something like:
>>
>> [...]
>> Added: svn:mergeinfo
>> Merged /subversion/branches/1.6.x/INSTALL:r1000000
>>
>>
>> The bug is that "svn diff" shows a mergeinfo diff assuming that the
>> previous merginfo was an explicit empty set of mergeinfo, but it wasn't,
>> it was inherited mergeinfo.
>>
>> - Julian
>>
>>
>
>
>
Re: Diff shows added svn:mergeinfo prop as lots of new merges
Posted by Julian Foad <ju...@wandisco.com>.
Can I file an issue for this?
Philip said the server makes (or used to make?) the same mistake
internally when processing mergeinfo - presumably in the guts of the
ra_get_mergeinfo2 API?
I assume the deletion (elision) of a mergeinfo prop would generate an
all-revs-unmerged diff output, but haven't tested that.
- Julian
On Thu, 2011-09-08 at 16:59 +0100, Julian Foad wrote:
> If Subversion creates subtree mergeinfo on a path that previously didn't
> have any, then "svn diff" incorrectly shows all the source revs in that
> mergeinfo as newly "merged".
>
> In a Subversion trunk WC, using 1.7.0-rc2:
>
> $ svn merge -c 1000000 ^/subversion/branches/1.6.x/INSTALL INSTALL
> --- Merging r1000000 into 'INSTALL':
> U INSTALL
> --- Recording mergeinfo for merge of r1000000 into 'INSTALL':
> G INSTALL
>
> [Note that r1000000 is a no-op on trunk, so in fact no content change
> was made despite the 'U' marker.]
>
> $ svn diff INSTALL
> Index: INSTALL
> ===================================================================
> --- INSTALL (revision 1166027)
> +++ INSTALL (working copy)
>
> Property changes on: INSTALL
> ___________________________________________________________________
> Added: svn:mergeinfo
> Merged /subversion/branches/atomic-revprop/INSTALL:r965046-1000689
> Merged /subversion/branches/diff-callbacks3/INSTALL:r870059-870761
> Merged /subversion/branches/bdb-reverse-deltas/INSTALL:r872050-872529
> Merged /subversion/branches/double-delete/INSTALL:r870511-872970
> [...]
>
> This is wrong. The property certainly was added, but that does not mean
> all those revisions were merged. The expected output is something like:
>
> [...]
> Added: svn:mergeinfo
> Merged /subversion/branches/1.6.x/INSTALL:r1000000
>
>
> The bug is that "svn diff" shows a mergeinfo diff assuming that the
> previous merginfo was an explicit empty set of mergeinfo, but it wasn't,
> it was inherited mergeinfo.
>
> - Julian
>
>