Re: [PATCH 3/4] check-non-portable-shell: improve `VAR=val shell-func` detection

From: Kyle Lippincott <spectral@google.com>
Date: Jul 22, 2024, 17:26 UTC
Message-ID: <CAO_smVg8+WCG0dWZNPVbDM4gBJLLHrg96nOCzje6B3hUGneDGg@mail.gmail.com>
In-Reply-To: <20240722065915.80760-4-ericsunshine@charter.net>

On Mon, Jul 22, 2024 at 12:01 AM Eric Sunshine <ericsunshine@charter.net> wrote:

Show 36 quoted lines

>
> From: Eric Sunshine <sunshine@sunshineco.com>
>
> Unlike "VAR=val cmd" one-shot environment variable assignments which
> exist only for the invocation of 'cmd', those assigned by "VAR=val
> shell-func" exist within the running shell and continue to do so until
> the process exits. check-non-portable-shell.pl warns when it detects
> such usage since, more often than not, the author who writes such an
> invocation is unaware of the undesirable behavior.
>
> However, a limitation of the check is that it only detects such
> invocations when variable assignment (i.e. `VAR=val`) is the first
> thing on the line. Thus, it can easily be fooled by an invocation such
> as:
>
>     echo X | VAR=val shell-func
>
> Address this shortcoming by loosening the check so that the variable
> assignment can be recognized even when not at the beginning of the line.
>
> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
> ---
>  t/check-non-portable-shell.pl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl
> index b2b28c2ced..44b23d6ddd 100755
> --- a/t/check-non-portable-shell.pl
> +++ b/t/check-non-portable-shell.pl
> @@ -49,7 +49,7 @@ sub err {
>         /\bexport\s+[A-Za-z0-9_]*=/ and err '"export FOO=bar" is not portable (use FOO=bar && export FOO)';
>         /\blocal\s+[A-Za-z0-9_]*=\$([A-Za-z0-9_{]|[(][^(])/ and
>                 err q(quote "$val" in 'local var=$val');
> -       /^\s*([A-Z0-9_]+=(\w*|(["']).*?\3)\s+)+(\w+)/ and exists($func{$4}) and
> +       /\b([A-Z0-9_]+=(\w*|(["']).*?\3)\s+)+(\w+)/ and !/test_env.+=/ and exists($func{$4}) and
>                 err '"FOO=bar shell_func" assignment extends beyond "shell_func"';

Is there an example of a shell on Linux that has this behavior that I can observe, and/or reproduction steps? `bash --posix` does not do this, as far as I can tell. I tried:

``` bash --posix echo_hi() { echo hi; } FOO=BAR echo_hi echo $FOO <no output> ```

and the simpler:

``` bash --posix FOO=BAR echo hi echo $FOO ```

Both attempts were done interactively, not via a script.

I'm asking mostly because of the recent "platform support document" patch series - this is a very surprising behavior (which is presumably why it was added to this test), and it's possible this was just a bug in a shell that isn't really used anymore. Having documentation on how to reproduce the issue lets us know when we can remove these kinds of restrictions on our codebase that we took in the name of compatibility, possibly over a decade ago.

Show 6 quoted lines

>         $line = '';
>         # this resets our $. for each file
> --
> 2.45.2
>
>