From 1f64cc452545fa6504d8cbb6fc09470f875e76d4 Mon Sep 17 00:00:00 2001 From: Avery Pennarun Date: Mon, 17 Dec 2018 12:35:32 +0000 Subject: [PATCH] shelltest.od: add more "set -e" tests and add a 'skip' return code. Based on the earlier t/000-set-minus-e bug in minimal/do on some shells, let's add some extra tests that reveal the weirdness on those shells. Unfortunately because they are so popular (including bash and zsh), we can't reject them outright for failing this one. While we're here, add a new return code, "skip", which notes that a test has failed but is not important enough to be considered a warning or failure. Previously we just had these commented out, which is not quite obvious enough. ...and I updated a few comments while reviewing some of the older tests. --- redo/sh.do | 2 + t/.gitignore | 1 + t/000-set-minus-e/all.do | 1 + t/clean.do | 2 +- t/shelltest.od | 91 ++++++++++++++++++++++++++++++++-------- 5 files changed, 79 insertions(+), 18 deletions(-) diff --git a/redo/sh.do b/redo/sh.do index 82c7507..cc8b685 100644 --- a/redo/sh.do +++ b/redo/sh.do @@ -36,9 +36,11 @@ for sh in dash /usr/xpg4/bin/sh ash posh \ #echo "line: '$line'" >&2 stripw=${line#warning: } stripf=${line#failed: } + strips=${line#skip: } crash=$line [ "$line" = "$stripw" ] || msgs="$msgs W$stripw" [ "$line" = "$stripf" ] || msgs="$msgs F$stripf" + [ "$line" = "$strips" ] || msgs="$msgs s$strips" done /dev/null 2>&1 || true diff --git a/t/clean.do b/t/clean.do index effd5ed..0267669 100644 --- a/t/clean.do +++ b/t/clean.do @@ -2,6 +2,6 @@ sed 's/\.do$//' | xargs redo -rm -f broken shellfile shelltest.warned shelltest.failed \ +rm -f broken shellfile shellfail shelltest.warned shelltest.failed \ *~ .*~ stress.log flush-cache rm -rf 'space home dir' diff --git a/t/shelltest.od b/t/shelltest.od index 1d94639..8957ee3 100644 --- a/t/shelltest.od +++ b/t/shelltest.od @@ -29,6 +29,12 @@ warn() : >shelltest.warned } +# For things that should be a warning, but which are just too common +skip() +{ + echo " skip: $1" +} + quiet_stderr() { if [ -n "$SHELLTEST_QUIET" ]; then @@ -185,17 +191,20 @@ c" [ "$t2" = "$WANT" ] || fail 45 -# Arguably, 'export' and 'local' shouldn't change variable assignment quoting -# rules, but in almost all shells (except bash), they do, and POSIX doesn't say -# anything about it. So let's not bother testing this, other than just letting -# it check our syntax. -# +# Arguably, 'export' and 'local' shouldn't change variable assignment +# quoting rules, but in many shells they do, and POSIX doesn't say anything +# about it. This leads to really confusing whitespace bugs in some shells, +# where +# local x; x=$a +# and +# local x=$a +# mean two different things when $a contains whitespace. bob="a b *" bob=$(quiet_stderr eval 'export bob=$bob:hello'; echo "$bob") -#[ "$bob" = "a b *:hello" ] || warn 46 +[ "$bob" = "a b *:hello" ] || skip 46 bob="a b *" nob=$(eval 'f() { local nob=$bob:hello; echo "$nob"; }'; quiet_stderr f) -#[ "$nob" = "a b *:hello" ] || warn 47 +[ "$nob" = "a b *:hello" ] || skip 47 # Someone pointed out that temporary variable assignments aren't @@ -247,14 +256,14 @@ echo "`printf 'foo\r\n'`"" bar" | diff -q - broken || fail 59 # -# This one is too obnoxious. dash and ash pass the test, but most shells don't, -# and this case is just too dumb to care about. Just don't do that! +# This one is too obnoxious. dash and ash pass the test, but most shells +# don't, and this case is just too dumb to care about. Just don't do that! # -#t=`echo $(case x in x) echo hello;; esac)` -#[ "$t" = "hello" ] || fail 60 +t=`echo $(case x in x) echo hello;; esac)` +[ "$t" = "hello" ] || skip 60 # -# Note that with the little-known optional left-paren, this *does* work. Let's -# try it to make sure that remains true. +# Note that with the little-known optional left-paren, this *does* work. +# Let's try it to make sure that remains true. t=`echo $(case x in (x) echo hello;; esac)` [ "$t" = "hello" ] || fail 60 @@ -328,6 +337,53 @@ rv=$? [ "$rv" = 0 ] || fail 89 +rm -f shellfail +echo "false; true" >shellfail + +# 'false' should *not* abort because outer context has "||". +# Then 'true' gives success. +set +e +( + set -e + false + true +) || fail 89a + +# 'false' still should not abort even in a subshell. +set +e +( + set -e + ( false; true ) + [ "$?" = 0 ] || fail 89b1 + true +) || fail 89b + +# Check whether "." scripts inherit "set -e" setting *and* the outer "||" +# context. +set +e +( + set -e + ( . ./shellfail ) + # There seems to be widespread disagreement on what should happen + # here. All shells seem to inherit "set -e" into a "." script, but + # they disagree about whether the outer "||" context (which + # effectively disables "set -e") should be inherited. ksh93, bash, + # and zsh inherit it, while ash, dash, and mksh (among others) throw + # it away, causing "set -e" to abort. + # + # Honestly, the latter behaviour seems a lot more like what I'd + # expect (a separate script doesn't want its "set -e" behaviour + # to depend on whether it's deep in a context with an "||" somewhere + # outside it), so let's warn on the former. But I'm willing to hear + # arguments either way. -- apenwarr + [ "$?" = 0 ] && warn 89c + # even if "." aborted and returned nonzero, our own context should + # not have aborted, so we expect to get here and avoid the outer + # fail below. + true +) || fail 89c1 + + # http://www.gnu.org/software/hello/manual/autoconf/Limitations-of-Builtins.html . /dev/null || fail 90 (! : | :) && fail 91 || true @@ -338,7 +394,7 @@ case frog.c in (*) t3=all ;; esac [ "$t3" = "c" ] || fail 93 -t4=$(echo '\n' | wc -l) +t4=$(echo '\n' | wc -l) # echo should *not* be parsing backslash-escapes [ "$t4" -eq 1 ] || warn 94 f5() { for arg; do @@ -400,9 +456,10 @@ false # this is actually a bash/kshism, but is allowed by POSIX: the parameters to # '.' should be passed to the sub-script. # -# We used to require this, because it's so useful despite being optional in POSIX. -# But unfortunately, too many shells (including dash) can't do it, so we ended up -# always using bash, which leads people to write .do scripts with bashisms. +# We used to require this, because it's so useful despite being optional in +# POSIX. But unfortunately, too many shells (including dash) can't do it, +# so we ended up always using bash, which leads people to write .do scripts +# with bashisms. set x y z # dotparams.od might warn 115 . ./dotparams.od a b || fail 117