Re: [BUG] git-submodule has bash-ism?

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 1, 2016, 20:56 UTC
Message-ID: <xmqq37owy6fr.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <xmqq8tyoy6se.fsf@gitster.mtv.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 14 quoted lines

> John Keeping <john@keeping.me.uk> writes:
>
>> Absolutely, my original point should have been prefixed with: I wonder
>> if the reason we haven't had any problems reported is because ...
>>
>> And we've got lucky because the clobbering of global variables happens
>> not to matter in these particular cases.
>
> Ah, I did misunderstand why you were making that statement, and now
> I fully agree with your conclusion (which is what Jeff spelled out
> in the latest message) that the fact that we saw no breakage report
> is not a datapoint that everybody's shell supports "local" at all.
>
> Thanks for clarification.

So here is the final version with a log message.

-- >8 --
Subject: [PATCH] t5500 & t7403: lose bash-ism "local"

In t5500::check_prot_host_port_path(), diagport is not a variable used elsewhere and the function is not recursively called so this can simply lose the "local", which may not be supported by shell (besides, the function liberally clobbers other variables without making them "local").

t7403::reset_submodule_urls() overrides the "root" variable used in the test framework for no good reason; its use is not about temporarily relocating where the test repositories are created. This assignment can be made not to clobber the varible by moving them into the subshells it already uses. Its value is always $TRASH_DIRECTORY, so we could use it instead there, and this function that is called only once and its two subshells may not be necessary (instead, the caller can use "git -C $there config" and set a value that is derived from $TRASH_DIRECTORY), but this is a minimum fix that is needed to lose "local".

Helped-by: John Keeping <john@keeping.me.uk>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t5500-fetch-pack.sh     | 1 -
 t/t7403-submodule-sync.sh | 4 ++--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 9b9bec4..dc305d6 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -556,7 +556,6 @@ check_prot_path () {
 }
 
 check_prot_host_port_path () {
-	local diagport
 	case "$2" in
 		*ssh*)
 		pp=ssh
diff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh
index 79bc135..5503ec0 100755
--- a/t/t7403-submodule-sync.sh
+++ b/t/t7403-submodule-sync.sh
@@ -62,13 +62,13 @@ test_expect_success 'change submodule' '
 '
 
 reset_submodule_urls () {
-	local root
-	root=$(pwd) &&
 	(
+		root=$(pwd) &&
 		cd super-clone/submodule &&
 		git config remote.origin.url "$root/submodule"
 	) &&
 	(
+		root=$(pwd) &&
 		cd super-clone/submodule/sub-submodule &&
 		git config remote.origin.url "$root/submodule"
 	)

-- 
2.9.0-rc1-223-gb1e1500