# [PATCH] t0000: do not use export X=Y 10 messages from 2013-07-08 to 2013-07-09. Participants: Torsten Bögershausen, Thomas Rast, Junio C Hamano. Thread: https://gitlist.dev/t/34378 ## Torsten Bögershausen, 2013-07-08 09:21 Subject: [PATCH] t0000: do not use export X=Y Message-ID: <201307081121.22769.tboegi@web.de> URL: https://gitlist.dev/e/201307081121.22769.tboegi%40web.de ``` The shell syntax "export X=Y A=B" is not understood by all shells. Signed-off-by: Torsten Bögershausen --- 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 ``` ## Thomas Rast, 2013-07-08 14:36 Subject: Re: [PATCH] t0000: do not use export X=Y Message-ID: <8761wli0fe.fsf@linux-k42r.v.cablecom.net> URL: https://gitlist.dev/e/8761wli0fe.fsf%40linux-k42r.v.cablecom.net In-Reply-To: <201307081121.22769.tboegi@web.de> ``` Torsten Bögershausen writes: > The shell syntax "export X=Y A=B" is not understood by all shells. > > Signed-off-by: Torsten Bögershausen > --- > 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 ``` ## Thomas Rast, 2013-07-08 15:20 Subject: [PATCH 1/2] t9902: fix 'test A == B' to use = operator Message-ID: URL: https://gitlist.dev/e/b9e889a27f648c44179ad39159240867f86525f3.1373296313.git.trast%40inf.ethz.ch In-Reply-To: <8761wli0fe.fsf@linux-k42r.v.cablecom.net> ``` 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 --- 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 ``` ## Thomas Rast, 2013-07-08 15:20 Subject: [PATCH 2/2] test-lint: detect 'export FOO=bar' Message-ID: <24ee18d3b06883c4185821f5ffb5faed26e6a436.1373296313.git.trast@inf.ethz.ch> URL: https://gitlist.dev/e/24ee18d3b06883c4185821f5ffb5faed26e6a436.1373296313.git.trast%40inf.ethz.ch In-Reply-To: ``` 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 --- I wrote: > Torsten Bögershausen 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 ``` ## Junio C Hamano, 2013-07-08 15:20 Subject: Re: [PATCH] t0000: do not use export X=Y Message-ID: <7vy59h13jc.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vy59h13jc.fsf%40alter.siamese.dyndns.org In-Reply-To: <8761wli0fe.fsf@linux-k42r.v.cablecom.net> ``` Thomas Rast writes: > Torsten Bögershausen writes: > >> The shell syntax "export X=Y A=B" is not understood by all shells. >> >> Signed-off-by: Torsten Bögershausen >> --- >> 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. ``` ## Junio C Hamano, 2013-07-08 17:20 Subject: Re: [PATCH 1/2] t9902: fix 'test A == B' to use = operator Message-ID: <7v1u790xzj.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7v1u790xzj.fsf%40alter.siamese.dyndns.org In-Reply-To: ``` Thomas Rast 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 > --- > 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 > } ``` ## Torsten Bögershausen, 2013-07-08 19:31 Subject: Re: [PATCH 2/2] test-lint: detect 'export FOO=bar' Message-ID: <51DB1399.9020307@web.de> URL: https://gitlist.dev/e/51DB1399.9020307%40web.de In-Reply-To: <24ee18d3b06883c4185821f5ffb5faed26e6a436.1373296313.git.trast@inf.ethz.ch> ``` 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 > --- > > I wrote: > >> Torsten Bögershausen 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 ``` ## Thomas Rast, 2013-07-09 09:27 Subject: Re: [PATCH 1/2] t9902: fix 'test A == B' to use = operator Message-ID: <87fvvogk1x.fsf@linux-k42r.v.cablecom.net> URL: https://gitlist.dev/e/87fvvogk1x.fsf%40linux-k42r.v.cablecom.net In-Reply-To: <7v1u790xzj.fsf@alter.siamese.dyndns.org> ``` Junio C Hamano writes: > Thomas Rast 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 ``` ## Thomas Rast, 2013-07-09 09:28 Subject: Re: [PATCH 2/2] test-lint: detect 'export FOO=bar' Message-ID: <87a9lwgk0e.fsf@linux-k42r.v.cablecom.net> URL: https://gitlist.dev/e/87a9lwgk0e.fsf%40linux-k42r.v.cablecom.net In-Reply-To: <51DB1399.9020307@web.de> ``` Torsten Bögershausen 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 ``` ## Torsten Bögershausen, 2013-07-09 09:53 Subject: Re: [PATCH 2/2] test-lint: detect 'export FOO=bar' Message-ID: <51DBDDA2.8070304@web.de> URL: https://gitlist.dev/e/51DBDDA2.8070304%40web.de In-Reply-To: <87a9lwgk0e.fsf@linux-k42r.v.cablecom.net> ``` On 09.07.13 11:28, Thomas Rast wrote: > Torsten Bögershausen 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 ```