Re: [PATCH 1/4] t3430: modernize one-shot "VAR=val shell-func" invocation
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Jul 26, 2024, 06:15 UTC
- Message-ID
- <CAPig+cQNfZp8Sh4s8ukF0Vto-+RqyHeTM0jJK4mOJhkmxz2s1Q@mail.gmail.com>
- In-Reply-To
- <c586f7dc-636b-45a3-acb2-faedfe1068e6@gmail.com>
On Mon, Jul 22, 2024 at 11:10 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
Show 23 quoted lines
> On 22/07/2024 07:59, Eric Sunshine wrote:
> > 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 (or are explicitly unset).
>
> I'm not sure I follow. If I run
>
> sh -c 'f() {
> echo "f: HELLO=$HELLO"
> env | grep HELLO
> }
> HELLO=x f; echo "HELLO=$HELLO"'
>
> Then I see
>
> f: HELLO=x
> HELLO=x
> HELLO=
>
> which seems to contradict the commit message as $HELLO is unset when the
> function returns. I see the same result if I replace "sh" (which is bash
> on my system) with an explicit "bash", "dash" or "zsh".I believe downstream discussion[1][2] established that the behavior is inconsistent between various shells and versions of shells, and is considered undefined by POSIX.
[1]: https://lore.kernel.org/git/xmqq34o1cn6b.fsf@gitster.g/ [2]: https://lore.kernel.org/git/xmqqbk2p9lwi.fsf_-_@gitster.g/
> I'm also confused as to why this caused a problem for Rubén's test as > $HELLO is set in the environment so I'm don't understand why git wasn't > picking up the right pager.
Junio summarized the problem and explanation[3].
[3]: https://lore.kernel.org/git/xmqq7cdd9l0m.fsf@gitster.g/
Show 7 quoted lines
> > A common way to work around the problem is to wrap a subshell around the > > variable assignments and function call, thus ensuring that the > > assignments are short-lived. However, these days, a more ergonomic > > approach is to employ test_env() which is tailor-made for this specific > > use-case. > > Oh, that sounds useful, I didn't know it existed.
I didn't know about it either, and only discovered it upon my initial attempt at making check-non-portable-shell.pl recognize the case Rubén identified, at which point it started showing false-positives on `test_env` invocations. Actually, considering that I was involved[4] in the conversation which led to the introduction[5] of `test_env` by Peff, it may be that I did know about it but forgot.
[4]: https://lore.kernel.org/git/CAPig+cR989yU4+JNTFREaeXqY61nusUOhufeBGGVCi29tR1P5w@mail.gmail.com/ [5]: https://lore.kernel.org/git/20160601070425.GA13648@sigill.intra.peff.net/