[Svnmerge] RESEND: [PATCH] Fix for bidirectional merging corner cases

[Raman Gupta]

My apologies -- I originally sent this email without line breaks (an
unfortunate result of messing around with my Thunderbird config). I am
forwarding it with line breaks, sorry for the inconvenience!

Cheers,
Raman

-------- Original Message --------
Subject: [PATCH] Fix for bidirectional merging corner cases
Date: Sat, 13 May 2006 13:16:48 -0400
From: Raman Gupta <rocketraman at fastmail.fm>
To: Svnmerge <svnmerge at orcaware.com>

A refactorization of the code in revision 18696 that checks for
reflected revisions broke it for a corner case. The current code may
incorrectly consider the first revision to be reflected even when it is
not. This can occur if the first revision included a change to the merge
property, but one that did not change the merged revision list for the
source branch.

See my patch below. Since old_revs is not initialized in the loop that
checks the merge_metadata_changed revs, the first rev to be checked is
ALWAYS considered reflected.

Note that the actual incidence of this bug is reduced (but not
eliminated) by a mistake elsewhere. In analyze_head_revs, the base
revision for analysis is selected:

    # Calculate the base of analysis. If there is a "1-XX" interval in
    # the merged_revs, we do not need to check those.
    base = 1
    r = opts["merged-revs"].normalized()
    if r and r[0][0] == 1:
        base = r[0][1]

Note that here, as long as the first merged revision is 1, the base is
selected to be an already merged revision. Since this may become the
first revision to be checked in analyze_revs (though redundantly because
it has already been merged), the algorithm works because old_revs is
initialized during that first redundant loop. However, there are two
situations in which this problem will still occur despite the error above:

1) if a revision list that only includes true candidate revisions is
passed to avail or merge with -r, and

2) if the last revision to be merged DID NOT include a change to the
merge properties (such as in my reproduction recipe).

I have attached a reproduction recipe for this problem. In an empty
directory, run template.sh first to create the template repo, and then
run test.sh as many times as you wish.

Here are the patches (they are independent, but I wouldn't recommend
applying the base + 1 patch without the first patch because it will only
increase the chance for buggy behavior):

A refactorization of the code in revision 18696 that checks for
reflected revisions broke it for some corner cases. The first revision
to be checked may be considered reflected even when it is not. This can
occur if the first revision that included a change to the merge property
did not affect the source branch.

* contrib/client-side/svnmerge.py
  (analyze_revs): If old_revs is not initialized in the loop that checks
    revisions in which the merge props changed, initialize it. This
    prevents the first revision from being considered reflected even if
    it is not.

Patch by: Raman Gupta <rocketraman at fastmail.fm>
Review by: ?

<patch attached as bidibug-v1.patch>

Here is a log message/patch for the base + 1 problem (yes, all the tests
pass):

Small optimization in obtaining the revisions to be checked for merging.
The last merged revision is included in analyze_head_revs, but later
excluded by the algorithm anyway, resulting in some unnecessary work.

* contrib/client-side/svnmerge.py
  (analyze_head_revs): Don't include the last merged revision in the
    list of revisions to be checked for merging.

Patch by: Raman Gupta <rocketraman at fastmail.fm>
Review by: ?

<patch attached as baseplusone-v1.patch>

Cheers,
Raman

-------------- next part --------------
A non-text attachment was scrubbed...
Name: template.sh
Type: application/x-shellscript
Size: 1175 bytes
Desc: not available
Url : /pipermail/svnmerge/attachments/20060513/188bd17f/attachment.bin 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: test.sh
Type: application/x-shellscript
Size: 2579 bytes
Desc: not available
Url : /pipermail/svnmerge/attachments/20060513/188bd17f/attachment-0001.bin 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bidibug-v1.patch
Type: text/x-patch
Size: 554 bytes
Desc: not available
Url : /pipermail/svnmerge/attachments/20060513/188bd17f/attachment-0002.bin 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: baseplusone-v1.patch
Type: text/x-patch
Size: 428 bytes
Desc: not available
Url : /pipermail/svnmerge/attachments/20060513/188bd17f/attachment-0003.bin