On 03/07/15 01:26, Jim Meyering wrote: > 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.
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? cheers, Pádraig.
From 31325c2582e69780e0cf2ece3c787b9da7326092 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email protected]> Date: Fri, 3 Jul 2015 04:44:05 +0100 Subject: [PATCH] tests: avoid side effects of $SHELL environment variable Since non interactive shells don't generally set $SHELL, its value is propagated through the tests and may cause issues with $PATH for example if $SHELL implicitly adjusts the $PATH when run. Instead we set $SHELL to that determined by the posix-shell module, and use that consistently for all test sub scripts. * tests/local.mk: Explicitly set $SHELL to $(PREFERABLY_POSIX_SHELL) which defaults to $CONFIG_SHELL and thus usually /bin/sh. * tests/envvar-check: Remove bash environment variables with side effects, in case /bin/bash was selected for $SHELL. * tests/misc/help-version.sh: Remove redundant initialization of $SHELL. * tests/install/strip-program.sh: Use $SHELL for sub script. * tests/misc/sort-compress-hang.sh: Likewise. * tests/misc/sort-compress-proc.sh: Likewise. * tests/misc/sort-compress.sh: Likewise. * tests/misc/timeout-group.sh: Likewise. * tests/rm/fail-eperm.xpl: Remove redundant elision of bash env vars. * tests/misc/pwd-long.sh: Likewise. --- tests/envvar-check | 2 ++ tests/install/strip-program.sh | 2 +- tests/local.mk | 2 +- tests/misc/help-version.sh | 6 ------ tests/misc/pwd-long.sh | 1 - tests/misc/sort-compress-hang.sh | 4 ++-- tests/misc/sort-compress-proc.sh | 8 ++++---- tests/misc/sort-compress.sh | 4 ++-- tests/misc/timeout-group.sh | 14 +++++++------- tests/rm/fail-eperm.xpl | 1 - 10 files changed, 19 insertions(+), 25 deletions(-) diff --git a/tests/envvar-check b/tests/envvar-check index 76becbf..28d95f3 100644 --- a/tests/envvar-check +++ b/tests/envvar-check @@ -28,12 +28,14 @@ vars=' _STDBUF_E _STDBUF_I _STDBUF_O + BASH_ENV BLOCKSIZE BLOCK_SIZE CDPATH COLUMNS DF_BLOCK_SIZE DU_BLOCK_SIZE + ENV LANGUAGE LS_BLOCK_SIZE LS_COLORS diff --git a/tests/install/strip-program.sh b/tests/install/strip-program.sh index 7ac2165..103f549 100755 --- a/tests/install/strip-program.sh +++ b/tests/install/strip-program.sh @@ -22,7 +22,7 @@ print_ver_ ginstall working_umask_or_skip_ cat <<EOF > b || framework_failure_ -#!$PREFERABLY_POSIX_SHELL +#!$SHELL sed s/b/B/ \$1 > \$1.t && mv \$1.t \$1 EOF chmod a+x b || framework_failure_ diff --git a/tests/local.mk b/tests/local.mk index 3cd8f92..7df04da 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -79,7 +79,7 @@ TESTS_ENVIRONMENT = \ MAKE=$(MAKE) \ PACKAGE_VERSION=$(PACKAGE_VERSION) \ PERL='$(PERL)' \ - PREFERABLY_POSIX_SHELL='$(PREFERABLY_POSIX_SHELL)' \ + SHELL='$(PREFERABLY_POSIX_SHELL)' \ ; test -d /usr/xpg4/bin && PATH='/usr/xpg4/bin$(PATH_SEPARATOR)'"$$PATH"; \ PATH='$(abs_top_builddir)/src$(PATH_SEPARATOR)'"$$PATH" \ ; 9>&2 diff --git a/tests/misc/help-version.sh b/tests/misc/help-version.sh index e0dd721..7815037 100755 --- a/tests/misc/help-version.sh +++ b/tests/misc/help-version.sh @@ -17,12 +17,6 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see <http://www.gnu.org/licenses/>. -# Ensure that $SHELL is set to *some* value and exported. -# This is required for dircolors, which would fail e.g., when -# invoked via debuild (which removes SHELL from the environment). -test "x$SHELL" = x && SHELL=/bin/sh -export SHELL - . "${srcdir=.}/tests/init.sh"; path_prepend_ ./src # Terminate any background processes diff --git a/tests/misc/pwd-long.sh b/tests/misc/pwd-long.sh index 927fdcf..3ced777 100755 --- a/tests/misc/pwd-long.sh +++ b/tests/misc/pwd-long.sh @@ -56,7 +56,6 @@ sub normalize_to_cwd_relative ($$$) } # Set up a safe, well-known environment -delete @ENV{qw(BASH_ENV CDPATH ENV)}; $ENV{IFS} = ''; # Taint checking requires a sanitized $PATH. This script performs no $PATH diff --git a/tests/misc/sort-compress-hang.sh b/tests/misc/sort-compress-hang.sh index 9cfd6cc..dc101fe 100755 --- a/tests/misc/sort-compress-hang.sh +++ b/tests/misc/sort-compress-hang.sh @@ -20,8 +20,8 @@ print_ver_ sort very_expensive_ -cat <<\EOF >compress || framework_failure_ -#!/bin/sh +cat <<EOF >compress || framework_failure_ +#!$SHELL tr 41 14 || exit touch ok EOF diff --git a/tests/misc/sort-compress-proc.sh b/tests/misc/sort-compress-proc.sh index 4ad42d5..e211909 100755 --- a/tests/misc/sort-compress-proc.sh +++ b/tests/misc/sort-compress-proc.sh @@ -32,11 +32,11 @@ insize=$(stat -c %s - <in) || fail=1 # This compressor's behavior is adjustable via environment variables. export PRE_COMPRESS= export POST_COMPRESS= -cat <<\EOF >compress || framework_failure_ -#!/bin/sh -eval "$PRE_COMPRESS" +cat <<EOF >compress || framework_failure_ +#!$SHELL +eval "\$PRE_COMPRESS" tr 41 14 || exit -eval "$POST_COMPRESS" +eval "\$POST_COMPRESS" EOF chmod +x compress diff --git a/tests/misc/sort-compress.sh b/tests/misc/sort-compress.sh index 605d539..584110e 100755 --- a/tests/misc/sort-compress.sh +++ b/tests/misc/sort-compress.sh @@ -27,8 +27,8 @@ sort -S 1k in > out || fail=1 compare exp out || fail=1 # Create our own gzip program that will be used as the default -cat <<\EOF > gzip || fail=1 -#!/bin/sh +cat <<EOF > gzip || fail=1 +#!$SHELL tr 41 14 touch ok EOF diff --git a/tests/misc/timeout-group.sh b/tests/misc/timeout-group.sh index 054c5ae..7d0ec66 100755 --- a/tests/misc/timeout-group.sh +++ b/tests/misc/timeout-group.sh @@ -30,20 +30,20 @@ print_ver_ timeout setsid true || skip_ "setsid required to control groups" -cat > timeout.cmd <<\EOF -#!/bin/sh +cat > timeout.cmd <<EOF +#!$SHELL trap 'touch int.received; exit' INT touch timeout.running -count=$1 -until test -e int.received || test $count = 0; do +count=\$1 +until test -e int.received || test \$count = 0; do sleep 1 - count=$(expr $count - 1) + count=\$(expr \$count - 1) done EOF chmod a+x timeout.cmd -cat > group.sh <<\EOF -#!/bin/sh +cat > group.sh <<EOF +#!$SHELL trap '' INT timeout --foreground 25 ./timeout.cmd 20& wait diff --git a/tests/rm/fail-eperm.xpl b/tests/rm/fail-eperm.xpl index 80299b4..7a5b9ce 100755 --- a/tests/rm/fail-eperm.xpl +++ b/tests/rm/fail-eperm.xpl @@ -32,7 +32,6 @@ my $verbose = $ENV{VERBOSE} && $ENV{VERBOSE} eq 'yes'; $ENV{LC_ALL} = 'C'; # Set up a safe, well-known environment -delete @ENV{qw(BASH_ENV CDPATH ENV)}; $ENV{IFS} = ''; # Taint checking requires a sanitized $PATH. This script performs no $PATH -- 2.4.1
