git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / Atom feed
* [PATCH] t0000: do not use export X=Y
@ 2013-07-08  9:21 Torsten Bögershausen
  2013-07-08 14:36 ` Thomas Rast
  0 siblings, 1 reply; 10+ messages in thread
From: Torsten Bögershausen @ 2013-07-08  9:21 UTC (permalink / raw)
  To: trast, git

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] t0000: do not use export X=Y
  2013-07-08  9:21 [PATCH] t0000: do not use export X=Y Torsten Bögershausen
@ 2013-07-08 14:36 ` Thomas Rast
  2013-07-08 15:20   ` [PATCH 1/2] t9902: fix 'test A == B' to use = operator Thomas Rast
  2013-07-08 15:20   ` [PATCH] t0000: do not use export X=Y Junio C Hamano
  0 siblings, 2 replies; 10+ messages in thread
From: Thomas Rast @ 2013-07-08 14:36 UTC (permalink / raw)
  To: Torsten Bögershausen; +Cc: git

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/2] t9902: fix 'test A == B' to use = operator
  2013-07-08 14:36 ` Thomas Rast
@ 2013-07-08 15:20   ` Thomas Rast
  2013-07-08 15:20     ` [PATCH 2/2] test-lint: detect 'export FOO=bar' Thomas Rast
  2013-07-08 17:20     ` [PATCH 1/2] t9902: fix 'test A == B' to use = operator Junio C Hamano
  2013-07-08 15:20   ` [PATCH] t0000: do not use export X=Y Junio C Hamano
  1 sibling, 2 replies; 10+ messages in thread
From: Thomas Rast @ 2013-07-08 15:20 UTC (permalink / raw)
  To: git; +Cc: Torsten Bögershausen

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 2/2] test-lint: detect 'export FOO=bar'
  2013-07-08 15:20   ` [PATCH 1/2] t9902: fix 'test A == B' to use = operator Thomas Rast
@ 2013-07-08 15:20     ` Thomas Rast
  2013-07-08 19:31       ` Torsten Bögershausen
  2013-07-08 17:20     ` [PATCH 1/2] t9902: fix 'test A == B' to use = operator Junio C Hamano
  1 sibling, 1 reply; 10+ messages in thread
From: Thomas Rast @ 2013-07-08 15:20 UTC (permalink / raw)
  To: git; +Cc: Torsten Bögershausen

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] t0000: do not use export X=Y
  2013-07-08 14:36 ` Thomas Rast
  2013-07-08 15:20   ` [PATCH 1/2] t9902: fix 'test A == B' to use = operator Thomas Rast
@ 2013-07-08 15:20   ` Junio C Hamano
  1 sibling, 0 replies; 10+ messages in thread
From: Junio C Hamano @ 2013-07-08 15:20 UTC (permalink / raw)
  To: Thomas Rast; +Cc: Torsten Bögershausen, git

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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] t9902: fix 'test A == B' to use = operator
  2013-07-08 15:20   ` [PATCH 1/2] t9902: fix 'test A == B' to use = operator Thomas Rast
  2013-07-08 15:20     ` [PATCH 2/2] test-lint: detect 'export FOO=bar' Thomas Rast
@ 2013-07-08 17:20     ` Junio C Hamano
  2013-07-09  9:27       ` Thomas Rast
  1 sibling, 1 reply; 10+ messages in thread
From: Junio C Hamano @ 2013-07-08 17:20 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git, Torsten Bögershausen

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
>  }

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] test-lint: detect 'export FOO=bar'
  2013-07-08 15:20     ` [PATCH 2/2] test-lint: detect 'export FOO=bar' Thomas Rast
@ 2013-07-08 19:31       ` Torsten Bögershausen
  2013-07-09  9:28         ` Thomas Rast
  0 siblings, 1 reply; 10+ messages in thread
From: Torsten Bögershausen @ 2013-07-08 19:31 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git, 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] t9902: fix 'test A == B' to use = operator
  2013-07-08 17:20     ` [PATCH 1/2] t9902: fix 'test A == B' to use = operator Junio C Hamano
@ 2013-07-09  9:27       ` Thomas Rast
  0 siblings, 0 replies; 10+ messages in thread
From: Thomas Rast @ 2013-07-09  9:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Torsten Bögershausen

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] test-lint: detect 'export FOO=bar'
  2013-07-08 19:31       ` Torsten Bögershausen
@ 2013-07-09  9:28         ` Thomas Rast
  2013-07-09  9:53           ` Torsten Bögershausen
  0 siblings, 1 reply; 10+ messages in thread
From: Thomas Rast @ 2013-07-09  9:28 UTC (permalink / raw)
  To: Torsten Bögershausen; +Cc: git

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] test-lint: detect 'export FOO=bar'
  2013-07-09  9:28         ` Thomas Rast
@ 2013-07-09  9:53           ` Torsten Bögershausen
  0 siblings, 0 replies; 10+ messages in thread
From: Torsten Bögershausen @ 2013-07-09  9:53 UTC (permalink / raw)
  To: Thomas Rast; +Cc: Torsten Bögershausen, git

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2013-07-09  9:53 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-08  9:21 [PATCH] t0000: do not use export X=Y Torsten Bögershausen
2013-07-08 14:36 ` Thomas Rast
2013-07-08 15:20   ` [PATCH 1/2] t9902: fix 'test A == B' to use = operator Thomas Rast
2013-07-08 15:20     ` [PATCH 2/2] test-lint: detect 'export FOO=bar' Thomas Rast
2013-07-08 19:31       ` Torsten Bögershausen
2013-07-09  9:28         ` Thomas Rast
2013-07-09  9:53           ` Torsten Bögershausen
2013-07-08 17:20     ` [PATCH 1/2] t9902: fix 'test A == B' to use = operator Junio C Hamano
2013-07-09  9:27       ` Thomas Rast
2013-07-08 15:20   ` [PATCH] t0000: do not use export X=Y Junio C Hamano

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ https://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for the project(s) associated with this inbox:

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

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git