git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* [PATCH] protocol: re-enable v2 protocol by default
@ 2020-09-25 18:34 Jeff King
  2020-09-25 18:41 ` Junio C Hamano
  2020-09-25 21:24 ` jrnieder
  0 siblings, 2 replies; 3+ messages in thread
From: Jeff King @ 2020-09-25 18:34 UTC (permalink / raw)
  To: git; +Cc: Jonathan Nieder, Jonathan Tan

Protocol v2 became the default in v2.26.0 via 684ceae32d (fetch: default
to protocol version 2, 2019-12-23). More widespread use turned up a
regression in negotiation. That was fixed in v2.27.0 via 4fa3f00abb
(fetch-pack: in protocol v2, in_vain only after ACK, 2020-04-27), but we
also reverted the default to v0 as a precuation in 11c7f2a30b (Revert
"fetch: default to protocol version 2", 2020-04-22).

In v2.28.0, we re-enabled it for experimental users with 3697caf4b9
(config: let feature.experimental imply protocol.version=2, 2020-05-20)
and haven't heard any complaints. v2.28 has only been out for 2 months,
but I'd generally expect people turning on feature.experimental to also
stay pretty up-to-date. So we're not likely to collect much more data by
waiting. In addition, we have no further reports from people running
v2.26.0, and of course some people have been setting protocol.version
manually for ages.

Let's move forward with v2 as the default again. It's possible there are
still lurking bugs, but we won't know until it gets more widespread use.
And we can find and squash them just like any other bug at this point.

Signed-off-by: Jeff King <peff@peff.net>
---
After seeing yet another round of "if you use the v2 protocol, all of
your issues fetching a non-tip object for submodules or CI will go away"
advice today, I was reminded that we haven't picked up the question
again of when we should look at v2 becoming the default again.

 Documentation/config/feature.txt  | 4 ----
 Documentation/config/protocol.txt | 3 +--
 protocol.c                        | 6 +-----
 3 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/Documentation/config/feature.txt b/Documentation/config/feature.txt
index c0cbf2bb1c..cdecd04e5b 100644
--- a/Documentation/config/feature.txt
+++ b/Documentation/config/feature.txt
@@ -14,10 +14,6 @@ feature.experimental::
 +
 * `fetch.negotiationAlgorithm=skipping` may improve fetch negotiation times by
 skipping more commits at a time, reducing the number of round trips.
-+
-* `protocol.version=2` speeds up fetches from repositories with many refs by
-allowing the client to specify which refs to list before the server lists
-them.
 
 feature.manyFiles::
 	Enable config options that optimize for repos with many files in the
diff --git a/Documentation/config/protocol.txt b/Documentation/config/protocol.txt
index c46e9b3d00..756591d77b 100644
--- a/Documentation/config/protocol.txt
+++ b/Documentation/config/protocol.txt
@@ -48,8 +48,7 @@ protocol.version::
 	If set, clients will attempt to communicate with a server
 	using the specified protocol version.  If the server does
 	not support it, communication falls back to version 0.
-	If unset, the default is `0`, unless `feature.experimental`
-	is enabled, in which case the default is `2`.
+	If unset, the default is `2`.
 	Supported versions:
 +
 --
diff --git a/protocol.c b/protocol.c
index 8d964fc65e..052d7edbb9 100644
--- a/protocol.c
+++ b/protocol.c
@@ -17,7 +17,6 @@ static enum protocol_version parse_protocol_version(const char *value)
 enum protocol_version get_protocol_version_config(void)
 {
 	const char *value;
-	int val;
 	const char *git_test_k = "GIT_TEST_PROTOCOL_VERSION";
 	const char *git_test_v;
 
@@ -31,9 +30,6 @@ enum protocol_version get_protocol_version_config(void)
 		return version;
 	}
 
-	if (!git_config_get_bool("feature.experimental", &val) && val)
-		return protocol_v2;
-
 	git_test_v = getenv(git_test_k);
 	if (git_test_v && *git_test_v) {
 		enum protocol_version env = parse_protocol_version(git_test_v);
@@ -43,7 +39,7 @@ enum protocol_version get_protocol_version_config(void)
 		return env;
 	}
 
-	return protocol_v0;
+	return protocol_v2;
 }
 
 enum protocol_version determine_protocol_version_server(void)
-- 
2.28.0.1085.g44a0350633

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] protocol: re-enable v2 protocol by default
  2020-09-25 18:34 [PATCH] protocol: re-enable v2 protocol by default Jeff King
@ 2020-09-25 18:41 ` Junio C Hamano
  2020-09-25 21:24 ` jrnieder
  1 sibling, 0 replies; 3+ messages in thread
From: Junio C Hamano @ 2020-09-25 18:41 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Jonathan Nieder, Jonathan Tan

Jeff King <peff@peff.net> writes:

> Let's move forward with v2 as the default again. It's possible there are
> still lurking bugs, but we won't know until it gets more widespread use.
> And we can find and squash them just like any other bug at this point.

Ack.  Thanks.

>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> After seeing yet another round of "if you use the v2 protocol, all of
> your issues fetching a non-tip object for submodules or CI will go away"
> advice today, I was reminded that we haven't picked up the question
> again of when we should look at v2 becoming the default again.
>
>  Documentation/config/feature.txt  | 4 ----
>  Documentation/config/protocol.txt | 3 +--
>  protocol.c                        | 6 +-----
>  3 files changed, 2 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/config/feature.txt b/Documentation/config/feature.txt
> index c0cbf2bb1c..cdecd04e5b 100644
> --- a/Documentation/config/feature.txt
> +++ b/Documentation/config/feature.txt
> @@ -14,10 +14,6 @@ feature.experimental::
>  +
>  * `fetch.negotiationAlgorithm=skipping` may improve fetch negotiation times by
>  skipping more commits at a time, reducing the number of round trips.
> -+
> -* `protocol.version=2` speeds up fetches from repositories with many refs by
> -allowing the client to specify which refs to list before the server lists
> -them.
>  
>  feature.manyFiles::
>  	Enable config options that optimize for repos with many files in the
> diff --git a/Documentation/config/protocol.txt b/Documentation/config/protocol.txt
> index c46e9b3d00..756591d77b 100644
> --- a/Documentation/config/protocol.txt
> +++ b/Documentation/config/protocol.txt
> @@ -48,8 +48,7 @@ protocol.version::
>  	If set, clients will attempt to communicate with a server
>  	using the specified protocol version.  If the server does
>  	not support it, communication falls back to version 0.
> -	If unset, the default is `0`, unless `feature.experimental`
> -	is enabled, in which case the default is `2`.
> +	If unset, the default is `2`.
>  	Supported versions:
>  +
>  --
> diff --git a/protocol.c b/protocol.c
> index 8d964fc65e..052d7edbb9 100644
> --- a/protocol.c
> +++ b/protocol.c
> @@ -17,7 +17,6 @@ static enum protocol_version parse_protocol_version(const char *value)
>  enum protocol_version get_protocol_version_config(void)
>  {
>  	const char *value;
> -	int val;
>  	const char *git_test_k = "GIT_TEST_PROTOCOL_VERSION";
>  	const char *git_test_v;
>  
> @@ -31,9 +30,6 @@ enum protocol_version get_protocol_version_config(void)
>  		return version;
>  	}
>  
> -	if (!git_config_get_bool("feature.experimental", &val) && val)
> -		return protocol_v2;
> -
>  	git_test_v = getenv(git_test_k);
>  	if (git_test_v && *git_test_v) {
>  		enum protocol_version env = parse_protocol_version(git_test_v);
> @@ -43,7 +39,7 @@ enum protocol_version get_protocol_version_config(void)
>  		return env;
>  	}
>  
> -	return protocol_v0;
> +	return protocol_v2;
>  }
>  
>  enum protocol_version determine_protocol_version_server(void)

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] protocol: re-enable v2 protocol by default
  2020-09-25 18:34 [PATCH] protocol: re-enable v2 protocol by default Jeff King
  2020-09-25 18:41 ` Junio C Hamano
@ 2020-09-25 21:24 ` jrnieder
  1 sibling, 0 replies; 3+ messages in thread
From: jrnieder @ 2020-09-25 21:24 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Jonathan Tan

Jeff King wrote:

> Let's move forward with v2 as the default again. It's possible there are
> still lurking bugs, but we won't know until it gets more widespread use.
> And we can find and squash them just like any other bug at this point.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  Documentation/config/feature.txt  | 4 ----
>  Documentation/config/protocol.txt | 3 +--
>  protocol.c                        | 6 +-----
>  3 files changed, 2 insertions(+), 11 deletions(-)

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Thanks.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-09-25 21:34 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-25 18:34 [PATCH] protocol: re-enable v2 protocol by default Jeff King
2020-09-25 18:41 ` Junio C Hamano
2020-09-25 21:24 ` jrnieder

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ https://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git