[PATCH] t0000: do not use export X=Y

[PATCH] t0000: do not use export X=Y

[Torsten Bögershausen]

The shell syntax "export X=Y A=B" is not understood by all shells.

Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
 t/t0000-basic.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 5c32288..10be52b 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -53,7 +53,8 @@ run_sub_test_lib_test () {
 		# Pretend we're a test harness.  This prevents
 		# test-lib from writing the counts to a file that will
 		# later be summarized, showing spurious "failed" tests
-		export HARNESS_ACTIVE=t &&
+		HARNESS_ACTIVE=t &&
+		export HARNESS_ACTIVE &&
 		cd "$name" &&
 		cat >"$name.sh" <<-EOF &&
 		#!$SHELL_PATH
-- 
1.8.3

Re: [PATCH] t0000: do not use export X=Y

[Thomas Rast]

Torsten Bögershausen <tboegi@web.de> writes:

> The shell syntax "export X=Y A=B" is not understood by all shells.
>
> Signed-off-by: Torsten Bögershausen <tboegi@web.de>
> ---
>  t/t0000-basic.sh | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
> index 5c32288..10be52b 100755
> --- a/t/t0000-basic.sh
> +++ b/t/t0000-basic.sh
> @@ -53,7 +53,8 @@ run_sub_test_lib_test () {
>  		# Pretend we're a test harness.  This prevents
>  		# test-lib from writing the counts to a file that will
>  		# later be summarized, showing spurious "failed" tests
> -		export HARNESS_ACTIVE=t &&
> +		HARNESS_ACTIVE=t &&
> +		export HARNESS_ACTIVE &&
>  		cd "$name" &&
>  		cat >"$name.sh" <<-EOF &&
>  		#!$SHELL_PATH

Ack.  Sorry for breaking this -- I suppose test-lint would have caught
me out?

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

[PATCH 1/2] t9902: fix 'test A == B' to use = operator

[Thomas Rast]

The == operator as an alias to = is not POSIX.  This doesn't actually
matter for the execution of the script, because it only runs when the
shell is bash.  However, it trips up test-lint, so it's nicer to use
the standard form.

Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
---
 t/t9902-completion.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index d9e3103..272a071 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -69,7 +69,7 @@ run_completion ()
 	local -a COMPREPLY _words
 	local _cword
 	_words=( $1 )
-	test "${1: -1}" == ' ' && _words+=('')
+	test "${1: -1}" = ' ' && _words+=('')
 	(( _cword = ${#_words[@]} - 1 ))
 	__git_wrap__git_main && print_comp
 }
-- 
1.8.3.2.947.g0347b11

[PATCH 2/2] test-lint: detect 'export FOO=bar'

[Thomas Rast]

Some shells do not understand the one-line construct, and instead need

  FOO=bar &&
  export FOO

Detect this in the test-lint target.

Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
---

I wrote:

> Torsten Bögershausen <tboegi@web.de> writes:
[...]
> > -		export HARNESS_ACTIVE=t &&
> > +		HARNESS_ACTIVE=t &&
> > +		export HARNESS_ACTIVE &&
[...]
> Ack.  Sorry for breaking this -- I suppose test-lint would have caught
> me out?

Well, no, not yet.


 t/check-non-portable-shell.pl | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl
index 8b5a71d..45971f4 100755
--- a/t/check-non-portable-shell.pl
+++ b/t/check-non-portable-shell.pl
@@ -21,6 +21,7 @@ while (<>) {
 	/^\s*declare\s+/ and err 'arrays/declare not portable';
 	/^\s*[^#]\s*which\s/ and err 'which is not portable (please use type)';
 	/test\s+[^=]*==/ and err '"test a == b" is not portable (please use =)';
+	/^\s*export\s+[^=]*=/ and err '"export FOO=bar" is not portable (please use FOO=bar && export FOO)';
 	# this resets our $. for each file
 	close ARGV if eof;
 }
-- 
1.8.3.2.947.g0347b11

Re: [PATCH 2/2] test-lint: detect 'export FOO=bar'

[Torsten Bögershausen]

On 2013-07-08 17.20, Thomas Rast wrote:
> Some shells do not understand the one-line construct, and instead need
> 
>   FOO=bar &&
>   export FOO
> 
> Detect this in the test-lint target.
> 
> Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
> ---
> 
> I wrote:
> 
>> Torsten Bögershausen <tboegi@web.de> writes:
> [...]
>>> -		export HARNESS_ACTIVE=t &&
>>> +		HARNESS_ACTIVE=t &&
>>> +		export HARNESS_ACTIVE &&
> [...]
>> Ack.  Sorry for breaking this -- I suppose test-lint would have caught
>> me out?
> 
> Well, no, not yet.
Thanks for working on this

> 
> 
>  t/check-non-portable-shell.pl | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl
> index 8b5a71d..45971f4 100755
> --- a/t/check-non-portable-shell.pl
> +++ b/t/check-non-portable-shell.pl
> @@ -21,6 +21,7 @@ while (<>) {
>  	/^\s*declare\s+/ and err 'arrays/declare not portable';
>  	/^\s*[^#]\s*which\s/ and err 'which is not portable (please use type)';
>  	/test\s+[^=]*==/ and err '"test a == b" is not portable (please use =)';
> +	/^\s*export\s+[^=]*=/ and err '"export FOO=bar" is not portable (please use FOO=bar && export FOO)';
I have a slightly tighter reg exp in my tree, but credits should go to Thomas: 

/^\s*export\s+\S+=\S+/

/Torsten

Re: [PATCH 2/2] test-lint: detect 'export FOO=bar'

[Thomas Rast]

Torsten Bögershausen <tboegi@web.de> writes:

>> +	/^\s*export\s+[^=]*=/ and err '"export FOO=bar" is not portable (please use FOO=bar && export FOO)';
> I have a slightly tighter reg exp in my tree, but credits should go to Thomas: 
>
> /^\s*export\s+\S+=\S+/

Hmm, is that correct?  I would expect shells that have problems with
'export FOO=bar' to also fail on 'export FOO=' (i.e. set to empty string
and export).

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: [PATCH 2/2] test-lint: detect 'export FOO=bar'

[Torsten Bögershausen]

On 09.07.13 11:28, Thomas Rast wrote:
> Torsten Bögershausen <tboegi@web.de> writes:
> 
>>> +	/^\s*export\s+[^=]*=/ and err '"export FOO=bar" is not portable (please use FOO=bar && export FOO)';
>> I have a slightly tighter reg exp in my tree, but credits should go to Thomas: 
>>
>> /^\s*export\s+\S+=\S+/
> 
> Hmm, is that correct?  I would expect shells that have problems with
> 'export FOO=bar' to also fail on 'export FOO=' (i.e. set to empty string
> and export).
> 
Good point, thanks

Re: [PATCH 1/2] t9902: fix 'test A == B' to use = operator

[Junio C Hamano]

Thomas Rast <trast@inf.ethz.ch> writes:

> The == operator as an alias to = is not POSIX.  This doesn't actually
> matter for the execution of the script, because it only runs when the
> shell is bash.  However, it trips up test-lint, so it's nicer to use
> the standard form.

OK, my knee-jerk reaction was "this is only for bash" as you said,
but the test-lint part I agree with.

But then test-lint _ought_ to also catch the use of "local" in the
ideal world, so perhaps in the longer term we would need to treat
this bash-only script differently from others anyway???

>
> Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
> ---
>  t/t9902-completion.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> index d9e3103..272a071 100755
> --- a/t/t9902-completion.sh
> +++ b/t/t9902-completion.sh
> @@ -69,7 +69,7 @@ run_completion ()
>  	local -a COMPREPLY _words
>  	local _cword
>  	_words=( $1 )
> -	test "${1: -1}" == ' ' && _words+=('')
> +	test "${1: -1}" = ' ' && _words+=('')
>  	(( _cword = ${#_words[@]} - 1 ))
>  	__git_wrap__git_main && print_comp
>  }

Re: [PATCH 1/2] t9902: fix 'test A == B' to use = operator

[Thomas Rast]

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

> Thomas Rast <trast@inf.ethz.ch> writes:
>
>> The == operator as an alias to = is not POSIX.  This doesn't actually
>> matter for the execution of the script, because it only runs when the
>> shell is bash.  However, it trips up test-lint, so it's nicer to use
>> the standard form.
>
> OK, my knee-jerk reaction was "this is only for bash" as you said,
> but the test-lint part I agree with.
>
> But then test-lint _ought_ to also catch the use of "local" in the
> ideal world, so perhaps in the longer term we would need to treat
> this bash-only script differently from others anyway???

True.  I didn't really think about wider implications; I just noticed
that there was an easy-to-fix complaint from test-lint :-)

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: [PATCH] t0000: do not use export X=Y

[Junio C Hamano]

Thomas Rast <trast@inf.ethz.ch> writes:

> Torsten Bögershausen <tboegi@web.de> writes:
>
>> The shell syntax "export X=Y A=B" is not understood by all shells.
>>
>> Signed-off-by: Torsten Bögershausen <tboegi@web.de>
>> ---
>>  t/t0000-basic.sh | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
>> index 5c32288..10be52b 100755
>> --- a/t/t0000-basic.sh
>> +++ b/t/t0000-basic.sh
>> @@ -53,7 +53,8 @@ run_sub_test_lib_test () {
>>  		# Pretend we're a test harness.  This prevents
>>  		# test-lib from writing the counts to a file that will
>>  		# later be summarized, showing spurious "failed" tests
>> -		export HARNESS_ACTIVE=t &&
>> +		HARNESS_ACTIVE=t &&
>> +		export HARNESS_ACTIVE &&
>>  		cd "$name" &&
>>  		cat >"$name.sh" <<-EOF &&
>>  		#!$SHELL_PATH
>
> Ack.  Sorry for breaking this -- I suppose test-lint would have caught
> me out?

Or I should have.  Thanks for catching.