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 <[EMAIL PROTECTED]>
Review by: ?

Index: svnmerge.py
===================================================================
--- svnmerge.py (revision 19635)
+++ svnmerge.py (working copy)
@@ -882,6 +882,8 @@
         old_revs = None
         for rev in merge_metadata.changed_revs():
             new_revs = merge_metadata.get(rev).get(target_dir)
+            if old_revs is None:
+                old_revs = get_revlist_prop(url, opts["prop"], 
rev-1).get(target_dir)
             if new_revs != old_revs:
                 reflected_revs.append("%s" % rev)
             old_revs = new_revs



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 <[EMAIL PROTECTED]>
Review by: ?

Index: svnmerge.py
===================================================================
--- svnmerge.py (revision 19635)
+++ svnmerge.py (working copy)
@@ -905,7 +907,7 @@
     base = 1
     r = opts["merged-revs"].normalized()
     if r and r[0][0] == 1:
-        base = r[0][1]
+        base = r[0][1] + 1

     # See if the user filtered the revision set. If so, we are not
     # interested in something outside that range.


Cheers,
Raman

Attachment: template.sh
Description: application/shellscript

Attachment: test.sh
Description: application/shellscript

_______________________________________________
Svnmerge mailing list
[email protected]
http://www.orcaware.com/mailman/listinfo/svnmerge

Reply via email to