Re: factor-parallel test failure with SHELL=zsh

[Jim Meyering]

On Thu, Jul 2, 2015 at 9:07 PM, Pádraig Brady <address@hidden> wrote:
> On 03/07/15 01:26, Jim Meyering wrote:
>> On Thu, Jul 2, 2015 at 9:59 AM, Pádraig Brady <address@hidden> wrote:
>>> On 02/07/15 17:46, Jim Meyering wrote:
>>>> On Thu, Jul 2, 2015 at 4:57 AM, Pádraig Brady <address@hidden> 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.
>
> Another possible disadvantage is that $SHELL might induce other
> undetected issues other than $PATH, like resetting environment variables
> removed in tests/envvar-check etc.
>
> Since $SHELL isn't reset by the non interactive shells in the tests,
> and it's used implicitly in various places in the tests, how about
> we set it consistently for all test sub scripts.  How about the attached
> which sets it to the value determined by the gnulib posix-shell module?

I prefer your patch. Thanks.
The only part I had qualms about was where <<\EOF was
changed to <<EOF for a sole use of $SHELL, but that meant
changing each of several $var uses to \$var.
How about just using a separate printf or echo to emit that
first "#!$SHELL" line and retaining/appending the remainder
via a <<\EOF quoted here-doc?