On 22/07/2024 16:09, Phillip Wood wrote:
Hi Eric
On 22/07/2024 07:59, Eric Sunshine wrote:
From: Eric Sunshine <sunshine@xxxxxxxxxxxxxx>
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).
Having seen the parallel discussion about the behavior of hash this
construct is non-portable because the behavior differs between shells so
perhaps the commit message could say something like
Unlike "VAR=val cmd" one-shot environment variable assignments which
only exist for the invocation of external command "cmd", the behavior of
"VAR=val func" where "func" is a shell function or builtin command
varies between shells and so we should not use it in our test suite.
Best Wishes
Phillip
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 <sunshine@xxxxxxxxxxxxxx>
---
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@xxxxxxxxxxx" \
- test_commit second-root) &&
+ test_env GIT_AUTHOR_NAME="Parsnip"
GIT_AUTHOR_EMAIL="root@xxxxxxxxxxx" \
+ test_commit second-root &&
test_commit third-root &&
cat >script-from-scratch <<-\EOF &&
pick third-root