On Wed, Sep 07, 2016 at 11:49:28AM -0700, Junio C Hamano wrote:
> Jeff King <p...@peff.net> writes:
> > 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 think that is probably a more technically correct interpretation
> of the history.
> I think upgrading "best-effort" to "guarantee" like you did is a
> right approach nevertheless. I think the "best-effort" we initially
> did was merely us being lazy.
Yeah, after sleeping on it, the conclusion I came to was that it does
not _hurt_ to have include-tag be a bit more careful.
I also wondered about the corner case I noted in the commit message. If
you have a tag chain of A->B->C, and you already have "C" (a commit),
but are fetching "B" (a tag), then include-tag does not notice "A".
That's OK for git-fetch. It will collect "A" during its backfill phase
(not because of "B" at all, but because it knows that "A" eventually
peels to "C", which it already has). "git-clone" does not have a
backfill, of course. But neither can it "already have" a commit. So
either we get "C" as part of the clone (in which case include-tag will
include "A"), or it does not (in which case we cannot be getting "B"
either, because "C" is reachable from it).
And of course that's only when single-branch is in use. Normally
git-clone just grabs all the tags blindly. :)
So I think everything Just Works after my patch, though we do still rely
on fetch backfill to pick up some obscure cases.