Subject: Re: [PATCH] archive: fix archive generation for empty trees

From: René Scharfe <[email-masked]>
Date: Fri, 09 Mar 2012 08:30:14 +0100

Am 09.03.2012 01:06, schrieb Brodie Rao:
> 2012/3/7 Jeff King<[email-masked]>:
>> On Wed, Mar 07, 2012 at 10:38:07PM -0800, Junio C Hamano wrote:
>>
>>>> ... However, prefix_pathspec does a lot of magic parsing;
>>>> it's unclear to me whether this is all in support of properly
>>>> adding the prefix, or if its side effects are important.
>>>
>>> These "magic" are for things like :(root)/path that will explicitly
>>> refuse the prefix when run from a subdirectory.
>>
>> Yeah, that was my impression. In that case, I would think we could get
>> rid of the get_pathspec call entirely, as it is purely about fixing-up
>> prefixes, and we know that we have none.
>
> Let me see if I've got this right: We're currently passing in ""/NULL
> to get_pathspec() because we handle the prefix beforehand in
> parse_treeish_args(). Once we get the tree object, every path is
> relative to it, so we don't need to continue using a prefix.
>
> Wouldn't it be better to continue using get_pathspec(), passing it the
> real prefix, and looking up tree entries relative to the top-level
> tree? The way it works now, you get weird behavior like this:
>
>    $ cd xdiff
>    $ git archive -v --format=tar HEAD ../t/t5000-tar-tree.sh>  /dev/null
>    fatal: '../t/t5000-tar-tree.sh' is outside repository
>    $ git archive -v --format=tar HEAD ..>  /dev/null
>    fatal: '..' is outside repository

With get_pathspec() gone you'd get this instead:

	fatal: path not found: ..

The message could be improved by mentioning the subdirectory and perhaps 
the tree, something like this:

	fatal: path not found in subdir 'xdiff' of 'HEAD': ..

However, you seem to expect such an invocation to succeed.  What should 
go into the created archive in that case and which pathes would be recorded?

René