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
