{"thread":{"id":"61818","subject":"[PATCH 2/4] t4034: fix use of one-shot variable assignment with shell function","startedAt":"2024-07-22T07:01:30Z","lastAt":"2024-07-27T05:37:02Z","messageCount":37,"participants":["Eric Sunshine","Rubén Justo","Phillip Wood","Kyle Lippincott","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"499040","messageId":"20240722065915.80760-3-ericsunshine@charter.net","threadId":"61818","inReplyTo":"20240722065915.80760-1-ericsunshine@charter.net","subject":"[PATCH 2/4] t4034: fix use of one-shot variable assignment with shell function","fromName":"Eric Sunshine","fromEmail":"ericsunshine@charter.net","sentAt":"2024-07-22T06:59:12Z","receivedAt":"2024-07-22T07:01:30Z","isPatch":true,"sender":{"key":"ericsunshine@charter.net","avatar":null},"body":"From: Eric Sunshine \n\nUnlike \"VAR=val cmd\" one-shot environment variable assignments which\nexist only for the invocation of 'cmd', those assigned by \"VAR=val\nshell-func\" exist within the running shell and continue to do so until\nthe process exits (or are explicitly unset). In most cases, it is\nunlikely that this behavior was intended by the test author, and, even\nif those leaked assignments do not impact other tests today, they can\nnegatively impact tests added later by authors unaware that the variable\nassignments are still hanging around. Address this shortcoming by\nensuring that the assignments are short-lived.\n\nSigned-off-by: Eric Sunshine \n---\n t/t4034-diff-words.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh\nindex 74586f3813..4dcd7e9925 100755\n--- a/t/t4034-diff-words.sh\n+++ b/t/t4034-diff-words.sh\n@@ -70,7 +70,7 @@ test_language_driver () {\n \t\tword_diff --color-words\n \t'\n \ttest_expect_success \"diff driver '$lang' in Islandic\" '\n-\t\tLANG=is_IS.UTF-8 LANGUAGE=is LC_ALL=\"$is_IS_locale\" \\\n+\t\ttest_env LANG=is_IS.UTF-8 LANGUAGE=is LC_ALL=\"$is_IS_locale\" \\\n \t\tword_diff --color-words\n \t'\n }\n-- \n2.45.2\n\n"},{"id":"499041","messageId":"20240722065915.80760-1-ericsunshine@charter.net","threadId":"61818","inReplyTo":null,"subject":"[PATCH 0/4] improve one-shot variable detection with shell function","fromName":"Eric Sunshine","fromEmail":"ericsunshine@charter.net","sentAt":"2024-07-22T06:59:10Z","receivedAt":"2024-07-22T07:01:30Z","isPatch":true,"sender":{"key":"ericsunshine@charter.net","avatar":null},"body":"From: Eric Sunshine \n\nThis series addresses a blind-spot of check-non-portable-shell's\ndetection of one-shot environment variable assignment with shell\nfunctions. In particular, although it correctly detects:\n\n VAR=val shell-func\n\nit will miss invocations such as:\n\n echo X | VAR=val shell-func\n\nReferences:\nhttps://lore.kernel.org/git/CAPig+cRyj8J7MZEufu34NUzwOL2n=w35nT1Ug7FGRwMC0=Qpwg@mail.gmail.com/\nhttps://lore.kernel.org/git/bc1b9cce-d04d-4a79-8fab-55ec3c8bae30@gmail.com/\n\nEric Sunshine (4):\n t3430: modernize one-shot \"VAR=val shell-func\" invocation\n t4034: fix use of one-shot variable assignment with shell function\n check-non-portable-shell: improve `VAR=val shell-func` detection\n check-non-portable-shell: suggest alternative for `VAR=val shell-func`\n\n t/check-non-portable-shell.pl | 4 ++--\n t/t3430-rebase-merges.sh | 4 ++--\n t/t4034-diff-words.sh | 2 +-\n 3 files changed, 5 insertions(+), 5 deletions(-)\n\n-- \n2.45.2\n\n"},{"id":"499044","messageId":"20240722065915.80760-2-ericsunshine@charter.net","threadId":"61818","inReplyTo":"20240722065915.80760-1-ericsunshine@charter.net","subject":"[PATCH 1/4] t3430: modernize one-shot \"VAR=val shell-func\" invocation","fromName":"Eric Sunshine","fromEmail":"ericsunshine@charter.net","sentAt":"2024-07-22T06:59:11Z","receivedAt":"2024-07-22T07:01:30Z","isPatch":true,"sender":{"key":"ericsunshine@charter.net","avatar":null},"body":"From: Eric Sunshine \n\nUnlike \"VAR=val cmd\" one-shot environment variable assignments which\nexist only for the invocation of 'cmd', those assigned by \"VAR=val\nshell-func\" exist within the running shell and continue to do so until\nthe process exits (or are explicitly unset). check-non-portable-shell.pl\nwarns when it detects such usage since, more often than not, the author\nwho writes such an invocation is unaware of the undesirable behavior.\n\nA common way to work around the problem is to wrap a subshell around the\nvariable assignments and function call, thus ensuring that the\nassignments are short-lived. However, these days, a more ergonomic\napproach is to employ test_env() which is tailor-made for this specific\nuse-case.\n\nSigned-off-by: Eric Sunshine \n---\n t/t3430-rebase-merges.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh\nindex 36ca126bcd..e851ede4f9 100755\n--- a/t/t3430-rebase-merges.sh\n+++ b/t/t3430-rebase-merges.sh\n@@ -392,8 +392,8 @@ test_expect_success 'refuse to merge ancestors of HEAD' '\n \n test_expect_success 'root commits' '\n \tgit checkout --orphan unrelated &&\n-\t(GIT_AUTHOR_NAME=\"Parsnip\" GIT_AUTHOR_EMAIL=\"root@example.com\" \\\n-\t test_commit second-root) &&\n+\ttest_env GIT_AUTHOR_NAME=\"Parsnip\" GIT_AUTHOR_EMAIL=\"root@example.com\" \\\n+\t\ttest_commit second-root &&\n \ttest_commit third-root &&\n \tcat >script-from-scratch <<-\\EOF &&\n \tpick third-root\n-- \n2.45.2\n\n"},{"id":"499042","messageId":"20240722065915.80760-4-ericsunshine@charter.net","threadId":"61818","inReplyTo":"20240722065915.80760-1-ericsunshine@charter.net","subject":"[PATCH 3/4] check-non-portable-shell: improve `VAR=val shell-func` detection","fromName":"Eric Sunshine","fromEmail":"ericsunshine@charter.net","sentAt":"2024-07-22T06:59:13Z","receivedAt":"2024-07-22T07:01:31Z","isPatch":true,"sender":{"key":"ericsunshine@charter.net","avatar":null},"body":"From: Eric Sunshine \n\nUnlike \"VAR=val cmd\" one-shot environment variable assignments which\nexist only for the invocation of 'cmd', those assigned by \"VAR=val\nshell-func\" exist within the running shell and continue to do so until\nthe process exits. check-non-portable-shell.pl warns when it detects\nsuch usage since, more often than not, the author who writes such an\ninvocation is unaware of the undesirable behavior.\n\nHowever, a limitation of the check is that it only detects such\ninvocations when variable assignment (i.e. `VAR=val`) is the first\nthing on the line. Thus, it can easily be fooled by an invocation such\nas:\n\n echo X | VAR=val shell-func\n\nAddress this shortcoming by loosening the check so that the variable\nassignment can be recognized even when not at the beginning of the line.\n\nSigned-off-by: Eric Sunshine \n---\n t/check-non-portable-shell.pl | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl\nindex b2b28c2ced..44b23d6ddd 100755\n--- a/t/check-non-portable-shell.pl\n+++ b/t/check-non-portable-shell.pl\n@@ -49,7 +49,7 @@ sub err {\n \t/\\bexport\\s+[A-Za-z0-9_]*=/ and err '\"export FOO=bar\" is not portable (use FOO=bar && export FOO)';\n \t/\\blocal\\s+[A-Za-z0-9_]*=\\$([A-Za-z0-9_{]|[(][^(])/ and\n \t\terr q(quote \"$val\" in 'local var=$val');\n-\t/^\\s*([A-Z0-9_]+=(\\w*|([\"']).*?\\3)\\s+)+(\\w+)/ and exists($func{$4}) and\n+\t/\\b([A-Z0-9_]+=(\\w*|([\"']).*?\\3)\\s+)+(\\w+)/ and !/test_env.+=/ and exists($func{$4}) and\n \t\terr '\"FOO=bar shell_func\" assignment extends beyond \"shell_func\"';\n \t$line = '';\n \t# this resets our $. for each file\n-- \n2.45.2\n\n"},{"id":"499043","messageId":"20240722065915.80760-5-ericsunshine@charter.net","threadId":"61818","inReplyTo":"20240722065915.80760-1-ericsunshine@charter.net","subject":"[PATCH 4/4] check-non-portable-shell: suggest alternative for `VAR=val shell-func`","fromName":"Eric Sunshine","fromEmail":"ericsunshine@charter.net","sentAt":"2024-07-22T06:59:14Z","receivedAt":"2024-07-22T07:01:31Z","isPatch":true,"sender":{"key":"ericsunshine@charter.net","avatar":null},"body":"From: Eric Sunshine \n\nMost problems reported by check-non-portable-shell are accompanied by\nadvice suggesting how the test author can repair the problem. For\ninstance:\n\n error: egrep/fgrep obsolescent (use grep -E/-F)\n\nHowever, when one-shot variable assignment is detected when calling a\nshell function (i.e. `VAR=val shell-func`), the problem is reported, but\nno advice is given. The lack of advice is particularly egregious since\nneither the problem nor the workaround are likely well-known by\nnewcomers to the project writing tests for the first time. Address this\nshortcoming by recommending the use of test_env() which is tailor made\nfor this specific use-case.\n\nSigned-off-by: Eric Sunshine \n---\n t/check-non-portable-shell.pl | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl\nindex 44b23d6ddd..56db7cc6ed 100755\n--- a/t/check-non-portable-shell.pl\n+++ b/t/check-non-portable-shell.pl\n@@ -50,7 +50,7 @@ sub err {\n \t/\\blocal\\s+[A-Za-z0-9_]*=\\$([A-Za-z0-9_{]|[(][^(])/ and\n \t\terr q(quote \"$val\" in 'local var=$val');\n \t/\\b([A-Z0-9_]+=(\\w*|([\"']).*?\\3)\\s+)+(\\w+)/ and !/test_env.+=/ and exists($func{$4}) and\n-\t\terr '\"FOO=bar shell_func\" assignment extends beyond \"shell_func\"';\n+\t\terr '\"FOO=bar shell_func\" assignment extends beyond \"shell_func\" (use test_env FOO=bar shell_func)';\n \t$line = '';\n \t# this resets our $. for each file\n \tclose ARGV if eof;\n-- \n2.45.2\n\n"},{"id":"499056","messageId":"8c6559cd-d18b-442d-b692-f1611f1907f4@gmail.com","threadId":"61818","inReplyTo":"20240722065915.80760-4-ericsunshine@charter.net","subject":"Re: [PATCH 3/4] check-non-portable-shell: improve `VAR=val shell-func` detection","fromName":"Rubén Justo","fromEmail":"rjusto@gmail.com","sentAt":"2024-07-22T14:46:46Z","receivedAt":"2024-07-22T14:46:49Z","isPatch":true,"sender":{"key":"rjusto@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5685487?v=4"},"body":"On Mon, Jul 22, 2024 at 02:59:13AM -0400, Eric Sunshine wrote:\n> From: Eric Sunshine \n> \n> Unlike \"VAR=val cmd\" one-shot environment variable assignments which\n> exist only for the invocation of 'cmd', those assigned by \"VAR=val\n> shell-func\" exist within the running shell and continue to do so until\n> the process exits. check-non-portable-shell.pl warns when it detects\n> such usage since, more often than not, the author who writes such an\n> invocation is unaware of the undesirable behavior.\n> \n> However, a limitation of the check is that it only detects such\n> invocations when variable assignment (i.e. `VAR=val`) is the first\n> thing on the line. Thus, it can easily be fooled by an invocation such\n> as:\n> \n> echo X | VAR=val shell-func\n> \n> Address this shortcoming by loosening the check so that the variable\n> assignment can be recognized even when not at the beginning of the line.\n> \n> Signed-off-by: Eric Sunshine \n> ---\n> t/check-non-portable-shell.pl | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl\n> index b2b28c2ced..44b23d6ddd 100755\n> --- a/t/check-non-portable-shell.pl\n> +++ b/t/check-non-portable-shell.pl\n> @@ -49,7 +49,7 @@ sub err {\n> \t/\\bexport\\s+[A-Za-z0-9_]*=/ and err '\"export FOO=bar\" is not portable (use FOO=bar && export FOO)';\n> \t/\\blocal\\s+[A-Za-z0-9_]*=\\$([A-Za-z0-9_{]|[(][^(])/ and\n> \t\terr q(quote \"$val\" in 'local var=$val');\n> -\t/^\\s*([A-Z0-9_]+=(\\w*|([\"']).*?\\3)\\s+)+(\\w+)/ and exists($func{$4}) and\n> +\t/\\b([A-Z0-9_]+=(\\w*|([\"']).*?\\3)\\s+)+(\\w+)/ and !/test_env.+=/ and exists($func{$4}) and\n\nLosing \"^\\s*\" means we'll cause false positives, such as:\n\n # VAR=VAL shell-func\n echo VAR=VAL shell-func\n\nRegardless of that, the regex will continue to pose problems with:\n\n VAR=$OTHER_VALUE shell-func\n VAR=$(cmd) shell-func\n VAR=VAL\\ UE shell-func\n VAR=\"\\\"val\\\" shell-func UE\" non-shell-func \n\nWhich, of course, should be cases that should be written in a more\northodox way. \n\nBut we will start to detect errors like the ones mentioned in the\nmessage, which are more likely to happen.\n\nI think this change is a good step forward, and I'm happy with it as it\nis.\n\nThanks\n\n> \t\terr '\"FOO=bar shell_func\" assignment extends beyond \"shell_func\"';\n> \t$line = '';\n> \t# this resets our $. for each file\n> -- \n> 2.45.2\n> \n"},{"id":"499057","messageId":"fd67d8c6-8d8d-4127-9833-5808909dd26b@gmail.com","threadId":"61818","inReplyTo":"20240722065915.80760-5-ericsunshine@charter.net","subject":"Re: [PATCH 4/4] check-non-portable-shell: suggest alternative for `VAR=val shell-func`","fromName":"Rubén Justo","fromEmail":"rjusto@gmail.com","sentAt":"2024-07-22T14:47:24Z","receivedAt":"2024-07-22T14:47:27Z","isPatch":true,"sender":{"key":"rjusto@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5685487?v=4"},"body":"On 7/22/24 8:59 AM, Eric Sunshine wrote:\n> From: Eric Sunshine \n> \n> Most problems reported by check-non-portable-shell are accompanied by\n> advice suggesting how the test author can repair the problem. For\n> instance:\n> \n> error: egrep/fgrep obsolescent (use grep -E/-F)\n> \n> However, when one-shot variable assignment is detected when calling a\n> shell function (i.e. `VAR=val shell-func`), the problem is reported, but\n> no advice is given. The lack of advice is particularly egregious since\n> neither the problem nor the workaround are likely well-known by\n> newcomers to the project writing tests for the first time. Address this\n> shortcoming by recommending the use of test_env() which is tailor made\n> for this specific use-case.\n> \n> Signed-off-by: Eric Sunshine \n> ---\n> t/check-non-portable-shell.pl | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl\n> index 44b23d6ddd..56db7cc6ed 100755\n> --- a/t/check-non-portable-shell.pl\n> +++ b/t/check-non-portable-shell.pl\n> @@ -50,7 +50,7 @@ sub err {\n> \t/\\blocal\\s+[A-Za-z0-9_]*=\\$([A-Za-z0-9_{]|[(][^(])/ and\n> \t\terr q(quote \"$val\" in 'local var=$val');\n> \t/\\b([A-Z0-9_]+=(\\w*|([\"']).*?\\3)\\s+)+(\\w+)/ and !/test_env.+=/ and exists($func{$4}) and\n> -\t\terr '\"FOO=bar shell_func\" assignment extends beyond \"shell_func\"';\n> +\t\terr '\"FOO=bar shell_func\" assignment extends beyond \"shell_func\" (use test_env FOO=bar shell_func)';\n\nThis is a nice improvement.\n\n> \t$line = '';\n> \t# this resets our $. for each file\n> \tclose ARGV if eof;\n"},{"id":"499058","messageId":"2e1c8fc6-86f0-404f-bef6-9502aa0d31d0@gmail.com","threadId":"61818","inReplyTo":"20240722065915.80760-1-ericsunshine@charter.net","subject":"Re: [PATCH 0/4] improve one-shot variable detection with shell function","fromName":"Rubén Justo","fromEmail":"rjusto@gmail.com","sentAt":"2024-07-22T14:50:30Z","receivedAt":"2024-07-22T14:50:33Z","isPatch":true,"sender":{"key":"rjusto@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5685487?v=4"},"body":"On Mon, Jul 22, 2024 at 02:59:10AM -0400, Eric Sunshine wrote:\n> From: Eric Sunshine \n> \n> This series addresses a blind-spot of check-non-portable-shell's\n> detection of one-shot environment variable assignment with shell\n> functions. In particular, although it correctly detects:\n> \n> VAR=val shell-func\n> \n> it will miss invocations such as:\n> \n> echo X | VAR=val shell-func\n> \n> References:\n> https://lore.kernel.org/git/CAPig+cRyj8J7MZEufu34NUzwOL2n=w35nT1Ug7FGRwMC0=Qpwg@mail.gmail.com/\n> https://lore.kernel.org/git/bc1b9cce-d04d-4a79-8fab-55ec3c8bae30@gmail.com/\n> \n> Eric Sunshine (4):\n> t3430: modernize one-shot \"VAR=val shell-func\" invocation\n> t4034: fix use of one-shot variable assignment with shell function\n> check-non-portable-shell: improve `VAR=val shell-func` detection\n> check-non-portable-shell: suggest alternative for `VAR=val shell-func`\n\nAll these changes look good to me.\n\nThanks.\n\n> \n> t/check-non-portable-shell.pl | 4 ++--\n> t/t3430-rebase-merges.sh | 4 ++--\n> t/t4034-diff-words.sh | 2 +-\n> 3 files changed, 5 insertions(+), 5 deletions(-)\n> \n> -- \n> 2.45.2\n> \n"},{"id":"499060","messageId":"c586f7dc-636b-45a3-acb2-faedfe1068e6@gmail.com","threadId":"61818","inReplyTo":"20240722065915.80760-2-ericsunshine@charter.net","subject":"Re: [PATCH 1/4] t3430: modernize one-shot \"VAR=val shell-func\" invocation","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-07-22T15:09:59Z","receivedAt":"2024-07-22T15:10:07Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Eric\n\nOn 22/07/2024 07:59, Eric Sunshine wrote:\n> From: Eric Sunshine \n> \n> Unlike \"VAR=val cmd\" one-shot environment variable assignments which\n> exist only for the invocation of 'cmd', those assigned by \"VAR=val\n> shell-func\" exist within the running shell and continue to do so until\n> the process exits (or are explicitly unset).\n\nI'm not sure I follow. If I run\n\nsh -c 'f() {\n echo \"f: HELLO=$HELLO\"\n env | grep HELLO\n}\nHELLO=x f; echo \"HELLO=$HELLO\"'\n\nThen I see\n\nf: HELLO=x\nHELLO=x\nHELLO=\n\nwhich seems to contradict the commit message as $HELLO is unset when the \nfunction returns. I see the same result if I replace \"sh\" (which is bash \non my system) with an explicit \"bash\", \"dash\" or \"zsh\".\n\nI'm also confused as to why this caused a problem for Rubén's test as \n$HELLO is set in the environment so I'm don't understand why git wasn't \npicking up the right pager.\n\n> check-non-portable-shell.pl\n> warns when it detects such usage since, more often than not, the author\n> who writes such an invocation is unaware of the undesirable behavior.\n> \n> A common way to work around the problem is to wrap a subshell around the\n> variable assignments and function call, thus ensuring that the\n> assignments are short-lived. However, these days, a more ergonomic\n> approach is to employ test_env() which is tailor-made for this specific\n> use-case.\n\nOh, that sounds useful, I didn't know it existed.\n\nBest Wishes\n\nPhillip\n\n> Signed-off-by: Eric Sunshine \n> ---\n> t/t3430-rebase-merges.sh | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh\n> index 36ca126bcd..e851ede4f9 100755\n> --- a/t/t3430-rebase-merges.sh\n> +++ b/t/t3430-rebase-merges.sh\n> @@ -392,8 +392,8 @@ test_expect_success 'refuse to merge ancestors of HEAD' '\n> \n> test_expect_success 'root commits' '\n> \tgit checkout --orphan unrelated &&\n> -\t(GIT_AUTHOR_NAME=\"Parsnip\" GIT_AUTHOR_EMAIL=\"root@example.com\" \\\n> -\t test_commit second-root) &&\n> +\ttest_env GIT_AUTHOR_NAME=\"Parsnip\" GIT_AUTHOR_EMAIL=\"root@example.com\" \\\n> +\t\ttest_commit second-root &&\n> \ttest_commit third-root &&\n> \tcat >script-from-scratch <<-\\EOF &&\n> \tpick third-root\n"},{"id":"499067","messageId":"CAO_smVg8+WCG0dWZNPVbDM4gBJLLHrg96nOCzje6B3hUGneDGg@mail.gmail.com","threadId":"61818","inReplyTo":"20240722065915.80760-4-ericsunshine@charter.net","subject":"Re: [PATCH 3/4] check-non-portable-shell: improve `VAR=val shell-func` detection","fromName":"Kyle Lippincott","fromEmail":"spectral@google.com","sentAt":"2024-07-22T17:26:10Z","receivedAt":"2024-07-22T17:26:26Z","isPatch":true,"sender":{"key":"spectral@google.com","avatar":"https://avatars.githubusercontent.com/u/6371650?v=4"},"body":"On Mon, Jul 22, 2024 at 12:01 AM Eric Sunshine wrote:\n>\n> From: Eric Sunshine \n>\n> Unlike \"VAR=val cmd\" one-shot environment variable assignments which\n> exist only for the invocation of 'cmd', those assigned by \"VAR=val\n> shell-func\" exist within the running shell and continue to do so until\n> the process exits. check-non-portable-shell.pl warns when it detects\n> such usage since, more often than not, the author who writes such an\n> invocation is unaware of the undesirable behavior.\n>\n> However, a limitation of the check is that it only detects such\n> invocations when variable assignment (i.e. `VAR=val`) is the first\n> thing on the line. Thus, it can easily be fooled by an invocation such\n> as:\n>\n> echo X | VAR=val shell-func\n>\n> Address this shortcoming by loosening the check so that the variable\n> assignment can be recognized even when not at the beginning of the line.\n>\n> Signed-off-by: Eric Sunshine \n> ---\n> t/check-non-portable-shell.pl | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl\n> index b2b28c2ced..44b23d6ddd 100755\n> --- a/t/check-non-portable-shell.pl\n> +++ b/t/check-non-portable-shell.pl\n> @@ -49,7 +49,7 @@ sub err {\n> /\\bexport\\s+[A-Za-z0-9_]*=/ and err '\"export FOO=bar\" is not portable (use FOO=bar && export FOO)';\n> /\\blocal\\s+[A-Za-z0-9_]*=\\$([A-Za-z0-9_{]|[(][^(])/ and\n> err q(quote \"$val\" in 'local var=$val');\n> - /^\\s*([A-Z0-9_]+=(\\w*|([\"']).*?\\3)\\s+)+(\\w+)/ and exists($func{$4}) and\n> + /\\b([A-Z0-9_]+=(\\w*|([\"']).*?\\3)\\s+)+(\\w+)/ and !/test_env.+=/ and exists($func{$4}) and\n> err '\"FOO=bar shell_func\" assignment extends beyond \"shell_func\"';\n\nIs there an example of a shell on Linux that has this behavior that I\ncan observe, and/or reproduction steps? `bash --posix` does not do\nthis, as far as I can tell. I tried:\n\n```\nbash --posix\necho_hi() { echo hi; }\nFOO=BAR echo_hi\necho $FOO\n\n```\n\nand the simpler:\n\n```\nbash --posix\nFOO=BAR echo hi\necho $FOO\n```\n\nBoth attempts were done interactively, not via a script.\n\nI'm asking mostly because of the recent \"platform support document\"\npatch series - this is a very surprising behavior (which is presumably\nwhy it was added to this test), and it's possible this was just a bug\nin a shell that isn't really used anymore. Having documentation on how\nto reproduce the issue lets us know when we can remove these kinds of\nrestrictions on our codebase that we took in the name of\ncompatibility, possibly over a decade ago.\n\n> $line = '';\n> # this resets our $. for each file\n> --\n> 2.45.2\n>\n>\n"},{"id":"499071","messageId":"xmqq34o1cn6b.fsf@gitster.g","threadId":"61818","inReplyTo":"CAO_smVg8+WCG0dWZNPVbDM4gBJLLHrg96nOCzje6B3hUGneDGg@mail.gmail.com","subject":"Re: [PATCH 3/4] check-non-portable-shell: improve `VAR=val shell-func` detection","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-07-22T18:10:52Z","receivedAt":"2024-07-22T18:10:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kyle Lippincott writes:\n\n> Is there an example of a shell on Linux that has this behavior that I\n> can observe, and/or reproduction steps?\n\nEvery once in a while this comes up and we fix, e.g.\n\nhttps://lore.kernel.org/git/528CE716.8060307@ramsay1.demon.co.uk/\nhttps://lore.kernel.org/git/c6efda03848abc00cf8bf8d84fc34ef0d652b64c.1264151435.git.mhagger@alum.mit.edu/\nhttps://lore.kernel.org/git/Koa4iojOlOQ_YENPwWXKt7G8Aa1x6UaBnFFtliKdZmpcrrqOBhY7NQ@cipher.nrlssc.navy.mil/\nhttps://lore.kernel.org/git/20180713055205.32351-2-sunshine@sunshineco.com/\nhttps://lore.kernel.org/git/574E27A4.6040804@ramsayjones.plus.com/\n\nwhich is from a query\n\n https://lore.kernel.org/git/?q=one-shot+export+shell+function\n\nbut unfortunately we do not document which exact shell the observed\nbreakage happened with.\n\nThe closest article I found that is suitable as a discussion\nreignitor talks about what POSIX requires, which may be more\nrelevant:\n\n https://lore.kernel.org/git/4B5027B8.2090507@viscovery.net/\n\n"},{"id":"499072","messageId":"xmqqv80xb7z1.fsf@gitster.g","threadId":"61818","inReplyTo":"20240722065915.80760-2-ericsunshine@charter.net","subject":"Re: [PATCH 1/4] t3430: modernize one-shot \"VAR=val shell-func\" invocation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-07-22T18:24:34Z","receivedAt":"2024-07-22T18:24:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> A common way to work around the problem is to wrap a subshell around the\n> variable assignments and function call, thus ensuring that the\n> assignments are short-lived. However, these days, a more ergonomic\n> approach is to employ test_env() which is tailor-made for this specific\n> use-case.\n\nOK. I am not sure if that is \"ergonomic\", though. An explict\nsubshell has a good documentation value that even though we call\ntest_commit there, we do not care about the committer timestamps\nsubsequent commits would record, and we do not mind losing the\neffect of test_tick from this invocation of test_commit. Hiding\nall of that behind test_env loses the documentation value.\n\nWe could resurrect it by explicitly passing \"--no-tick\" to\ntest_commit, though ;-).\n\n> Signed-off-by: Eric Sunshine \n> ---\n> t/t3430-rebase-merges.sh | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh\n> index 36ca126bcd..e851ede4f9 100755\n> --- a/t/t3430-rebase-merges.sh\n> +++ b/t/t3430-rebase-merges.sh\n> @@ -392,8 +392,8 @@ test_expect_success 'refuse to merge ancestors of HEAD' '\n> \n> test_expect_success 'root commits' '\n> \tgit checkout --orphan unrelated &&\n> -\t(GIT_AUTHOR_NAME=\"Parsnip\" GIT_AUTHOR_EMAIL=\"root@example.com\" \\\n> -\t test_commit second-root) &&\n> +\ttest_env GIT_AUTHOR_NAME=\"Parsnip\" GIT_AUTHOR_EMAIL=\"root@example.com\" \\\n> +\t\ttest_commit second-root &&\n> \ttest_commit third-root &&\n> \tcat >script-from-scratch <<-\\EOF &&\n> \tpick third-root\n"},{"id":"499086","messageId":"CAO_smVhd_fWkC1=9r_ASCEPoM_rRap3DAWq--nq+6dQ8M8qzjQ@mail.gmail.com","threadId":"61818","inReplyTo":"xmqq34o1cn6b.fsf@gitster.g","subject":"Re: [PATCH 3/4] check-non-portable-shell: improve `VAR=val shell-func` detection","fromName":"Kyle Lippincott","fromEmail":"spectral@google.com","sentAt":"2024-07-22T21:35:18Z","receivedAt":"2024-07-22T21:35:31Z","isPatch":true,"sender":{"key":"spectral@google.com","avatar":"https://avatars.githubusercontent.com/u/6371650?v=4"},"body":"On Mon, Jul 22, 2024 at 11:10 AM Junio C Hamano wrote:\n>\n> Kyle Lippincott writes:\n>\n> > Is there an example of a shell on Linux that has this behavior that I\n> > can observe, and/or reproduction steps?\n>\n> Every once in a while this comes up and we fix, e.g.\n>\n> https://lore.kernel.org/git/528CE716.8060307@ramsay1.demon.co.uk/\n> https://lore.kernel.org/git/c6efda03848abc00cf8bf8d84fc34ef0d652b64c.1264151435.git.mhagger@alum.mit.edu/\n> https://lore.kernel.org/git/Koa4iojOlOQ_YENPwWXKt7G8Aa1x6UaBnFFtliKdZmpcrrqOBhY7NQ@cipher.nrlssc.navy.mil/\n> https://lore.kernel.org/git/20180713055205.32351-2-sunshine@sunshineco.com/\n\nThanks, this one leads to\nhttps://lore.kernel.org/git/20180713055205.32351-1-sunshine@sunshineco.com/,\nwhich references\nhttps://public-inbox.org/git/xmqqefg8w73c.fsf@gitster-ct.c.googlers.com/T/,\nwhich claims that `dash` has this behavior 6 years ago. The version of\n`dash` I have on my machine right now doesn't seem to have this issue,\nbut I can believe some older version does.\n\n> https://lore.kernel.org/git/574E27A4.6040804@ramsayjones.plus.com/\n>\n> which is from a query\n>\n> https://lore.kernel.org/git/?q=one-shot+export+shell+function\n>\n> but unfortunately we do not document which exact shell the observed\n> breakage happened with.\n>\n> The closest article I found that is suitable as a discussion\n> reignitor talks about what POSIX requires, which may be more\n> relevant:\n>\n> https://lore.kernel.org/git/4B5027B8.2090507@viscovery.net/\n\nThis claims that `ksh` \"gets it right\", and I can confirm that ksh\ndoes behave this way on my Linux machine.\n\nHaving just looked at the POSIX standard (I don't think I'm allowed to\ncopy from this document), the POSIX standard (POSIX.1-2024, at least)\nexplicitly leaves it unspecified whether the variable assignments\nremain in effect after function execution.\n\nThanks for indulging my curiosity; should we include a statement in\nthe linter along the lines of `# POSIX.1-2024 explicitly does not\nspecify if variable assignment persists after executing a shell\nfunction; some shells, such as ksh, have these variables remain.`?\n"},{"id":"499087","messageId":"xmqq34o19jj1.fsf@gitster.g","threadId":"61818","inReplyTo":"CAO_smVhd_fWkC1=9r_ASCEPoM_rRap3DAWq--nq+6dQ8M8qzjQ@mail.gmail.com","subject":"Re: [PATCH 3/4] check-non-portable-shell: improve `VAR=val shell-func` detection","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-07-22T21:57:54Z","receivedAt":"2024-07-22T21:58:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kyle Lippincott writes:\n\n> Having just looked at the POSIX standard (I don't think I'm allowed to\n> copy from this document), the POSIX standard (POSIX.1-2024, at least)\n> explicitly leaves it unspecified whether the variable assignments\n> remain in effect after function execution.\n\nTrue.\n\nhttps://pubs.opengroup.org/onlinepubs/9799919799/utilities/V3_chap02.html#tag_19_09_01_02\n\nalso says that it is unspecified if the variable gets exported, and\nolder version of dash that comes on Ubuntu 20.04 chooses *not* to\nexport, which was the test breakage that triggered this whole\ndiscussion. The thread can be seen here:\n\nhttps://lore.kernel.org/git/xmqqbk2p9lwi.fsf_-_@gitster.g/\n\n> Thanks for indulging my curiosity; should we include a statement in\n> the linter along the lines of `# POSIX.1-2024 explicitly does not\n> specify if variable assignment persists after executing a shell\n> function; some shells, such as ksh, have these variables remain.`?\n\nGiving a review (either positive or negative is fine, as long as it\nis constructive) on the update to CodingGuidelines\n\n https://lore.kernel.org/git/xmqqbk2p9lwi.fsf_-_@gitster.g/\n\nmay be a good place to start.\n\nThanks.\n"},{"id":"499142","messageId":"1b672247-c05d-44f5-967a-9861d715040b@gmail.com","threadId":"61818","inReplyTo":"c586f7dc-636b-45a3-acb2-faedfe1068e6@gmail.com","subject":"Re: [PATCH 1/4] t3430: modernize one-shot \"VAR=val shell-func\" invocation","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-07-23T09:26:50Z","receivedAt":"2024-07-23T09:26:53Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 22/07/2024 16:09, Phillip Wood wrote:\n> Hi Eric\n> \n> On 22/07/2024 07:59, Eric Sunshine wrote:\n>> From: Eric Sunshine \n>>\n>> Unlike \"VAR=val cmd\" one-shot environment variable assignments which\n>> exist only for the invocation of 'cmd', those assigned by \"VAR=val\n>> shell-func\" exist within the running shell and continue to do so until\n>> the process exits (or are explicitly unset).\n\nHaving seen the parallel discussion about the behavior of hash this \nconstruct is non-portable because the behavior differs between shells so \nperhaps the commit message could say something like\n\nUnlike \"VAR=val cmd\" one-shot environment variable assignments which \nonly exist for the invocation of external command \"cmd\", the behavior of \n\"VAR=val func\" where \"func\" is a shell function or builtin command \nvaries between shells and so we should not use it in our test suite.\n\nBest Wishes\n\nPhillip\n\n> I'm not sure I follow. If I run\n> \n> sh -c 'f() {\n>     echo \"f: HELLO=$HELLO\"\n>     env | grep HELLO\n> }\n> HELLO=x f; echo \"HELLO=$HELLO\"'\n> \n> Then I see\n> \n> f: HELLO=x\n> HELLO=x\n> HELLO=\n> \n> which seems to contradict the commit message as $HELLO is unset when the \n> function returns. I see the same result if I replace \"sh\" (which is bash \n> on my system) with an explicit \"bash\", \"dash\" or \"zsh\".\n> \n> I'm also confused as to why this caused a problem for Rubén's test as \n> $HELLO is set in the environment so I'm don't understand why git wasn't \n> picking up the right pager.\n> \n>> check-non-portable-shell.pl\n>> warns when it detects such usage since, more often than not, the author\n>> who writes such an invocation is unaware of the undesirable behavior.\n>>\n>> A common way to work around the problem is to wrap a subshell around the\n>> variable assignments and function call, thus ensuring that the\n>> assignments are short-lived. However, these days, a more ergonomic\n>> approach is to employ test_env() which is tailor-made for this specific\n>> use-case.\n> \n> Oh, that sounds useful, I didn't know it existed.\n> \n> Best Wishes\n> \n> Phillip\n> \n>> Signed-off-by: Eric Sunshine \n>> ---\n>>   t/t3430-rebase-merges.sh | 4 ++--\n>>   1 file changed, 2 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh\n>> index 36ca126bcd..e851ede4f9 100755\n>> --- a/t/t3430-rebase-merges.sh\n>> +++ b/t/t3430-rebase-merges.sh\n>> @@ -392,8 +392,8 @@ test_expect_success 'refuse to merge ancestors of \n>> HEAD' '\n>>   test_expect_success 'root commits' '\n>>       git checkout --orphan unrelated &&\n>> -    (GIT_AUTHOR_NAME=\"Parsnip\" GIT_AUTHOR_EMAIL=\"root@example.com\" \\\n>> -     test_commit second-root) &&\n>> +    test_env GIT_AUTHOR_NAME=\"Parsnip\" \n>> GIT_AUTHOR_EMAIL=\"root@example.com\" \\\n>> +        test_commit second-root &&\n>>       test_commit third-root &&\n>>       cat >script-from-scratch <<-\\EOF &&\n>>       pick third-root\n\n"},{"id":"499392","messageId":"CAPig+cQNfZp8Sh4s8ukF0Vto-+RqyHeTM0jJK4mOJhkmxz2s1Q@mail.gmail.com","threadId":"61818","inReplyTo":"c586f7dc-636b-45a3-acb2-faedfe1068e6@gmail.com","subject":"Re: [PATCH 1/4] t3430: modernize one-shot \"VAR=val shell-func\" invocation","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-07-26T06:15:59Z","receivedAt":"2024-07-26T06:16:11Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Jul 22, 2024 at 11:10 AM Phillip Wood wrote:\n> On 22/07/2024 07:59, Eric Sunshine wrote:\n> > Unlike \"VAR=val cmd\" one-shot environment variable assignments which\n> > exist only for the invocation of 'cmd', those assigned by \"VAR=val\n> > shell-func\" exist within the running shell and continue to do so until\n> > the process exits (or are explicitly unset).\n>\n> I'm not sure I follow. If I run\n>\n> sh -c 'f() {\n> echo \"f: HELLO=$HELLO\"\n> env | grep HELLO\n> }\n> HELLO=x f; echo \"HELLO=$HELLO\"'\n>\n> Then I see\n>\n> f: HELLO=x\n> HELLO=x\n> HELLO=\n>\n> which seems to contradict the commit message as $HELLO is unset when the\n> function returns. I see the same result if I replace \"sh\" (which is bash\n> on my system) with an explicit \"bash\", \"dash\" or \"zsh\".\n\nI believe downstream discussion[1][2] established that the behavior is\ninconsistent between various shells and versions of shells, and is\nconsidered undefined by POSIX.\n\n[1]: https://lore.kernel.org/git/xmqq34o1cn6b.fsf@gitster.g/\n[2]: https://lore.kernel.org/git/xmqqbk2p9lwi.fsf_-_@gitster.g/\n\n> I'm also confused as to why this caused a problem for Rubén's test as\n> $HELLO is set in the environment so I'm don't understand why git wasn't\n> picking up the right pager.\n\nJunio summarized the problem and explanation[3].\n\n[3]: https://lore.kernel.org/git/xmqq7cdd9l0m.fsf@gitster.g/\n\n> > A common way to work around the problem is to wrap a subshell around the\n> > variable assignments and function call, thus ensuring that the\n> > assignments are short-lived. However, these days, a more ergonomic\n> > approach is to employ test_env() which is tailor-made for this specific\n> > use-case.\n>\n> Oh, that sounds useful, I didn't know it existed.\n\nI didn't know about it either, and only discovered it upon my initial\nattempt at making check-non-portable-shell.pl recognize the case Rubén\nidentified, at which point it started showing false-positives on\n`test_env` invocations. Actually, considering that I was involved[4]\nin the conversation which led to the introduction[5] of `test_env` by\nPeff, it may be that I did know about it but forgot.\n\n[4]: https://lore.kernel.org/git/CAPig+cR989yU4+JNTFREaeXqY61nusUOhufeBGGVCi29tR1P5w@mail.gmail.com/\n[5]: https://lore.kernel.org/git/20160601070425.GA13648@sigill.intra.peff.net/\n"},{"id":"499394","messageId":"CAPig+cSYDM-B4wLWX2euAEVH2FutOeFy4sTRJnWk_Qm7Y7so3g@mail.gmail.com","threadId":"61818","inReplyTo":"xmqqv80xb7z1.fsf@gitster.g","subject":"Re: [PATCH 1/4] t3430: modernize one-shot \"VAR=val shell-func\" invocation","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-07-26T06:30:54Z","receivedAt":"2024-07-26T06:31:06Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Jul 22, 2024 at 2:24 PM Junio C Hamano wrote:\n> Eric Sunshine writes:\n> > A common way to work around the problem is to wrap a subshell around the\n> > variable assignments and function call, thus ensuring that the\n> > assignments are short-lived. However, these days, a more ergonomic\n> > approach is to employ test_env() which is tailor-made for this specific\n> > use-case.\n>\n> OK. I am not sure if that is \"ergonomic\", though. An explict\n> subshell has a good documentation value that even though we call\n> test_commit there, we do not care about the committer timestamps\n> subsequent commits would record, and we do not mind losing the\n> effect of test_tick from this invocation of test_commit. Hiding\n> all of that behind test_env loses the documentation value.\n>\n> We could resurrect it by explicitly passing \"--no-tick\" to\n> test_commit, though ;-).\n\nYour mention of `test_commit` reminded me that, these days, it accepts\nan --author switch which seems tailor-made for what this chunk of code\nin this test is actually checking: namely, that the root commit of the\norphan branch has \"Parsnip \" as its author. So an\neven simpler approach is to change it to:\n\n test_commit --author \"Parsnip \" second-root &&\n\nThat conveys precisely that the test is interested in overriding the\nauthor for that one commit.\n"},{"id":"499395","messageId":"CAPig+cRfMVZv2hMQRtPmy+HYpxV5=4XBWypPHAL4uwmv8WmT=w@mail.gmail.com","threadId":"61818","inReplyTo":"1b672247-c05d-44f5-967a-9861d715040b@gmail.com","subject":"Re: [PATCH 1/4] t3430: modernize one-shot \"VAR=val shell-func\" invocation","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-07-26T06:33:30Z","receivedAt":"2024-07-26T06:33:42Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Jul 23, 2024 at 5:26 AM Phillip Wood wrote:\n> On 22/07/2024 16:09, Phillip Wood wrote:\n> > On 22/07/2024 07:59, Eric Sunshine wrote:\n> >> Unlike \"VAR=val cmd\" one-shot environment variable assignments which\n> >> exist only for the invocation of 'cmd', those assigned by \"VAR=val\n> >> shell-func\" exist within the running shell and continue to do so until\n> >> the process exits (or are explicitly unset).\n>\n> Having seen the parallel discussion about the behavior of hash this\n> construct is non-portable because the behavior differs between shells so\n> perhaps the commit message could say something like\n>\n> Unlike \"VAR=val cmd\" one-shot environment variable assignments which\n> only exist for the invocation of external command \"cmd\", the behavior of\n> \"VAR=val func\" where \"func\" is a shell function or builtin command\n> varies between shells and so we should not use it in our test suite.\n\nIndeed, given all the subsequent discussion, it is now apparent that\nmultiple undesirable behaviors have been experienced, not just the one\nmentioned by this commit message, and that POSIX states that the\nbehavior is undefined.\n"},{"id":"499396","messageId":"CAPig+cRH+mVgCf3UMQmiG6QueELCrAKGMikc6OtZMK845QDccA@mail.gmail.com","threadId":"61818","inReplyTo":"8c6559cd-d18b-442d-b692-f1611f1907f4@gmail.com","subject":"Re: [PATCH 3/4] check-non-portable-shell: improve `VAR=val shell-func` detection","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-07-26T06:45:59Z","receivedAt":"2024-07-26T06:46:10Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Jul 22, 2024 at 10:46 AM Rubén Justo wrote:\n> On Mon, Jul 22, 2024 at 02:59:13AM -0400, Eric Sunshine wrote:\n> > - /^\\s*([A-Z0-9_]+=(\\w*|([\"']).*?\\3)\\s+)+(\\w+)/ and exists($func{$4}) and\n> > + /\\b([A-Z0-9_]+=(\\w*|([\"']).*?\\3)\\s+)+(\\w+)/ and !/test_env.+=/ and exists($func{$4}) and\n>\n> Losing \"^\\s*\" means we'll cause false positives, such as:\n>\n> # VAR=VAL shell-func\n> echo VAR=VAL shell-func\n\nTrue, though, considering that \"shell-func\" in these examples must\nmatch the name of a function actually defined in one of the input\nfiles, one would expect (or at least hope) that this sort of\nfalse-positive will be exceedingly rare. Indeed, there are no such\nfalse-positives in the existing test scripts. Of course, we can always\ntighten the regex later if it proves to be problematic.\n\n> Regardless of that, the regex will continue to pose problems with:\n>\n> VAR=$OTHER_VALUE shell-func\n> VAR=$(cmd) shell-func\n> VAR=VAL\\ UE shell-func\n> VAR=\"\\\"val\\\" shell-func UE\" non-shell-func\n>\n> Which, of course, should be cases that should be written in a more\n> orthodox way.\n\nYes, it can be difficult to be thorough when \"linting\" a programming\nlanguage merely via regular-expressions, and this particular\nexpression is already almost unreadable. The effort involved in trying\nto make it perfect may very well outweigh the potential gain in\ncoverage.\n\n> But we will start to detect errors like the ones mentioned in the\n> message, which are more likely to happen.\n\nIndeed.\n"},{"id":"499401","messageId":"20240726081522.28015-3-ericsunshine@charter.net","threadId":"61818","inReplyTo":"20240726081522.28015-1-ericsunshine@charter.net","subject":"[PATCH v2 2/5] t4034: fix use of one-shot variable assignment with shell function","fromName":"Eric Sunshine","fromEmail":"ericsunshine@charter.net","sentAt":"2024-07-26T08:15:19Z","receivedAt":"2024-07-26T08:17:50Z","isPatch":true,"sender":{"key":"ericsunshine@charter.net","avatar":null},"body":"From: Eric Sunshine \n\nThe behavior of a one-shot environment variable assignment of the form\n\"VAR=val cmd\" is undefined according to POSIX when \"cmd\" is a shell\nfunction. Indeed the behavior differs between shell implementations and\neven different versions of the same shell, thus should be avoided.\n\nSigned-off-by: Eric Sunshine \n---\n t/t4034-diff-words.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh\nindex 74586f3813..4dcd7e9925 100755\n--- a/t/t4034-diff-words.sh\n+++ b/t/t4034-diff-words.sh\n@@ -70,7 +70,7 @@ test_language_driver () {\n \t\tword_diff --color-words\n \t'\n \ttest_expect_success \"diff driver '$lang' in Islandic\" '\n-\t\tLANG=is_IS.UTF-8 LANGUAGE=is LC_ALL=\"$is_IS_locale\" \\\n+\t\ttest_env LANG=is_IS.UTF-8 LANGUAGE=is LC_ALL=\"$is_IS_locale\" \\\n \t\tword_diff --color-words\n \t'\n }\n-- \n2.45.2\n\n"},{"id":"499402","messageId":"20240726081522.28015-1-ericsunshine@charter.net","threadId":"61818","inReplyTo":"20240722065915.80760-1-ericsunshine@charter.net","subject":"[PATCH v2 0/5] improve one-shot variable detection with shell function","fromName":"Eric Sunshine","fromEmail":"ericsunshine@charter.net","sentAt":"2024-07-26T08:15:17Z","receivedAt":"2024-07-26T08:17:50Z","isPatch":true,"sender":{"key":"ericsunshine@charter.net","avatar":null},"body":"From: Eric Sunshine \n\nThis is a reroll of [1] which improves check-non-portable-shell's\ndetection of one-shot environment variable assignment with shell\nfunctions. Changes since v1:\n\n* commit messages now state the behavior is undefined according to POSIX\n rather than focusing only on the original reason given (that the\n assignments could outlive the shell function invocation)\n\n* t3430 simplified by dropping the subshell altogether in favor of\n `test_commit --author`\n\n* new commit to improve the error message when one-shot assignment with\n shell function is detected\n\n[1]: https://lore.kernel.org/git/20240722065915.80760-1-ericsunshine@charter.net/\n\nEric Sunshine (5):\n t3430: drop unnecessary one-shot \"VAR=val shell-func\" invocation\n t4034: fix use of one-shot variable assignment with shell function\n check-non-portable-shell: loosen one-shot assignment error message\n check-non-portable-shell: suggest alternative for `VAR=val shell-func`\n check-non-portable-shell: improve `VAR=val shell-func` detection\n\n t/check-non-portable-shell.pl | 4 ++--\n t/t3430-rebase-merges.sh | 3 +--\n t/t4034-diff-words.sh | 2 +-\n 3 files changed, 4 insertions(+), 5 deletions(-)\n\nRange-diff against v1:\n1: 5bb6811f68 ! 1: 0d3c0593c9 t3430: modernize one-shot \"VAR=val shell-func\" invocation\n @@ Metadata\n Author: Eric Sunshine \n \n ## Commit message ##\n - t3430: modernize one-shot \"VAR=val shell-func\" invocation\n + t3430: drop unnecessary one-shot \"VAR=val shell-func\" invocation\n \n - Unlike \"VAR=val cmd\" one-shot environment variable assignments which\n - exist only for the invocation of 'cmd', those assigned by \"VAR=val\n - shell-func\" exist within the running shell and continue to do so until\n - the process exits (or are explicitly unset). check-non-portable-shell.pl\n - warns when it detects such usage since, more often than not, the author\n - who writes such an invocation is unaware of the undesirable behavior.\n + The behavior of a one-shot environment variable assignment of the form\n + \"VAR=val cmd\" is undefined according to POSIX when \"cmd\" is a shell\n + function. Indeed the behavior differs between shell implementations and\n + even different versions of the same shell. One such ill-defined behavior\n + is that, with some shells, the assignment will outlive the invocation of\n + the function, thus may potentially impact subsequent commands in the\n + test, as well as subsequent tests. A common way to work around the\n + problem is to wrap a subshell around the one-shot assignment, thus\n + ensuring that the assignment is short-lived.\n \n - A common way to work around the problem is to wrap a subshell around the\n - variable assignments and function call, thus ensuring that the\n - assignments are short-lived. However, these days, a more ergonomic\n - approach is to employ test_env() which is tailor-made for this specific\n - use-case.\n + In this test, the subshell is employed precisely for this purpose; other\n + side-effects of the subshell, such as losing the effect of `test_tick`\n + which is invoked by `test_commit`, are immaterial.\n +\n + These days, we can take advantage of `test_commit --author` to more\n + clearly convey that the test is interested only in overriding the author\n + of the commit.\n \n Signed-off-by: Eric Sunshine \n \n @@ t/t3430-rebase-merges.sh: test_expect_success 'refuse to merge ancestors of HEAD\n \tgit checkout --orphan unrelated &&\n -\t(GIT_AUTHOR_NAME=\"Parsnip\" GIT_AUTHOR_EMAIL=\"root@example.com\" \\\n -\t test_commit second-root) &&\n -+\ttest_env GIT_AUTHOR_NAME=\"Parsnip\" GIT_AUTHOR_EMAIL=\"root@example.com\" \\\n -+\t\ttest_commit second-root &&\n ++\ttest_commit --author \"Parsnip \" second-root &&\n \ttest_commit third-root &&\n \tcat >script-from-scratch <<-\\EOF &&\n \tpick third-root\n2: 1f35449847 ! 2: 19ee8295ef t4034: fix use of one-shot variable assignment with shell function\n @@ Metadata\n ## Commit message ##\n t4034: fix use of one-shot variable assignment with shell function\n \n - Unlike \"VAR=val cmd\" one-shot environment variable assignments which\n - exist only for the invocation of 'cmd', those assigned by \"VAR=val\n - shell-func\" exist within the running shell and continue to do so until\n - the process exits (or are explicitly unset). In most cases, it is\n - unlikely that this behavior was intended by the test author, and, even\n - if those leaked assignments do not impact other tests today, they can\n - negatively impact tests added later by authors unaware that the variable\n - assignments are still hanging around. Address this shortcoming by\n - ensuring that the assignments are short-lived.\n + The behavior of a one-shot environment variable assignment of the form\n + \"VAR=val cmd\" is undefined according to POSIX when \"cmd\" is a shell\n + function. Indeed the behavior differs between shell implementations and\n + even different versions of the same shell, thus should be avoided.\n \n Signed-off-by: Eric Sunshine \n \n-: ---------- > 3: 220ca26d4f check-non-portable-shell: loosen one-shot assignment error message\n-: ---------- > 4: 4910756aab check-non-portable-shell: suggest alternative for `VAR=val shell-func`\n3: 89621f72a2 ! 5: 7a15553a5a check-non-portable-shell: improve `VAR=val shell-func` detection\n @@ Metadata\n ## Commit message ##\n check-non-portable-shell: improve `VAR=val shell-func` detection\n \n - Unlike \"VAR=val cmd\" one-shot environment variable assignments which\n - exist only for the invocation of 'cmd', those assigned by \"VAR=val\n - shell-func\" exist within the running shell and continue to do so until\n - the process exits. check-non-portable-shell.pl warns when it detects\n - such usage since, more often than not, the author who writes such an\n - invocation is unaware of the undesirable behavior.\n + The behavior of a one-shot environment variable assignment of the form\n + \"VAR=val cmd\" is undefined according to POSIX when \"cmd\" is a shell\n + function. Indeed the behavior differs between shell implementations and\n + even different versions of the same shell, thus should be avoided.\n \n + As such, check-non-portable-shell.pl warns when it detects such usage.\n However, a limitation of the check is that it only detects such\n - invocations when variable assignment (i.e. `VAR=val`) is the first\n - thing on the line. Thus, it can easily be fooled by an invocation such\n - as:\n + invocations when variable assignment (i.e. `VAR=val`) is the first thing\n + on the line. Thus, it can easily be fooled by an invocation such as:\n \n echo X | VAR=val shell-func\n \n @@ t/check-non-portable-shell.pl: sub err {\n \t\terr q(quote \"$val\" in 'local var=$val');\n -\t/^\\s*([A-Z0-9_]+=(\\w*|([\"']).*?\\3)\\s+)+(\\w+)/ and exists($func{$4}) and\n +\t/\\b([A-Z0-9_]+=(\\w*|([\"']).*?\\3)\\s+)+(\\w+)/ and !/test_env.+=/ and exists($func{$4}) and\n - \t\terr '\"FOO=bar shell_func\" assignment extends beyond \"shell_func\"';\n + \t\terr '\"FOO=bar shell_func\" is not portable (use test_env FOO=bar shell_func)';\n \t$line = '';\n \t# this resets our $. for each file\n4: 7b2e1dd895 < -: ---------- check-non-portable-shell: suggest alternative for `VAR=val shell-func`\n-- \n2.45.2\n\n"},{"id":"499403","messageId":"20240726081522.28015-4-ericsunshine@charter.net","threadId":"61818","inReplyTo":"20240726081522.28015-1-ericsunshine@charter.net","subject":"[PATCH v2 3/5] check-non-portable-shell: loosen one-shot assignment error message","fromName":"Eric Sunshine","fromEmail":"ericsunshine@charter.net","sentAt":"2024-07-26T08:15:20Z","receivedAt":"2024-07-26T08:17:50Z","isPatch":true,"sender":{"key":"ericsunshine@charter.net","avatar":null},"body":"From: Eric Sunshine \n\nWhen a0a630192d (t/check-non-portable-shell: detect \"FOO=bar\nshell_func\", 2018-07-13) added the check for one-shot environment\nvariable assignment for shell functions, the primary reason given for\navoiding them was that, under some shells, the assignment outlives the\ninvocation of the shell function, thus could potentially negatively\nimpact subsequent commands in the same test, as well as subsequent\ntests.\n\nHowever, it has recently become apparent that this is not the only\npotential problem with one-shot assignments and shell functions. Another\nproblem is that some shells do not actually export the variable to\ncommands which the function invokes[1]. More significantly, however, the\nbehavior of one-shot assignments with shell functions is considered\nundefined by POSIX[2].\n\nGiven this new understanding, the presented error message (\"assignment\nextends beyond 'shell_func'\") is too specific and potentially\nmisleading. Address this by emitting a less specific error message.\n\n(Note that the wording \"is not portable\" is chosen over the more\nspecific \"has undefined behavior according to POSIX\" for consistency\nwith almost all other error message issued by this \"lint\" script.)\n\n[1]: https://lore.kernel.org/git/xmqqbk2p9lwi.fsf_-_@gitster.g/\n[2]: https://lore.kernel.org/git/xmqq34o19jj1.fsf@gitster.g/\n\nSigned-off-by: Eric Sunshine \n---\n t/check-non-portable-shell.pl | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl\nindex b2b28c2ced..179efaa39d 100755\n--- a/t/check-non-portable-shell.pl\n+++ b/t/check-non-portable-shell.pl\n@@ -50,7 +50,7 @@ sub err {\n \t/\\blocal\\s+[A-Za-z0-9_]*=\\$([A-Za-z0-9_{]|[(][^(])/ and\n \t\terr q(quote \"$val\" in 'local var=$val');\n \t/^\\s*([A-Z0-9_]+=(\\w*|([\"']).*?\\3)\\s+)+(\\w+)/ and exists($func{$4}) and\n-\t\terr '\"FOO=bar shell_func\" assignment extends beyond \"shell_func\"';\n+\t\terr '\"FOO=bar shell_func\" is not portable';\n \t$line = '';\n \t# this resets our $. for each file\n \tclose ARGV if eof;\n-- \n2.45.2\n\n"},{"id":"499405","messageId":"20240726081522.28015-2-ericsunshine@charter.net","threadId":"61818","inReplyTo":"20240726081522.28015-1-ericsunshine@charter.net","subject":"[PATCH v2 1/5] t3430: drop unnecessary one-shot \"VAR=val shell-func\" invocation","fromName":"Eric Sunshine","fromEmail":"ericsunshine@charter.net","sentAt":"2024-07-26T08:15:18Z","receivedAt":"2024-07-26T08:17:50Z","isPatch":true,"sender":{"key":"ericsunshine@charter.net","avatar":null},"body":"From: Eric Sunshine \n\nThe behavior of a one-shot environment variable assignment of the form\n\"VAR=val cmd\" is undefined according to POSIX when \"cmd\" is a shell\nfunction. Indeed the behavior differs between shell implementations and\neven different versions of the same shell. One such ill-defined behavior\nis that, with some shells, the assignment will outlive the invocation of\nthe function, thus may potentially impact subsequent commands in the\ntest, as well as subsequent tests. A common way to work around the\nproblem is to wrap a subshell around the one-shot assignment, thus\nensuring that the assignment is short-lived.\n\nIn this test, the subshell is employed precisely for this purpose; other\nside-effects of the subshell, such as losing the effect of `test_tick`\nwhich is invoked by `test_commit`, are immaterial.\n\nThese days, we can take advantage of `test_commit --author` to more\nclearly convey that the test is interested only in overriding the author\nof the commit.\n\nSigned-off-by: Eric Sunshine \n---\n t/t3430-rebase-merges.sh | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh\nindex 36ca126bcd..2aa8593f77 100755\n--- a/t/t3430-rebase-merges.sh\n+++ b/t/t3430-rebase-merges.sh\n@@ -392,8 +392,7 @@ test_expect_success 'refuse to merge ancestors of HEAD' '\n \n test_expect_success 'root commits' '\n \tgit checkout --orphan unrelated &&\n-\t(GIT_AUTHOR_NAME=\"Parsnip\" GIT_AUTHOR_EMAIL=\"root@example.com\" \\\n-\t test_commit second-root) &&\n+\ttest_commit --author \"Parsnip \" second-root &&\n \ttest_commit third-root &&\n \tcat >script-from-scratch <<-\\EOF &&\n \tpick third-root\n-- \n2.45.2\n\n"},{"id":"499406","messageId":"20240726081522.28015-5-ericsunshine@charter.net","threadId":"61818","inReplyTo":"20240726081522.28015-1-ericsunshine@charter.net","subject":"[PATCH v2 4/5] check-non-portable-shell: suggest alternative for `VAR=val shell-func`","fromName":"Eric Sunshine","fromEmail":"ericsunshine@charter.net","sentAt":"2024-07-26T08:15:21Z","receivedAt":"2024-07-26T08:17:50Z","isPatch":true,"sender":{"key":"ericsunshine@charter.net","avatar":null},"body":"From: Eric Sunshine \n\nMost problems reported by check-non-portable-shell are accompanied by\nadvice suggesting how the test author can repair the problem. For\ninstance:\n\n error: egrep/fgrep obsolescent (use grep -E/-F)\n\nHowever, when one-shot variable assignment is detected when calling a\nshell function (i.e. `VAR=val shell-func`), the problem is reported, but\nno advice is given. The lack of advice is particularly egregious since\nneither the problem nor the workaround are likely well-known by\nnewcomers to the project writing tests for the first time. Address this\nshortcoming by recommending the use of `test_env` which is tailor made\nfor this specific use-case.\n\nSigned-off-by: Eric Sunshine \n---\n t/check-non-portable-shell.pl | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl\nindex 179efaa39d..903af14294 100755\n--- a/t/check-non-portable-shell.pl\n+++ b/t/check-non-portable-shell.pl\n@@ -50,7 +50,7 @@ sub err {\n \t/\\blocal\\s+[A-Za-z0-9_]*=\\$([A-Za-z0-9_{]|[(][^(])/ and\n \t\terr q(quote \"$val\" in 'local var=$val');\n \t/^\\s*([A-Z0-9_]+=(\\w*|([\"']).*?\\3)\\s+)+(\\w+)/ and exists($func{$4}) and\n-\t\terr '\"FOO=bar shell_func\" is not portable';\n+\t\terr '\"FOO=bar shell_func\" is not portable (use test_env FOO=bar shell_func)';\n \t$line = '';\n \t# this resets our $. for each file\n \tclose ARGV if eof;\n-- \n2.45.2\n\n"},{"id":"499404","messageId":"20240726081522.28015-6-ericsunshine@charter.net","threadId":"61818","inReplyTo":"20240726081522.28015-1-ericsunshine@charter.net","subject":"[PATCH v2 5/5] check-non-portable-shell: improve `VAR=val shell-func` detection","fromName":"Eric Sunshine","fromEmail":"ericsunshine@charter.net","sentAt":"2024-07-26T08:15:22Z","receivedAt":"2024-07-26T08:17:51Z","isPatch":true,"sender":{"key":"ericsunshine@charter.net","avatar":null},"body":"From: Eric Sunshine \n\nThe behavior of a one-shot environment variable assignment of the form\n\"VAR=val cmd\" is undefined according to POSIX when \"cmd\" is a shell\nfunction. Indeed the behavior differs between shell implementations and\neven different versions of the same shell, thus should be avoided.\n\nAs such, check-non-portable-shell.pl warns when it detects such usage.\nHowever, a limitation of the check is that it only detects such\ninvocations when variable assignment (i.e. `VAR=val`) is the first thing\non the line. Thus, it can easily be fooled by an invocation such as:\n\n echo X | VAR=val shell-func\n\nAddress this shortcoming by loosening the check so that the variable\nassignment can be recognized even when not at the beginning of the line.\n\nSigned-off-by: Eric Sunshine \n---\n t/check-non-portable-shell.pl | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl\nindex 903af14294..6ee7700eb4 100755\n--- a/t/check-non-portable-shell.pl\n+++ b/t/check-non-portable-shell.pl\n@@ -49,7 +49,7 @@ sub err {\n \t/\\bexport\\s+[A-Za-z0-9_]*=/ and err '\"export FOO=bar\" is not portable (use FOO=bar && export FOO)';\n \t/\\blocal\\s+[A-Za-z0-9_]*=\\$([A-Za-z0-9_{]|[(][^(])/ and\n \t\terr q(quote \"$val\" in 'local var=$val');\n-\t/^\\s*([A-Z0-9_]+=(\\w*|([\"']).*?\\3)\\s+)+(\\w+)/ and exists($func{$4}) and\n+\t/\\b([A-Z0-9_]+=(\\w*|([\"']).*?\\3)\\s+)+(\\w+)/ and !/test_env.+=/ and exists($func{$4}) and\n \t\terr '\"FOO=bar shell_func\" is not portable (use test_env FOO=bar shell_func)';\n \t$line = '';\n \t# this resets our $. for each file\n-- \n2.45.2\n\n"},{"id":"499441","messageId":"9d96b89f-f4e4-49f4-aa59-2c229d3988e4@gmail.com","threadId":"61818","inReplyTo":"20240726081522.28015-5-ericsunshine@charter.net","subject":"Re: [PATCH v2 4/5] check-non-portable-shell: suggest alternative for `VAR=val shell-func`","fromName":"Rubén Justo","fromEmail":"rjusto@gmail.com","sentAt":"2024-07-26T13:11:34Z","receivedAt":"2024-07-26T13:11:37Z","isPatch":true,"sender":{"key":"rjusto@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5685487?v=4"},"body":"On Fri, Jul 26, 2024 at 04:15:21AM -0400, Eric Sunshine wrote:\n> From: Eric Sunshine \n> \n> Most problems reported by check-non-portable-shell are accompanied by\n> advice suggesting how the test author can repair the problem. For\n> instance:\n> \n> error: egrep/fgrep obsolescent (use grep -E/-F)\n> \n> However, when one-shot variable assignment is detected when calling a\n> shell function (i.e. `VAR=val shell-func`), the problem is reported, but\n> no advice is given. The lack of advice is particularly egregious since\n> neither the problem nor the workaround are likely well-known by\n> newcomers to the project writing tests for the first time. Address this\n> shortcoming by recommending the use of `test_env` which is tailor made\n> for this specific use-case.\n> \n> Signed-off-by: Eric Sunshine \n> ---\n> t/check-non-portable-shell.pl | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl\n> index 179efaa39d..903af14294 100755\n> --- a/t/check-non-portable-shell.pl\n> +++ b/t/check-non-portable-shell.pl\n> @@ -50,7 +50,7 @@ sub err {\n> \t/\\blocal\\s+[A-Za-z0-9_]*=\\$([A-Za-z0-9_{]|[(][^(])/ and\n> \t\terr q(quote \"$val\" in 'local var=$val');\n> \t/^\\s*([A-Z0-9_]+=(\\w*|([\"']).*?\\3)\\s+)+(\\w+)/ and exists($func{$4}) and\n> -\t\terr '\"FOO=bar shell_func\" is not portable';\n> +\t\terr '\"FOO=bar shell_func\" is not portable (use test_env FOO=bar shell_func)';\n\nWhen someone blames this line in the future, the message of this commit\nwill appear and be informative. However, I think the message of the\nprevious patch [3/5], which also touches this line, would also be\nrelevant for this context. And it won't be so obvious to get to that\nmessage. Therefore, it might be worth combining this commit with the\nprevious one. But I'm not sure the change is worth it to have a new\niteration of this series.\n\nAnyway, the change in the err() is a convenient improvement.\n\n> \t$line = '';\n> \t# this resets our $. for each file\n> \tclose ARGV if eof;\n> -- \n> 2.45.2\n> \n"},{"id":"499442","messageId":"5b47c334-f536-48c6-a8ae-9f89910c51c8@gmail.com","threadId":"61818","inReplyTo":"CAPig+cRH+mVgCf3UMQmiG6QueELCrAKGMikc6OtZMK845QDccA@mail.gmail.com","subject":"Re: [PATCH 3/4] check-non-portable-shell: improve `VAR=val shell-func` detection","fromName":"Rubén Justo","fromEmail":"rjusto@gmail.com","sentAt":"2024-07-26T13:15:55Z","receivedAt":"2024-07-26T13:15:58Z","isPatch":true,"sender":{"key":"rjusto@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5685487?v=4"},"body":"On Fri, Jul 26, 2024 at 02:45:59AM -0400, Eric Sunshine wrote:\n> On Mon, Jul 22, 2024 at 10:46 AM Rubén Justo wrote:\n> > On Mon, Jul 22, 2024 at 02:59:13AM -0400, Eric Sunshine wrote:\n> > > - /^\\s*([A-Z0-9_]+=(\\w*|([\"']).*?\\3)\\s+)+(\\w+)/ and exists($func{$4}) and\n> > > + /\\b([A-Z0-9_]+=(\\w*|([\"']).*?\\3)\\s+)+(\\w+)/ and !/test_env.+=/ and exists($func{$4}) and\n> >\n> > Losing \"^\\s*\" means we'll cause false positives, such as:\n> >\n> > # VAR=VAL shell-func\n> > echo VAR=VAL shell-func\n> \n> True, though, considering that \"shell-func\" in these examples must\n> match the name of a function actually defined in one of the input\n> files, one would expect (or at least hope) that this sort of\n> false-positive will be exceedingly rare. Indeed, there are no such\n> false-positives in the existing test scripts. Of course, we can always\n> tighten the regex later if it proves to be problematic.\n> \n> > Regardless of that, the regex will continue to pose problems with:\n> >\n> > VAR=$OTHER_VALUE shell-func\n> > VAR=$(cmd) shell-func\n> > VAR=VAL\\ UE shell-func\n> > VAR=\"\\\"val\\\" shell-func UE\" non-shell-func\n> >\n> > Which, of course, should be cases that should be written in a more\n> > orthodox way.\n> \n> Yes, it can be difficult to be thorough when \"linting\" a programming\n> language merely via regular-expressions, and this particular\n> expression is already almost unreadable. The effort involved in trying\n> to make it perfect may very well outweigh the potential gain in\n> coverage.\n\nI tried to be exhaustive in the analysis of the change and explicit in\nthe conclusions so that it is clear, and documented in the list, that we\nacknowledge the magnitude of the change.\n\nI agree. I don't think it's worth refining the regex any further. It\nmight even be counterproductive. It covers the cases it was already\ncovering and the new ones that have occurred.\n\nA simple 'Acked-by: Rubén Justo ' didn't seem\nsufficient to me :), but perhaps it would have been clearer.\n\nI have the same positive opinion of your new iteration:\n20240722065915.80760-5-ericsunshine@charter.net\n\n> \n> > But we will start to detect errors like the ones mentioned in the\n> > message, which are more likely to happen.\n> \n> Indeed.\n"},{"id":"499462","messageId":"xmqq34nwuhg0.fsf@gitster.g","threadId":"61818","inReplyTo":"20240726081522.28015-1-ericsunshine@charter.net","subject":"Re: [PATCH v2 0/5] improve one-shot variable detection with shell function","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-07-26T18:38:39Z","receivedAt":"2024-07-26T18:38:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> @@ t/t3430-rebase-merges.sh: test_expect_success 'refuse to merge ancestors of HEAD\n> \tgit checkout --orphan unrelated &&\n> -\t(GIT_AUTHOR_NAME=\"Parsnip\" GIT_AUTHOR_EMAIL=\"root@example.com\" \\\n> -\t test_commit second-root) &&\n> -+\ttest_env GIT_AUTHOR_NAME=\"Parsnip\" GIT_AUTHOR_EMAIL=\"root@example.com\" \\\n> -+\t\ttest_commit second-root &&\n> ++\ttest_commit --author \"Parsnip \" second-root &&\n\nVery pleasing to the eyes.\n\n> @@ t/check-non-portable-shell.pl: sub err {\n> \t\terr q(quote \"$val\" in 'local var=$val');\n> -\t/^\\s*([A-Z0-9_]+=(\\w*|([\"']).*?\\3)\\s+)+(\\w+)/ and exists($func{$4}) and\n> +\t/\\b([A-Z0-9_]+=(\\w*|([\"']).*?\\3)\\s+)+(\\w+)/ and !/test_env.+=/ and exists($func{$4}) and\n> - \t\terr '\"FOO=bar shell_func\" assignment extends beyond \"shell_func\"';\n> + \t\terr '\"FOO=bar shell_func\" is not portable (use test_env FOO=bar shell_func)';\n\nOK.\n\nThanks. Will queue.\n"},{"id":"499463","messageId":"xmqqplr0t2bo.fsf@gitster.g","threadId":"61818","inReplyTo":"20240726081522.28015-2-ericsunshine@charter.net","subject":"Re: [PATCH v2 1/5] t3430: drop unnecessary one-shot \"VAR=val shell-func\" invocation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-07-26T18:50:35Z","receivedAt":"2024-07-26T18:50:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> From: Eric Sunshine \n>\n> The behavior of a one-shot environment variable assignment of the form\n> \"VAR=val cmd\" is undefined according to POSIX when \"cmd\" is a shell\n\nPlease use the right word to describe what the standard says.\n\nThroughout the topic's discussion, you seem to be repeating\n\"undefined\", but the word POSIX uses for this particular unportable\nbehaviour is \"unspecified\". The differences are subtle, and for\nprograms that want to be conformant, there is no practical\ndifference (in other words, we should not rely on the existence or\nvalidity of the value or behaviour if we wanted to be portable).\n\nThe former is what results from use of an invalid construct or\nfeeding an invalid data input. The implementation can do whatever\nit wants to do once you trigger an undefined behaviour. The latter\nis what results from use of a valid construct or valid data input,\nbut outcome may differ across implementations. An \"unspecified\"\nbehaviour often are still consistent and sensible within a single\nconformant implementation.\n\n"},{"id":"499465","messageId":"CAPig+cTKnWjaTkh_TwCKqu9Yks9KwOfe_+3xON+ErGYucLny9g@mail.gmail.com","threadId":"61818","inReplyTo":"9d96b89f-f4e4-49f4-aa59-2c229d3988e4@gmail.com","subject":"Re: [PATCH v2 4/5] check-non-portable-shell: suggest alternative for `VAR=val shell-func`","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-07-26T19:31:09Z","receivedAt":"2024-07-26T19:31:21Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jul 26, 2024 at 9:11 AM Rubén Justo wrote:\n> On Fri, Jul 26, 2024 at 04:15:21AM -0400, Eric Sunshine wrote:\n> > - err '\"FOO=bar shell_func\" is not portable';\n> > + err '\"FOO=bar shell_func\" is not portable (use test_env FOO=bar shell_func)';\n>\n> When someone blames this line in the future, the message of this commit\n> will appear and be informative. However, I think the message of the\n> previous patch [3/5], which also touches this line, would also be\n> relevant for this context. And it won't be so obvious to get to that\n> message. Therefore, it might be worth combining this commit with the\n> previous one. But I'm not sure the change is worth it to have a new\n> iteration of this series.\n\nI did consider combining the two patches but decided against it.\nDespite the fact that both patches touch the same line/message, they\nreally are two distinct \"fixes\" as evidenced by the fact that the\nexplanation provided by each commit message is entirely orthogonal to\nthe other.\n"},{"id":"499466","messageId":"CAPig+cSOggARypGJzjKBe82DtdFGz9OGKm55sB5_pj2d79fD=Q@mail.gmail.com","threadId":"61818","inReplyTo":"xmqqplr0t2bo.fsf@gitster.g","subject":"Re: [PATCH v2 1/5] t3430: drop unnecessary one-shot \"VAR=val shell-func\" invocation","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-07-26T19:32:17Z","receivedAt":"2024-07-26T19:32:30Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jul 26, 2024 at 2:50 PM Junio C Hamano wrote:\n> Eric Sunshine writes:\n> > The behavior of a one-shot environment variable assignment of the form\n> > \"VAR=val cmd\" is undefined according to POSIX when \"cmd\" is a shell\n>\n> Please use the right word to describe what the standard says.\n>\n> Throughout the topic's discussion, you seem to be repeating\n> \"undefined\", but the word POSIX uses for this particular unportable\n> behaviour is \"unspecified\". The differences are subtle, and for\n> programs that want to be conformant, there is no practical\n> difference (in other words, we should not rely on the existence or\n> validity of the value or behaviour if we wanted to be portable).\n>\n> The former is what results from use of an invalid construct or\n> feeding an invalid data input. The implementation can do whatever\n> it wants to do once you trigger an undefined behaviour. The latter\n> is what results from use of a valid construct or valid data input,\n> but outcome may differ across implementations. An \"unspecified\"\n> behaviour often are still consistent and sensible within a single\n> conformant implementation.\n\nMakes sense. Will adjust the commit messages.\n"},{"id":"499476","messageId":"20240727053509.34339-1-ericsunshine@charter.net","threadId":"61818","inReplyTo":"20240726081522.28015-1-ericsunshine@charter.net","subject":"[PATCH v3 0/5] improve one-shot variable detection with shell function","fromName":"Eric Sunshine","fromEmail":"ericsunshine@charter.net","sentAt":"2024-07-27T05:35:04Z","receivedAt":"2024-07-27T05:37:00Z","isPatch":true,"sender":{"key":"ericsunshine@charter.net","avatar":null},"body":"From: Eric Sunshine \n\nThis is a reroll of [1] which improves check-non-portable-shell's\ndetection of one-shot environment variable assignment with shell\nfunctions.\n\nThe only difference from v2 is that the commit messages have been\nadjusted to use more accurate terminology[2]. In particular, they now\nsay that the behavior of one-shot variable assignment with a\nshell-function is _unspecified_, not _undefined_.\n\n[1]: https://lore.kernel.org/git/20240726081522.28015-1-ericsunshine@charter.net/\n[2]: https://lore.kernel.org/git/xmqqplr0t2bo.fsf@gitster.g/\n\nEric Sunshine (5):\n t3430: drop unnecessary one-shot \"VAR=val shell-func\" invocation\n t4034: fix use of one-shot variable assignment with shell function\n check-non-portable-shell: loosen one-shot assignment error message\n check-non-portable-shell: suggest alternative for `VAR=val shell-func`\n check-non-portable-shell: improve `VAR=val shell-func` detection\n\n t/check-non-portable-shell.pl | 4 ++--\n t/t3430-rebase-merges.sh | 3 +--\n t/t4034-diff-words.sh | 2 +-\n 3 files changed, 4 insertions(+), 5 deletions(-)\n\nRange-diff against v2:\n1: 0d3c0593c9 ! 1: 3bf12762a5 t3430: drop unnecessary one-shot \"VAR=val shell-func\" invocation\n @@ Commit message\n t3430: drop unnecessary one-shot \"VAR=val shell-func\" invocation\n \n The behavior of a one-shot environment variable assignment of the form\n - \"VAR=val cmd\" is undefined according to POSIX when \"cmd\" is a shell\n + \"VAR=val cmd\" is unspecified according to POSIX when \"cmd\" is a shell\n function. Indeed the behavior differs between shell implementations and\n - even different versions of the same shell. One such ill-defined behavior\n + even different versions of the same shell. One such problematic behavior\n is that, with some shells, the assignment will outlive the invocation of\n the function, thus may potentially impact subsequent commands in the\n test, as well as subsequent tests. A common way to work around the\n2: 19ee8295ef ! 2: cb77c3dc66 t4034: fix use of one-shot variable assignment with shell function\n @@ Commit message\n t4034: fix use of one-shot variable assignment with shell function\n \n The behavior of a one-shot environment variable assignment of the form\n - \"VAR=val cmd\" is undefined according to POSIX when \"cmd\" is a shell\n + \"VAR=val cmd\" is unspecified according to POSIX when \"cmd\" is a shell\n function. Indeed the behavior differs between shell implementations and\n even different versions of the same shell, thus should be avoided.\n \n3: 220ca26d4f ! 3: 0b3716cfb3 check-non-portable-shell: loosen one-shot assignment error message\n @@ Commit message\n potential problem with one-shot assignments and shell functions. Another\n problem is that some shells do not actually export the variable to\n commands which the function invokes[1]. More significantly, however, the\n - behavior of one-shot assignments with shell functions is considered\n - undefined by POSIX[2].\n + behavior of one-shot assignments with shell functions is not specified\n + by POSIX[2].\n \n Given this new understanding, the presented error message (\"assignment\n extends beyond 'shell_func'\") is too specific and potentially\n misleading. Address this by emitting a less specific error message.\n \n (Note that the wording \"is not portable\" is chosen over the more\n - specific \"has undefined behavior according to POSIX\" for consistency\n - with almost all other error message issued by this \"lint\" script.)\n + specific \"behavior not specified by POSIX\" for consistency with almost\n + all other error message issued by this \"lint\" script.)\n \n [1]: https://lore.kernel.org/git/xmqqbk2p9lwi.fsf_-_@gitster.g/\n [2]: https://lore.kernel.org/git/xmqq34o19jj1.fsf@gitster.g/\n4: 4910756aab = 4: 24ae9be947 check-non-portable-shell: suggest alternative for `VAR=val shell-func`\n5: 7a15553a5a ! 5: 38cd3556c5 check-non-portable-shell: improve `VAR=val shell-func` detection\n @@ Commit message\n check-non-portable-shell: improve `VAR=val shell-func` detection\n \n The behavior of a one-shot environment variable assignment of the form\n - \"VAR=val cmd\" is undefined according to POSIX when \"cmd\" is a shell\n + \"VAR=val cmd\" is unspecified according to POSIX when \"cmd\" is a shell\n function. Indeed the behavior differs between shell implementations and\n even different versions of the same shell, thus should be avoided.\n \n-- \n2.45.2\n\n"},{"id":"499477","messageId":"20240727053509.34339-2-ericsunshine@charter.net","threadId":"61818","inReplyTo":"20240727053509.34339-1-ericsunshine@charter.net","subject":"[PATCH v3 1/5] t3430: drop unnecessary one-shot \"VAR=val shell-func\" invocation","fromName":"Eric Sunshine","fromEmail":"ericsunshine@charter.net","sentAt":"2024-07-27T05:35:05Z","receivedAt":"2024-07-27T05:37:00Z","isPatch":true,"sender":{"key":"ericsunshine@charter.net","avatar":null},"body":"From: Eric Sunshine \n\nThe behavior of a one-shot environment variable assignment of the form\n\"VAR=val cmd\" is unspecified according to POSIX when \"cmd\" is a shell\nfunction. Indeed the behavior differs between shell implementations and\neven different versions of the same shell. One such problematic behavior\nis that, with some shells, the assignment will outlive the invocation of\nthe function, thus may potentially impact subsequent commands in the\ntest, as well as subsequent tests. A common way to work around the\nproblem is to wrap a subshell around the one-shot assignment, thus\nensuring that the assignment is short-lived.\n\nIn this test, the subshell is employed precisely for this purpose; other\nside-effects of the subshell, such as losing the effect of `test_tick`\nwhich is invoked by `test_commit`, are immaterial.\n\nThese days, we can take advantage of `test_commit --author` to more\nclearly convey that the test is interested only in overriding the author\nof the commit.\n\nSigned-off-by: Eric Sunshine \n---\n t/t3430-rebase-merges.sh | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh\nindex 36ca126bcd..2aa8593f77 100755\n--- a/t/t3430-rebase-merges.sh\n+++ b/t/t3430-rebase-merges.sh\n@@ -392,8 +392,7 @@ test_expect_success 'refuse to merge ancestors of HEAD' '\n \n test_expect_success 'root commits' '\n \tgit checkout --orphan unrelated &&\n-\t(GIT_AUTHOR_NAME=\"Parsnip\" GIT_AUTHOR_EMAIL=\"root@example.com\" \\\n-\t test_commit second-root) &&\n+\ttest_commit --author \"Parsnip \" second-root &&\n \ttest_commit third-root &&\n \tcat >script-from-scratch <<-\\EOF &&\n \tpick third-root\n-- \n2.45.2\n\n"},{"id":"499478","messageId":"20240727053509.34339-3-ericsunshine@charter.net","threadId":"61818","inReplyTo":"20240727053509.34339-1-ericsunshine@charter.net","subject":"[PATCH v3 2/5] t4034: fix use of one-shot variable assignment with shell function","fromName":"Eric Sunshine","fromEmail":"ericsunshine@charter.net","sentAt":"2024-07-27T05:35:06Z","receivedAt":"2024-07-27T05:37:01Z","isPatch":true,"sender":{"key":"ericsunshine@charter.net","avatar":null},"body":"From: Eric Sunshine \n\nThe behavior of a one-shot environment variable assignment of the form\n\"VAR=val cmd\" is unspecified according to POSIX when \"cmd\" is a shell\nfunction. Indeed the behavior differs between shell implementations and\neven different versions of the same shell, thus should be avoided.\n\nSigned-off-by: Eric Sunshine \n---\n t/t4034-diff-words.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh\nindex 74586f3813..4dcd7e9925 100755\n--- a/t/t4034-diff-words.sh\n+++ b/t/t4034-diff-words.sh\n@@ -70,7 +70,7 @@ test_language_driver () {\n \t\tword_diff --color-words\n \t'\n \ttest_expect_success \"diff driver '$lang' in Islandic\" '\n-\t\tLANG=is_IS.UTF-8 LANGUAGE=is LC_ALL=\"$is_IS_locale\" \\\n+\t\ttest_env LANG=is_IS.UTF-8 LANGUAGE=is LC_ALL=\"$is_IS_locale\" \\\n \t\tword_diff --color-words\n \t'\n }\n-- \n2.45.2\n\n"},{"id":"499479","messageId":"20240727053509.34339-5-ericsunshine@charter.net","threadId":"61818","inReplyTo":"20240727053509.34339-1-ericsunshine@charter.net","subject":"[PATCH v3 4/5] check-non-portable-shell: suggest alternative for `VAR=val shell-func`","fromName":"Eric Sunshine","fromEmail":"ericsunshine@charter.net","sentAt":"2024-07-27T05:35:08Z","receivedAt":"2024-07-27T05:37:02Z","isPatch":true,"sender":{"key":"ericsunshine@charter.net","avatar":null},"body":"From: Eric Sunshine \n\nMost problems reported by check-non-portable-shell are accompanied by\nadvice suggesting how the test author can repair the problem. For\ninstance:\n\n error: egrep/fgrep obsolescent (use grep -E/-F)\n\nHowever, when one-shot variable assignment is detected when calling a\nshell function (i.e. `VAR=val shell-func`), the problem is reported, but\nno advice is given. The lack of advice is particularly egregious since\nneither the problem nor the workaround are likely well-known by\nnewcomers to the project writing tests for the first time. Address this\nshortcoming by recommending the use of `test_env` which is tailor made\nfor this specific use-case.\n\nSigned-off-by: Eric Sunshine \n---\n t/check-non-portable-shell.pl | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl\nindex 179efaa39d..903af14294 100755\n--- a/t/check-non-portable-shell.pl\n+++ b/t/check-non-portable-shell.pl\n@@ -50,7 +50,7 @@ sub err {\n \t/\\blocal\\s+[A-Za-z0-9_]*=\\$([A-Za-z0-9_{]|[(][^(])/ and\n \t\terr q(quote \"$val\" in 'local var=$val');\n \t/^\\s*([A-Z0-9_]+=(\\w*|([\"']).*?\\3)\\s+)+(\\w+)/ and exists($func{$4}) and\n-\t\terr '\"FOO=bar shell_func\" is not portable';\n+\t\terr '\"FOO=bar shell_func\" is not portable (use test_env FOO=bar shell_func)';\n \t$line = '';\n \t# this resets our $. for each file\n \tclose ARGV if eof;\n-- \n2.45.2\n\n"},{"id":"499480","messageId":"20240727053509.34339-4-ericsunshine@charter.net","threadId":"61818","inReplyTo":"20240727053509.34339-1-ericsunshine@charter.net","subject":"[PATCH v3 3/5] check-non-portable-shell: loosen one-shot assignment error message","fromName":"Eric Sunshine","fromEmail":"ericsunshine@charter.net","sentAt":"2024-07-27T05:35:07Z","receivedAt":"2024-07-27T05:37:02Z","isPatch":true,"sender":{"key":"ericsunshine@charter.net","avatar":null},"body":"From: Eric Sunshine \n\nWhen a0a630192d (t/check-non-portable-shell: detect \"FOO=bar\nshell_func\", 2018-07-13) added the check for one-shot environment\nvariable assignment for shell functions, the primary reason given for\navoiding them was that, under some shells, the assignment outlives the\ninvocation of the shell function, thus could potentially negatively\nimpact subsequent commands in the same test, as well as subsequent\ntests.\n\nHowever, it has recently become apparent that this is not the only\npotential problem with one-shot assignments and shell functions. Another\nproblem is that some shells do not actually export the variable to\ncommands which the function invokes[1]. More significantly, however, the\nbehavior of one-shot assignments with shell functions is not specified\nby POSIX[2].\n\nGiven this new understanding, the presented error message (\"assignment\nextends beyond 'shell_func'\") is too specific and potentially\nmisleading. Address this by emitting a less specific error message.\n\n(Note that the wording \"is not portable\" is chosen over the more\nspecific \"behavior not specified by POSIX\" for consistency with almost\nall other error message issued by this \"lint\" script.)\n\n[1]: https://lore.kernel.org/git/xmqqbk2p9lwi.fsf_-_@gitster.g/\n[2]: https://lore.kernel.org/git/xmqq34o19jj1.fsf@gitster.g/\n\nSigned-off-by: Eric Sunshine \n---\n t/check-non-portable-shell.pl | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl\nindex b2b28c2ced..179efaa39d 100755\n--- a/t/check-non-portable-shell.pl\n+++ b/t/check-non-portable-shell.pl\n@@ -50,7 +50,7 @@ sub err {\n \t/\\blocal\\s+[A-Za-z0-9_]*=\\$([A-Za-z0-9_{]|[(][^(])/ and\n \t\terr q(quote \"$val\" in 'local var=$val');\n \t/^\\s*([A-Z0-9_]+=(\\w*|([\"']).*?\\3)\\s+)+(\\w+)/ and exists($func{$4}) and\n-\t\terr '\"FOO=bar shell_func\" assignment extends beyond \"shell_func\"';\n+\t\terr '\"FOO=bar shell_func\" is not portable';\n \t$line = '';\n \t# this resets our $. for each file\n \tclose ARGV if eof;\n-- \n2.45.2\n\n"},{"id":"499481","messageId":"20240727053509.34339-6-ericsunshine@charter.net","threadId":"61818","inReplyTo":"20240727053509.34339-1-ericsunshine@charter.net","subject":"[PATCH v3 5/5] check-non-portable-shell: improve `VAR=val shell-func` detection","fromName":"Eric Sunshine","fromEmail":"ericsunshine@charter.net","sentAt":"2024-07-27T05:35:09Z","receivedAt":"2024-07-27T05:37:02Z","isPatch":true,"sender":{"key":"ericsunshine@charter.net","avatar":null},"body":"From: Eric Sunshine \n\nThe behavior of a one-shot environment variable assignment of the form\n\"VAR=val cmd\" is unspecified according to POSIX when \"cmd\" is a shell\nfunction. Indeed the behavior differs between shell implementations and\neven different versions of the same shell, thus should be avoided.\n\nAs such, check-non-portable-shell.pl warns when it detects such usage.\nHowever, a limitation of the check is that it only detects such\ninvocations when variable assignment (i.e. `VAR=val`) is the first thing\non the line. Thus, it can easily be fooled by an invocation such as:\n\n echo X | VAR=val shell-func\n\nAddress this shortcoming by loosening the check so that the variable\nassignment can be recognized even when not at the beginning of the line.\n\nSigned-off-by: Eric Sunshine \n---\n t/check-non-portable-shell.pl | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl\nindex 903af14294..6ee7700eb4 100755\n--- a/t/check-non-portable-shell.pl\n+++ b/t/check-non-portable-shell.pl\n@@ -49,7 +49,7 @@ sub err {\n \t/\\bexport\\s+[A-Za-z0-9_]*=/ and err '\"export FOO=bar\" is not portable (use FOO=bar && export FOO)';\n \t/\\blocal\\s+[A-Za-z0-9_]*=\\$([A-Za-z0-9_{]|[(][^(])/ and\n \t\terr q(quote \"$val\" in 'local var=$val');\n-\t/^\\s*([A-Z0-9_]+=(\\w*|([\"']).*?\\3)\\s+)+(\\w+)/ and exists($func{$4}) and\n+\t/\\b([A-Z0-9_]+=(\\w*|([\"']).*?\\3)\\s+)+(\\w+)/ and !/test_env.+=/ and exists($func{$4}) and\n \t\terr '\"FOO=bar shell_func\" is not portable (use test_env FOO=bar shell_func)';\n \t$line = '';\n \t# this resets our $. for each file\n-- \n2.45.2\n\n"}]}