Re: Multiple fetches when unshallowing a shallow clone

[Jeff King <peff@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.