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