"tag from <refname>" gives KeyError

Bug #401249 reported by Samuel Bronson on 2009-07-19
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Bazaar Fast Import
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]

Samuel Bronson (naesten) wrote :
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.

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

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

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  Edit
Everyone can see this information.

Other bug subscribers

Bug attachments