Possible bug in selecting the best revision as base for a tag or branch

Anders Pilegaard <[email protected]> Fri, 28 Jan 2011 13:36:10 +0100
Newsgroups gmane.comp.version-control.subversion.cvs2svn.devel
Message-ID <[email protected]>
As I've mentioned in another mail thread, I'm in the process of
converting a large, old CVS repository to Git.  As part of that I'm
doing test conversions and looking at the result.

I'm currently using revision 5318 from the git2svn SVN repository.

I've come across some tags and branches that are getting fixup commits
- where it shouldn't be necessary.  One example was a tag that was
created with a fixup commit deleting one file.  That would have been
fine if the tag hadn't been set on the complete repository - but in
this case it had.  And looking at the history there was actually
another commit which would have been a perfect match - because the
"missing" file was actually deleted in a later commit.

After looking a bit at the code my guess is that the problem lies in
the functions that try to select the best SVN revision to use.  For a
given tag the code loops through all files touched by the tag, finding
the range of SVN revisions that have that particular CVS revision on
this file.  If things are simple the intersection of these ranges is
non-empty, and the lowest numbered SVN revision in the range is
selected.

This is fine in the normal cases.  But when some later revision in the
range deletes a file which *isn't* covered by the tag, we risk getting
a suboptimal base revision.

To further debug this I created a small test repository, which I have
attached (both the repository 'ex3.tar.gz' and the script creating it
'create_ex3.sh').  As can also be seen from the script, the tag 'tag2'
and the branch 'branch2' should both have a perfect match - namely the
commit with the message "branch1 delta3".  Instead both are created
via fixup tags.  And not only that - they are also created off the
main trunk instead of off branch1 where they should belong!

When I started debugging this a bit, things started looking really
strange.  I've attached output from the conversion ('log.txt') which
has a bit extra debug output from the patch 'dbgdiff.txt'.  My options
file is 'cvs2git-svn.options'.

As can be seen from the output of pass 13, the following SVN revisions
are created:

  r1 Project init
  r2 Commit "first"
  r3 Create tag2
  r4 Create branch1
  r5 Create branch2
  r6 Commit "branch1 delta1" (changes file1)
  r7 Commit "MAIN delta2"  (changes file1+file2)
  r8 Commit "branch1 delta3" (removes file1)
  r9 Commit "branch2 delta4" (changes file2)

Here is probably the first problem - tag2 and branch2 both have lower
SVN revision numbers than the revision they ideally should match,
which is r8.

This can partially be seen from the extra debug output - the ones
where the first line starts with '--- Ranges for'.  The range for eg
the tag commit (#3) ends up as [2:].  I'm not entirely sure how that
works out later, but the comments in the code in 'get_range_map' of
'openings_closings.py' clearly describes that the range shouldn't
extend beyond the revision number of the tag.  This is probably
enforced in the code using this range - I haven't debugged this
completely ...

My next thought was to try to see what the range would be before it
was limited.  That is the extra debug output where the first line
starts with '### Ranges for'.  Here the range for #3 becomes [2:7].
This is better - but the revision it really belongs to is r8, which is
still not in the range.  (Unless of course I'm confusing a 0-based and
a 1-based numbering ... :-) ...)

So why do the tags get so low revision numbers?  I haven't tracked it
down, but I'm guessing it's due to some timestamp sorting.  Since tags
(and branches) don't have timestamps in CVS, the conversion is forced
to guess a timestamp.  The easiest guess would be to use the highest
timestamp of the tagged revisions.  But for the above sort to work it
should really be the lowest timestamp of the *next* revision after the
tagged.

Please comment - especially if I'm on the wrong track somewhere! ... :-)

I'll continue digging into this to see what I can come up with.

Best regards,
  Anders Pilegaard

------------------------------------------------------
http://cvs2svn.tigris.org/ds/viewMessage.do?dsForumId=1667&dsMessageId=2701167

To unsubscribe from this discussion, e-mail: [[email protected]].
log.txt (text/plain, 14.2 KB)
0.150756: Running command ['cvs', '-Q', '-R', '-f', '--version']
0.155126: Conversion start time: 2011-01-28 12:57:46 CET
0.157260: ----- pass 1 (CollectRevsPass) -----
0.157347: Examining all CVS ',v' files...
0.158280: /tmp/repo/foo/file1,v
0.160817: /tmp/repo/foo/file2,v
0.162718: Processed 2 files
0.163087: Flushing cache for RecordTable('cvs2svn-tmp/metadata-index.dat')
0.163746: Done
0.163838: Time for pass1 (CollectRevsPass): 0.007 seconds.
0.170764: ----- pass 2 (CleanMetadataPass) -----
0.171141: Converting metadata to UTF8...
0.172220: Flushing cache for RecordTable('cvs2svn-tmp/metadata-clean-index.dat')
0.172388: Flushing cache for RecordTable('cvs2svn-tmp/metadata-index.dat')
0.172471: Done
0.172548: Time for pass2 (CleanMetadataPass): 0.009 seconds.
0.172834: Deleting cvs2svn-tmp/statistics-01.pck
0.172936: Deleting cvs2svn-tmp/metadata-index.dat
0.173030: Deleting cvs2svn-tmp/metadata.pck
0.179270: ----- pass 3 (CollateSymbolsPass) -----
0.180085: Converting symbol branch1 as a branch because it is always used as a branch.
0.180180: The preferred parent of Branch('branch1') is Trunk
0.180292: Converting symbol tag2 as a tag because it is always used as a tag.
0.180395: The preferred parent of Tag('tag2') is Trunk
0.180730: Converting symbol branch2 as a branch because it is always used as a branch.
0.180824: The preferred parent of Branch('branch2') is Trunk
0.181041: Checking for forced tags with commits...
0.181298: Done
0.181381: Time for pass3 (CollateSymbolsPass): 0.009 seconds.
0.181675: Deleting cvs2svn-tmp/statistics-02.pck
0.181774: Deleting cvs2svn-tmp/symbol-statistics.pck
0.187999: ----- pass 4 (FilterSymbolsPass) -----
0.189233: Filtering out excluded symbols and summarizing items...
0.189499: /tmp/repo/foo/file1,v
0.189991: Running command ['cvs', '-Q', '-R', '-f', '-d', ':local:/tmp/repo', 'co', '-r1.1.2.1', '-p', 'foo/file1']
0.194717: Running command ['cvs', '-Q', '-R', '-f', '-d', ':local:/tmp/repo', 'co', '-r1.1', '-p', 'foo/file1']
0.199672: Running command ['cvs', '-Q', '-R', '-f', '-d', ':local:/tmp/repo', 'co', '-r1.2', '-p', 'foo/file1']
0.204680: /tmp/repo/foo/file2,v
0.205339: Running command ['cvs', '-Q', '-R', '-f', '-d', ':local:/tmp/repo', 'co', '-r1.1.4.1', '-p', 'foo/file2']
0.210000: Running command ['cvs', '-Q', '-R', '-f', '-d', ':local:/tmp/repo', 'co', '-r1.1', '-p', 'foo/file2']
0.214200: Running command ['cvs', '-Q', '-R', '-f', '-d', ':local:/tmp/repo', 'co', '-r1.2', '-p', 'foo/file2']
0.219247: Done
0.219690: Time for pass4 (FilterSymbolsPass): 0.038 seconds.
0.220160: Deleting cvs2svn-tmp/cvs-items.pck
0.220372: Deleting cvs2svn-tmp/statistics-03.pck
0.227282: ----- pass 5 (SortRevisionsPass) -----
0.227993: Sorting CVS revision summaries...
0.229255: Done
0.229590: Time for pass5 (SortRevisionsPass): 0.010 seconds.
0.230297: Deleting cvs2svn-tmp/statistics-04.pck
0.230628: Deleting cvs2svn-tmp/revs.dat
0.240790: ----- pass 6 (SortSymbolsPass) -----
0.241564: Sorting CVS symbol summaries...
0.242396: Done
0.242685: Time for pass6 (SortSymbolsPass): 0.013 seconds.
0.243419: Deleting cvs2svn-tmp/symbols.dat
0.243763: Deleting cvs2svn-tmp/statistics-05.pck
0.263798: ----- pass 7 (InitializeChangesetsPass) -----
0.264351: Creating preliminary commit sets...
0.267041: RevisionChangeset<1> [2, 9]
0.267693: RevisionChangeset<2> [3, a]
0.268211: RevisionChangeset<3> [4]
0.268633: RevisionChangeset<4> [5]
0.268984: RevisionChangeset<5> [b]
0.269588: BranchChangeset<6>("Branch('branch1')") [1, 8]
0.270080: TagChangeset<7>("Tag('tag2')") [6]
0.270410: BranchChangeset<8>("Branch('branch2')") [7]
0.270693: Flushing cache for RecordTable('cvs2svn-tmp/cvs-items-sorted-index.dat')
0.270994: Flushing cache for RecordTable('cvs2svn-tmp/cvs-item-to-changeset.dat')
0.271252: Flushing cache for RecordTable('cvs2svn-tmp/changesets-index.dat')
0.271533: Done
0.271680: Time for pass7 (InitializeChangesetsPass): 0.029 seconds.
0.272250: Deleting cvs2svn-tmp/item-serializer.pck
0.272423: Deleting cvs2svn-tmp/symbols-s.dat
0.272592: Deleting cvs2svn-tmp/revs-s.dat
0.272738: Deleting cvs2svn-tmp/statistics-06.pck
0.283367: ----- pass 8 (BreakRevisionChangesetCyclesPass) -----
0.283922: Breaking revision changeset dependency cycles...
0.289368: Flushing cache for RecordTable('cvs2svn-tmp/changesets-index.dat')
0.290027: Consumed changeset ids 2, 3, 1, 4, 5
0.290133: Flushing cache for RecordTable('cvs2svn-tmp/cvs-item-to-changeset-revbroken.dat')
0.290261: Flushing cache for RecordTable('cvs2svn-tmp/changesets-revbroken-index.dat')
0.290536: Flushing cache for RecordTable('cvs2svn-tmp/cvs-items-sorted-index.dat')
0.290682: Done
0.290820: Time for pass8 (BreakRevisionChangesetCyclesPass): 0.019 seconds.
0.291429: Deleting cvs2svn-tmp/cvs-item-to-changeset.dat
0.291609: Deleting cvs2svn-tmp/changesets.pck
0.291769: Deleting cvs2svn-tmp/changesets-index.dat
0.291925: Deleting cvs2svn-tmp/statistics-07.pck
0.302104: ----- pass 9 (RevisionTopologicalSortPass) -----
0.302698: Generating CVSRevisions in commit order...
0.309922: Flushing cache for RecordTable('cvs2svn-tmp/cvs-item-to-changeset-revbroken.dat')
0.310107: Flushing cache for RecordTable('cvs2svn-tmp/changesets-revbroken-index.dat')
0.310256: Flushing cache for RecordTable('cvs2svn-tmp/changesets-revsorted-index.dat')
0.310549: Flushing cache for RecordTable('cvs2svn-tmp/cvs-items-sorted-index.dat')
0.310702: Done
0.310838: Time for pass9 (RevisionTopologicalSortPass): 0.020 seconds.
0.311462: Deleting cvs2svn-tmp/changesets-revbroken.pck
0.311643: Deleting cvs2svn-tmp/changesets-revbroken-index.dat
0.311810: Deleting cvs2svn-tmp/statistics-08.pck
0.321942: ----- pass 10 (BreakSymbolChangesetCyclesPass) -----
0.322536: Breaking symbol changeset dependency cycles...
0.327294: Flushing cache for RecordTable('cvs2svn-tmp/changesets-revsorted-index.dat')
0.327833: Consumed changeset ids 7, 6, 8
0.327934: Flushing cache for RecordTable('cvs2svn-tmp/cvs-item-to-changeset-symbroken.dat')
0.328052: Flushing cache for RecordTable('cvs2svn-tmp/changesets-symbroken-index.dat')
0.328331: Flushing cache for RecordTable('cvs2svn-tmp/cvs-items-sorted-index.dat')
0.328488: Done
0.328631: Time for pass10 (BreakSymbolChangesetCyclesPass): 0.018 seconds.
0.329253: Deleting cvs2svn-tmp/cvs-item-to-changeset-revbroken.dat
0.329434: Deleting cvs2svn-tmp/changesets-revsorted.pck
0.329590: Deleting cvs2svn-tmp/changesets-revsorted-index.dat
0.329743: Deleting cvs2svn-tmp/statistics-09.pck
0.342017: ----- pass 11 (BreakAllChangesetCyclesPass) -----
0.342606: Breaking CVSSymbol dependency loops...
0.348747: Flushing cache for RecordTable('cvs2svn-tmp/changesets-symbroken-index.dat')
0.349768: Consumed changeset ids 2, 7, 6, 8, 3, 1, 4, 5
0.349875: Flushing cache for RecordTable('cvs2svn-tmp/cvs-item-to-changeset-allbroken.dat')
0.349995: Flushing cache for RecordTable('cvs2svn-tmp/changesets-allbroken-index.dat')
0.350260: Done
0.350396: Time for pass11 (BreakAllChangesetCyclesPass): 0.022 seconds.
0.351089: Deleting cvs2svn-tmp/changesets-symbroken.pck
0.351275: Deleting cvs2svn-tmp/changesets-symbroken-index.dat
0.351430: Deleting cvs2svn-tmp/cvs-item-to-changeset-symbroken.dat
0.351580: Deleting cvs2svn-tmp/statistics-10.pck
0.362226: ----- pass 12 (TopologicalSortPass) -----
0.362840: Generating CVSRevisions in commit order...
0.367797: Timestamp "Fri Jan 28 09:36:15 2011" adjusted to "Fri Jan 28 09:36:16 2011" to ensure monotonicity.
0.368142: Flushing cache for RecordTable('cvs2svn-tmp/cvs-item-to-changeset-allbroken.dat')
0.368283: Flushing cache for RecordTable('cvs2svn-tmp/changesets-allbroken-index.dat')
0.368531: Flushing cache for RecordTable('cvs2svn-tmp/cvs-items-sorted-index.dat')
0.368679: Done
0.368809: Time for pass12 (TopologicalSortPass): 0.018 seconds.
0.369442: Deleting cvs2svn-tmp/cvs-item-to-changeset-allbroken.dat
0.369618: Deleting cvs2svn-tmp/statistics-11.pck
0.379659: ----- pass 13 (CreateRevsPass) -----
0.380288: Mapping CVS revisions to Subversion commits...
0.383147: Creating Subversion r1 (Project initialization)
0.383352: ------------------------------------------------------------
0.383441: CVS Revision grouping:
0.383547:   Time: Fri Jan 28 10:36:12 2011
0.383893: Creating Subversion r2 (commit)
0.384009:  file1 1.1
0.384099:  file2 1.1
0.384684: Creating Subversion r3 (copying to tag 'tag2')
0.385049: Creating Subversion r4 (copying to branch 'branch1')
0.385292: Creating Subversion r5 (copying to branch 'branch2')
0.385509: ------------------------------------------------------------
0.385557: CVS Revision grouping:
0.385613:   Time: Fri Jan 28 10:36:16 2011
0.385733: Creating Subversion r6 (commit)
0.385790:  file1 1.1.2.1
0.386073: ------------------------------------------------------------
0.386121: CVS Revision grouping:
0.386175:   Time: Fri Jan 28 10:36:19 2011
0.386338: Creating Subversion r7 (commit)
0.386395:  file1 1.2
0.386445:  file2 1.2
0.386706: ------------------------------------------------------------
0.386755: CVS Revision grouping:
0.386808:   Time: Fri Jan 28 10:36:21 2011
0.386929: Creating Subversion r8 (commit)
0.386985:  file1 1.1.2.2
0.387209: ------------------------------------------------------------
0.387256: CVS Revision grouping:
0.387309:   Time: Fri Jan 28 10:36:23 2011
0.387426: Creating Subversion r9 (commit)
0.387485:  file2 1.1.4.1
0.387617: Flushing cache for RecordTable('cvs2svn-tmp/changesets-allbroken-index.dat')
0.387704: Flushing cache for RecordTable('cvs2svn-tmp/cvs-revs-to-svn-revnums.dat')
0.387864: Flushing cache for RecordTable('cvs2svn-tmp/svn-commits.pck')
0.388077: Flushing cache for RecordTable('cvs2svn-tmp/cvs-items-sorted-index.dat')
0.388160: Done
0.388240: Time for pass13 (CreateRevsPass): 0.019 seconds.
0.388633: Deleting cvs2svn-tmp/changesets-s.txt
0.388737: Deleting cvs2svn-tmp/statistics-12.pck
0.388832: Deleting cvs2svn-tmp/changesets-allbroken.pck
0.388922: Deleting cvs2svn-tmp/changesets-allbroken-index.dat
0.395407: ----- pass 14 (SortSymbolOpeningsClosingsPass) -----
0.395848: Sorting symbolic name source revisions...
0.396335: Done
0.396423: Time for pass14 (SortSymbolOpeningsClosingsPass): 0.008 seconds.
0.396806: Deleting cvs2svn-tmp/statistics-13.pck
0.396908: Deleting cvs2svn-tmp/symbolic-names.txt
0.404064: ----- pass 15 (IndexSymbolsPass) -----
0.404501: Determining offsets for all symbolic names...
0.404828:   branch1
0.404893:   tag2
0.404945:   branch2
0.405126: Done.
0.405200: Time for pass15 (IndexSymbolsPass): 0.009 seconds.
0.405551: Deleting cvs2svn-tmp/statistics-14.pck
0.412429: ----- pass 16 (OutputPass) -----
### Ranges for #3 (SVNCommit #: 3
   debug description: copying to tag 'tag2'
   symbolic name: tag2
)
{CVSTag(file2:Tag('tag2')<6>): [2:7]}
--- Ranges for #3 (SVNCommit #: 3
   debug description: copying to tag 'tag2'
   symbolic name: tag2
)
{CVSTag(file2:Tag('tag2')<6>): [2:]}
0.415866: Tag('tag2') will be created via a fixup branch
### Ranges for #4 (SVNCommit #: 4
   debug description: copying to branch 'branch1'
   symbolic name: branch1
)
{CVSBranch(file1:Branch('branch1'):1.1.2<1>): [2:7],
 CVSBranch(file2:Branch('branch1'):1.1.2<8>): [2:7]}
--- Ranges for #4 (SVNCommit #: 4
   debug description: copying to branch 'branch1'
   symbolic name: branch1
)
{CVSBranch(file1:Branch('branch1'):1.1.2<1>): [2:],
 CVSBranch(file2:Branch('branch1'):1.1.2<8>): [2:]}
0.417307: Branch('branch1') will be created via a simple copy from Trunk:r2
### Ranges for #5 (SVNCommit #: 5
   debug description: copying to branch 'branch2'
   symbolic name: branch2
)
{CVSBranch(file2:Branch('branch2'):1.1.4<7>): [2:7]}
--- Ranges for #5 (SVNCommit #: 5
   debug description: copying to branch 'branch2'
   symbolic name: branch2
)
{CVSBranch(file2:Branch('branch2'):1.1.4<7>): [2:]}
0.417987: Branch('branch2') will be created via fixup commit(s)
0.419798: Flushing cache for RecordTable('cvs2svn-tmp/mirror-nodes-index.dat')
0.420033: Flushing cache for RecordTable('cvs2svn-tmp/cvs-revs-to-svn-revnums.dat')
0.420095: Flushing cache for RecordTable('cvs2svn-tmp/svn-commits.pck')
0.420173: Flushing cache for RecordTable('cvs2svn-tmp/cvs-items-sorted-index.dat')
0.420246: Flushing cache for RecordTable('cvs2svn-tmp/metadata-clean-index.dat')
0.420348: Time for pass16 (OutputPass): 0.015 seconds.
0.420779: Deleting cvs2svn-tmp/metadata-clean-index.dat
0.420881: Deleting cvs2svn-tmp/metadata-clean.pck
0.420971: Deleting cvs2svn-tmp/symbols.pck
0.421057: Deleting cvs2svn-tmp/svn-commits-index.dat
0.421144: Deleting cvs2svn-tmp/svn-commits.pck
0.421236: Deleting cvs2svn-tmp/cvs-revs-to-svn-revnums.dat
0.421323: Deleting cvs2svn-tmp/projects.pck
0.421410: Deleting cvs2svn-tmp/cvs-paths.pck
0.421497: Deleting cvs2svn-tmp/symbol-offsets.pck
0.421582: Deleting cvs2svn-tmp/cvs-items-sorted.pck
0.421664: Deleting cvs2svn-tmp/mirror-nodes-index.dat
0.421750: Deleting cvs2svn-tmp/mirror-nodes.pck
0.421833: Deleting cvs2svn-tmp/symbolic-names-s.txt
0.421918: Deleting cvs2svn-tmp/cvs-items-sorted-index.dat
0.422005: Deleting cvs2svn-tmp/statistics-15.pck
0.422090: Deleting cvs2svn-tmp/statistics-16.pck
0.428648: 
0.428648: cvs2svn Statistics:
0.428648: ------------------
0.428648: Total CVS Files:                 2
0.428648: Total CVS Revisions:             7
0.428648: Total CVS Branches:              3
0.428648: Total CVS Tags:                  1
0.428648: Total Unique Tags:               1
0.428648: Total Unique Branches:           2
0.428648: CVS Repos Size in KB:            1
0.428648: Total SVN Commits:               9
0.428648: First Revision Date:    Fri Jan 28 10:36:12 2011
0.428648: Last Revision Date:     Fri Jan 28 10:36:23 2011
0.428648: ------------------
0.429019: Timings (seconds):
0.429019: ------------------
0.429019: 0.007   pass1    CollectRevsPass
0.429019: 0.009   pass2    CleanMetadataPass
0.429019: 0.009   pass3    CollateSymbolsPass
0.429019: 0.038   pass4    FilterSymbolsPass
0.429019: 0.010   pass5    SortRevisionsPass
0.429019: 0.013   pass6    SortSymbolsPass
0.429019: 0.029   pass7    InitializeChangesetsPass
0.429019: 0.019   pass8    BreakRevisionChangesetCyclesPass
0.429019: 0.020   pass9    RevisionTopologicalSortPass
0.429019: 0.018   pass10   BreakSymbolChangesetCyclesPass
0.429019: 0.022   pass11   BreakAllChangesetCyclesPass
0.429019: 0.018   pass12   TopologicalSortPass
0.429019: 0.019   pass13   CreateRevsPass
0.429019: 0.008   pass14   SortSymbolOpeningsClosingsPass
0.429019: 0.009   pass15   IndexSymbolsPass
0.429019: 0.015   pass16   OutputPass
0.429019: 0.263   total
create_ex3.sh (application/x-sh, 699 B)
#!/bin/bash

cd /tmp
rm -rf repo sandbox initial
mkdir repo
cvs -d /tmp/repo init
mkdir initial
(cd initial; cvs -d /tmp/repo import -m 'Create empty repo' foo vendor init)
cvs -d /tmp/repo checkout -d sandbox foo
cd sandbox

echo file1 > file1
echo file2 > file2
cvs add file1 file2
cvs commit -m first

cvs tag -b branch1
cvs update -r branch1
echo "delta1" >> file1
sleep 2
cvs commit -m "branch1 delta1"

cvs update -A
echo "delta2" >> file1
echo "delta2" >> file2
sleep 2
cvs commit -m "MAIN delta2"

cvs update -r branch1
rm file1
cvs rm file1
cvs commit -m "branch1 delta3"

cvs tag -b branch2
cvs tag tag2

cvs update -r branch2
echo "delta4" >> file2
sleep 2
cvs commit -m "branch2 delta4"
ex3.tar.gz (application/x-gzip, 10 KB) - not displayed
dbgdiff.txt (text/plain, 913 B)
Index: cvs2svn_lib/openings_closings.py
===================================================================
--- cvs2svn_lib/openings_closings.py	(revision 5318)
+++ cvs2svn_lib/openings_closings.py	(working copy)
@@ -213,6 +213,10 @@
               )
         range.add_closing(revnum)
 
+    print "### Ranges for #%d (%s)" % (svn_symbol_commit.revnum, svn_symbol_commit)
+    from pprint import pprint
+    pprint(range_map)
+
     # Make sure that all CVSSymbols are accounted for, and adjust the
     # closings to be not later than svn_symbol_commit.revnum.
     for cvs_symbol in cvs_symbol_map.itervalues():
@@ -231,6 +235,10 @@
              and range.closing_revnum > svn_symbol_commit.revnum:
         range.closing_revnum = None
 
+    print "--- Ranges for #%d (%s)" % (svn_symbol_commit.revnum, svn_symbol_commit)
+    from pprint import pprint
+    pprint(range_map)
+    
     return range_map
cvs2git-svn.options (application/octet-stream, 26.7 KB) - not displayed