Hi, Jonathan Tan wrote:
> --- a/builtin/fetch.c > +++ b/builtin/fetch.c > @@ -359,7 +359,7 @@ static struct ref *get_ref_map(struct transport > *transport, > refspec_ref_prefixes(&transport->remote->fetch, &ref_prefixes); > > if (ref_prefixes.argc && > - (tags == TAGS_SET || (tags == TAGS_DEFAULT && !rs->nr))) { > + (tags == TAGS_SET || tags == TAGS_DEFAULT)) { Makes a lot of sense. This means that if I run git fetch origin master then the ls-refs step will now include refs/tags/* in addition to refs/heads/master, erasing some of the gain that protocol v2 brought. But since the user didn't pass --no-tags, that is what the user asked for. One could argue that the user didn't *mean* to ask for that. In other words, I wonder if the following would better match the user intent: - introduce a new tagopt value, --tags=auto or something, that means that tags are only followed when no refspec is supplied. In other words, git fetch --tags=auto <remote> would perform tag auto-following, while git fetch --tags=auto <remote> <branch> would act like fetch --no-tags. - make --tags=auto the default for new clones. What do you think? Some related thoughts on tag auto-following: It would be tempting to not use tag auto-following at all in the default configuration and just include refs/tags/*:refs/tags/* in the default clone refspec. Unfortunately, that would be a pretty significant behavior change from the present behavior, since - it would fetch tags pointing to objects unrelated to the fetched history - if we have ref-in-want *with* pattern support, it would mean we try to fetch all tags on every fetch. As discussed in the thread [1], this is expensive and difficult to get right. - if an unannotated tag is fast-forwarded, it would allow existing tags to be updated - it interacts poorly with --prune - ... I actually suspect it's a good direction in the long term, but until we address those downsides (see also the occasional discussions on tag namespaces), we're not ready for it. The --tags=auto described above is a sort of half approximation. Anyway, this is all a long way of saying that despite the performance regression I believe this patch heads in the right direction. The performance regression is inevitable in what the user is (unintentionally) requesting, and we can address it separately by changing the defaults to better match what the user intends to request. Thanks, Jonathan [1] https://public-inbox.org/git/20180605175144.4225-1-bmw...@google.com/