{"thread":{"id":"61636","subject":"Add warning when v0 protocol is used/downgraded","startedAt":"2024-06-16T11:47:55Z","lastAt":"2024-06-19T03:38:04Z","messageCount":5,"participants":["Devste Devste","Jonathan Nieder","Junio C Hamano","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"497221","messageId":"CANM0SV3CQPRyJCDanB8JFpkAMwuoo-mg3A=_L743_GXJtoFtQA@mail.gmail.com","threadId":"61636","inReplyTo":null,"subject":"Add warning when v0 protocol is used/downgraded","fromName":"Devste Devste","fromEmail":"devstemail@gmail.com","sentAt":"2024-06-16T11:47:41Z","receivedAt":"2024-06-16T11:47:55Z","isPatch":false,"sender":{"key":"devstemail@gmail.com","avatar":null},"body":"- When \"git config protocol.version 2\" is used, there is no\nwarning/message when the remote returns a response in v0 format. This\nleads to any issues related to slow(er) git caused by old protocol use\nbeing unnoticed, leading to wasted time debugging.\n\n- v2 protocol has been standard since 2.26\nhttps://github.com/git/git/blob/master/Documentation/RelNotes/2.26.0.txt#L101\nHowever, there are still large providers (that rhyme with\nNuntucket...) that do not support it/have actively disabled it now\n(years after the release)\nAdditionally, we encountered various self-hosted git servers that had\nthe protocol version restricted to 1 in their initial setup and this\nbeing forgotten about. This led to unnecessarily slow fetches by their\nusers unaware of this problem, since git just silently accepts v1 (0)\nprotocol.\n\nSince v2 is the default protocol, I think it would be expected that if\na non-default protocol reply is returned, there is a message shown to\nthe user (like e.g. the detached head warning) to make the user aware\nthat an outdated git protocol was used making git slow.\n\nOtherwise, this (currently) leads to reports that e.g. git fetch is\ngetting slower and slower (as repo sizes increase over time). However\nthe issue in all cases we have handled so far, has always been that\nthe old protocol was used without the user being aware of it and not\nan issue with git itself.\n\ne.g.\nIf\nprotocol.version is not explicitly set or v2\nand both the local and server git version are >=2.26\nand the reply is not in v2 protocol format\n"},{"id":"497225","messageId":"Zm8EqOfc_v4KBVVK@google.com","threadId":"61636","inReplyTo":"CANM0SV3CQPRyJCDanB8JFpkAMwuoo-mg3A=_L743_GXJtoFtQA@mail.gmail.com","subject":"Re: Add warning when v0 protocol is used/downgraded","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2024-06-16T15:33:41Z","receivedAt":"2024-06-16T15:33:45Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nDevste Devste wrote:\n\n> - When \"git config protocol.version 2\" is used, there is no\n> warning/message when the remote returns a response in v0 format. This\n> leads to any issues related to slow(er) git caused by old protocol use\n> being unnoticed, leading to wasted time debugging.\n\nSpecifying protocol version is meant to be backward compatible, and\nthere are cases where the old protocol still needs to be used - for\nexample, if an SSH server doesn't support transmitting the\nGIT_PROTOCOL environment variable, then having the fallback to v0 is\nstill useful there.\n\nSo I'd be concerned that printing the protocol version in the default\ncase would be overly disruptive for such cases.  This would be even\nmore so for protocol v2 for push, which doesn't exist yet - once it\nexists, it wouldn't be great if all pushes using existing servers\nproduced an extra piece of noisy output. :)\n\nThat said, I'm sympathetic to the debugging use case you've described\nhere.  Do tools like GIT_TRACE_PACKET, GIT_TRACE2_EVENT, and \"git\nbugreport\" produce the right information in these scenarios?  Would\n\"git fetch -v\" (i.e., when the user has explicitly asked git to be\nmore verbose) be a good place to provide some additional diagnostic\noutput?\n\n> If\n> protocol.version is not explicitly set or v2\n> and both the local and server git version are >=2.26\n> and the reply is not in v2 protocol format\n\nInteresting!  We haven't previously used the \"agent\" field (server\nversion) for anything other than logging it verbatim; I'd worry a bit\nabout getting into the same kind of mess as User-Agent parsing on the\nweb if we go that direction.  But I would expect the main obstacles to\nupdating protocol version support to be in (a) reimplementations of\ngit protocol rather than the standard git reference implementation and\n(b) plumbing such as httpd and sshd around git, rather than git\nitself.\n\nThanks and hope that helps,\nJonathan\n"},{"id":"497230","messageId":"xmqqjziobc2w.fsf@gitster.g","threadId":"61636","inReplyTo":"Zm8EqOfc_v4KBVVK@google.com","subject":"Re: Add warning when v0 protocol is used/downgraded","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-06-17T01:19:19Z","receivedAt":"2024-06-17T01:19:27Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> Specifying protocol version is meant to be backward compatible, and\n> there are cases where the old protocol still needs to be used - for\n> ...\n> more so for protocol v2 for push, which doesn't exist yet - once it\n> exists, it wouldn't be great if all pushes using existing servers\n> produced an extra piece of noisy output. :)\n\nI do not think it is a great idea to add this as a warning, as if\nsomething bad is happening, either.\n\nI also agree that it is a legitimate debugging issue.  When the user\nsees some symptom, after learning that the same symptom was reported\nto be associated with the use of v2 on the Internet somewhere, it is\nreasonable for the user to want to see what protocol is being used,\nin order to debug the configuration, especially when the user thinks\nthey configured to use v0 (or vice versa)\n\nSo I am all for (1) adding to, if it is not already done, this kind\nof information to the GIT_TRACE* output, and (2) advertising and\nadvocating GIT_TRACE* stuff as a useful debugging tool.\n\nThanks.\n"},{"id":"497296","messageId":"20240618182415.GA178291@coredump.intra.peff.net","threadId":"61636","inReplyTo":"Zm8EqOfc_v4KBVVK@google.com","subject":"Re: Add warning when v0 protocol is used/downgraded","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-06-18T18:24:15Z","receivedAt":"2024-06-18T18:24:22Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Jun 16, 2024 at 03:33:41PM +0000, Jonathan Nieder wrote:\n\n> Specifying protocol version is meant to be backward compatible, and\n> there are cases where the old protocol still needs to be used - for\n> example, if an SSH server doesn't support transmitting the\n> GIT_PROTOCOL environment variable, then having the fallback to v0 is\n> still useful there.\n> \n> So I'd be concerned that printing the protocol version in the default\n> case would be overly disruptive for such cases.  This would be even\n> more so for protocol v2 for push, which doesn't exist yet - once it\n> exists, it wouldn't be great if all pushes using existing servers\n> produced an extra piece of noisy output. :)\n> \n> That said, I'm sympathetic to the debugging use case you've described\n> here.  Do tools like GIT_TRACE_PACKET, GIT_TRACE2_EVENT, and \"git\n> bugreport\" produce the right information in these scenarios?  Would\n> \"git fetch -v\" (i.e., when the user has explicitly asked git to be\n> more verbose) be a good place to provide some additional diagnostic\n> output?\n\nYou can certainly distinguish v2 with GIT_TRACE_PACKET; the first line\nof the v2 response is \"version 2\". But recognizing v0 as \"not v2\" is\nharder for the layman. Plus it generates a ton of otherwise confusing\noutput. I do agree that \"fetch -v\" might be a reasonable spot for this.\n\n> > If\n> > protocol.version is not explicitly set or v2\n> > and both the local and server git version are >=2.26\n> > and the reply is not in v2 protocol format\n> \n> Interesting!  We haven't previously used the \"agent\" field (server\n> version) for anything other than logging it verbatim; I'd worry a bit\n> about getting into the same kind of mess as User-Agent parsing on the\n> web if we go that direction.  But I would expect the main obstacles to\n> updating protocol version support to be in (a) reimplementations of\n> git protocol rather than the standard git reference implementation and\n> (b) plumbing such as httpd and sshd around git, rather than git\n> itself.\n\nYeah, I'd really prefer if we can keep \"agent\" as purely informative, at\nleast by default. But having a debug/verbose mode that says \"looks like\nyou should both support v2, but it wasn't used for some reason\" seems\nreasonable to me.\n\nWe don't distinguish right now between the default behavior and\nexplicitly setting \"protocol.version\" to \"2\". We could perhaps take the\nlatter as a hint to be a bit more chatty about falling back to v0.\n \nI do think that v2 isn't going to make that big a difference in many\ncases. For most clients the main benefit is the reduced advertisement,\nbut that's probably only meaningful if the server has a ton of refs\n(often refs/changes or refs/pull, since you end up seeing all of \"heads\"\nand \"tags\" anyway). There are other features (like fetching individual\nblobs for partial clones) that some clients might care about, and where\nfinding the v0/v2 distinction would be valuable for debugging. But\ncomplaining any time we fall back to v0 seems a bit excessive to me.\n\nThere may be some error messages we could improve there (e.g., if the\nserver comes back with \"not our ref\" and v0 is in use, we might give a\nhint that the protocol version is the problem).\n\n-Peff\n"},{"id":"497325","messageId":"CANM0SV2N3-uRbPG=VuEuUhL_BdgbCkoWPxzhwoa_g2s7ejujvA@mail.gmail.com","threadId":"61636","inReplyTo":"20240618182415.GA178291@coredump.intra.peff.net","subject":"Re: Add warning when v0 protocol is used/downgraded","fromName":"Devste Devste","fromEmail":"devstemail@gmail.com","sentAt":"2024-06-19T03:37:51Z","receivedAt":"2024-06-19T03:38:04Z","isPatch":false,"sender":{"key":"devstemail@gmail.com","avatar":null},"body":"At least for cases where there is a difference expected in output.\ne.g. we deal mostly in huge monorepos and there is a massive (= 2\nseconds per fetch!) difference between v0 and v2, since v0 returns\ntons of data in a fetch that you don't get included by default in v2.\n\n\nOn Tue, 18 Jun 2024 at 20:24, Jeff King <peff@peff.net> wrote:\n>\n> On Sun, Jun 16, 2024 at 03:33:41PM +0000, Jonathan Nieder wrote:\n>\n> > Specifying protocol version is meant to be backward compatible, and\n> > there are cases where the old protocol still needs to be used - for\n> > example, if an SSH server doesn't support transmitting the\n> > GIT_PROTOCOL environment variable, then having the fallback to v0 is\n> > still useful there.\n> >\n> > So I'd be concerned that printing the protocol version in the default\n> > case would be overly disruptive for such cases.  This would be even\n> > more so for protocol v2 for push, which doesn't exist yet - once it\n> > exists, it wouldn't be great if all pushes using existing servers\n> > produced an extra piece of noisy output. :)\n> >\n> > That said, I'm sympathetic to the debugging use case you've described\n> > here.  Do tools like GIT_TRACE_PACKET, GIT_TRACE2_EVENT, and \"git\n> > bugreport\" produce the right information in these scenarios?  Would\n> > \"git fetch -v\" (i.e., when the user has explicitly asked git to be\n> > more verbose) be a good place to provide some additional diagnostic\n> > output?\n>\n> You can certainly distinguish v2 with GIT_TRACE_PACKET; the first line\n> of the v2 response is \"version 2\". But recognizing v0 as \"not v2\" is\n> harder for the layman. Plus it generates a ton of otherwise confusing\n> output. I do agree that \"fetch -v\" might be a reasonable spot for this.\n>\n> > > If\n> > > protocol.version is not explicitly set or v2\n> > > and both the local and server git version are >=2.26\n> > > and the reply is not in v2 protocol format\n> >\n> > Interesting!  We haven't previously used the \"agent\" field (server\n> > version) for anything other than logging it verbatim; I'd worry a bit\n> > about getting into the same kind of mess as User-Agent parsing on the\n> > web if we go that direction.  But I would expect the main obstacles to\n> > updating protocol version support to be in (a) reimplementations of\n> > git protocol rather than the standard git reference implementation and\n> > (b) plumbing such as httpd and sshd around git, rather than git\n> > itself.\n>\n> Yeah, I'd really prefer if we can keep \"agent\" as purely informative, at\n> least by default. But having a debug/verbose mode that says \"looks like\n> you should both support v2, but it wasn't used for some reason\" seems\n> reasonable to me.\n>\n> We don't distinguish right now between the default behavior and\n> explicitly setting \"protocol.version\" to \"2\". We could perhaps take the\n> latter as a hint to be a bit more chatty about falling back to v0.\n>\n> I do think that v2 isn't going to make that big a difference in many\n> cases. For most clients the main benefit is the reduced advertisement,\n> but that's probably only meaningful if the server has a ton of refs\n> (often refs/changes or refs/pull, since you end up seeing all of \"heads\"\n> and \"tags\" anyway). There are other features (like fetching individual\n> blobs for partial clones) that some clients might care about, and where\n> finding the v0/v2 distinction would be valuable for debugging. But\n> complaining any time we fall back to v0 seems a bit excessive to me.\n>\n> There may be some error messages we could improve there (e.g., if the\n> server comes back with \"not our ref\" and v0 is in use, we might give a\n> hint that the protocol version is the problem).\n>\n> -Peff\n"}]}