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 <pt...@gmail.com> on 2009/12/09 21:40:27 UTC
[RFC] Add notifications when mergeinfo is set to describe a merge
The svn merge subcommand can create local svn:mergeinfo modifications
in two basic ways:
1) The application of a diff -- If the merge sources have differences
in the svn:mergeinfo property, then they are applied just like any
other versioned property.
2) Describing the merge performed -- A merge tracking aware merge
records mergeinfo describing the merged source(s) and revision(s).
E.g. 'svn merge ^/trunk branch-wc -r500:588' sets the mergeinfo
'/trunk:501-588' on branch-wc*
Changes of the first type result in notifications, as they always
have, even prior to merge tracking.
Changes of the second type produce no notifications. As Philip and I
discussed here,
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2425507,
this includes --record-only merges. In that thread Philip suggested
that --record-only merges should produce notifications for the "type
2" changes, since --record-only merges typically produce no output at
all. I've come around to agree with this position, but I think it
should be expanded to "regular" (non --record-only) merges too.
There has been no shortage of confusion regarding type 2 mergeinfo
changes, especially when it comes to subtrees of the merge target.
The changes in 1.7 limiting the recording of subtree mergeinfo
(http://svn.haxx.se/dev/archive-2009-08/0045.shtml) should alleviate
much of this confusion, but I think producing notifications for type 2
changes in all cases will help more.
Here is an example to demonstrate what I have in mind:
# We have a working copy with no mergeinfo on the
# root, but some subtree mergeinfo from earlier
# subtree merges:
>svn pg svn:mergeinfo -vR
Properties on 'build':
svn:mergeinfo
/trunk/build:35021
Properties on 'subversion':
svn:mergeinfo
/trunk/subversion:35000-35020
Properties on 'subversion\tests':
svn:mergeinfo
/trunk/subversion/tests:35000-35020,35022
# We merge a single revision which affects a
# single file:
>svn log -qv -r35049 ^^/
------------------------------------------------------------------------
r35049 | pburba | 2009-01-06 12:11:11 -0500 (Tue, 06 Jan 2009)
Changed paths:
M /trunk/subversion/tests/cmdline/svntest/main.py
------------------------------------------------------------------------
# Currently all we see is the "type 1" notification
# from the application of the diff:
>svn merge ^^/trunk . -c35049
--- Merging r35049 into 'subversion\tests':
U subversion\tests\cmdline\svntest\main.py
# But status and diff show that the subtrees affected
# by the merge had their mergeinfo updated and the
# merge target itself got mergeinfo added, all of which
# describe the merge that was just done. Note that the
# mergeinfo on 'build' did not change because that
# subtree was not affected by the merge (new in 1.7):
>svn st
M .
M subversion
M subversion\tests
M subversion\tests\cmdline\svntest\main.py
>svn diff . subversion subversion\tests --depth empty
Property changes on: .
___________________________________________________________________
Added: svn:mergeinfo
Merged /trunk:r35049
Property changes on: subversion
___________________________________________________________________
Modified: svn:mergeinfo
Merged /trunk/subversion:r35049
Property changes on: subversion\tests
___________________________________________________________________
Modified: svn:mergeinfo
Merged /trunk/subversion/tests:r35049
# Instead of silence on the type 2 mergeinfo changes,
# what I am proposing is something like this:
>svn merge ^^/trunk . -c35049
--- Merging r35049 into 'subversion\tests':
U subversion\tests\cmdline\svntest\main.py
--- Recording mergeinfo for merge of r35049 into '.':
A .
G subversion
G subversion\tests
Specifically, the special header (for a single rev)
--- Recording mergeinfo for merge of rX into 'path':
or (for multiple revisions)
--- Recording mergeinfo for merge of rM through rN into 'path':
would be printed after all type 1 notifications. Then all the type 2
notifications would be grouped together.
Obviously I'm open to what language to use for the header, but do
folks like the concept? Do we think it would make clearer what is
happening during a merge or does it just add noise?
Thoughts appreciated,
Paul
* Yes, it might add/merge that mergeinfo to existing mergeinfo, there
might be subtree mergeinfo too, it might even be a foreign repos, etc.
Let's just assume this is the simple (obvious) case where branch-wc
is a copy of ^/trunk that has never been merged to and has no explicit
mergeinfo prior to the merge.
Re: [RFC] Add notifications when mergeinfo is set to describe a
merge
Posted by Stefan Sperling <st...@elego.de>.
On Sat, Jan 23, 2010 at 06:49:49PM -0500, Paul Burba wrote:
> I committed this enhancement in r902509.
It's lovely, thank you :)
Stefan
Re: [RFC] Add notifications when mergeinfo is set to describe a
merge
Posted by Julian Foad <ju...@wandisco.com>.
On Sat, 2010-01-23, Paul Burba wrote:
> I committed this enhancement in r902509.
Thanks, Paul.
> A few things to note:
>
> 1) In my example at the start of this thread I used an ' A'
> notification to denote the addition of a mergeinfo property. That
> does not agree with how we notify property additions in other cases;
> we use ' U' for the addition, deletion, or modification of a property
> and ' G' for a change to a locally modified property. My patch sticks
> to those conventions.
Good.
> 2) I added a separate header when mergeinfo is elided vs. recorded to
> describe a merge, e.g.:
>
> >svn pg svn:mergeinfo -vR A_COPY_2
> Properties on 'A_COPY_2\D':
> svn:mergeinfo
> /A/D:5
>
> >svn merge ^^/A A_COPY_2 -c5
> --- Recording mergeinfo for merge of r5 into 'A_COPY_2':
> U A_COPY_2
> --- Eliding mergeinfo from 'A_COPY_2\D':
> U A_COPY_2\D
Perhaps more detail than the user really needs to know, but OK.
> >svn diff
>
> Property changes on: A_COPY_2
> ___________________________________________________________________
> Added: svn:mergeinfo
> Merged /A:r5
>
> Property changes on: A_COPY_2\D
> ___________________________________________________________________
> Deleted: svn:mergeinfo
> Reverse-merged /A/D:r5
Ew, describing that diff as a "reverse merge" is ugly. Not related to
this patch, though.
> 3) There are still some mergeinfo updates that we don't notify. For
> example, when a subtree has non-inheritable mergeinfo, the subtree's
> immediate children without explicit mergeinfo get (possibly temporary)
> explicit mergeinfo set which reflects the inheritable portion of it's
> parent. Ultimately this is reported (or removed silently) at the end
> of the merge, so I've kept these updates quiet; notifying of these
> changes would IMHO cross the line between "informative" and "confusing
> noise".
Good. I agree.
> 4) --dry-run merges currently do *not* produce the new notifications.
> It doesn't make any sense to me that they should, but I'm open to
> arguments to the contrary.
I tend to think the opposite - that a dry run should show me exactly
what is going to happen, in as much detail and accuracy as possible, to
save me the bother of running the merge and then reverting it - but I
don't feel that strongly about it.
- Julian
Re: [RFC] Add notifications when mergeinfo is set to describe a merge
Posted by Paul Burba <pt...@gmail.com>.
On Thu, Dec 10, 2009 at 8:53 AM, Julian Foad <ju...@btopenworld.com> wrote:
> Mark Phippard wrote:
>> On Thu, Dec 10, 2009 at 5:30 AM, Julian Foad <ju...@btopenworld.com> wrote:
>> > It is worth considering printing an abbreviated notification.
>>
>> Given that Paul's proposal calls for adding a new section to the
>> notification process to show the paths that mergeinfo is applied to,
>> don't you think he accomplishes this goal? Even if the notifications
>> are perceived as overwhelming, they are still segregated to their own
>> area where you can easily ignore them.
>>
>> In addition, the trunk code no longer updates subtrees with mergeinfo
>> unless they are modified by the merge. So there really should not be
>> too much extra noise.
>
> Ah, thanks, I hadn't remembered that change, and that makes a huge
> difference. Great. It's fine to issue lots of subtree mergeinfo
> notifications if lots of files were touched by the merge.
>
> +1 to adding the notifications as proposed.
>
> - Julian
I committed this enhancement in r902509.
A few things to note:
1) In my example at the start of this thread I used an ' A'
notification to denote the addition of a mergeinfo property. That
does not agree with how we notify property additions in other cases;
we use ' U' for the addition, deletion, or modification of a property
and ' G' for a change to a locally modified property. My patch sticks
to those conventions.
2) I added a separate header when mergeinfo is elided vs. recorded to
describe a merge, e.g.:
>svn pg svn:mergeinfo -vR A_COPY_2
Properties on 'A_COPY_2\D':
svn:mergeinfo
/A/D:5
>svn merge ^^/A A_COPY_2 -c5
--- Recording mergeinfo for merge of r5 into 'A_COPY_2':
U A_COPY_2
--- Eliding mergeinfo from 'A_COPY_2\D':
U A_COPY_2\D
>svn diff
Property changes on: A_COPY_2
___________________________________________________________________
Added: svn:mergeinfo
Merged /A:r5
Property changes on: A_COPY_2\D
___________________________________________________________________
Deleted: svn:mergeinfo
Reverse-merged /A/D:r5
3) There are still some mergeinfo updates that we don't notify. For
example, when a subtree has non-inheritable mergeinfo, the subtree's
immediate children without explicit mergeinfo get (possibly temporary)
explicit mergeinfo set which reflects the inheritable portion of it's
parent. Ultimately this is reported (or removed silently) at the end
of the merge, so I've kept these updates quiet; notifying of these
changes would IMHO cross the line between "informative" and "confusing
noise".
4) --dry-run merges currently do *not* produce the new notifications.
It doesn't make any sense to me that they should, but I'm open to
arguments to the contrary.
Paul
Re: [RFC] Add notifications when mergeinfo is set to describe a
merge
Posted by Julian Foad <ju...@btopenworld.com>.
Mark Phippard wrote:
> On Thu, Dec 10, 2009 at 5:30 AM, Julian Foad <ju...@btopenworld.com> wrote:
> > It is worth considering printing an abbreviated notification.
>
> Given that Paul's proposal calls for adding a new section to the
> notification process to show the paths that mergeinfo is applied to,
> don't you think he accomplishes this goal? Even if the notifications
> are perceived as overwhelming, they are still segregated to their own
> area where you can easily ignore them.
>
> In addition, the trunk code no longer updates subtrees with mergeinfo
> unless they are modified by the merge. So there really should not be
> too much extra noise.
Ah, thanks, I hadn't remembered that change, and that makes a huge
difference. Great. It's fine to issue lots of subtree mergeinfo
notifications if lots of files were touched by the merge.
+1 to adding the notifications as proposed.
- Julian
Re: [RFC] Add notifications when mergeinfo is set to describe a merge
Posted by Mark Phippard <ma...@gmail.com>.
On Thu, Dec 10, 2009 at 5:30 AM, Julian Foad <ju...@btopenworld.com> wrote:
> Stefan Sperling wrote:
>> On Wed, Dec 09, 2009 at 04:40:27PM -0500, Paul Burba wrote:
>> > # Instead of silence on the type 2 mergeinfo changes,
>> > # what I am proposing is something like this:
>> >
>> > >svn merge ^^/trunk . -c35049
>> > --- Merging r35049 into 'subversion\tests':
>> > U subversion\tests\cmdline\svntest\main.py
>> > --- Recording mergeinfo for merge of r35049 into '.':
>> > A .
>> > G subversion
>> > G subversion\tests
>> >
>> > Specifically, the special header (for a single rev)
>> >
>> > --- Recording mergeinfo for merge of rX into 'path':
>> >
>> > or (for multiple revisions)
>> >
>> > --- Recording mergeinfo for merge of rM through rN into 'path':
>> >
>> > would be printed after all type 1 notifications. Then all the type 2
>> > notifications would be grouped together.
>>
>> I like your proposal.
>> I think it's a good idea to always tell users what is happening with
>> mergeinfo. The UI should show mergeinfo changes wherever possible.
>> It's important that users do not overlook mergeinfo changes and end
>> up not committing them.
>
> True, but also we must cautiously balance the helpfulness of
> notifications against the possibility for a mass of uninteresting
> mergeinfo notifications to overwhelm the user and make it hard to see
> the interesting changes. Some users have hundreds of nodes with subtree
> mergeinfo.
>
> It is worth considering printing an abbreviated notification.
Given that Paul's proposal calls for adding a new section to the
notification process to show the paths that mergeinfo is applied to,
don't you think he accomplishes this goal? Even if the notifications
are perceived as overwhelming, they are still segregated to their own
area where you can easily ignore them.
In addition, the trunk code no longer updates subtrees with mergeinfo
unless they are modified by the merge. So there really should not be
too much extra noise.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
Re: [RFC] Add notifications when mergeinfo is set to describe a
merge
Posted by Julian Foad <ju...@btopenworld.com>.
Stefan Sperling wrote:
> On Wed, Dec 09, 2009 at 04:40:27PM -0500, Paul Burba wrote:
> > # Instead of silence on the type 2 mergeinfo changes,
> > # what I am proposing is something like this:
> >
> > >svn merge ^^/trunk . -c35049
> > --- Merging r35049 into 'subversion\tests':
> > U subversion\tests\cmdline\svntest\main.py
> > --- Recording mergeinfo for merge of r35049 into '.':
> > A .
> > G subversion
> > G subversion\tests
> >
> > Specifically, the special header (for a single rev)
> >
> > --- Recording mergeinfo for merge of rX into 'path':
> >
> > or (for multiple revisions)
> >
> > --- Recording mergeinfo for merge of rM through rN into 'path':
> >
> > would be printed after all type 1 notifications. Then all the type 2
> > notifications would be grouped together.
>
> I like your proposal.
> I think it's a good idea to always tell users what is happening with
> mergeinfo. The UI should show mergeinfo changes wherever possible.
> It's important that users do not overlook mergeinfo changes and end
> up not committing them.
True, but also we must cautiously balance the helpfulness of
notifications against the possibility for a mass of uninteresting
mergeinfo notifications to overwhelm the user and make it hard to see
the interesting changes. Some users have hundreds of nodes with subtree
mergeinfo.
It is worth considering printing an abbreviated notification.
- Julian
Re: [RFC] Add notifications when mergeinfo is set to describe a
merge
Posted by Stefan Sperling <st...@elego.de>.
On Wed, Dec 09, 2009 at 04:40:27PM -0500, Paul Burba wrote:
> # Instead of silence on the type 2 mergeinfo changes,
> # what I am proposing is something like this:
>
> >svn merge ^^/trunk . -c35049
> --- Merging r35049 into 'subversion\tests':
> U subversion\tests\cmdline\svntest\main.py
> --- Recording mergeinfo for merge of r35049 into '.':
> A .
> G subversion
> G subversion\tests
>
> Specifically, the special header (for a single rev)
>
> --- Recording mergeinfo for merge of rX into 'path':
>
> or (for multiple revisions)
>
> --- Recording mergeinfo for merge of rM through rN into 'path':
>
> would be printed after all type 1 notifications. Then all the type 2
> notifications would be grouped together.
I like your proposal.
I think it's a good idea to always tell users what is happening with
mergeinfo. The UI should show mergeinfo changes wherever possible.
It's important that users do not overlook mergeinfo changes and end
up not committing them.
Stefan