Re: [PATCH v2 2/2] fetch: send "refs/tags/" prefix upon CLI refspecs
- From
- Jonathan Tan <jonathantanmy@google.com>
- Date
- Jun 18, 2018, 19:47 UTC
- Message-ID
- <20180618194750.137341-1-jonathantanmy@google.com>
- In-Reply-To
- <CAGZ79kbB0Tv8wb_7j0=OdQqGU78KHp3JzuCeD01JTF4EHwOH0w@mail.gmail.com>
> okay. Thinking long term, we may want to introduce a capability that > can filter the tag space, e.g. "want-refs-since <date> refs/tags/*" > as a client directive as then the client only asks for new (newly > created/appearing) tags instead of all tags.
I don't think refs normally have a created/modified time.
Show 6 quoted lines
> > since tag refs are sent by > > default now, the test has been switched to using branch refs instead. > > That mismatches what I would expect to read below. > I would expect the client to always ask for refs/tags/* now and > instead of the server just giving them.
I don't understand why there is a mismatch, so I'll just answer all your questions. Yes, the client indeed always asks for refs/tags/* now.
By the server just giving them, do you mean (1) giving them based on what is being fetched, or (2) giving them as if refs/tags/* was sent? If (1), this is possible, but requires the server to precompute all the commits that would be sent (and this would be an overestimate, since negotiation has not yet taken place). If (2), I think it's better to let the client control this (since the client could just as easily want --no-tags, then sending tags would be wasted).
> Oh, the problem is in that other test to restrict the refs/tags > to *not* be sent?
The problem is that refs/tags/* is sent by default. Maybe adding --no-tags would work (I haven't tried it, though), but I think it's clearer for the test to just operate on branches, than to have --no-tags and let the reader wonder what --no-tags has to do with what's being tested.
> Maybe we can only ask for refs/tags/* if we do not have any > "refs/tags/" on the CLI: if I invoke "git fetch refs/tags/v1" > I would not get an advertisement for refs/tags/v2 but if I omit > all tags from the refspec, I'd get all its advertisements (v1+v2)
This would cause different behavior. For example, if I run "git fetch refs/heads/master refs/tags/foo", I would expect tag following to work even if the tag's name does not start with refs/tags/foo.
Show 16 quoted lines
> > @@ -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)) {
>
> Oh, I see. This always asks for refs/tags/ whereas before we only
> asked for them if there were *no* refspec given. Maybe we can
> change this to
>
> refspec_any_item_contains("refs/tags/")
>
> instead of always asking for the tags?
> (that function would need to be written; I guess for a short term bugfix
> this is good enough)Same answer as above.
> How is the tag following documented (i.e. when is the user least > surprised that we do not tag-follow all and unconditionally)?
It's documented under the --no-tags option in the man page for git fetch. I'm not sure what you mean by the user being surprised.