[PATCH 1/8] tests: define GIT_TEST_PROTOCOL_VERSION

[PATCH 1/8] tests: define GIT_TEST_PROTOCOL_VERSION

[Jonathan Tan]

Define a GIT_TEST_PROTOCOL_VERSION environment variable meant to be used
from tests. When set, this ensures protocol.version is at least the
given value, allowing the entire test suite to be run as if this
configuration is in place for all repositories.

As of this patch, all tests pass whether GIT_TEST_PROTOCOL_VERSION is
unset, set to 0, or set to 1. Some tests fail when
GIT_TEST_PROTOCOL_VERSION is set to 2, but this will be dealt with in
subsequent patches.

This is based on work by Ævar Arnfjörð Bjarmason.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
 protocol.c                  | 17 +++++++++++++++--
 t/README                    |  3 +++
 t/t5400-send-pack.sh        |  2 +-
 t/t5551-http-fetch-smart.sh |  3 ++-
 4 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/protocol.c b/protocol.c
index 5664bd7a05..c7a735bfa2 100644
--- a/protocol.c
+++ b/protocol.c
@@ -42,6 +42,10 @@ static const char *format_protocol_version(enum protocol_version version)
 enum protocol_version get_protocol_version_config(void)
 {
 	const char *value;
+	enum protocol_version retval = protocol_v0;
+	const char *git_test_k = "GIT_TEST_PROTOCOL_VERSION";
+	const char *git_test_v = getenv(git_test_k);
+
 	if (!git_config_get_string_const("protocol.version", &value)) {
 		enum protocol_version version = parse_protocol_version(value);
 
@@ -49,10 +53,19 @@ enum protocol_version get_protocol_version_config(void)
 			die("unknown value for config 'protocol.version': %s",
 			    value);
 
-		return version;
+		retval = version;
+	}
+
+	if (git_test_v && strlen(git_test_v)) {
+		enum protocol_version env = parse_protocol_version(git_test_v);
+
+		if (env == protocol_unknown_version)
+			die("unknown value for %s: %s", git_test_k, git_test_v);
+		if (retval < env)
+			retval = env;
 	}
 
-	return protocol_v0;
+	return retval;
 }
 
 void register_allowed_protocol_version(enum protocol_version version)
diff --git a/t/README b/t/README
index 25864ec883..21e941eb94 100644
--- a/t/README
+++ b/t/README
@@ -327,6 +327,9 @@ marked strings" in po/README for details.
 GIT_TEST_SPLIT_INDEX=<boolean> forces split-index mode on the whole
 test suite. Accept any boolean values that are accepted by git-config.
 
+GIT_TEST_PROTOCOL_VERSION=<n>, when set, overrides the
+'protocol.version' setting to n if it is less than n.
+
 GIT_TEST_FULL_IN_PACK_ARRAY=<boolean> exercises the uncommon
 pack-objects code path where there are more than 1024 packs even if
 the actual number of packs in repository is below this limit. Accept
diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh
index f1932ea431..571d620aed 100755
--- a/t/t5400-send-pack.sh
+++ b/t/t5400-send-pack.sh
@@ -288,7 +288,7 @@ test_expect_success 'receive-pack de-dupes .have lines' '
 	$shared .have
 	EOF
 
-	GIT_TRACE_PACKET=$(pwd)/trace \
+	GIT_TRACE_PACKET=$(pwd)/trace GIT_TEST_PROTOCOL_VERSION= \
 	    git push \
 		--receive-pack="unset GIT_TRACE_PACKET; git-receive-pack" \
 		fork HEAD:foo &&
diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
index a60dd907bd..8f620e0a35 100755
--- a/t/t5551-http-fetch-smart.sh
+++ b/t/t5551-http-fetch-smart.sh
@@ -44,7 +44,8 @@ test_expect_success 'clone http repository' '
 	< Cache-Control: no-cache, max-age=0, must-revalidate
 	< Content-Type: application/x-git-upload-pack-result
 	EOF
-	GIT_TRACE_CURL=true git clone --quiet $HTTPD_URL/smart/repo.git clone 2>err &&
+	GIT_TRACE_CURL=true GIT_TEST_PROTOCOL_VERSION= \
+		git clone --quiet $HTTPD_URL/smart/repo.git clone 2>err &&
 	test_cmp file clone/file &&
 	tr '\''\015'\'' Q <err |
 	sed -e "
-- 
2.19.0.271.gfe8321ec05.dirty

Re: [PATCH 1/8] tests: define GIT_TEST_PROTOCOL_VERSION

[Ævar Arnfjörð Bjarmason]

On Wed, Feb 06 2019, Jonathan Tan wrote:

> Define a GIT_TEST_PROTOCOL_VERSION environment variable meant to be used
> from tests. When set, this ensures protocol.version is at least the
> given value, allowing the entire test suite to be run as if this
> configuration is in place for all repositories.
>
> As of this patch, all tests pass whether GIT_TEST_PROTOCOL_VERSION is
> unset, set to 0, or set to 1. Some tests fail when
> GIT_TEST_PROTOCOL_VERSION is set to 2, but this will be dealt with in
> subsequent patches.
>
> This is based on work by Ævar Arnfjörð Bjarmason.
>
> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
> ---
>  protocol.c                  | 17 +++++++++++++++--
>  t/README                    |  3 +++
>  t/t5400-send-pack.sh        |  2 +-
>  t/t5551-http-fetch-smart.sh |  3 ++-
>  4 files changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/protocol.c b/protocol.c
> index 5664bd7a05..c7a735bfa2 100644
> --- a/protocol.c
> +++ b/protocol.c
> @@ -42,6 +42,10 @@ static const char *format_protocol_version(enum protocol_version version)
>  enum protocol_version get_protocol_version_config(void)
>  {
>  	const char *value;
> +	enum protocol_version retval = protocol_v0;
> +	const char *git_test_k = "GIT_TEST_PROTOCOL_VERSION";
> +	const char *git_test_v = getenv(git_test_k);
> +
>  	if (!git_config_get_string_const("protocol.version", &value)) {
>  		enum protocol_version version = parse_protocol_version(value);
>
> @@ -49,10 +53,19 @@ enum protocol_version get_protocol_version_config(void)
>  			die("unknown value for config 'protocol.version': %s",
>  			    value);
>
> -		return version;
> +		retval = version;
> +	}
> +
> +	if (git_test_v && strlen(git_test_v)) {
> +		enum protocol_version env = parse_protocol_version(git_test_v);
> +
> +		if (env == protocol_unknown_version)
> +			die("unknown value for %s: %s", git_test_k, git_test_v);
> +		if (retval < env)
> +			retval = env;
>  	}
>
> -	return protocol_v0;
> +	return retval;
>  }
>
>  void register_allowed_protocol_version(enum protocol_version version)
> diff --git a/t/README b/t/README
> index 25864ec883..21e941eb94 100644
> --- a/t/README
> +++ b/t/README
> @@ -327,6 +327,9 @@ marked strings" in po/README for details.
>  GIT_TEST_SPLIT_INDEX=<boolean> forces split-index mode on the whole
>  test suite. Accept any boolean values that are accepted by git-config.
>
> +GIT_TEST_PROTOCOL_VERSION=<n>, when set, overrides the
> +'protocol.version' setting to n if it is less than n.
> +

In my version
(https://public-inbox.org/git/20181213155817.27666-6-avarab@gmail.com/)
I didn't have this "if it is less than n" caveat. I expect that helped
with making some tests that were setting e.g. protocol.version=2 Just
Work, is that the reason for this?

Mine also had more docs here, but maybe telling people that they can use
"env" is too much...


>  GIT_TEST_FULL_IN_PACK_ARRAY=<boolean> exercises the uncommon
>  pack-objects code path where there are more than 1024 packs even if
>  the actual number of packs in repository is below this limit. Accept
> diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh
> index f1932ea431..571d620aed 100755
> --- a/t/t5400-send-pack.sh
> +++ b/t/t5400-send-pack.sh
> @@ -288,7 +288,7 @@ test_expect_success 'receive-pack de-dupes .have lines' '
>  	$shared .have
>  	EOF
>
> -	GIT_TRACE_PACKET=$(pwd)/trace \
> +	GIT_TRACE_PACKET=$(pwd)/trace GIT_TEST_PROTOCOL_VERSION= \
>  	    git push \
>  		--receive-pack="unset GIT_TRACE_PACKET; git-receive-pack" \
>  		fork HEAD:foo &&
> diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
> index a60dd907bd..8f620e0a35 100755
> --- a/t/t5551-http-fetch-smart.sh
> +++ b/t/t5551-http-fetch-smart.sh
> @@ -44,7 +44,8 @@ test_expect_success 'clone http repository' '
>  	< Cache-Control: no-cache, max-age=0, must-revalidate
>  	< Content-Type: application/x-git-upload-pack-result
>  	EOF
> -	GIT_TRACE_CURL=true git clone --quiet $HTTPD_URL/smart/repo.git clone 2>err &&
> +	GIT_TRACE_CURL=true GIT_TEST_PROTOCOL_VERSION= \
> +		git clone --quiet $HTTPD_URL/smart/repo.git clone 2>err &&
>  	test_cmp file clone/file &&
>  	tr '\''\015'\'' Q <err |
>  	sed -e "

Re: [PATCH 1/8] tests: define GIT_TEST_PROTOCOL_VERSION

[Jonathan Tan]

> > @@ -327,6 +327,9 @@ marked strings" in po/README for details.
> >  GIT_TEST_SPLIT_INDEX=<boolean> forces split-index mode on the whole
> >  test suite. Accept any boolean values that are accepted by git-config.
> >
> > +GIT_TEST_PROTOCOL_VERSION=<n>, when set, overrides the
> > +'protocol.version' setting to n if it is less than n.
> > +
> 
> In my version
> (https://public-inbox.org/git/20181213155817.27666-6-avarab@gmail.com/)
> I didn't have this "if it is less than n" caveat. I expect that helped
> with making some tests that were setting e.g. protocol.version=2 Just
> Work, is that the reason for this?

Yes, that's right. I thought that there is not much value in testing
tests that are explicitly protocol v2 as another protocol, since there
was a reason in the first place why the test writer wanted to test it
with v2.

> Mine also had more docs here, but maybe telling people that they can use
> "env" is too much...

With the ability to set =0 to effectively disable the option (because
the minimum of 0 and 0/1/2 is 0/1/2), I thought it was less important.

Re: [PATCH 1/8] tests: define GIT_TEST_PROTOCOL_VERSION

[Jeff King]

On Tue, Feb 05, 2019 at 04:21:15PM -0800, Jonathan Tan wrote:

> Define a GIT_TEST_PROTOCOL_VERSION environment variable meant to be used
> from tests. When set, this ensures protocol.version is at least the
> given value, allowing the entire test suite to be run as if this
> configuration is in place for all repositories.
> 
> As of this patch, all tests pass whether GIT_TEST_PROTOCOL_VERSION is
> unset, set to 0, or set to 1. Some tests fail when
> GIT_TEST_PROTOCOL_VERSION is set to 2, but this will be dealt with in
> subsequent patches.

Makes sense. The "at least" part made me scratch my head at first, but
your explanation in response to Ævar made sense.

Two minor nits:

> diff --git a/protocol.c b/protocol.c
> index 5664bd7a05..c7a735bfa2 100644
> --- a/protocol.c
> +++ b/protocol.c
> @@ -42,6 +42,10 @@ static const char *format_protocol_version(enum protocol_version version)
>  enum protocol_version get_protocol_version_config(void)
>  {
>  	const char *value;
> +	enum protocol_version retval = protocol_v0;
> +	const char *git_test_k = "GIT_TEST_PROTOCOL_VERSION";
> +	const char *git_test_v = getenv(git_test_k);

We've discussed recently how the return value from getenv() isn't
stable. It looks like we could assign it much closer to the point-of-use
here (which still isn't 100% foolproof, but I think is something we
could encourage as a general pattern, and mostly works due to our
ring-buffer technique).

I.e., right before this conditional:

> +
> +	if (git_test_v && strlen(git_test_v)) {

It's more idiomatic in our code base to check for a non-empty string as:

  if (git_test_v && *git_test_v)

though obviously that's pretty minor.

-Peff

[PATCH 0/8] GIT_TEST_PROTOCOL_VERSION, this time on master

[Jonathan Tan]

There's a version of this in Junio's Git repository as
jt/test-protocol-version, blocked by js/protocol-advertise-multi. After
chatting with Josh (the author of js/protocol-advertise-multi) last
week, it seems that that branch might take a while, so I have decided to
rebase my patches on master.

This necessitates patch 2, which contains additional updates to t5601,
so that all tests pass with GIT_TEST_PROTOCOL_VERSION=1.
(js/protocol-advertise-multi eliminates the need to do that, which is
why these changes did not appear in the previous version.) When
js/protocol-advertise-multi is merged, we can and should revert patch 2.

I have incorporated a small suggestion by Peff [1], and he seems overall
OK with the patch set [2].

I have based my work on the existing contents of
jt/test-protocol-version, so most commits include a "Signed-off-by" line by
Junio. Let me know if you prefer it to be removed.

[1] https://public-inbox.org/git/20190211201959.GA9072@sigill.intra.peff.net/
[2] https://public-inbox.org/git/20190211203827.GA9010@sigill.intra.peff.net/

Here is a range-diff.

1:  7ed58eb98b ! 1:  4dcd898813 tests: define GIT_TEST_PROTOCOL_VERSION
    @@ -8,9 +8,8 @@
         configuration is in place for all repositories.
     
         As of this patch, all tests pass whether GIT_TEST_PROTOCOL_VERSION is
    -    unset, set to 0, or set to 1. Some tests fail when
    -    GIT_TEST_PROTOCOL_VERSION is set to 2, but this will be dealt with in
    -    subsequent patches.
    +    unset or set to 0. Some tests fail when GIT_TEST_PROTOCOL_VERSION is set
    +    to 1 or 2, but this will be dealt with in subsequent patches.
     
         This is based on work by Ævar Arnfjörð Bjarmason.
     
    @@ -39,7 +38,7 @@
     +		retval = version;
     +	}
     +
    -+	if (git_test_v && strlen(git_test_v)) {
    ++	if (git_test_v && *git_test_v) {
     +		enum protocol_version env = parse_protocol_version(git_test_v);
     +
     +		if (env == protocol_unknown_version)
    @@ -52,7 +51,7 @@
     +	return retval;
      }
      
    - void register_allowed_protocol_version(enum protocol_version version)
    + enum protocol_version determine_protocol_version_server(void)
     
      diff --git a/t/README b/t/README
      --- a/t/README
-:  ---------- > 2:  1c865e4ae9 t5601: check ssh command only with protocol v0
2:  1b3c299528 = 3:  227a88aa8c tests: always test fetch of unreachable with v0
3:  526670b1d5 = 4:  8f510c5c78 t5503: fix overspecification of trace expectation
4:  1548be81a5 = 5:  9aaabdac52 t5512: compensate for v0 only sending HEAD symrefs
5:  1da961b33a = 6:  6c29e64bc3 t5700: only run with protocol version 1
6:  3c22aaad5b = 7:  d4638e9418 tests: fix protocol version for overspecifications
7:  188754ca3f = 8:  629a243c7f t5552: compensate for v2 filtering ref adv.

Jonathan Tan (8):
  tests: define GIT_TEST_PROTOCOL_VERSION
  t5601: check ssh command only with protocol v0
  tests: always test fetch of unreachable with v0
  t5503: fix overspecification of trace expectation
  t5512: compensate for v0 only sending HEAD symrefs
  t5700: only run with protocol version 1
  tests: fix protocol version for overspecifications
  t5552: compensate for v2 filtering ref adv.

 protocol.c                           | 17 ++++++++--
 t/README                             |  3 ++
 t/t5400-send-pack.sh                 |  2 +-
 t/t5500-fetch-pack.sh                |  4 ++-
 t/t5503-tagfollow.sh                 |  2 +-
 t/t5512-ls-remote.sh                 | 18 ++++++++---
 t/t5515-fetch-merge-logic.sh         |  4 +++
 t/t5516-fetch-push.sh                | 22 ++++++++++---
 t/t5539-fetch-http-shallow.sh        |  5 ++-
 t/t5541-http-push-smart.sh           | 14 +++++++--
 t/t5551-http-fetch-smart.sh          | 47 +++++++++++++++++++++-------
 t/t5552-skipping-fetch-negotiator.sh |  5 ++-
 t/t5601-clone.sh                     | 16 +++++-----
 t/t5700-protocol-v1.sh               |  3 ++
 t/t7406-submodule-update.sh          |  5 ++-
 15 files changed, 128 insertions(+), 39 deletions(-)

-- 
2.19.0.271.gfe8321ec05.dirty

[PATCH 1/8] tests: define GIT_TEST_PROTOCOL_VERSION

[Jonathan Tan]

Define a GIT_TEST_PROTOCOL_VERSION environment variable meant to be used
from tests. When set, this ensures protocol.version is at least the
given value, allowing the entire test suite to be run as if this
configuration is in place for all repositories.

As of this patch, all tests pass whether GIT_TEST_PROTOCOL_VERSION is
unset or set to 0. Some tests fail when GIT_TEST_PROTOCOL_VERSION is set
to 1 or 2, but this will be dealt with in subsequent patches.

This is based on work by Ævar Arnfjörð Bjarmason.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 protocol.c                  | 17 +++++++++++++++--
 t/README                    |  3 +++
 t/t5400-send-pack.sh        |  2 +-
 t/t5551-http-fetch-smart.sh |  3 ++-
 4 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/protocol.c b/protocol.c
index 5e636785d1..9741f05750 100644
--- a/protocol.c
+++ b/protocol.c
@@ -17,6 +17,10 @@ static enum protocol_version parse_protocol_version(const char *value)
 enum protocol_version get_protocol_version_config(void)
 {
 	const char *value;
+	enum protocol_version retval = protocol_v0;
+	const char *git_test_k = "GIT_TEST_PROTOCOL_VERSION";
+	const char *git_test_v = getenv(git_test_k);
+
 	if (!git_config_get_string_const("protocol.version", &value)) {
 		enum protocol_version version = parse_protocol_version(value);
 
@@ -24,10 +28,19 @@ enum protocol_version get_protocol_version_config(void)
 			die("unknown value for config 'protocol.version': %s",
 			    value);
 
-		return version;
+		retval = version;
+	}
+
+	if (git_test_v && *git_test_v) {
+		enum protocol_version env = parse_protocol_version(git_test_v);
+
+		if (env == protocol_unknown_version)
+			die("unknown value for %s: %s", git_test_k, git_test_v);
+		if (retval < env)
+			retval = env;
 	}
 
-	return protocol_v0;
+	return retval;
 }
 
 enum protocol_version determine_protocol_version_server(void)
diff --git a/t/README b/t/README
index 886bbec5bc..3125bfa6b6 100644
--- a/t/README
+++ b/t/README
@@ -341,6 +341,9 @@ marked strings" in po/README for details.
 GIT_TEST_SPLIT_INDEX=<boolean> forces split-index mode on the whole
 test suite. Accept any boolean values that are accepted by git-config.
 
+GIT_TEST_PROTOCOL_VERSION=<n>, when set, overrides the
+'protocol.version' setting to n if it is less than n.
+
 GIT_TEST_FULL_IN_PACK_ARRAY=<boolean> exercises the uncommon
 pack-objects code path where there are more than 1024 packs even if
 the actual number of packs in repository is below this limit. Accept
diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh
index f1932ea431..571d620aed 100755
--- a/t/t5400-send-pack.sh
+++ b/t/t5400-send-pack.sh
@@ -288,7 +288,7 @@ test_expect_success 'receive-pack de-dupes .have lines' '
 	$shared .have
 	EOF
 
-	GIT_TRACE_PACKET=$(pwd)/trace \
+	GIT_TRACE_PACKET=$(pwd)/trace GIT_TEST_PROTOCOL_VERSION= \
 	    git push \
 		--receive-pack="unset GIT_TRACE_PACKET; git-receive-pack" \
 		fork HEAD:foo &&
diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
index ba83e567e5..a852684e16 100755
--- a/t/t5551-http-fetch-smart.sh
+++ b/t/t5551-http-fetch-smart.sh
@@ -43,7 +43,8 @@ test_expect_success 'clone http repository' '
 	< Cache-Control: no-cache, max-age=0, must-revalidate
 	< Content-Type: application/x-git-upload-pack-result
 	EOF
-	GIT_TRACE_CURL=true git clone --quiet $HTTPD_URL/smart/repo.git clone 2>err &&
+	GIT_TRACE_CURL=true GIT_TEST_PROTOCOL_VERSION= \
+		git clone --quiet $HTTPD_URL/smart/repo.git clone 2>err &&
 	test_cmp file clone/file &&
 	tr '\''\015'\'' Q <err |
 	sed -e "
-- 
2.19.0.271.gfe8321ec05.dirty

[PATCH 2/8] t5601: check ssh command only with protocol v0

[Jonathan Tan]

When running the SSH command as part of a fetch, Git will write "SendEnv
GIT_PROTOCOL" as an option if protocol v1 or v2 is used, but not v0.
Update all tests that check this to run Git with
GIT_TEST_PROTOCOL_VERSION=0.

I chose not to do a more thorough fix (for example, checking the value of
GIT_TEST_PROTOCOL_VERSION to see if the SendEnv check needs to be done)
because a set of patches [1] that unifies the handling of SSH options,
including writing "SendEnv GIT_PROTOCOL" regardless of protocol version,
is in progress. When that is done, this patch should be reverted, since
the functionality in here is no longer needed.

As of this patch, all tests pass if GIT_TEST_PROTOCOL_VERSION is set to
1.

[1] https://public-inbox.org/git/cover.1545342797.git.steadmon@google.com/

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
 t/t5601-clone.sh | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index d6948cbdab..a454b143ea 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -345,7 +345,7 @@ expect_ssh () {
 }
 
 test_expect_success 'clone myhost:src uses ssh' '
-	git clone myhost:src ssh-clone &&
+	GIT_TEST_PROTOCOL_VERSION=0 git clone myhost:src ssh-clone &&
 	expect_ssh myhost src
 '
 
@@ -356,12 +356,12 @@ test_expect_success !MINGW,!CYGWIN 'clone local path foo:bar' '
 '
 
 test_expect_success 'bracketed hostnames are still ssh' '
-	git clone "[myhost:123]:src" ssh-bracket-clone &&
+	GIT_TEST_PROTOCOL_VERSION=0 git clone "[myhost:123]:src" ssh-bracket-clone &&
 	expect_ssh "-p 123" myhost src
 '
 
 test_expect_success 'OpenSSH variant passes -4' '
-	git clone -4 "[myhost:123]:src" ssh-ipv4-clone &&
+	GIT_TEST_PROTOCOL_VERSION=0 git clone -4 "[myhost:123]:src" ssh-ipv4-clone &&
 	expect_ssh "-4 -p 123" myhost src
 '
 
@@ -405,7 +405,7 @@ test_expect_success 'OpenSSH-like uplink is treated as ssh' '
 	test_when_finished "rm -f \"\$TRASH_DIRECTORY/uplink\"" &&
 	GIT_SSH="$TRASH_DIRECTORY/uplink" &&
 	test_when_finished "GIT_SSH=\"\$TRASH_DIRECTORY/ssh\$X\"" &&
-	git clone "[myhost:123]:src" ssh-bracket-clone-sshlike-uplink &&
+	GIT_TEST_PROTOCOL_VERSION=0 git clone "[myhost:123]:src" ssh-bracket-clone-sshlike-uplink &&
 	expect_ssh "-p 123" myhost src
 '
 
@@ -444,14 +444,14 @@ test_expect_success 'single quoted plink.exe in GIT_SSH_COMMAND' '
 
 test_expect_success 'GIT_SSH_VARIANT overrides plink detection' '
 	copy_ssh_wrapper_as "$TRASH_DIRECTORY/plink" &&
-	GIT_SSH_VARIANT=ssh \
-	git clone "[myhost:123]:src" ssh-bracket-clone-variant-1 &&
+	GIT_TEST_PROTOCOL_VERSION=0 GIT_SSH_VARIANT=ssh \
+		git clone "[myhost:123]:src" ssh-bracket-clone-variant-1 &&
 	expect_ssh "-p 123" myhost src
 '
 
 test_expect_success 'ssh.variant overrides plink detection' '
 	copy_ssh_wrapper_as "$TRASH_DIRECTORY/plink" &&
-	git -c ssh.variant=ssh \
+	GIT_TEST_PROTOCOL_VERSION=0 git -c ssh.variant=ssh \
 		clone "[myhost:123]:src" ssh-bracket-clone-variant-2 &&
 	expect_ssh "-p 123" myhost src
 '
@@ -482,7 +482,7 @@ counter=0
 # $3 path
 test_clone_url () {
 	counter=$(($counter + 1))
-	test_might_fail git clone "$1" tmp$counter &&
+	test_might_fail env GIT_TEST_PROTOCOL_VERSION=0 git clone "$1" tmp$counter &&
 	shift &&
 	expect_ssh "$@"
 }
-- 
2.19.0.271.gfe8321ec05.dirty

[PATCH 3/8] tests: always test fetch of unreachable with v0

[Jonathan Tan]

Some tests check that fetching an unreachable object fails, but protocol
v2 allows such fetches. Unset GIT_TEST_PROTOCOL_VERSION so that these
tests are always run using protocol v0.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t5500-fetch-pack.sh       |  4 +++-
 t/t5516-fetch-push.sh       | 22 +++++++++++++++++-----
 t/t5551-http-fetch-smart.sh | 10 ++++++++--
 t/t7406-submodule-update.sh |  5 ++++-
 4 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 49c540b1e1..0ef4d6f20c 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -636,7 +636,9 @@ test_expect_success 'fetch-pack cannot fetch a raw sha1 that is not advertised a
 	test_commit -C server 6 &&
 
 	git init client &&
-	test_must_fail git -C client fetch-pack ../server \
+	# Some protocol versions (e.g. 2) support fetching
+	# unadvertised objects, so restrict this test to v0.
+	test_must_fail env GIT_TEST_PROTOCOL_VERSION= git -C client fetch-pack ../server \
 		$(git -C server rev-parse refs/heads/master^) 2>err &&
 	test_i18ngrep "Server does not allow request for unadvertised object" err
 '
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 37e8e80893..4bfbb79654 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1147,8 +1147,12 @@ test_expect_success 'fetch exact SHA1' '
 		git prune &&
 		test_must_fail git cat-file -t $the_commit &&
 
+		# Some protocol versions (e.g. 2) support fetching
+		# unadvertised objects, so restrict this test to v0.
+
 		# fetching the hidden object should fail by default
-		test_must_fail git fetch -v ../testrepo $the_commit:refs/heads/copy 2>err &&
+		test_must_fail env GIT_TEST_PROTOCOL_VERSION= \
+			git fetch -v ../testrepo $the_commit:refs/heads/copy 2>err &&
 		test_i18ngrep "Server does not allow request for unadvertised object" err &&
 		test_must_fail git rev-parse --verify refs/heads/copy &&
 
@@ -1204,7 +1208,10 @@ do
 		mk_empty shallow &&
 		(
 			cd shallow &&
-			test_must_fail git fetch --depth=1 ../testrepo/.git $SHA1 &&
+			# Some protocol versions (e.g. 2) support fetching
+			# unadvertised objects, so restrict this test to v0.
+			test_must_fail env GIT_TEST_PROTOCOL_VERSION= \
+				git fetch --depth=1 ../testrepo/.git $SHA1 &&
 			git --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&
 			git fetch --depth=1 ../testrepo/.git $SHA1 &&
 			git cat-file commit $SHA1
@@ -1232,15 +1239,20 @@ do
 		mk_empty shallow &&
 		(
 			cd shallow &&
-			test_must_fail ok=sigpipe git fetch ../testrepo/.git $SHA1_3 &&
-			test_must_fail ok=sigpipe git fetch ../testrepo/.git $SHA1_1 &&
+			# Some protocol versions (e.g. 2) support fetching
+			# unadvertised objects, so restrict this test to v0.
+			test_must_fail ok=sigpipe env GIT_TEST_PROTOCOL_VERSION= \
+				git fetch ../testrepo/.git $SHA1_3 &&
+			test_must_fail ok=sigpipe env GIT_TEST_PROTOCOL_VERSION= \
+				git fetch ../testrepo/.git $SHA1_1 &&
 			git --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&
 			git fetch ../testrepo/.git $SHA1_1 &&
 			git cat-file commit $SHA1_1 &&
 			test_must_fail git cat-file commit $SHA1_2 &&
 			git fetch ../testrepo/.git $SHA1_2 &&
 			git cat-file commit $SHA1_2 &&
-			test_must_fail ok=sigpipe git fetch ../testrepo/.git $SHA1_3
+			test_must_fail ok=sigpipe env GIT_TEST_PROTOCOL_VERSION= \
+				git fetch ../testrepo/.git $SHA1_3
 		)
 	'
 done
diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
index a852684e16..f02ae3b797 100755
--- a/t/t5551-http-fetch-smart.sh
+++ b/t/t5551-http-fetch-smart.sh
@@ -307,7 +307,10 @@ test_expect_success 'test allowreachablesha1inwant with unreachable' '
 
 	git init --bare test_reachable.git &&
 	git -C test_reachable.git remote add origin "$HTTPD_URL/smart/repo.git" &&
-	test_must_fail git -C test_reachable.git fetch origin "$(git rev-parse HEAD)"
+	# Some protocol versions (e.g. 2) support fetching
+	# unadvertised objects, so restrict this test to v0.
+	test_must_fail env GIT_TEST_PROTOCOL_VERSION= \
+		git -C test_reachable.git fetch origin "$(git rev-parse HEAD)"
 '
 
 test_expect_success 'test allowanysha1inwant with unreachable' '
@@ -326,7 +329,10 @@ test_expect_success 'test allowanysha1inwant with unreachable' '
 
 	git init --bare test_reachable.git &&
 	git -C test_reachable.git remote add origin "$HTTPD_URL/smart/repo.git" &&
-	test_must_fail git -C test_reachable.git fetch origin "$(git rev-parse HEAD)" &&
+	# Some protocol versions (e.g. 2) support fetching
+	# unadvertised objects, so restrict this test to v0.
+	test_must_fail env GIT_TEST_PROTOCOL_VERSION= \
+		git -C test_reachable.git fetch origin "$(git rev-parse HEAD)" &&
 
 	git -C "$server" config uploadpack.allowanysha1inwant 1 &&
 	git -C test_reachable.git fetch origin "$(git rev-parse HEAD)"
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index e87164aa8f..c973278300 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -943,7 +943,10 @@ test_expect_success 'submodule update clone shallow submodule outside of depth'
 		cd super3 &&
 		sed -e "s#url = ../#url = file://$pwd/#" <.gitmodules >.gitmodules.tmp &&
 		mv -f .gitmodules.tmp .gitmodules &&
-		test_must_fail git submodule update --init --depth=1 2>actual &&
+		# Some protocol versions (e.g. 2) support fetching
+		# unadvertised objects, so restrict this test to v0.
+		test_must_fail env GIT_TEST_PROTOCOL_VERSION= \
+			git submodule update --init --depth=1 2>actual &&
 		test_i18ngrep "Direct fetching of that commit failed." actual &&
 		git -C ../submodule config uploadpack.allowReachableSHA1InWant true &&
 		git submodule update --init --depth=1 >actual &&
-- 
2.19.0.271.gfe8321ec05.dirty

[PATCH 4/8] t5503: fix overspecification of trace expectation

[Jonathan Tan]

In order to extract the wants from a trace, a loop in t5503 currently
breaks if "0000" is found. This works for protocol v0 and v1, but not
v2. Instead, teach t5503 to look specifically for the "want" string,
which is compatible with all protocols.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t5503-tagfollow.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t5503-tagfollow.sh b/t/t5503-tagfollow.sh
index 4ca48f0276..6041a4dd32 100755
--- a/t/t5503-tagfollow.sh
+++ b/t/t5503-tagfollow.sh
@@ -47,7 +47,7 @@ get_needs () {
 	test -s "$1" &&
 	perl -alne '
 		next unless $F[1] eq "upload-pack<";
-		last if $F[2] eq "0000";
+		next unless $F[2] eq "want";
 		print $F[2], " ", $F[3];
 	' "$1"
 }
-- 
2.19.0.271.gfe8321ec05.dirty

[PATCH 5/8] t5512: compensate for v0 only sending HEAD symrefs

[Jonathan Tan]

Protocol v2 supports sending non-HEAD symrefs, but this is not true of
protocol v0. Some tests expect protocol v0 behavior, so fix them to use
protocol v0.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t5512-ls-remote.sh | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
index ced15ae122..e3c4a48c85 100755
--- a/t/t5512-ls-remote.sh
+++ b/t/t5512-ls-remote.sh
@@ -223,7 +223,9 @@ test_expect_success 'ls-remote --symref' '
 	$(git rev-parse refs/tags/mark1.10)	refs/tags/mark1.10
 	$(git rev-parse refs/tags/mark1.2)	refs/tags/mark1.2
 	EOF
-	git ls-remote --symref >actual &&
+	# Protocol v2 supports sending symrefs for refs other than HEAD, so use
+	# protocol v0 here.
+	GIT_TEST_PROTOCOL_VERSION= git ls-remote --symref >actual &&
 	test_cmp expect actual
 '
 
@@ -232,7 +234,9 @@ test_expect_success 'ls-remote with filtered symref (refname)' '
 	ref: refs/heads/master	HEAD
 	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	HEAD
 	EOF
-	git ls-remote --symref . HEAD >actual &&
+	# Protocol v2 supports sending symrefs for refs other than HEAD, so use
+	# protocol v0 here.
+	GIT_TEST_PROTOCOL_VERSION= git ls-remote --symref . HEAD >actual &&
 	test_cmp expect actual
 '
 
@@ -243,7 +247,9 @@ test_expect_failure 'ls-remote with filtered symref (--heads)' '
 	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/heads/foo
 	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/heads/master
 	EOF
-	git ls-remote --symref --heads . >actual &&
+	# Protocol v2 supports sending symrefs for refs other than HEAD, so use
+	# protocol v0 here.
+	GIT_TEST_PROTOCOL_VERSION= git ls-remote --symref --heads . >actual &&
 	test_cmp expect actual
 '
 
@@ -252,9 +258,11 @@ test_expect_success 'ls-remote --symref omits filtered-out matches' '
 	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/heads/foo
 	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/heads/master
 	EOF
-	git ls-remote --symref --heads . >actual &&
+	# Protocol v2 supports sending symrefs for refs other than HEAD, so use
+	# protocol v0 here.
+	GIT_TEST_PROTOCOL_VERSION= git ls-remote --symref --heads . >actual &&
 	test_cmp expect actual &&
-	git ls-remote --symref . "refs/heads/*" >actual &&
+	GIT_TEST_PROTOCOL_VERSION= git ls-remote --symref . "refs/heads/*" >actual &&
 	test_cmp expect actual
 '
 
-- 
2.19.0.271.gfe8321ec05.dirty

[PATCH 6/8] t5700: only run with protocol version 1

[Jonathan Tan]

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t5700-protocol-v1.sh | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/t/t5700-protocol-v1.sh b/t/t5700-protocol-v1.sh
index ba86a44eb1..d5ed196bfd 100755
--- a/t/t5700-protocol-v1.sh
+++ b/t/t5700-protocol-v1.sh
@@ -4,6 +4,9 @@ test_description='test git wire-protocol transition'
 
 TEST_NO_CREATE_REPO=1
 
+# This is a protocol-specific test.
+GIT_TEST_PROTOCOL_VERSION=
+
 . ./test-lib.sh
 
 # Test protocol v1 with 'git://' transport
-- 
2.19.0.271.gfe8321ec05.dirty

[PATCH 7/8] tests: fix protocol version for overspecifications

[Jonathan Tan]

These tests are also marked with a NEEDSWORK comment.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t5515-fetch-merge-logic.sh  |  4 ++++
 t/t5539-fetch-http-shallow.sh |  5 ++++-
 t/t5541-http-push-smart.sh    | 14 ++++++++++++--
 t/t5551-http-fetch-smart.sh   | 34 ++++++++++++++++++++++++++--------
 4 files changed, 46 insertions(+), 11 deletions(-)

diff --git a/t/t5515-fetch-merge-logic.sh b/t/t5515-fetch-merge-logic.sh
index 36b0dbc01c..e55d8474ef 100755
--- a/t/t5515-fetch-merge-logic.sh
+++ b/t/t5515-fetch-merge-logic.sh
@@ -6,6 +6,10 @@
 
 test_description='Merge logic in fetch'
 
+# NEEDSWORK: If the overspecification of the expected result is reduced, we
+# might be able to run this test in all protocol versions.
+GIT_TEST_PROTOCOL_VERSION=
+
 . ./test-lib.sh
 
 LF='
diff --git a/t/t5539-fetch-http-shallow.sh b/t/t5539-fetch-http-shallow.sh
index 5fbf67c446..cdb687b93a 100755
--- a/t/t5539-fetch-http-shallow.sh
+++ b/t/t5539-fetch-http-shallow.sh
@@ -67,7 +67,10 @@ test_expect_success 'no shallow lines after receiving ACK ready' '
 		cd clone &&
 		git checkout --orphan newnew &&
 		test_commit new-too &&
-		GIT_TRACE_PACKET="$TRASH_DIRECTORY/trace" git fetch --depth=2 &&
+		# NEEDSWORK: If the overspecification of the expected result is reduced, we
+		# might be able to run this test in all protocol versions.
+		GIT_TRACE_PACKET="$TRASH_DIRECTORY/trace" GIT_TEST_PROTOCOL_VERSION= \
+			git fetch --depth=2 &&
 		grep "fetch-pack< ACK .* ready" ../trace &&
 		! grep "fetch-pack> done" ../trace
 	)
diff --git a/t/t5541-http-push-smart.sh b/t/t5541-http-push-smart.sh
index 5475afc052..0e3055ab98 100755
--- a/t/t5541-http-push-smart.sh
+++ b/t/t5541-http-push-smart.sh
@@ -47,7 +47,12 @@ test_expect_success 'no empty path components' '
 	cd "$ROOT_PATH" &&
 	git clone $HTTPD_URL/smart/test_repo.git/ test_repo_clone &&
 
-	check_access_log exp
+	# NEEDSWORK: If the overspecification of the expected result is reduced, we
+	# might be able to run this test in all protocol versions.
+	if test -z "$GIT_TEST_PROTOCOL_VERSION"
+	then
+		check_access_log exp
+	fi
 '
 
 test_expect_success 'clone remote repository' '
@@ -128,7 +133,12 @@ GET  /smart/test_repo.git/info/refs?service=git-receive-pack HTTP/1.1 200
 POST /smart/test_repo.git/git-receive-pack HTTP/1.1 200
 EOF
 test_expect_success 'used receive-pack service' '
-	check_access_log exp
+	# NEEDSWORK: If the overspecification of the expected result is reduced, we
+	# might be able to run this test in all protocol versions.
+	if test -z "$GIT_TEST_PROTOCOL_VERSION"
+	then
+		check_access_log exp
+	fi
 '
 
 test_http_push_nonff "$HTTPD_DOCUMENT_ROOT_PATH"/test_repo.git \
diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
index f02ae3b797..a685d3edb6 100755
--- a/t/t5551-http-fetch-smart.sh
+++ b/t/t5551-http-fetch-smart.sh
@@ -81,12 +81,18 @@ test_expect_success 'clone http repository' '
 		/^< Content-Length: /d
 		/^< Transfer-Encoding: /d
 	" >actual &&
-	sed -e "s/^> Accept-Encoding: .*/> Accept-Encoding: ENCODINGS/" \
-			actual >actual.smudged &&
-	test_cmp exp actual.smudged &&
 
-	grep "Accept-Encoding:.*gzip" actual >actual.gzip &&
-	test_line_count = 2 actual.gzip
+	# NEEDSWORK: If the overspecification of the expected result is reduced, we
+	# might be able to run this test in all protocol versions.
+	if test -z "$GIT_TEST_PROTOCOL_VERSION"
+	then
+		sed -e "s/^> Accept-Encoding: .*/> Accept-Encoding: ENCODINGS/" \
+				actual >actual.smudged &&
+		test_cmp exp actual.smudged &&
+
+		grep "Accept-Encoding:.*gzip" actual >actual.gzip &&
+		test_line_count = 2 actual.gzip
+	fi
 '
 
 test_expect_success 'fetch changes via http' '
@@ -104,7 +110,13 @@ test_expect_success 'used upload-pack service' '
 	GET  /smart/repo.git/info/refs?service=git-upload-pack HTTP/1.1 200
 	POST /smart/repo.git/git-upload-pack HTTP/1.1 200
 	EOF
-	check_access_log exp
+
+	# NEEDSWORK: If the overspecification of the expected result is reduced, we
+	# might be able to run this test in all protocol versions.
+	if test -z "$GIT_TEST_PROTOCOL_VERSION"
+	then
+		check_access_log exp
+	fi
 '
 
 test_expect_success 'follow redirects (301)' '
@@ -216,8 +228,14 @@ test_expect_success 'cookies stored in http.cookiefile when http.savecookies set
 	git config http.cookiefile cookies.txt &&
 	git config http.savecookies true &&
 	git ls-remote $HTTPD_URL/smart_cookies/repo.git master &&
-	tail -3 cookies.txt | sort >cookies_tail.txt &&
-	test_cmp expect_cookies.txt cookies_tail.txt
+
+	# NEEDSWORK: If the overspecification of the expected result is reduced, we
+	# might be able to run this test in all protocol versions.
+	if test -z "$GIT_TEST_PROTOCOL_VERSION"
+	then
+		tail -3 cookies.txt | sort >cookies_tail.txt &&
+		test_cmp expect_cookies.txt cookies_tail.txt
+	fi
 '
 
 test_expect_success 'transfer.hiderefs works over smart-http' '
-- 
2.19.0.271.gfe8321ec05.dirty

[PATCH 8/8] t5552: compensate for v2 filtering ref adv.

[Jonathan Tan]

Protocol v2 filters the ref advertisement, but protocol v0 does not. A
test in t5552 uses the ref advertisement, so fix it to use protocol v0.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t5552-skipping-fetch-negotiator.sh | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/t/t5552-skipping-fetch-negotiator.sh b/t/t5552-skipping-fetch-negotiator.sh
index 30857b84a8..8a14be51a1 100755
--- a/t/t5552-skipping-fetch-negotiator.sh
+++ b/t/t5552-skipping-fetch-negotiator.sh
@@ -127,7 +127,10 @@ test_expect_success 'use ref advertisement to filter out commits' '
 	# not need to send any ancestors of "c3", but we still need to send "c3"
 	# itself.
 	test_config -C client fetch.negotiationalgorithm skipping &&
-	trace_fetch client origin to_fetch &&
+
+	# The ref advertisement itself is filtered when protocol v2 is used, so
+	# use v0.
+	GIT_TEST_PROTOCOL_VERSION= trace_fetch client origin to_fetch &&
 	have_sent c5 c4^ c2side &&
 	have_not_sent c4 c4^^ c4^^^
 '
-- 
2.19.0.271.gfe8321ec05.dirty