Re: [PATCH v2 2/2] fetch: send "refs/tags/" prefix upon CLI refspecs

Brandon Williams [off-list ref] writes:

On 07/09, Junio C Hamano wrote:

quoted

Brandon Williams [off-list ref] writes:

quoted

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.

I thought JTan's [ref]
showed us a way forward to reduce the overhead even further without
having to be sad ;-).  Am I mistaken?

That's true, what Jonathan mentioned there would avoid having to send
"refs/tags/*" when requesting the refs.  The question is do we wait on
implementing that functionality (as another feature to fetch) or do we
fix this now?

It's not like the earlier v2 protocol used to be super efficient and
correct, whose performance benefit is destroyed with this "fix"
irreversibly.  It was a fast but sometimes incorrect implementation,
and we'd protect users of the still-under-development feature with
these two patches while updating the protocol further in order to
become truly efficient and correct, so I do not see it a wrong move
to take a hit like this patch does in the meantime.

What I would see a wrong move would be to leave it very long as-is
after we apply this fix, and declare to flip the default not to
follow tags, using the performance hit as an excuse.

So I do not care too deeply either way, whether we wait to regain
efficiency or taking this safe fix as the first step, as long as it
is fixed in the longer term.  I had an impression that the sentiment
in the thread was that it was OK to accept the inefficiency for now
and fix it later, and I am personally fine with that approach.