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

From: Brodie Rao <brodie@sf.io>
Date: Mar 9, 2012, 00:06 UTC
Message-ID: <CAJjwD-2=pEfk2WQ2=cKy8eUSwbx8y86jEo_kyiQWsxVTqVFQEg@mail.gmail.com>
In-Reply-To: <20120308071559.GF7643@sigill.intra.peff.net>

2012/3/7 Jeff King <peff@peff.net>:

Show 12 quoted lines

> 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