threads / patch / 28252

patcht6030: use $SHELL_PATH to invoke user's preferred shell instead of bare sh

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

## tl;dr

4 messages between Aug 29, 2011 and Aug 30, 2011. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Brandon Casey· Aug 29, 2011, 20:09 UTC · lore

From: Brandon Casey <drafnel@gmail.com>

Some platforms (IRIX, Solaris) provide an ancient /bin/sh which chokes on modern shell syntax like $(). SHELL_PATH is provided to allow the user to specify a working sh, let's use it here.

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
 t/t6030-bisect-porcelain.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Show changes to t/t6030-bisect-porcelain.sh +1 −1

diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 62125ec..c53af62 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -732,7 +732,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 sh -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.\$\$

-- 
1.7.6.1

Junio C Hamano· Aug 30, 2011, 20:15 UTC · re: Brandon Casey · lore

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

Brandon Casey <casey@nrlssc.navy.mil> writes:

Show 24 quoted lines

> From: Brandon Casey <drafnel@gmail.com>
>
> Some platforms (IRIX, Solaris) provide an ancient /bin/sh which chokes on
> modern shell syntax like $().  SHELL_PATH is provided to allow the user to
> specify a working sh, let's use it here.
>
> Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
> ---
>  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 62125ec..c53af62 100755
> --- a/t/t6030-bisect-porcelain.sh
> +++ b/t/t6030-bisect-porcelain.sh
> @@ -732,7 +732,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 sh -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.\$\$

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?

Junio C Hamano· Aug 30, 2011, 20:27 UTC · re: Junio C Hamano · lore

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

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

Show 9 quoted lines

>> +	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(-)

Show changes to t/t6030-bisect-porcelain.sh +1 −1

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.\$\$

Brandon Casey· Aug 30, 2011, 20:55 UTC · re: Junio C Hamano · lore

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

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

Show 11 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
> 
>>> +	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

Show 28 quoted lines

> 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.\$\$