Add warning when v0 protocol is used/downgraded

Add warning when v0 protocol is used/downgraded

[Devste Devste]

- When "git config protocol.version 2" is used, there is no
warning/message when the remote returns a response in v0 format. This
leads to any issues related to slow(er) git caused by old protocol use
being unnoticed, leading to wasted time debugging.

- v2 protocol has been standard since 2.26
https://github.com/git/git/blob/master/Documentation/RelNotes/2.26.0.txt#L101
However, there are still large providers (that rhyme with
Nuntucket...) that do not support it/have actively disabled it now
(years after the release)
Additionally, we encountered various self-hosted git servers that had
the protocol version restricted to 1 in their initial setup and this
being forgotten about. This led to unnecessarily slow fetches by their
users unaware of this problem, since git just silently accepts v1 (0)
protocol.

Since v2 is the default protocol, I think it would be expected that if
a non-default protocol reply is returned, there is a message shown to
the user (like e.g. the detached head warning) to make the user aware
that an outdated git protocol was used making git slow.

Otherwise, this (currently) leads to reports that e.g. git fetch is
getting slower and slower (as repo sizes increase over time). However
the issue in all cases we have handled so far, has always been that
the old protocol was used without the user being aware of it and not
an issue with git itself.

e.g.
If
protocol.version is not explicitly set or v2
and both the local and server git version are >=2.26
and the reply is not in v2 protocol format

Re: Add warning when v0 protocol is used/downgraded

[Jonathan Nieder]

Hi,

Devste Devste wrote:

> - When "git config protocol.version 2" is used, there is no
> warning/message when the remote returns a response in v0 format. This
> leads to any issues related to slow(er) git caused by old protocol use
> being unnoticed, leading to wasted time debugging.

Specifying protocol version is meant to be backward compatible, and
there are cases where the old protocol still needs to be used - for
example, if an SSH server doesn't support transmitting the
GIT_PROTOCOL environment variable, then having the fallback to v0 is
still useful there.

So I'd be concerned that printing the protocol version in the default
case would be overly disruptive for such cases.  This would be even
more so for protocol v2 for push, which doesn't exist yet - once it
exists, it wouldn't be great if all pushes using existing servers
produced an extra piece of noisy output. :)

That said, I'm sympathetic to the debugging use case you've described
here.  Do tools like GIT_TRACE_PACKET, GIT_TRACE2_EVENT, and "git
bugreport" produce the right information in these scenarios?  Would
"git fetch -v" (i.e., when the user has explicitly asked git to be
more verbose) be a good place to provide some additional diagnostic
output?

> If
> protocol.version is not explicitly set or v2
> and both the local and server git version are >=2.26
> and the reply is not in v2 protocol format

Interesting!  We haven't previously used the "agent" field (server
version) for anything other than logging it verbatim; I'd worry a bit
about getting into the same kind of mess as User-Agent parsing on the
web if we go that direction.  But I would expect the main obstacles to
updating protocol version support to be in (a) reimplementations of
git protocol rather than the standard git reference implementation and
(b) plumbing such as httpd and sshd around git, rather than git
itself.

Thanks and hope that helps,
Jonathan

Re: Add warning when v0 protocol is used/downgraded

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> Specifying protocol version is meant to be backward compatible, and
> there are cases where the old protocol still needs to be used - for
> ...
> more so for protocol v2 for push, which doesn't exist yet - once it
> exists, it wouldn't be great if all pushes using existing servers
> produced an extra piece of noisy output. :)

I do not think it is a great idea to add this as a warning, as if
something bad is happening, either.

I also agree that it is a legitimate debugging issue.  When the user
sees some symptom, after learning that the same symptom was reported
to be associated with the use of v2 on the Internet somewhere, it is
reasonable for the user to want to see what protocol is being used,
in order to debug the configuration, especially when the user thinks
they configured to use v0 (or vice versa)

So I am all for (1) adding to, if it is not already done, this kind
of information to the GIT_TRACE* output, and (2) advertising and
advocating GIT_TRACE* stuff as a useful debugging tool.

Thanks.

Re: Add warning when v0 protocol is used/downgraded

[Jeff King]

On Sun, Jun 16, 2024 at 03:33:41PM +0000, Jonathan Nieder wrote:

> Specifying protocol version is meant to be backward compatible, and
> there are cases where the old protocol still needs to be used - for
> example, if an SSH server doesn't support transmitting the
> GIT_PROTOCOL environment variable, then having the fallback to v0 is
> still useful there.
> 
> So I'd be concerned that printing the protocol version in the default
> case would be overly disruptive for such cases.  This would be even
> more so for protocol v2 for push, which doesn't exist yet - once it
> exists, it wouldn't be great if all pushes using existing servers
> produced an extra piece of noisy output. :)
> 
> That said, I'm sympathetic to the debugging use case you've described
> here.  Do tools like GIT_TRACE_PACKET, GIT_TRACE2_EVENT, and "git
> bugreport" produce the right information in these scenarios?  Would
> "git fetch -v" (i.e., when the user has explicitly asked git to be
> more verbose) be a good place to provide some additional diagnostic
> output?

You can certainly distinguish v2 with GIT_TRACE_PACKET; the first line
of the v2 response is "version 2". But recognizing v0 as "not v2" is
harder for the layman. Plus it generates a ton of otherwise confusing
output. I do agree that "fetch -v" might be a reasonable spot for this.

> > If
> > protocol.version is not explicitly set or v2
> > and both the local and server git version are >=2.26
> > and the reply is not in v2 protocol format
> 
> Interesting!  We haven't previously used the "agent" field (server
> version) for anything other than logging it verbatim; I'd worry a bit
> about getting into the same kind of mess as User-Agent parsing on the
> web if we go that direction.  But I would expect the main obstacles to
> updating protocol version support to be in (a) reimplementations of
> git protocol rather than the standard git reference implementation and
> (b) plumbing such as httpd and sshd around git, rather than git
> itself.

Yeah, I'd really prefer if we can keep "agent" as purely informative, at
least by default. But having a debug/verbose mode that says "looks like
you should both support v2, but it wasn't used for some reason" seems
reasonable to me.

We don't distinguish right now between the default behavior and
explicitly setting "protocol.version" to "2". We could perhaps take the
latter as a hint to be a bit more chatty about falling back to v0.
 
I do think that v2 isn't going to make that big a difference in many
cases. For most clients the main benefit is the reduced advertisement,
but that's probably only meaningful if the server has a ton of refs
(often refs/changes or refs/pull, since you end up seeing all of "heads"
and "tags" anyway). There are other features (like fetching individual
blobs for partial clones) that some clients might care about, and where
finding the v0/v2 distinction would be valuable for debugging. But
complaining any time we fall back to v0 seems a bit excessive to me.

There may be some error messages we could improve there (e.g., if the
server comes back with "not our ref" and v0 is in use, we might give a
hint that the protocol version is the problem).

-Peff

Re: Add warning when v0 protocol is used/downgraded

[Devste Devste]

At least for cases where there is a difference expected in output.
e.g. we deal mostly in huge monorepos and there is a massive (= 2
seconds per fetch!) difference between v0 and v2, since v0 returns
tons of data in a fetch that you don't get included by default in v2.


On Tue, 18 Jun 2024 at 20:24, Jeff King <peff@peff.net> wrote:
>
> On Sun, Jun 16, 2024 at 03:33:41PM +0000, Jonathan Nieder wrote:
>
> > Specifying protocol version is meant to be backward compatible, and
> > there are cases where the old protocol still needs to be used - for
> > example, if an SSH server doesn't support transmitting the
> > GIT_PROTOCOL environment variable, then having the fallback to v0 is
> > still useful there.
> >
> > So I'd be concerned that printing the protocol version in the default
> > case would be overly disruptive for such cases.  This would be even
> > more so for protocol v2 for push, which doesn't exist yet - once it
> > exists, it wouldn't be great if all pushes using existing servers
> > produced an extra piece of noisy output. :)
> >
> > That said, I'm sympathetic to the debugging use case you've described
> > here.  Do tools like GIT_TRACE_PACKET, GIT_TRACE2_EVENT, and "git
> > bugreport" produce the right information in these scenarios?  Would
> > "git fetch -v" (i.e., when the user has explicitly asked git to be
> > more verbose) be a good place to provide some additional diagnostic
> > output?
>
> You can certainly distinguish v2 with GIT_TRACE_PACKET; the first line
> of the v2 response is "version 2". But recognizing v0 as "not v2" is
> harder for the layman. Plus it generates a ton of otherwise confusing
> output. I do agree that "fetch -v" might be a reasonable spot for this.
>
> > > If
> > > protocol.version is not explicitly set or v2
> > > and both the local and server git version are >=2.26
> > > and the reply is not in v2 protocol format
> >
> > Interesting!  We haven't previously used the "agent" field (server
> > version) for anything other than logging it verbatim; I'd worry a bit
> > about getting into the same kind of mess as User-Agent parsing on the
> > web if we go that direction.  But I would expect the main obstacles to
> > updating protocol version support to be in (a) reimplementations of
> > git protocol rather than the standard git reference implementation and
> > (b) plumbing such as httpd and sshd around git, rather than git
> > itself.
>
> Yeah, I'd really prefer if we can keep "agent" as purely informative, at
> least by default. But having a debug/verbose mode that says "looks like
> you should both support v2, but it wasn't used for some reason" seems
> reasonable to me.
>
> We don't distinguish right now between the default behavior and
> explicitly setting "protocol.version" to "2". We could perhaps take the
> latter as a hint to be a bit more chatty about falling back to v0.
>
> I do think that v2 isn't going to make that big a difference in many
> cases. For most clients the main benefit is the reduced advertisement,
> but that's probably only meaningful if the server has a ton of refs
> (often refs/changes or refs/pull, since you end up seeing all of "heads"
> and "tags" anyway). There are other features (like fetching individual
> blobs for partial clones) that some clients might care about, and where
> finding the v0/v2 distinction would be valuable for debugging. But
> complaining any time we fall back to v0 seems a bit excessive to me.
>
> There may be some error messages we could improve there (e.g., if the
> server comes back with "not our ref" and v0 is in use, we might give a
> hint that the protocol version is the problem).
>
> -Peff