[BUG+PATCH] [cvs2svn] Warning! Branches can contain incorrect data!

[Max Bowsher <ma...@ukf.net>]

An incorrect optimization in cvs2svn's score_revisions causes some branches
to use incorrect revisions to copy from.

The "if not closings:  return openings" 'easy-out' is in fact invalid.

Reproduction:

Add print statements:
[[[
Index: cvs2svn.py
===================================================================
--- cvs2svn.py (revision 6897)
+++ cvs2svn.py (working copy)
@@ -984,11 +984,15 @@
                           'Node-copyfrom-path: /%s\n'
                           '\n'
                           % (svn_dst_path, change.copyfrom_rev,
svn_src_path))
+      sys.stdout.write('DUMPFILE COPY: %s <- r%d /%s\n'
+                          % (svn_dst_path, change.copyfrom_rev,
svn_src_path))

       for ent in change.deleted_entries:
         self.dumpfile.write('Node-path: %s\n'
                             'Node-action: delete\n'
                             '\n' % (svn_dst_path + '/' + ent))
+        sys.stdout.write('DUMPFILE PRUNE: %s'
+                            '\n' % (svn_dst_path + '/' + ent))

   def prune_entries(self, path, expected):
     """Delete any entries in PATH that are not in list EXPECTED.
@@ -1438,7 +1442,8 @@
     if not openings:
       return []
     if not closings:
-      return openings
+      closings = []
+    #   return openings

     # No easy out, so wish for lexical closures and calculate the scores
:-).
     scores = []
@@ -1457,6 +1462,11 @@
           scores[j] = (opening_pair[0], opening_pair[1] - closing_score)
         else:
           min = j + 1
+    if not closings and len(scores) > 1:
+      print "FAKE EASY-OUT ANALYSIS:"
+      print "%s <%s>" % (openings, self.best_rev(openings))
+      print "%s <%s>" % (scores, self.best_rev(scores))
+      return openings
     return scores

   def best_rev(self, scores):
]]]

If the easy-out was valid, then the two lines printed after "FAKE EASY-OUT
ANALYSIS:" should be identical. The "DUMPFILE xxx:" prints serve to
demonstrate the caused incorrect behaviour.

Here is example output from running cvs2svn on test-data/main-cvsrepos:

[[[
committing: Tue Jun  3 04:20:31 2003, over 0 seconds
FAKE EASY-OUT ANALYSIS:
[(19, 5), (22, 1)] <19>
[(19, 5), (22, 6)] <22>
DUMPFILE COPY: branches/B_SPLIT <- r19 /trunk
DUMPFILE PRUNE: branches/B_SPLIT/partial-prune
DUMPFILE PRUNE: branches/B_SPLIT/single-files
...
    new revision: 27
]]]

See that the copy is made from r19, but it should have come from r22.

Here's what happened in r22:

[[[
committing: Fri May 23 01:48:51 2003, over 0 seconds
    adding or changing 1.1.2.2 :
branches/B_MIXED/proj/sub2/branch_B_MIXED_only
    adding or changing 1.3 : trunk/proj/sub2/default
    new revision: 22
]]]

See that cvsrev 1.3 of proj/sub2/default was committed.
In the RCS file: " B_SPLIT:1.3.0.2 "

The copy has copied the wrong revision into the branch.

Proposed fix below:

[[[
* tools/cvs2svn/cvs2svn.py (SymbolicNameTracker.score_revisions):
    Do not do invalid optimization.

Index: cvs2svn.py
===================================================================
--- cvs2svn.py (revision 6897)
+++ cvs2svn.py (working copy)
@@ -1431,14 +1431,15 @@
     which are not correct and need to be deleted or recopied; those
     can only be detected by descending and examining their scores.

-    If OPENINGS is false, return the empty list, else if CLOSINGS is
-    false, return OPENINGS."""
+    If OPENINGS is false, return the empty list."""

     # First look for easy outs.
     if not openings:
       return []
-    if not closings:
-      return openings
+
+    # Must be able to call len(closings) below.
+    if closings == None:
+      closings = []

     # No easy out, so wish for lexical closures and calculate the scores
:-).
     scores = []
]]]

Max.





---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org