On Thu, Jul 2, 2015 at 9:59 AM, Pádraig Brady <[email protected]> wrote:
> On 02/07/15 17:46, Jim Meyering wrote:
>> On Thu, Jul 2, 2015 at 4:57 AM, Pádraig Brady <[email protected]> wrote:
>>> On 02/07/15 08:08, Pádraig Brady wrote:
>>>> On 02/07/15 06:19, Jim Meyering wrote:
>>>>> For me, an obvious work-around is to set SHELL=/bin/sh
>>>>> or similar.
>>>>
>>>> Yes or to avoid the slight chance of /bin/sh also resetting the path
>>>> you could directly reference the binary as follows.
>>>> I slightly prefer setting the SHELL though.
>>>
>>> Yes given we should also set SHELL in tests/split/filter.sh,
>>> I'll apply the attached in your name later on.
>>
>> Thanks, but I would prefer to change init.sh to reject (or at least
>> deprioritize) zsh in that case. I'll propose a patch soon.
>
> Or I suppose any $SHELL so configured to reset the $PATH.
> I suppose we'd need the change to the tests also in case
> another shell was not found?

I realized that the problem was not that we were actually using
zsh, but that SHELL was set to /bin/zsh in spite of that script
being interpreted by /bin/sh. It appears that SHELL is set to
/bin/zsh at login, and no shell resets it.

This suggests a minimal change would be to add SHELL to
the list of envvars that we ensure are unset via
tests/envvar-check, and that does solve my problem.
However, I wonder if that is too large a sledgehammer.

An alternative would be to unset it only upon detecting this
misbehavior, e.g., adding these lines to that file:

diff --git a/tests/envvar-check b/tests/envvar-check
...
+# If invoking $SHELL -c 'CODE' puts a modified PATH in the environment,
+# as zsh does when ~/.zshenv modifies PATH, also unset SHELL.
+( PATH=$PATH:/no-such; $SHELL -c 'test '"$PATH"' = "$PATH"') \
+  || vars="$vars SHELL"
+
 for var in $vars

The disadvantage of that approach is that it imposes the cost
of a subshell and fork-exec of $SHELL prior to each and every test.

What do you think?
From 64c0f180c51c3dc940348dc82fbec87d37ed628b Mon Sep 17 00:00:00 2001
From: Jim Meyering <[email protected]>
Date: Thu, 2 Jul 2015 17:07:17 -0700
Subject: [PATCH] tests: avoid false failure of factor-parallel.sh with
 SHELL=zsh

With SHELL=/bin/zsh and a PATH-modifying ~/.zshenv, the
factor-parallel.sh test would fail when the factor program found via
the ~/.zshenv-set PATH did not have the latest buffering fix ("factor:
ensure atomic output through pipes", commit v8.23-242-gf2db30e). That
happened to me because my .zshenv set PATH so that the prior version of
factor was used, due to the effective invocation of "$SHELL -c factor"
via the test's "...|split -nr/4 --filter=factor|..." invocation.
* tests/envvar-check: If the offending usage of $SHELL -c modifies PATH,
undefine that variable.
---
 tests/envvar-check | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tests/envvar-check b/tests/envvar-check
index 76becbf..9c28693 100644
--- a/tests/envvar-check
+++ b/tests/envvar-check
@@ -48,6 +48,12 @@ vars='
   TMPDIR
   VERSION_CONTROL
 '
+
+# If invoking $SHELL -c 'CODE' puts a modified PATH in the environment,
+# as zsh does when ~/.zshenv modifies PATH, also unset SHELL.
+( PATH=$PATH:/no-such; $SHELL -c 'test '"$PATH"' = "$PATH"') \
+  || vars="$vars SHELL"
+
 for var in $vars
 do
   $as_unset $var
-- 
2.3.7

Reply via email to