Re: [PATCH 5/5] pack-objects: walk tag chains for --include-tag

From: Jeff King <peff@peff.net>
Date: Sep 5, 2016, 21:59 UTC
Message-ID: <20160905215939.hriu6ev3m332qhp6@sigill.intra.peff.net>
In-Reply-To: <20160905215226.m6vv2tk5pe2qt4ui@sigill.intra.peff.net>

On Mon, Sep 05, 2016 at 05:52:26PM -0400, Jeff King wrote:

Show 11 quoted lines

> When pack-objects is given --include-tag, it peels each tag
> ref down to a non-tag object, and if that non-tag object is
> going to be packed, we include the tag, too. But what
> happens if we have a chain of tags (e.g., tag "A" points to
> tag "B", which points to commit "C")?
> 
> We'll peel down to "C" and realize that we want to include
> tag "A", but we do not ever consider tag "B", leading to a
> broken pack (assuming "B" was not otherwise selected).
> Instead, we have to walk the whole chain, adding any tags we
> find to the pack.

So technically one might argue that this pack isn't "broken", in that it _is_ a valid pack. It's just that it doesn't contain what the user was asking for.

As explained further in the commit message, "fetch" is robust to this, because it does a real connectivity check and follow-on fetch before writing anything it thinks it got via include-tag. So perhaps one could argue that pack-objects is correct; include-tag is best-effort, and it is the client's job to make sure it has everything it needs. And that would mean the bug is in git-clone, which should be doing the connectivity check and follow-on fetch.

I dunno. This seems like the most elegant place to fix it, though it does mean that pack-objects will go to some slight extra work when auto-packing a tag (it has to parse the tag to find out whether it's a chain). I'm doubt it matters much in practice.

-Peff