-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Patch looks fine Paul!.
+1 to commit.
Apart from one typo(revsion -> 'revision') and one code indentation
issue(prepare_subtree_ranges one if(err) clause has subsequent lines
with 3 spaces) everything looks fine.
Sorry for the delay, needed some continuous quality time to acquire the
original context with this problem/patch.
With regards
Kamesh Jayachandran
Paul Burba wrote:
> 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
>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFIYmcW3WHvyO0YTCwRAt7UAKCHPmsLD1BVl4Ea0V5C5lEdge4lewCgg/wx
ZhWfC1kbM2njMdHuNv8lD54=
=JBPn
-----END PGP SIGNATURE-----
---------------------------------------------------------------------
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-25 17:41:58 CEST