From: Phillip Wood Date: Mon, 22 Jul 2024 15:09:59 GMT Subject: Re: [PATCH 1/4] t3430: modernize one-shot "VAR=val shell-func" invocation Message-ID: In-Reply-To: <20240722065915.80760-2-ericsunshine@charter.net> Hi Eric On 22/07/2024 07:59, Eric Sunshine wrote: > From: Eric Sunshine > > 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'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. > 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. > > 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. Best Wishes Phillip > Signed-off-by: Eric Sunshine > --- > t/t3430-rebase-merges.sh | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh > index 36ca126bcd..e851ede4f9 100755 > --- a/t/t3430-rebase-merges.sh > +++ b/t/t3430-rebase-merges.sh > @@ -392,8 +392,8 @@ test_expect_success 'refuse to merge ancestors of HEAD' ' > > test_expect_success 'root commits' ' > git checkout --orphan unrelated && > - (GIT_AUTHOR_NAME="Parsnip" GIT_AUTHOR_EMAIL="root@example.com" \ > - test_commit second-root) && > + test_env GIT_AUTHOR_NAME="Parsnip" GIT_AUTHOR_EMAIL="root@example.com" \ > + test_commit second-root && > test_commit third-root && > cat >script-from-scratch <<-\EOF && > pick third-root