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

On 01/28/2011 01:36 PM, Anders Pilegaard wrote:
> 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.

Yes, this is a known bug:

http://cvs2svn.tigris.org/issues/show_bug.cgi?id=139

Feel free to add information to that bug report.

It is covered by the test suite in the test called "tagging_after_delete()".

Your analysis is very good and agrees with my own understanding of the
problem on all points.  I've made some comments below.

> 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.

Yes.  For files that were actually tagged, this is pretty
straightforward, because the revision that was tagged has a single,
contiguous range of SVN revisions where it existed.  Recording these
ranges is straightforward and requires a foreseeable amount of space.

A file that was not tagged, on the other hand, could correspond to a
revision before the file was first created, or from any branch from a
revision before the branch was created, or from any branch before the
file was added to the branch in question, or from any revision during
which the file was "dead".  The bookkeeping code would have to be made
smarter.  Moreover, the parent-choosing code is quite expensive in terms
of data that need to be recorded and in terms of processing time.  Any
changes here would have to be done carefully to avoid becoming too
expensive.

> 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.

Yes, the presence of a file that shouldn't be in a tag is completely
disregarded when choosing the tag point.

> 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!

Yes, the presence of a file that shouldn't be in a tag is also
disregarded when choosing a tag's preferred parent.

> [...] 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 ... :-) ...)

Your interpretation is correct; these numbers correspond directly to SVN
revision numbers.

> 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.

When commits are being formed in CreateRevsPass, tag commits are created
as soon as all of the file revisions that appear within the tag have
been committed.  This is done based on the topological ordering, not on
guessing a timestamp.  Only in OutputPass are timestamps chosen for tag
commits, simply by setting the timestamp equal to the timestamp of the
previous commit.

So tags get low revision numbers because cvs2svn isn't aware of the
reason to give them higher revision numbers, because it doesn't even
collect information about file deletions.

It would be great to get this problem fixed.  A partial fix (for git
only) is provided by the contrib/git-move-refs.py script.  Have you
tried that yet?  Did it help?

But even if git-move-refs.py does its thing, it is not a perfect
solution, because the same problem occurs for other VCSs.  It also does
its work without considering all of the information available to
cvs2svn; for example, I believe that its choices could contradict the
"possible parents" constraints.

I don't have time to write more now, but let me know if you have more
questions.  If you want to work on improvements here, I'd be happy to
share some thoughts about how this area could be improved.

By the way, issue 54 is also in this area and it might be possible to
come up with improvements that will fix both issues.

http://cvs2svn.tigris.org/issues/show_bug.cgi?id=54

Yours,
Michael

-- 
Michael Haggerty
[email protected]
http://softwareswirl.blogspot.com/

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

To unsubscribe from this discussion, e-mail: [[email protected]].