On Mon, Jul 22, 2024 at 11:10 AM Phillip Wood <phillip.wood123@xxxxxxxxx> wrote: > 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/ > > 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@xxxxxxxxxxxxxx/ [5]: https://lore.kernel.org/git/20160601070425.GA13648@xxxxxxxxxxxxxxxxxxxxx/