Re: [PATCH] t6030: use $SHELL_PATH to invoke user's preferred shell instead of bare sh

Junio C Hamano [off-list ref] writes:

quoted

+	git bisect run \"$SHELL_PATH\" -c '
 		GOOD=\$(git for-each-ref \"--format=%(objectname)\" refs/bisect/good-*) &&
 		git rev-list --objects BISECT_HEAD --not \$GOOD >tmp.\$\$ &&
 		git pack-objects --stdout >/dev/null < tmp.\$\$

Hmm, shouldn't we also be quoting '$' in front of SHELL_PATH to make the
test_expect_success interpolate the value of SHELL_PATH, instead of hoping
that SHELL_PATH does not have double-quote in it when the shell that forms
parameters given to test_expect_success interpolates it?

By doing this silly thing:

	$ mkdir '/var/tmp/a"b c/'
        $ ln -s /bin/sh '/var/tmp/a"b c/shell'

and then adding

	SHELL_PATH='/var/tmp/a"b c/shell'

after we source "test-lib.sh" in t6030, I see this breaks when the shell
parses to figure out what parameters to call test_expect_success with.

 t/t6030-bisect-porcelain.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 68c0e97..cd9de63 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh

@@ -701,7 +701,7 @@ test_expect_success 'bisect: demonstrate identification of damage boundary' "
 	git bisect reset &&
 	git checkout broken &&
 	git bisect start broken master --no-checkout &&
-	git bisect run \"$SHELL_PATH\" -c '
+	git bisect run \"\$SHELL_PATH\" -c '
 		GOOD=\$(git for-each-ref \"--format=%(objectname)\" refs/bisect/good-*) &&
 		git rev-list --objects BISECT_HEAD --not \$GOOD >tmp.\$\$ &&
 		git pack-objects --stdout >/dev/null < tmp.\$\$

On 08/30/2011 03:27 PM, Junio C Hamano wrote:

Junio C Hamano [off-list ref] writes:

quoted

quoted

+	git bisect run \"$SHELL_PATH\" -c '
 		GOOD=\$(git for-each-ref \"--format=%(objectname)\" refs/bisect/good-*) &&
 		git rev-list --objects BISECT_HEAD --not \$GOOD >tmp.\$\$ &&
 		git pack-objects --stdout >/dev/null < tmp.\$\$

Hmm, shouldn't we also be quoting '$' in front of SHELL_PATH to make the
test_expect_success interpolate the value of SHELL_PATH, instead of hoping
that SHELL_PATH does not have double-quote in it when the shell that forms
parameters given to test_expect_success interpolates it?

Yep, makes sense.  Good catch.  Thanks.

-Brandon