Re: fast-import bug?

On Fri, Jun 21, 2013 at 02:21:47AM -0700, Dave Abrahams wrote:

The docs for fast-import seem to imply that I can use "ls" to get the
SHA1 of a commit for which I have a mark:

       Reading from a named tree
           The <dataref> can be a mark reference (:<idnum>) or the full 40-byte
           SHA-1 of a Git tag, commit, or tree object, preexisting or waiting to
           be written. The path is relative to the top level of the tree named by
           <dataref>.

                       'ls' SP <dataref> SP <path> LF

       See filemodify above for a detailed description of <path>.

       Output uses the same format as git ls-tree <tree> -- <path>:

           <mode> SP ('blob' | 'tree' | 'commit') SP <dataref> HT <path> LF

       The <dataref> represents the blob, tree, or commit object at <path> and
                                                   ^^^^^^
       can be used in later cat-blob, filemodify, or ls commands.

but I can't get it to work.  It's not entirely clear it's supposed to
work.  What path would I pass?  Passing an empty path simply causes git
to report "missing ".

Which version of Git are you using?  I just tried this and get the error
"fatal: Empty path component found in input", which seems to be from
commit 178e1de (fast-import: don't allow 'ls' of path with empty
components, 2012-03-09), which is included in Git 1.7.9.5.

It seems to be slightly more complicated than that though, because after
allowing empty trees I get the "missing" message for the root tree.
This seems to be because its mode is 0 and not S_IFDIR.

With the patch below, things are working as I expect but I don't
understand why the mode of the root is not set correctly at this point.
Perhaps someone more familiar with fast-import will have some insight...

-- >8 --

diff --git a/fast-import.c b/fast-import.c
index 23f625f..bcce651 100644
--- a/fast-import.c
+++ b/fast-import.c

@@ -1626,6 +1626,15 @@ del_entry:
 	return 1;
 }
 
+static void copy_tree_entry(struct tree_entry *dst, struct tree_entry *src)
+{
+	memcpy(dst, src, sizeof(*dst));
+	if (src->tree && is_null_sha1(src->versions[1].sha1))
+		dst->tree = dup_tree_content(src->tree);
+	else
+		dst->tree = NULL;
+}
+
 static int tree_content_get(
 	struct tree_entry *root,
 	const char *p,

@@ -1651,11 +1660,7 @@ static int tree_content_get(
 		e = t->entries[i];
 		if (e->name->str_len == n && !strncmp_icase(p, e->name->str_dat, n)) {
 			if (!slash1) {
-				memcpy(leaf, e, sizeof(*leaf));
-				if (e->tree && is_null_sha1(e->versions[1].sha1))
-					leaf->tree = dup_tree_content(e->tree);
-				else
-					leaf->tree = NULL;
+				copy_tree_entry(leaf, e);
 				return 1;
 			}
 			if (!S_ISDIR(e->versions[1].mode))

@@ -3065,7 +3070,11 @@ static void parse_ls(struct branch *b)
 			die("Garbage after path in: %s", command_buf.buf);
 		p = uq.buf;
 	}
-	tree_content_get(root, p, &leaf);
+	if (!*p) {
+		copy_tree_entry(&leaf, root);
+		leaf.versions[1].mode = S_IFDIR;
+	} else
+		tree_content_get(root, p, &leaf);
 	/*
 	 * A directory in preparation would have a sha1 of zero
 	 * until it is saved.  Save, for simplicity.