{"thread":{"id":"54300","subject":"[PATCH] protocol: re-enable v2 protocol by default","startedAt":"2020-09-25T18:34:39Z","lastAt":"2020-09-25T21:24:35Z","messageCount":3,"participants":["Jeff King","Junio C Hamano","jrnieder@gmail.com"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"406375","messageId":"20200925183436.GA659633@coredump.intra.peff.net","threadId":"54300","inReplyTo":null,"subject":"[PATCH] protocol: re-enable v2 protocol by default","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-25T18:34:36Z","receivedAt":"2020-09-25T18:34:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Protocol v2 became the default in v2.26.0 via 684ceae32d (fetch: default\nto protocol version 2, 2019-12-23). More widespread use turned up a\nregression in negotiation. That was fixed in v2.27.0 via 4fa3f00abb\n(fetch-pack: in protocol v2, in_vain only after ACK, 2020-04-27), but we\nalso reverted the default to v0 as a precuation in 11c7f2a30b (Revert\n\"fetch: default to protocol version 2\", 2020-04-22).\n\nIn v2.28.0, we re-enabled it for experimental users with 3697caf4b9\n(config: let feature.experimental imply protocol.version=2, 2020-05-20)\nand haven't heard any complaints. v2.28 has only been out for 2 months,\nbut I'd generally expect people turning on feature.experimental to also\nstay pretty up-to-date. So we're not likely to collect much more data by\nwaiting. In addition, we have no further reports from people running\nv2.26.0, and of course some people have been setting protocol.version\nmanually for ages.\n\nLet's move forward with v2 as the default again. It's possible there are\nstill lurking bugs, but we won't know until it gets more widespread use.\nAnd we can find and squash them just like any other bug at this point.\n\nSigned-off-by: Jeff King \n---\nAfter seeing yet another round of \"if you use the v2 protocol, all of\nyour issues fetching a non-tip object for submodules or CI will go away\"\nadvice today, I was reminded that we haven't picked up the question\nagain of when we should look at v2 becoming the default again.\n\n Documentation/config/feature.txt | 4 ----\n Documentation/config/protocol.txt | 3 +--\n protocol.c | 6 +-----\n 3 files changed, 2 insertions(+), 11 deletions(-)\n\ndiff --git a/Documentation/config/feature.txt b/Documentation/config/feature.txt\nindex c0cbf2bb1c..cdecd04e5b 100644\n--- a/Documentation/config/feature.txt\n+++ b/Documentation/config/feature.txt\n@@ -14,10 +14,6 @@ feature.experimental::\n +\n * `fetch.negotiationAlgorithm=skipping` may improve fetch negotiation times by\n skipping more commits at a time, reducing the number of round trips.\n-+\n-* `protocol.version=2` speeds up fetches from repositories with many refs by\n-allowing the client to specify which refs to list before the server lists\n-them.\n \n feature.manyFiles::\n \tEnable config options that optimize for repos with many files in the\ndiff --git a/Documentation/config/protocol.txt b/Documentation/config/protocol.txt\nindex c46e9b3d00..756591d77b 100644\n--- a/Documentation/config/protocol.txt\n+++ b/Documentation/config/protocol.txt\n@@ -48,8 +48,7 @@ protocol.version::\n \tIf set, clients will attempt to communicate with a server\n \tusing the specified protocol version. If the server does\n \tnot support it, communication falls back to version 0.\n-\tIf unset, the default is `0`, unless `feature.experimental`\n-\tis enabled, in which case the default is `2`.\n+\tIf unset, the default is `2`.\n \tSupported versions:\n +\n --\ndiff --git a/protocol.c b/protocol.c\nindex 8d964fc65e..052d7edbb9 100644\n--- a/protocol.c\n+++ b/protocol.c\n@@ -17,7 +17,6 @@ static enum protocol_version parse_protocol_version(const char *value)\n enum protocol_version get_protocol_version_config(void)\n {\n \tconst char *value;\n-\tint val;\n \tconst char *git_test_k = \"GIT_TEST_PROTOCOL_VERSION\";\n \tconst char *git_test_v;\n \n@@ -31,9 +30,6 @@ enum protocol_version get_protocol_version_config(void)\n \t\treturn version;\n \t}\n \n-\tif (!git_config_get_bool(\"feature.experimental\", &val) && val)\n-\t\treturn protocol_v2;\n-\n \tgit_test_v = getenv(git_test_k);\n \tif (git_test_v && *git_test_v) {\n \t\tenum protocol_version env = parse_protocol_version(git_test_v);\n@@ -43,7 +39,7 @@ enum protocol_version get_protocol_version_config(void)\n \t\treturn env;\n \t}\n \n-\treturn protocol_v0;\n+\treturn protocol_v2;\n }\n \n enum protocol_version determine_protocol_version_server(void)\n-- \n2.28.0.1085.g44a0350633\n"},{"id":"406377","messageId":"xmqq4knllmwr.fsf@gitster.c.googlers.com","threadId":"54300","inReplyTo":"20200925183436.GA659633@coredump.intra.peff.net","subject":"Re: [PATCH] protocol: re-enable v2 protocol by default","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-25T18:41:08Z","receivedAt":"2020-09-25T18:41:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Let's move forward with v2 as the default again. It's possible there are\n> still lurking bugs, but we won't know until it gets more widespread use.\n> And we can find and squash them just like any other bug at this point.\n\nAck. Thanks.\n\n>\n> Signed-off-by: Jeff King \n> ---\n> After seeing yet another round of \"if you use the v2 protocol, all of\n> your issues fetching a non-tip object for submodules or CI will go away\"\n> advice today, I was reminded that we haven't picked up the question\n> again of when we should look at v2 becoming the default again.\n>\n> Documentation/config/feature.txt | 4 ----\n> Documentation/config/protocol.txt | 3 +--\n> protocol.c | 6 +-----\n> 3 files changed, 2 insertions(+), 11 deletions(-)\n>\n> diff --git a/Documentation/config/feature.txt b/Documentation/config/feature.txt\n> index c0cbf2bb1c..cdecd04e5b 100644\n> --- a/Documentation/config/feature.txt\n> +++ b/Documentation/config/feature.txt\n> @@ -14,10 +14,6 @@ feature.experimental::\n> +\n> * `fetch.negotiationAlgorithm=skipping` may improve fetch negotiation times by\n> skipping more commits at a time, reducing the number of round trips.\n> -+\n> -* `protocol.version=2` speeds up fetches from repositories with many refs by\n> -allowing the client to specify which refs to list before the server lists\n> -them.\n> \n> feature.manyFiles::\n> \tEnable config options that optimize for repos with many files in the\n> diff --git a/Documentation/config/protocol.txt b/Documentation/config/protocol.txt\n> index c46e9b3d00..756591d77b 100644\n> --- a/Documentation/config/protocol.txt\n> +++ b/Documentation/config/protocol.txt\n> @@ -48,8 +48,7 @@ protocol.version::\n> \tIf set, clients will attempt to communicate with a server\n> \tusing the specified protocol version. If the server does\n> \tnot support it, communication falls back to version 0.\n> -\tIf unset, the default is `0`, unless `feature.experimental`\n> -\tis enabled, in which case the default is `2`.\n> +\tIf unset, the default is `2`.\n> \tSupported versions:\n> +\n> --\n> diff --git a/protocol.c b/protocol.c\n> index 8d964fc65e..052d7edbb9 100644\n> --- a/protocol.c\n> +++ b/protocol.c\n> @@ -17,7 +17,6 @@ static enum protocol_version parse_protocol_version(const char *value)\n> enum protocol_version get_protocol_version_config(void)\n> {\n> \tconst char *value;\n> -\tint val;\n> \tconst char *git_test_k = \"GIT_TEST_PROTOCOL_VERSION\";\n> \tconst char *git_test_v;\n> \n> @@ -31,9 +30,6 @@ enum protocol_version get_protocol_version_config(void)\n> \t\treturn version;\n> \t}\n> \n> -\tif (!git_config_get_bool(\"feature.experimental\", &val) && val)\n> -\t\treturn protocol_v2;\n> -\n> \tgit_test_v = getenv(git_test_k);\n> \tif (git_test_v && *git_test_v) {\n> \t\tenum protocol_version env = parse_protocol_version(git_test_v);\n> @@ -43,7 +39,7 @@ enum protocol_version get_protocol_version_config(void)\n> \t\treturn env;\n> \t}\n> \n> -\treturn protocol_v0;\n> +\treturn protocol_v2;\n> }\n> \n> enum protocol_version determine_protocol_version_server(void)\n"},{"id":"406400","messageId":"20200925212427.GA1270882@google.com","threadId":"54300","inReplyTo":"20200925183436.GA659633@coredump.intra.peff.net","subject":"Re: [PATCH] protocol: re-enable v2 protocol by default","fromName":"","fromEmail":"jrnieder@gmail.com","sentAt":"2020-09-25T21:24:27Z","receivedAt":"2020-09-25T21:24:35Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n\n> Let's move forward with v2 as the default again. It's possible there are\n> still lurking bugs, but we won't know until it gets more widespread use.\n> And we can find and squash them just like any other bug at this point.\n>\n> Signed-off-by: Jeff King \n> ---\n> Documentation/config/feature.txt | 4 ----\n> Documentation/config/protocol.txt | 3 +--\n> protocol.c | 6 +-----\n> 3 files changed, 2 insertions(+), 11 deletions(-)\n\nReviewed-by: Jonathan Nieder \n\nThanks.\n"}]}