git fast-import not verifying commit author lines?

mtn git_export produces this output on a simple repo:

blob
mark :1
data 8
content

commit refs/heads/com.example.badexport
mark :2
author <somename> 1261454209 +0000
committer <somename> 1261454209 +0000
data 8
acommit

M 100644 :1 "afile"
progress revision 9b0e11e4d66eba8a3cf26095fb573116b886cd37 (1/1)
#############################################################

The author and committer lines are missing the names (I've filed this as a bug with monotone). git commit-tree refuses to to produce a commit object like this, so it seems like git fast-import should detect and report this instead of silently writing the invalid commit object to the repository.

--David

Nope.

These objects can still be processed by git commands, you just can't normally create them with Git tools. To some extent fast-import allows the caller to import data that you wouldn't otherwise create in Git because we know you are coming from a foreign system where the data might not reasonably exist.

Shawn.

But shouldn't there still be an extra space? One to separate "author" from the empty name, and one to separate the empty name from the email? If not, then I think this change should be made. (I couldn't find any authoritative documentation on what constitutes a valid commit object.)

(Sorry, this has been sitting in my outbox for a week.)

--David

index 288032c..6917739 100644
@@ -312,6 +312,6 @@ change to the project.
        'commit' SP <ref> LF
        mark?
-       ('author' SP <name> SP LT <email> GT SP <when> LF)?
-       'committer' SP <name> SP LT <email> GT SP <when> LF
+       ('author' (SP <name>)? SP LT <email> GT SP <when> LF)?
+       'committer' (SP <name>)? SP LT <email> GT SP <when> LF
        data
        ('from' SP <committish> LF)?

The fast-import parser does not validate that the author, committer or tagger name component contains both a name and an email address. Therefore the name component has always been optional. Correct the documentation to match the implementation.

---

Yes, we should do this.

 Documentation/git-fast-import.txt |    6 +++---
 fast-import.c                     |    6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

index 288032c..e6d364f 100644
@@ -311,8 +311,8 @@ change to the project.
 ....
 	'commit' SP <ref> LF
 	mark?
-	('author' SP <name> SP LT <email> GT SP <when> LF)?
-	'committer' SP <name> SP LT <email> GT SP <when> LF
+	('author' (SP <name>)? SP LT <email> GT SP <when> LF)?
+	'committer' (SP <name>)? SP LT <email> GT SP <when> LF
 	data
 	('from' SP <committish> LF)?
 	('merge' SP <committish> LF)?

@@ -657,7 +657,7 @@ lightweight (non-annotated) tags see the `reset` command below.
 ....
 	'tag' SP <name> LF
 	'from' SP <committish> LF
-	'tagger' SP <name> SP LT <email> GT SP <when> LF
+	'tagger' (SP <name>)? SP LT <email> GT SP <when> LF
 	data
 ....
 

index dd3c99d..cd87049 100644
@@ -19,8 +19,8 @@ Format of STDIN stream:
 
   new_commit ::= 'commit' sp ref_str lf
     mark?
-    ('author' sp name sp '<' email '>' sp when lf)?
-    'committer' sp name sp '<' email '>' sp when lf
+    ('author' (sp name)? sp '<' email '>' sp when lf)?
+    'committer' (sp name)? sp '<' email '>' sp when lf
     commit_msg
     ('from' sp committish lf)?
     ('merge' sp committish lf)*

@@ -47,7 +47,7 @@ Format of STDIN stream:
 
   new_tag ::= 'tag' sp tag_str lf
     'from' sp committish lf
-    ('tagger' sp name sp '<' email '>' sp when lf)?
+    ('tagger' (sp name)? sp '<' email '>' sp when lf)?
     tag_msg;
   tag_msg ::= data;
 

1.6.6.307.gba67

-- 
Shawn.

Thanks.