Bazaar Fast Import

"tag from <refname>" gives KeyError

Bug #401249 reported by Samuel Bronson
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Bazaar Fast Import
Triaged
Medium
Unassigned
python-fastimport
Invalid
Undecided
Unassigned

Bug Description

This code in processors/generic_processor.py appears to assume that the "from" revision for a "tag" command will always be expressed as a "mark", like :1. This is not the case; for example see the following slightly altered (to avoid bug #400960) and severely abbreviated output from a slightly altered version of git's contrib/fast-import/import-tars.perl (also provided as an attachment):

commit refs/heads/import-tars
author Matthew Luckie <email address hidden> 1087095178 +0000
committer Samuel Bronson <email address hidden> 1247888914 +0000
data 43
Imported from scamper-cvs-20040613.tar.gz.

deleteall

tag scamper-cvs-20040613
from refs/heads/import-tars
tagger Matthew Luckie <email address hidden> 1087095178 +0000
data 29
Package scamper-cvs-20040613

This is documented in the git-fast-import(1) manpage:

           Here <committish> is any of the following:

           · The name of an existing branch already in fast-import’s
               internal branch table. If fast-import doesn’t know the name,
               its treated as a SHA-1 expression.

           · A mark reference, :<idnum>, where <idnum> is the mark number.

               The reason fast-import uses : to denote a mark reference is
               this character is not legal in a Git branch name. The leading :
               makes it easy to distinguish between the mark 42 (:42) and the
               branch 42 (42 or refs/heads/42), or an abbreviated SHA-1 which
               happened to consist only of base-10 digits.

[two other, fairly git-specific forms are also documented]

Tags: import
Revision history for this message
Samuel Bronson (naesten) wrote :
Revision history for this message
Samuel Bronson (naesten) wrote :

Okay, I've written a test on branch lp:~naesten/bzr-fastimport/401249-tag-from-ref.

As for a fix, I'm looking at the code, and it looks to me as though there ought to be a method on cache_manager.CacheManager to look up a committish -- as it stands, the logic seems to be tied up in the head tracking code, which is *probably* not something we want to call from GenericProcessor._set_tag(), though I'm not positive about that.

Revision history for this message
Samuel Bronson (naesten) wrote :

It turns out that this issue also affects commits, meaning that the logic is not tangled up in the head tracking code -- it's just *not there*.

Pushed a change to the test to lp:~naesten/bzr-fastimport/401249-tag-from-ref to demonstrate this; I should probably rename the test, but I'm not sure to what ...

Revision history for this message
Ian Clatworthy (ian-clatworthy) wrote :

At a minimum, we need to trap the KeyError. For tags, I'd be ok with reporting a warning, skipping that tag and continuing. For other uses of from (e.g. merge commits), we need to make the call as to whether that's acceptable of not.

tags: added: import
Changed in bzr-fastimport:
importance: Undecided → Medium
status: New → Triaged
Revision history for this message
Samuel Bronson (naesten) wrote :

This bug is about failure to implement the following form of committish:

           · The name of an existing branch already in fast-import’s
               internal branch table. If fast-import doesn’t know the name,
               its treated as a SHA-1 expression.

If you want a bug about failure to trap the KeyError, you can make your own bug ;-).

Revision history for this message
Jelmer Vernooij (jelmer) wrote :

I've merged the test case (and made it raise KnownFailure), but the bug is not fixed yet

Changed in python-fastimport:
status: New → Invalid
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Bug attachments

Remote bug watches

Bug watches keep track of this bug in other bug trackers.