{"thread":{"id":"34378","subject":"[PATCH] t0000: do not use export X=Y","startedAt":"2013-07-08T09:21:22Z","lastAt":"2013-07-09T09:53:38Z","messageCount":10,"participants":["Torsten Bögershausen","Thomas Rast","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"222803","messageId":"201307081121.22769.tboegi@web.de","threadId":"34378","inReplyTo":null,"subject":"[PATCH] t0000: do not use export X=Y","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2013-07-08T09:21:22Z","receivedAt":"2013-07-08T09:21:22Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"The shell syntax \"export X=Y A=B\" is not understood by all shells.\n\nSigned-off-by: Torsten Bögershausen \n---\n t/t0000-basic.sh | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\nindex 5c32288..10be52b 100755\n--- a/t/t0000-basic.sh\n+++ b/t/t0000-basic.sh\n@@ -53,7 +53,8 @@ run_sub_test_lib_test () {\n \t\t# Pretend we're a test harness. This prevents\n \t\t# test-lib from writing the counts to a file that will\n \t\t# later be summarized, showing spurious \"failed\" tests\n-\t\texport HARNESS_ACTIVE=t &&\n+\t\tHARNESS_ACTIVE=t &&\n+\t\texport HARNESS_ACTIVE &&\n \t\tcd \"$name\" &&\n \t\tcat >\"$name.sh\" <<-EOF &&\n \t\t#!$SHELL_PATH\n-- \n1.8.3\n"},{"id":"222829","messageId":"8761wli0fe.fsf@linux-k42r.v.cablecom.net","threadId":"34378","inReplyTo":"201307081121.22769.tboegi@web.de","subject":"Re: [PATCH] t0000: do not use export X=Y","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-07-08T14:36:05Z","receivedAt":"2013-07-08T14:36:05Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Torsten Bögershausen writes:\n\n> The shell syntax \"export X=Y A=B\" is not understood by all shells.\n>\n> Signed-off-by: Torsten Bögershausen \n> ---\n> t/t0000-basic.sh | 3 ++-\n> 1 file changed, 2 insertions(+), 1 deletion(-)\n>\n> diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\n> index 5c32288..10be52b 100755\n> --- a/t/t0000-basic.sh\n> +++ b/t/t0000-basic.sh\n> @@ -53,7 +53,8 @@ run_sub_test_lib_test () {\n> \t\t# Pretend we're a test harness. This prevents\n> \t\t# test-lib from writing the counts to a file that will\n> \t\t# later be summarized, showing spurious \"failed\" tests\n> -\t\texport HARNESS_ACTIVE=t &&\n> +\t\tHARNESS_ACTIVE=t &&\n> +\t\texport HARNESS_ACTIVE &&\n> \t\tcd \"$name\" &&\n> \t\tcat >\"$name.sh\" <<-EOF &&\n> \t\t#!$SHELL_PATH\n\nAck. Sorry for breaking this -- I suppose test-lint would have caught\nme out?\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"222833","messageId":"b9e889a27f648c44179ad39159240867f86525f3.1373296313.git.trast@inf.ethz.ch","threadId":"34378","inReplyTo":"8761wli0fe.fsf@linux-k42r.v.cablecom.net","subject":"[PATCH 1/2] t9902: fix 'test A == B' to use = operator","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-07-08T15:20:31Z","receivedAt":"2013-07-08T15:20:31Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"The == operator as an alias to = is not POSIX. This doesn't actually\nmatter for the execution of the script, because it only runs when the\nshell is bash. However, it trips up test-lint, so it's nicer to use\nthe standard form.\n\nSigned-off-by: Thomas Rast \n---\n t/t9902-completion.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex d9e3103..272a071 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -69,7 +69,7 @@ run_completion ()\n \tlocal -a COMPREPLY _words\n \tlocal _cword\n \t_words=( $1 )\n-\ttest \"${1: -1}\" == ' ' && _words+=('')\n+\ttest \"${1: -1}\" = ' ' && _words+=('')\n \t(( _cword = ${#_words[@]} - 1 ))\n \t__git_wrap__git_main && print_comp\n }\n-- \n1.8.3.2.947.g0347b11\n"},{"id":"222834","messageId":"24ee18d3b06883c4185821f5ffb5faed26e6a436.1373296313.git.trast@inf.ethz.ch","threadId":"34378","inReplyTo":"b9e889a27f648c44179ad39159240867f86525f3.1373296313.git.trast@inf.ethz.ch","subject":"[PATCH 2/2] test-lint: detect 'export FOO=bar'","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-07-08T15:20:32Z","receivedAt":"2013-07-08T15:20:32Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Some shells do not understand the one-line construct, and instead need\n\n FOO=bar &&\n export FOO\n\nDetect this in the test-lint target.\n\nSigned-off-by: Thomas Rast \n---\n\nI wrote:\n\n> Torsten Bögershausen writes:\n[...]\n> > -\t\texport HARNESS_ACTIVE=t &&\n> > +\t\tHARNESS_ACTIVE=t &&\n> > +\t\texport HARNESS_ACTIVE &&\n[...]\n> Ack. Sorry for breaking this -- I suppose test-lint would have caught\n> me out?\n\nWell, no, not yet.\n\n\n t/check-non-portable-shell.pl | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl\nindex 8b5a71d..45971f4 100755\n--- a/t/check-non-portable-shell.pl\n+++ b/t/check-non-portable-shell.pl\n@@ -21,6 +21,7 @@ while (<>) {\n \t/^\\s*declare\\s+/ and err 'arrays/declare not portable';\n \t/^\\s*[^#]\\s*which\\s/ and err 'which is not portable (please use type)';\n \t/test\\s+[^=]*==/ and err '\"test a == b\" is not portable (please use =)';\n+\t/^\\s*export\\s+[^=]*=/ and err '\"export FOO=bar\" is not portable (please use FOO=bar && export FOO)';\n \t# this resets our $. for each file\n \tclose ARGV if eof;\n }\n-- \n1.8.3.2.947.g0347b11\n"},{"id":"222836","messageId":"7vy59h13jc.fsf@alter.siamese.dyndns.org","threadId":"34378","inReplyTo":"8761wli0fe.fsf@linux-k42r.v.cablecom.net","subject":"Re: [PATCH] t0000: do not use export X=Y","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-08T15:20:55Z","receivedAt":"2013-07-08T15:20:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast writes:\n\n> Torsten Bögershausen writes:\n>\n>> The shell syntax \"export X=Y A=B\" is not understood by all shells.\n>>\n>> Signed-off-by: Torsten Bögershausen \n>> ---\n>> t/t0000-basic.sh | 3 ++-\n>> 1 file changed, 2 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\n>> index 5c32288..10be52b 100755\n>> --- a/t/t0000-basic.sh\n>> +++ b/t/t0000-basic.sh\n>> @@ -53,7 +53,8 @@ run_sub_test_lib_test () {\n>> \t\t# Pretend we're a test harness. This prevents\n>> \t\t# test-lib from writing the counts to a file that will\n>> \t\t# later be summarized, showing spurious \"failed\" tests\n>> -\t\texport HARNESS_ACTIVE=t &&\n>> +\t\tHARNESS_ACTIVE=t &&\n>> +\t\texport HARNESS_ACTIVE &&\n>> \t\tcd \"$name\" &&\n>> \t\tcat >\"$name.sh\" <<-EOF &&\n>> \t\t#!$SHELL_PATH\n>\n> Ack. Sorry for breaking this -- I suppose test-lint would have caught\n> me out?\n\nOr I should have. Thanks for catching.\n"},{"id":"222847","messageId":"7v1u790xzj.fsf@alter.siamese.dyndns.org","threadId":"34378","inReplyTo":"b9e889a27f648c44179ad39159240867f86525f3.1373296313.git.trast@inf.ethz.ch","subject":"Re: [PATCH 1/2] t9902: fix 'test A == B' to use = operator","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-08T17:20:48Z","receivedAt":"2013-07-08T17:20:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast writes:\n\n> The == operator as an alias to = is not POSIX. This doesn't actually\n> matter for the execution of the script, because it only runs when the\n> shell is bash. However, it trips up test-lint, so it's nicer to use\n> the standard form.\n\nOK, my knee-jerk reaction was \"this is only for bash\" as you said,\nbut the test-lint part I agree with.\n\nBut then test-lint _ought_ to also catch the use of \"local\" in the\nideal world, so perhaps in the longer term we would need to treat\nthis bash-only script differently from others anyway???\n\n>\n> Signed-off-by: Thomas Rast \n> ---\n> t/t9902-completion.sh | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\n> index d9e3103..272a071 100755\n> --- a/t/t9902-completion.sh\n> +++ b/t/t9902-completion.sh\n> @@ -69,7 +69,7 @@ run_completion ()\n> \tlocal -a COMPREPLY _words\n> \tlocal _cword\n> \t_words=( $1 )\n> -\ttest \"${1: -1}\" == ' ' && _words+=('')\n> +\ttest \"${1: -1}\" = ' ' && _words+=('')\n> \t(( _cword = ${#_words[@]} - 1 ))\n> \t__git_wrap__git_main && print_comp\n> }\n"},{"id":"222856","messageId":"51DB1399.9020307@web.de","threadId":"34378","inReplyTo":"24ee18d3b06883c4185821f5ffb5faed26e6a436.1373296313.git.trast@inf.ethz.ch","subject":"Re: [PATCH 2/2] test-lint: detect 'export FOO=bar'","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2013-07-08T19:31:37Z","receivedAt":"2013-07-08T19:31:37Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 2013-07-08 17.20, Thomas Rast wrote:\n> Some shells do not understand the one-line construct, and instead need\n> \n> FOO=bar &&\n> export FOO\n> \n> Detect this in the test-lint target.\n> \n> Signed-off-by: Thomas Rast \n> ---\n> \n> I wrote:\n> \n>> Torsten Bögershausen writes:\n> [...]\n>>> -\t\texport HARNESS_ACTIVE=t &&\n>>> +\t\tHARNESS_ACTIVE=t &&\n>>> +\t\texport HARNESS_ACTIVE &&\n> [...]\n>> Ack. Sorry for breaking this -- I suppose test-lint would have caught\n>> me out?\n> \n> Well, no, not yet.\nThanks for working on this\n\n> \n> \n> t/check-non-portable-shell.pl | 1 +\n> 1 file changed, 1 insertion(+)\n> \n> diff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl\n> index 8b5a71d..45971f4 100755\n> --- a/t/check-non-portable-shell.pl\n> +++ b/t/check-non-portable-shell.pl\n> @@ -21,6 +21,7 @@ while (<>) {\n> \t/^\\s*declare\\s+/ and err 'arrays/declare not portable';\n> \t/^\\s*[^#]\\s*which\\s/ and err 'which is not portable (please use type)';\n> \t/test\\s+[^=]*==/ and err '\"test a == b\" is not portable (please use =)';\n> +\t/^\\s*export\\s+[^=]*=/ and err '\"export FOO=bar\" is not portable (please use FOO=bar && export FOO)';\nI have a slightly tighter reg exp in my tree, but credits should go to Thomas: \n\n/^\\s*export\\s+\\S+=\\S+/\n\n/Torsten\n"},{"id":"222890","messageId":"87fvvogk1x.fsf@linux-k42r.v.cablecom.net","threadId":"34378","inReplyTo":"7v1u790xzj.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] t9902: fix 'test A == B' to use = operator","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-07-09T09:27:22Z","receivedAt":"2013-07-09T09:27:22Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Junio C Hamano writes:\n\n> Thomas Rast writes:\n>\n>> The == operator as an alias to = is not POSIX. This doesn't actually\n>> matter for the execution of the script, because it only runs when the\n>> shell is bash. However, it trips up test-lint, so it's nicer to use\n>> the standard form.\n>\n> OK, my knee-jerk reaction was \"this is only for bash\" as you said,\n> but the test-lint part I agree with.\n>\n> But then test-lint _ought_ to also catch the use of \"local\" in the\n> ideal world, so perhaps in the longer term we would need to treat\n> this bash-only script differently from others anyway???\n\nTrue. I didn't really think about wider implications; I just noticed\nthat there was an easy-to-fix complaint from test-lint :-)\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"222891","messageId":"87a9lwgk0e.fsf@linux-k42r.v.cablecom.net","threadId":"34378","inReplyTo":"51DB1399.9020307@web.de","subject":"Re: [PATCH 2/2] test-lint: detect 'export FOO=bar'","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-07-09T09:28:17Z","receivedAt":"2013-07-09T09:28:17Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Torsten Bögershausen writes:\n\n>> +\t/^\\s*export\\s+[^=]*=/ and err '\"export FOO=bar\" is not portable (please use FOO=bar && export FOO)';\n> I have a slightly tighter reg exp in my tree, but credits should go to Thomas: \n>\n> /^\\s*export\\s+\\S+=\\S+/\n\nHmm, is that correct? I would expect shells that have problems with\n'export FOO=bar' to also fail on 'export FOO=' (i.e. set to empty string\nand export).\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"222895","messageId":"51DBDDA2.8070304@web.de","threadId":"34378","inReplyTo":"87a9lwgk0e.fsf@linux-k42r.v.cablecom.net","subject":"Re: [PATCH 2/2] test-lint: detect 'export FOO=bar'","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2013-07-09T09:53:38Z","receivedAt":"2013-07-09T09:53:38Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 09.07.13 11:28, Thomas Rast wrote:\n> Torsten Bögershausen writes:\n> \n>>> +\t/^\\s*export\\s+[^=]*=/ and err '\"export FOO=bar\" is not portable (please use FOO=bar && export FOO)';\n>> I have a slightly tighter reg exp in my tree, but credits should go to Thomas: \n>>\n>> /^\\s*export\\s+\\S+=\\S+/\n> \n> Hmm, is that correct? I would expect shells that have problems with\n> 'export FOO=bar' to also fail on 'export FOO=' (i.e. set to empty string\n> and export).\n> \nGood point, thanks\n"}]}