[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Fix for issue 3067

From: Kamesh Jayachandran <kamesh_at_collab.net>
Date: Wed, 25 Jun 2008 21:11:10 +0530

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
Version: GnuPG v1.2.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org


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

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.