On Thu, Jun 5, 2008 at 2:29 PM, Paul Burba <ptburba_at_gmail.com> wrote:
> On Fri, May 9, 2008 at 12:02 PM, Kamesh Jayachandran <kamesh_at_collab.net> wrote:
>> Hi All,
>>
>> I hereby attach the fix to issue 3067.
>>
>> I am bit skeptical about performance of 'guess_live_ranges' in this patch.
>> But it would perform poorly only for the situations when
>> merge_src_subtree_at_revision2 does not exist and that too for one invocation.
>>
>> Would like to know what others would think about it.
>>
>> Thanks
>> With regards
>> Kamesh Jayachandran
>
> Hi Kamesh,
>
> Sorry for the long delay, while working on this I've found other
> related problems (issue #3199 and #3157) that needed to be addressed
> first. But *finally* I reworked our patch, using a simpler approach
> that avoids the round trip overhead of guess_live_ranges() entirely.
> But first I should mention a few things:
>
> 1) I added/expanded a couple of merge tests in r31575 to cover a wider
> range of scenarios where the subtree of a target needs different
> ranges applied than the target. Our existing tests were lacking in
> this area.
>
> 2) In r31594 and r31601 I made some improvements that solve some of
> the problems in 1). The core problem of 3067, where we describe
> describe invalid subtrees to the merge report editor, still remains
> and this attached patch addresses that.
>
> Ok then, there are two key parts to this patch:
>
> The first is the new helper for filter_merged_revisions() called
> prepare_subtree_ranges(). See the doc string for
> prepare_subtree_ranges for an explanation of what I'm trying to do.
> Please give some thought to the question posed in the section titled
> '### This is tricky, sort of the inverse of B' as that is the one
> thing about this patch I'm not entirely confident in.
>
> The second is in filter_merged_revisions(), see the code in the block
> 'A little trick: If CHILD is a subtree which will be deleted by'.
>
> Let me know what you (or anyone else interested in looking at this) think.
Hi Kamesh,
As discussed today in IRC, here is my latest version of the patch
which now addresses the problems described in
http://subversion.tigris.org/issues/show_bug.cgi?id=3067#desc34.
Everything I said regarding the previous patch still holds (including
the key questions). The changes are in two areas:
1) Handle a merge range that predates the subtree's existence under
it's current name - this addresses the problem 'MERGE FAILS' in
3067#desc34. This is just a subset of the problem 'B) Part of
requested range predates subtree's existance.' in
merge.c:prepare_subtree_ranges(). And yes, one of your earlier
patches had logic similar to this...not quite sure why I ever removed
it :-\
2) No more preemptive elision of empty mergeinfo in
merge.c:get_mergeinfo_walk_cb() - this addresses the problem
'INCORRECT MERGEINFO SET' in 3067#desc34. When we are happy with this
patch we can probably commit this fix separately.
Paul
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-06-20 22:02:27 CEST