Stefano Lattarini wrote: > At Friday 11 June 2010, Jim Meyering wrote: >> > diff --git a/tests/init.sh b/tests/init.sh >> > index ef0957c..4eb4a95 100644 >> > --- a/tests/init.sh >> > +++ b/tests/init.sh >> > @@ -97,12 +97,15 @@ else >> > for re_shell_ in "${CONFIG_SHELL:-no_shell}" /bin/sh bash >> > dash zsh pdksh fail do >> > test "$re_shell_" = no_shell && continue >> > - test "$re_shell_" = fail && skip_ failed to find an >> > adequate shell + if test "$re_shell_" = fail; then >> > + echo "$0: skipped test: failed to find an adequate >> > shell" 1>&2 + exit 77 >> >> Did you deliberately choose not to use the "(exit 77); exit 77" >> idiom from Exit()? If so, why?
> I thought the '(exit ...); exit ...' idiom to be necessary only when > an exit trap is in place, to ensure that the code in the trap is > passed the correct exit status in $?. Did I miss something? While not the recommended use case, some script may well set up a trap prior to sourcing init.sh. >> Did you consider simply hoisting the definitions >> of Exit and/or ME_ to precede their first use? > No, I just tried to fix the problem with the minimal amount of changes. > Anyway, the absence of a decent shell and the failure of the exec call > are very very unlikely situations, so IMHO it's not a big deal if the > program name (`$0' instead of `$ME_') is suboptimal in the error > messages associated with those situations. Sure, but I prefer a more invasive change that keeps the code (already rather obfuscated) a little cleaner. Here's a patch that does that at the expense of using shell functions *before* guaranteeing we have a decent shell. I think that is ok, these days. >From 0ae8d6338cc686b1fca0da26aefadebc0875cdf9 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyer...@redhat.com> Date: Fri, 11 Jun 2010 13:41:31 +0200 Subject: [PATCH] init.sh: don't use $ME_ or skip_ before they are defined * tests/init.sh: Hoist definitions of $ME_ and skip_ to precede their first uses. Also hoist their companions: warn_, fail_, framework_failure_, $stderr_fileno. Prompted by a patch from Stefano Lattarini. --- ChangeLog | 6 ++++++ tests/init.sh | 43 ++++++++++++++++++++++--------------------- 2 files changed, 28 insertions(+), 21 deletions(-) diff --git a/ChangeLog b/ChangeLog index 726e66a..5f93588 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,11 @@ 2010-06-11 Jim Meyering <meyer...@redhat.com> + init.sh: don't use $ME_ or skip_ before they are defined + * tests/init.sh: Hoist definitions of $ME_ and skip_ to precede + their first uses. Also hoist their companions: warn_, fail_, + framework_failure_, $stderr_fileno. Prompted by a patch from + Stefano Lattarini. + test-sys_socket: avoid set-but-not-used warnings from gcc * tests/test-sys_socket.c (main): Use "i" and "x", in order to avoid warning about set-but-not-used variables. diff --git a/tests/init.sh b/tests/init.sh index ef0957c..286bbf1 100644 --- a/tests/init.sh +++ b/tests/init.sh @@ -57,6 +57,28 @@ # 4. Finally # $ exit +ME_=`expr "./$0" : '.*/\(.*\)$'` + +# We use a trap below for cleanup. This requires us to go through +# hoops to get the right exit status transported through the handler. +# So use `Exit STATUS' instead of `exit STATUS' inside of the tests. +# Turn off errexit here so that we don't trip the bug with OSF1/Tru64 +# sh inside this function. +Exit () { set +e; (exit $1); exit $1; } + +# Print warnings (e.g., about skipped and failed tests) to this file number. +# Override by defining to say, 9, in init.cfg, and putting say, +# "export ...ENVVAR_SETTINGS...; exec 9>&2; $(SHELL)" in the definition +# of TESTS_ENVIRONMENT in your tests/Makefile.am file. +# This is useful when using automake's parallel tests mode, to print +# the reason for skip/failure to console, rather than to the .log files. +: ${stderr_fileno_=2} + +warn_() { echo "$@" 1>&$stderr_fileno_; } +fail_() { warn_ "$ME_: failed test: $@"; Exit 1; } +skip_() { warn_ "$ME_: skipped test: $@"; Exit 77; } +framework_failure_() { warn_ "$ME_: set-up failure: $@"; Exit 1; } + # We require $(...) support unconditionally. # We require a few additional shell features only when $EXEEXT is nonempty, # in order to support automatic $EXEEXT emulation: @@ -118,26 +140,6 @@ test -n "$EXEEXT" && shopt -s expand_aliases : ${MALLOC_PERTURB_=87} export MALLOC_PERTURB_ -# We use a trap below for cleanup. This requires us to go through -# hoops to get the right exit status transported through the handler. -# So use `Exit STATUS' instead of `exit STATUS' inside of the tests. -# Turn off errexit here so that we don't trip the bug with OSF1/Tru64 -# sh inside this function. -Exit () { set +e; (exit $1); exit $1; } - -# Print warnings (e.g., about skipped and failed tests) to this file number. -# Override by defining to say, 9, in init.cfg, and putting say, -# "export ...ENVVAR_SETTINGS...; exec 9>&2; $(SHELL)" in the definition -# of TESTS_ENVIRONMENT in your tests/Makefile.am file. -# This is useful when using automake's parallel tests mode, to print -# the reason for skip/failure to console, rather than to the .log files. -: ${stderr_fileno_=2} - -warn_() { echo "$@" 1>&$stderr_fileno_; } -fail_() { warn_ "$ME_: failed test: $@"; Exit 1; } -skip_() { warn_ "$ME_: skipped test: $@"; Exit 77; } -framework_failure_() { warn_ "$ME_: set-up failure: $@"; Exit 1; } - # This is a stub function that is run upon trap (upon regular exit and # interrupt). Override it with a per-test function, e.g., to unmount # a partition, or to undo any other global state changes. @@ -247,7 +249,6 @@ setup_() test "$VERBOSE" = yes && set -x initial_cwd_=$PWD - ME_=`expr "./$0" : '.*/\(.*\)$'` pfx_=`testdir_prefix_` test_dir_=`mktempd_ "$initial_cwd_" "$pfx_-$ME_.XXXX"` \ -- 1.7.1.501.g23b46