Re: Multiple fetches when unshallowing a shallow clone
- From
Jeff King <peff@peff.net>
- Date
- Dec 6, 2015, 07:01 UTC
- Message-ID
- <20151206070144.GA17902@sigill.intra.peff.net>
- In-Reply-To
- <20151206063718.GA549@sigill.intra.peff.net>
On Sun, Dec 06, 2015 at 01:37:18AM -0500, Jeff King wrote:
> And indeed, replacing the logic with what I wrote does make the backfill > go away in my test case. But it's so far from what is there that I feel > like I must be missing something.
I think one thing I was missing is that we need to just grab the _object_, but we need to realize that the ref needs updating[1]. So we cannot skip backfill of any tag that we do not already have, even if we already have the tag object.
Which made me wonder why this:
git init parent && git -C parent commit --allow-empty -m one && git clone parent child && git -C parent commit --allow-empty -m two && git -C parent tag -m mytag foo && git -C parent commit --allow-empty -m three && git -C child fetch
does not appear to need to backfill to pick up refs/tags/foo. But it does. It's just that it hits the quickfetch() code path and does not have to ask the other side for a pack. And that explains why it does hit in the --shallow case: we explicitly disable quickfetch in such cases.
For the unshallow case, of course we could use it (but only for the second, backfill fetch). Something like this seems to work for me:
diff --git a/builtin/fetch.c b/builtin/fetch.c index ed84963..b33b90f 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -881,6 +881,8 @@ static void backfill_tags(struct transport *transport, struct ref *ref_map) transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL); transport_set_option(transport, TRANS_OPT_DEPTH, "0"); + if (unshallow) + depth = NULL; fetch_refs(transport, ref_map); if (gsecondary) { But I admit I am not at all confident that it doesn't cause other problems, or that it covers all cases. Even in a shallow repo, we should be able to quickfetch individual tags, shouldn't we? I wonder if we could just always set "depth = NULL" here. -Peff [1] I'm still puzzled why find_non_local_tags uses has_sha1_file() on the tag object at all, then.