threads / discuss / 61636

Add warning when v0 protocol is used/downgraded

Subject: Add warning when v0 protocol is used/downgraded

## tl;dr

5 messages between Jun 16, 2024 and Jun 19, 2024.

replies: 4people: 4as markdown or json

Devste Devste· Jun 16, 2024, 11:47 UTC · lore
- 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

Jonathan Nieder· Jun 16, 2024, 15:33 UTC · re: Devste Devste · lore

Re: Add warning when v0 protocol is used/downgraded

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

Junio C Hamano· Jun 17, 2024, 01:19 UTC · re: Jonathan Nieder · lore

Re: Add warning when v0 protocol is used/downgraded

Jonathan Nieder <jrnieder@gmail.com> writes:
Show 6 quoted lines
> 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.
Jeff King· Jun 18, 2024, 18:24 UTC · re: Jonathan Nieder · lore

Re: Add warning when v0 protocol is used/downgraded

On Sun, Jun 16, 2024 at 03:33:41PM +0000, Jonathan Nieder wrote:
Show 18 quoted lines
> 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.

Show 13 quoted lines
> > 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
Devste Devste· Jun 19, 2024, 03:37 UTC · re: Jeff King · lore

Re: Add warning when v0 protocol is used/downgraded

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:
Show 64 quoted lines
>
> 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

← back to recent threads