Re: fast-import bug?

[Dave Abrahams <dave@boostpro.com>]

on Sat Jun 22 2013, John Keeping <john-AT-keeping.me.uk> wrote:

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

,----[ git --version ]
| git version 1.8.3.1
`----

> I just tried this and get the error
> "fatal: Empty path component found in input", 

I get that too.

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

Yes, that's at least part of the issue.  I notice git-fast-import
rejects the root path "" for other commands, e.g. when used as the
source of a filecopy we get the same issue.  I also note that the docs
don't make it clear that quoting the path is mandatory if it might turn
out to be empty.

> It seems to be slightly more complicated than that though, because after
> allowing empty trees I get the "missing" message for the root tree.

Yeah, I've tried to patch Git to solve this but ran into that problem
and gave up.

> This seems to be because its mode is 0 and not S_IFDIR.

Aha.

> With the patch below, things are working as I expect 

Awesome; works for me, too!

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

Yeah... there's no bug tracker for Git, right?  So if nobody pays
attention to this thread, the problem will persist?

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

-- 
Dave Abrahams