On 07/09, Jonathan Nieder wrote:
> 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.
I agree with this observation, though I'm a bit sad about it. I think
that having tag auto-following the default is a little confusing (and
hurts perf[1] when using proto v2) but since thats the way its always been
we'll have to live with it for now. I think exploring changing the
defaults might be a good thing to do in the future. But for now we've
had enough people comment on this lacking functionality that we should
fix it.
[1] Thankfully it doesn't completely undo what protocol v2 did, as we
still are able to eliminate refs/changes or refs/pull or other various
refs which significantly add to the number of refs advertised during
fetch.
--
Brandon Williams
