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/10/19 17:39:40 UTC
Re: [RFC/PATCH] Allow merge to apply only svn:mergeinfo diffs
On Mon, Sep 28, 2009 at 3:32 PM, Mark Phippard <ma...@gmail.com> wrote:
> On Mon, Sep 28, 2009 at 2:16 PM, Paul Burba <pt...@gmail.com> wrote:
>
>> 1) Any problem with implementing something like this?
>>
>> 2) If the general idea is ok:
>>
>> 2a) Should any notifications be shown? The existing patch shows
>> notifications for paths with mergeinfo changes, but I'm not sure how
>> useful these are. Perhaps, like --record-only, there should be no
>> notifications?
>
> I like seeing notifications. I do not see what it hurts and it can
> help scripts and tools. In Subclipse, as an example, the
> notifications lets us know what paths have been changed so that we can
> refresh their status.
>
>> 2b) Should this be a new option to svn merge as in the patch? Or
>> should it be the default behavior of --record-only? If the latter
>> then we need to consider that this will slow --record-only merges down
>> compared with today's simple-no-editor-drive approach. Though I don't
>> think the performance hit will be too severe, since a lot of the
>> performance problems related to merge (Issue #3443, implicit mergeinfo
>> lookups (r36509), etc.) are fixed on trunk or don't apply here (e.g.
>> multiple editor drives during a singe merge). Or maybe there is some
>> other solution, --record-only = ARG, where ARG is two choices, one
>> representing the old simple way, one the new restricted editor drive.
>>
>> Any thoughts are appreciated, thanks,
>
> It is probably opening a can of worms we are not ready to deal with,
> but what about using --block ? As your own description explains, that
> is exactly the kind of scenario where this is needed.
>
> The big problem I see ( can of worms ) is that eventually people want
> to see what they have blocked and so this would not be enough.
Mark and I talked about this recently and came to the conclusion that
making the proposed behavior the *default* for --record-only is
probably the best approach. Merge tracking is complicated enough
without introducing new options and, as described above, the
performance price for this change should be reasonable.
The attached patch makes --record-only apply only svn:mergeinfo diffs
by default. It shows notifications for any path updated (unlike
--record-only's current behavior which never shows any notifications).
It also adds a new merge test to demonstrate the new functionality.
Any thoughts, questions, and/or objections to this change let me know.
Barring any objections I'll be committing this sometime this week.
Paul
[[[
Let --record-only merges apply svn:mergeinfo diffs.
See http://svn.haxx.se/dev/archive-2009-09/0520.shtml for discussion.
* subversion/libsvn_client/merge.c
(merge_props_changed): Only allow svn:mergeinfo prop changes to be changed.
(merge_file_changed,
merge_file_added,
merge_file_deleted,
merge_dir_added,
merge_dir_deleted):
Exit early if this is a --record-only merge.
(notification_receiver): If performing a --record-only merge, limit
notifications to mergeinfo changes.
(do_directory_merge): Actually drive the editor for --record-only merges.
* subversion/tests/cmdline/merge_tests.py
(record_only_merge): New test.
(test_list): Add record_only_merge.
]]]
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2409057
Re: [RFC/PATCH] Allow merge to apply only svn:mergeinfo diffs
Posted by Mark Phippard <ma...@gmail.com>.
On Tue, Oct 27, 2009 at 9:37 AM, Paul Burba <pt...@gmail.com> wrote:
> On Tue, Oct 27, 2009 at 8:25 AM, Mark Phippard <ma...@gmail.com> wrote:
>> On Thu, Oct 22, 2009 at 1:17 PM, Paul Burba <pt...@gmail.com> wrote:
>>
>>> Committed r40190. Only differences between that and the previously
>>> posted patch are new help text on the --record-only option and a doc
>>> string tweak for svn_client_merge3().
>>
>> Something I just thought of ... did you take care not to commit
>> self-referential mergeinfo when applying these diffs? A common
>> use-case for --record-only is to use it after a reintegrate merge to
>> record that revision on the source branch so that you do not have to
>> delete it.
>
> This should not be a problem. Since we actually are doing an editor
> drive, all the code that normally prevents self-referential mergeinfo
> during a "normal" merge is still used. For example (using
> trunk@40230):
Great. I figured you'd have taken care of it. Just wanted to be sure.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2411671
Re: [RFC/PATCH] Allow merge to apply only svn:mergeinfo diffs
Posted by Paul Burba <pt...@gmail.com>.
On Tue, Oct 27, 2009 at 8:25 AM, Mark Phippard <ma...@gmail.com> wrote:
> On Thu, Oct 22, 2009 at 1:17 PM, Paul Burba <pt...@gmail.com> wrote:
>
>> Committed r40190. Only differences between that and the previously
>> posted patch are new help text on the --record-only option and a doc
>> string tweak for svn_client_merge3().
>
> Something I just thought of ... did you take care not to commit
> self-referential mergeinfo when applying these diffs? A common
> use-case for --record-only is to use it after a reintegrate merge to
> record that revision on the source branch so that you do not have to
> delete it.
Mark,
This should not be a problem. Since we actually are doing an editor
drive, all the code that normally prevents self-referential mergeinfo
during a "normal" merge is still used. For example (using
trunk@40230):
# Make a change on a "branch"
>echo a change on a branch > A_COPY\mu
>svn ci -m ""
Sending A_COPY\mu
Transmitting file data .
Committed revision 8.
>svn up
At revision 8.
# Synch that "branch" with "trunk"
>svn merge ^^/A A_COPY --accept theirs-conflict
--- Merging r2 through r8 into 'A_COPY':
U A_COPY\B\E\beta
U A_COPY\D\G\rho
U A_COPY\D\H\omega
U A_COPY\D\H\psi
>svn pl -vR
Properties on 'A_COPY':
svn:mergeinfo
/A:2-8
>svn ci -m ""
Sending A_COPY
Sending A_COPY\B\E\beta
Sending A_COPY\D\G\rho
Sending A_COPY\D\H\omega
Sending A_COPY\D\H\psi
Transmitting file data ....
Committed revision 9.
>svn up
At revision 9.
# Reintegrate the "branch" back to "trunk"
>svn merge ^^/A_COPY A --reintegrate
--- Merging differences between repository URLs into 'A':
U A\mu
>svn ci -m ""
Sending A
Sending A\mu
Transmitting file data .
Committed revision 10.
>svn up
At revision 10.
# Now mark r10 as already merged from "trunk" to "branch"
# so we can continue merging in that direction without recreating
# "branch".
#
# r10 includes the self-referential mergeinfo '/A_COPY:r2-9' which
# should *not* be recorded...
>svn diff -r9:10 --depth empty ^^/A
Property changes on: .
___________________________________________________________________
Added: svn:mergeinfo
Merged /A_COPY:r2-9
# ...and is not:
>svn merge ^^/A A_COPY --record-only -c10
>svn diff
Property changes on: A_COPY
___________________________________________________________________
Modified: svn:mergeinfo
Merged /A:r10
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2411670
Re: [RFC/PATCH] Allow merge to apply only svn:mergeinfo diffs
Posted by Mark Phippard <ma...@gmail.com>.
On Thu, Oct 22, 2009 at 1:17 PM, Paul Burba <pt...@gmail.com> wrote:
> Committed r40190. Only differences between that and the previously
> posted patch are new help text on the --record-only option and a doc
> string tweak for svn_client_merge3().
Something I just thought of ... did you take care not to commit
self-referential mergeinfo when applying these diffs? A common
use-case for --record-only is to use it after a reintegrate merge to
record that revision on the source branch so that you do not have to
delete it.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2411656
Re: [RFC/PATCH] Allow merge to apply only svn:mergeinfo diffs
Posted by Paul Burba <pt...@gmail.com>.
On Mon, Oct 19, 2009 at 1:39 PM, Paul Burba <pt...@gmail.com> wrote:
> On Mon, Sep 28, 2009 at 3:32 PM, Mark Phippard <ma...@gmail.com> wrote:
>> On Mon, Sep 28, 2009 at 2:16 PM, Paul Burba <pt...@gmail.com> wrote:
>>
>>> 1) Any problem with implementing something like this?
>>>
>>> 2) If the general idea is ok:
>>>
>>> 2a) Should any notifications be shown? The existing patch shows
>>> notifications for paths with mergeinfo changes, but I'm not sure how
>>> useful these are. Perhaps, like --record-only, there should be no
>>> notifications?
>>
>> I like seeing notifications. I do not see what it hurts and it can
>> help scripts and tools. In Subclipse, as an example, the
>> notifications lets us know what paths have been changed so that we can
>> refresh their status.
>>
>>> 2b) Should this be a new option to svn merge as in the patch? Or
>>> should it be the default behavior of --record-only? If the latter
>>> then we need to consider that this will slow --record-only merges down
>>> compared with today's simple-no-editor-drive approach. Though I don't
>>> think the performance hit will be too severe, since a lot of the
>>> performance problems related to merge (Issue #3443, implicit mergeinfo
>>> lookups (r36509), etc.) are fixed on trunk or don't apply here (e.g.
>>> multiple editor drives during a singe merge). Or maybe there is some
>>> other solution, --record-only = ARG, where ARG is two choices, one
>>> representing the old simple way, one the new restricted editor drive.
>>>
>>> Any thoughts are appreciated, thanks,
>>
>> It is probably opening a can of worms we are not ready to deal with,
>> but what about using --block ? As your own description explains, that
>> is exactly the kind of scenario where this is needed.
>>
>> The big problem I see ( can of worms ) is that eventually people want
>> to see what they have blocked and so this would not be enough.
>
> Mark and I talked about this recently and came to the conclusion that
> making the proposed behavior the *default* for --record-only is
> probably the best approach. Merge tracking is complicated enough
> without introducing new options and, as described above, the
> performance price for this change should be reasonable.
>
> The attached patch makes --record-only apply only svn:mergeinfo diffs
> by default. It shows notifications for any path updated (unlike
> --record-only's current behavior which never shows any notifications).
> It also adds a new merge test to demonstrate the new functionality.
>
> Any thoughts, questions, and/or objections to this change let me know.
> Barring any objections I'll be committing this sometime this week.
Committed r40190. Only differences between that and the previously
posted patch are new help text on the --record-only option and a doc
string tweak for svn_client_merge3().
> Paul
>
> [[[
> Let --record-only merges apply svn:mergeinfo diffs.
>
> See http://svn.haxx.se/dev/archive-2009-09/0520.shtml for discussion.
>
> * subversion/libsvn_client/merge.c
> (merge_props_changed): Only allow svn:mergeinfo prop changes to be changed.
>
> (merge_file_changed,
> merge_file_added,
> merge_file_deleted,
> merge_dir_added,
> merge_dir_deleted):
> Exit early if this is a --record-only merge.
>
> (notification_receiver): If performing a --record-only merge, limit
> notifications to mergeinfo changes.
>
> (do_directory_merge): Actually drive the editor for --record-only merges.
>
> * subversion/tests/cmdline/merge_tests.py
> (record_only_merge): New test.
>
> (test_list): Add record_only_merge.
> ]]]
>
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2410319