> -----Original Message-----
> From: C. Michael Pilato [mailto:cmpilato@collab.net]
> Sent: Friday, October 26, 2007 4:28 AM
> To: Paul Burba
> Cc: Subversion Developers
> Subject: I think r27412 undid something not meant to be undone.
>
> Paul, I draw your attention to the log message of my r27412 commit:
>
> --------------------------------------------------------------
> In a nutshell, rework the changes made in r27133 by moving
> the iteration over revision ranges to just inside the
> svn_client_merge_peg3 API.
>
> The unfortunately reality is that this meant rolling this file back to
> r27132 and cherry-picking changes I wanted to re-apply from
> after that point, then reworking the basic notion of r27133.
> This is for correctness -- having the main worker functions
> accept two URLs and range of revisions makes it extremely
> different for those worker functions to deal with merges from
> sources which suffered location changes (renames) somewhere
> between the first and last revisions merged.
>
> NOTE: r27133 seems to have actually been about two different changes.
> One of them was the change of the public merge API to accept
> a revision range list instead of just a pair of revisions --
> that's what I meant to rework. The second change is
> something to do with talking about 3-way-merges instead of
> rollbacks. I don't know why the two changes had to be
> bundled. And I don't why the fact that I reverted both
> changes and then reworked only the first hasn't caused the
> test suite to fail at all. And I'm sad to say that I didn't
> even realize there was this dual change thing going on until
> I started composing this log message just prior to commit.
> Something tells me that second change will need to be remade,
> but I'm going to defer that until another commit.
>
> ...
> --------------------------------------------------------------
>
> I can't think of any reason why the API change (to support
> lists of ranges) would have itself necessitated the
> is_rollback -> is_three_way_merge change.
> Are they, in fact, distinct changes that happened to be
> committed at the same time? If so, would you a) explain to
> me the gist of the change so I can try to remake it
Mike,
These were not intended as independent changes.
In r27412 you iterate over the cherry picked ranges in
svn_client_merge_peg3(). In r27133 I pushed these iterations down into
the 'big three' do_single_file_merge(), do_merge(),
discover_and_merge_children() -- so then these three might be consuming
rangelists with some forward ranges and some rollbacks. Just passing
'is_rollback' was no longer sufficient to describe the ranges.
Some of these three's helper functions, e.g.
calculate_remaining_ranges(), started taking rangelists too or started
taking rangelists that were no longer guaranteed to be made up entirely
of forward or rollback ranges, e.g. calculate_merge_ranges().
My intent with the replacement of is_rollback with is_three_way_merge
was to communicate down to the big three and their helpers that if this
merge was not a three way merge, then each range processed would need to
be examined independently to determine if it was a rollback or not and
appropriate action taken as before.
Clearly *all* this was a needlessly complex approach (as r27412 proves).
And looking at r27133 again I see some areas where this could definitely
be simplified/made clearer. But I don't want to spend any more time on
it now since with r27412 my *entire* reason for replacing is_rollback
with is_three_way_merge is gone, since the big three are back to
handling single revision ranges only and their helpers are too or only
operate on ranges that are all in the same direction.
> b) tell
> me how to test whatever it is that this change is supposed to
> be testing, and
Any 2-way merge with cherry picked ranges which (once compacted) have
both forward and reverse ranges to be applied. merge_tests.py 71
'cherry_picking' already tests this.
> c) don't combine distinct changes like that
> in the future. :-)
There were problems in r27133, but that might be one I'm not actually
guilty of :-/
Paul
P.S. I see r27412 was committed at 4:17 AM EST. Sorry about the late
night :-( Unless you've moved to Hawaii that is, in that case...
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Oct 26 18:06:52 2007