git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Jonathan Tan <jonathantanmy@google.com>,
	Jeff King <peff@peff.net>,
	Jeff Hostetler <jeffhost@microsoft.com>
Subject: [PATCH 1/2] fetch test: avoid use of "VAR= cmd" with a shell function
Date: Thu, 26 Dec 2019 11:55:10 -0800	[thread overview]
Message-ID: <20191226195510.GB170890@google.com> (raw)
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


  reply	other threads:[~2019-12-26 19:55 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-24  0:58 [PATCH 0/5] Enable protocol v2 by default Jonathan Nieder
2019-12-24  0:59 ` [PATCH 1/5] fetch test: use more robust test for filtered objects Jonathan Nieder
2019-12-26 14:29   ` Derrick Stolee
2019-12-24  1:00 ` [PATCH 2/5] config doc: protocol.version is not experimental Jonathan Nieder
2019-12-24  1:01 ` [PATCH 3/5] test: request GIT_TEST_PROTOCOL_VERSION=0 when appropriate Jonathan Nieder
2019-12-26 19:26   ` Junio C Hamano
2019-12-26 19:53     ` [PATCH 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
2019-12-26 19:55       ` Jonathan Nieder [this message]
2019-12-26 19:57       ` [PATCH 2/2] t/check-non-portable-shell: detect "FOO= shell_func", too Jonathan Nieder
2019-12-26 20:08         ` Junio C Hamano
2019-12-26 20:18         ` Junio C Hamano
2019-12-26 22:37           ` Jonathan Nieder
2019-12-26 23:12           ` [PATCH jn/test-lint-one-shot-export-to-shell-function] fetch test: mark test of "skipping" haves as v0-only Jonathan Nieder
2019-12-26 20:39         ` [PATCH 2/2] t/check-non-portable-shell: detect "FOO= shell_func", too Eric Sunshine
2019-12-24  1:02 ` [PATCH 4/5] protocol test: let protocol.version override GIT_TEST_PROTOCOL_VERSION Jonathan Nieder
2019-12-24  1:04 ` [PATCH 5/5] fetch: default to protocol version 2 Jonathan Nieder
2019-12-26 14:30 ` [PATCH 0/5] Enable protocol v2 by default Derrick Stolee

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191226195510.GB170890@google.com \
    --to=jrnieder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jeffhost@microsoft.com \
    --cc=jonathantanmy@google.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).