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-bmwill@xxxxxxxxxx/
