git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 1/2] fetch test: avoid use of "VAR= cmd" with a shell function

From
Jonathan Nieder <jrnieder@gmail.com>
Date
Dec 26, 2019, 19:55 UTC
Message-ID
<20191226195510.GB170890@google.com>
In-Reply-To
<20191226195357.GA170890@google.com>

Just like assigning a nonempty value, assigning an empty value to a shell variable when calling a function produces non-portable behavior: in some shells, the assignment lasts for the duration of the function invocation, and in others, it persists after the function returns.

Use an explicit subshell with the envvar exported to make the behavior consistent across shells and crystal clear.

All previous instances of this pattern used "VAR=value" (with nonempty `value`), which is already diagnosed automatically by "make test-lint" since a0a630192d (t/check-non-portable-shell: detect "FOO=bar shell_func", 2018-07-13).

Noticed using an improved "make test-lint".
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 t/t5552-skipping-fetch-negotiator.sh | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/t/t5552-skipping-fetch-negotiator.sh b/t/t5552-skipping-fetch-negotiator.sh
index f70cbcc9ca..a452fe32fa 100755
--- a/t/t5552-skipping-fetch-negotiator.sh
+++ b/t/t5552-skipping-fetch-negotiator.sh
@@ -107,7 +107,11 @@ test_expect_success 'use ref advertisement to filter out commits' '
 
 	# The ref advertisement itself is filtered when protocol v2 is used, so
 	# use v0.
-	GIT_TEST_PROTOCOL_VERSION= trace_fetch client origin to_fetch &&
+	(
+		GIT_TEST_PROTOCOL_VERSION= &&
+		export GIT_TEST_PROTOCOL_VERSION &&
+		trace_fetch client origin to_fetch
+	) &&
 	have_sent c5 c4^ c2side &&
 	have_not_sent c4 c4^^ c4^^^
 '
-- 
2.24.1.735.g03f4e72817
Previous: Jonathan NiederNext: Jonathan Nieder
Message 8 of 17 in “Enable protocol v2 by default”
  1. 0/5 Enable protocol v2 by defaultJonathan Nieder, Dec 24, 2019
  2. 1/5 fetch test: use more robust test for filtered objectsJonathan Nieder, Dec 24, 2019
  3. Derrick StoleeDec 26, 2019
  4. 2/5 config doc: protocol.version is not experimentalJonathan Nieder, Dec 24, 2019
  5. 3/5 test: request GIT_TEST_PROTOCOL_VERSION=0 when appropriateJonathan Nieder, Dec 24, 2019
  6. Junio C HamanoDec 26, 2019
  7. 0/2 avoid use of "VAR= cmd" with a shell function (Re: [PATCH 3/5] test: request GIT_TEST_PROTOCOL_VERSION=0 when appropriate)Jonathan Nieder, Dec 26, 2019
  8. 1/2 fetch test: avoid use of "VAR= cmd" with a shell functionJonathan Nieder, Dec 26, 2019
  9. 2/2 t/check-non-portable-shell: detect "FOO= shell_func", tooJonathan Nieder, Dec 26, 2019
  10. Junio C HamanoDec 26, 2019
  11. Junio C HamanoDec 26, 2019
  12. Jonathan NiederDec 26, 2019
  13. fetch test: mark test of "skipping" haves as v0-onlyJonathan Nieder, Dec 26, 2019
  14. Eric SunshineDec 26, 2019
  15. 4/5 protocol test: let protocol.version override GIT_TEST_PROTOCOL_VERSIONJonathan Nieder, Dec 24, 2019
  16. 5/5 fetch: default to protocol version 2Jonathan Nieder, Dec 24, 2019
  17. Derrick StoleeDec 26, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.