On Sun, Dec 6, 2015 at 8:01 AM, Jeff King <peff@xxxxxxxx> wrote:
> 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.
It's probably worth adding a few comment lines about this. I did
search back commit history but did not get this.
> 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?
Yes. depth is only non-NULL when you pass --depth (or --unshallow).
quickfetch should happen when you fetch without those options.
> I wonder if we could just always set "depth = NULL" here.
--unshallow is essentially --depth=<max>, so I don't see why
--unshallow should be singled out here. We probably want to restore
depth back (or pass a flag to explicitly ignore the "depth" exception
in quickfetch). For multiple fetches, we spawn new commands so depth
being NULL does not harm. Just in case somebody tries to fetch a
couple more times in the same process in future.
--
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html