Use Exit, not exit, in test suite

[Ralf Wildenhues]

Since we added the exit trap in tests/defs.in (to clean up the per-test
directories), we now have to cater to the exit status differences
between shells.  This is documented in the Autoconf manual, here's a
quick recap:  With

   trap 'e=$?
         ...
         exit $e' 0
   ...
   $prev_command
   $command
   exit $state

some shells (e.g., bash) will exit the script with $state, and some with
the status of $command (e.g., Solaris sh, OSF1 sh).  This can be fixed
by replacing each `exit' in the tests with `(exit $state); exit $state'.

As far as I understand all the shell function pitfalls documented in the
Autoconf manual, it is portable to use a function Exit for these two
commands.

There's a further twist though: when errexit aka. `set -e' is in effect,
then OSF1/Tru64 sh will already abort the normal execution list after
the `(exit $state)'.  Which means, the exit status given to the trap
will be the one of the command that was executed before (i.e., of
$command), and that may be anything undesired).  `set +e' fixes this.

However, then things still don't work with OSF1 sh.  When `set -e' is in
effect, and $command fails, the trap will receive the exit status of
$prev_command, which in all likelyhood is 0, so test failures go
unnoticed.  (Were it not for a couple of XPASSes, I wouldn't have seen
this.)

Luckily I found this issue to be a problem only with the Tru64/OSF 5.1
sh, but not elsewhere (tested Solaris 2.6, IRIX 6.5, AIX 4.3.3).  There
are several ways to address this:

- Stop relying on `set -e' and manually add `|| Exit 1' whereever it may
matter.  Auditing all those several hundred instances seems daunting.

- Detect this shell at configure time and disable the trap for it.
This means, on this systems all tests will have leftover directories.

- Search for a better shell to use (e.g. via TESTS_ENVIRONMENT) on this
system.  This requires adapting a couple of tests.

I'm thinking of going the second and maybe the third option.

Meanwhile, the patch below introduces a shell function Exit in defs.in,
and modifies the test suite with this (GNU sed-specific) script:

for f in *.test
do
  sed -i -e '
    /<<.*END/,/^END/b
    /<<.*EOF/,/^EOF/b
    /^\$PERL -ne/b
    s/e\(xit [$0-9]\)/E\1/g
    ' $f
done

and then audited instances of perl scripts manually.

Applied to master, after verifying that this causes no regression on
GNU/Linux.  This fixes about 60 spurious failures on Solaris.  FWIW,
the full patch (including the humongous ChangeLog entry listing all
changed files) is soon expected to be available here:
<http://lists.gnu.org/archive/html/automake-commit/2008-09/msg00005.html>.

FWIW2, I did not update copyright years on the test scripts.  I do
not think it is technically needed for trivial changes, so I'm
sparing myself that thankless task.

Cheers,
Ralf

    Use `Exit' instead of `exit' in test suite.
    
    Cater to Bourne shells like Solaris sh that do not pass the
    `exit' argument as status to the cleanup trap.
    * Makefile.am (maintainer-check): Check that here-documents
    use only `END' or `EOF' as delimiter in the test suite.
    Check that, outside of here-documents, the tests do not use
    `exit' with an argument, but use `Exit' instead.
    * tests/defs.in (Exit): New function.  Use it throughout,
    starting with the introduction of the exit trap.
    * tests/*.test: Use `Exit $arg' instead of `exit $arg'
    throughout, except inside created files.
    
    Signed-off-by: Ralf Wildenhues <address@hidden>

diff --git a/Makefile.am b/Makefile.am
index 2589adc..3924daa 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -228,8 +228,28 @@ maintainer-check: automake aclocal
          echo 'Do not run "automake" in the above tests.  Use "$$AUTOMAKE" 
instead.' 1>&2;  \
          exit 1; \
        fi
+## Tests should only use END and EOF for here documents
+## (so that the next test is effective).
+       @if grep '<<' $(srcdir)/tests/*.test | grep -v 'END' | grep -v 'EOF'; 
then \
+         echo 'Use here documents with "END" and "EOF" only, for 
greppability.' 1>&2; \
+         exit 1; \
+       fi
+## Tests should never call exit directly, but use Exit.
+## This is so that the exit status is transported correctly across the 0 trap.
+## Ignore comments, and ignore one perl line in ext2.test.
+       @found=false; for file in $(srcdir)/tests/*.test; do \
+         res=`sed -n '/^#/d; /^\$$PERL/d; /<<.*END/,/^END/{b;}; 
/<<.*EOF/,/^EOF/{b;}; /exit [$$0-9]/p' $$file`; \
+         if test -n "$$res"; then \
+           echo "$$file:$$res"; \
+           found=true; \
+         fi; \
+       done; \
+       if $$found; then \
+         echo 'Do not call plain "exit", use "Exit" instead, in above tests.' 
1>&2; \
+         exit 1; \
+       fi
 ## Use AUTOMAKE_fails when appropriate
-       @if grep -v '^#' $(srcdir)/tests/*.test | grep '\$$AUTOMAKE.*&&.*exit'; 
then \
+       @if grep -v '^#' $(srcdir)/tests/*.test | grep 
'\$$AUTOMAKE.*&&.*[eE]xit'; then \
          echo 'Use AUTOMAKE_fails + grep to catch automake failures in the 
above tests.' 1>&2;  \
          exit 1; \
        fi
diff --git a/tests/defs.in b/tests/defs.in
index a797a82..9172427 100644
--- a/tests/defs.in
+++ b/tests/defs.in
@@ -231,6 +231,18 @@ case "$srcdir" in
     ;;
 esac
 
+# We use a trap below for cleanup.  This requires us to go through
+# hoops to get the right exit status transported through the signal.
+# 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
+}
+
 curdir=`pwd`
 testSubDir=$me.dir
 chmod -R u+rwx $testSubDir > /dev/null 2>&1
@@ -250,13 +262,13 @@ trap 'exit_status=$?
   exit $exit_status
 ' 0
 for signal in 1 2 13 15; do
-  trap 'signal='$signal'; { (exit 1); exit 1; }' $signal
+  trap 'signal='$signal'; { Exit 1; }' $signal
 done
 signal=0
 
 # Copy in some files we need.
 for file in install-sh missing depcomp; do
-   cp "$srcdir/../lib/$file" "$testSubDir/$file" || exit 1
+   cp "$srcdir/../lib/$file" "$testSubDir/$file" || Exit 1
 done
 
 cd ./$testSubDir
@@ -313,14 +325,14 @@ case $required in
        fi
     done
     case $required in
-      *libtool* ) test $libtool_found = yes || exit 77 ;;
-      *gettext* ) test $gettext_found = yes || exit 77 ;;
+      *libtool* ) test $libtool_found = yes || Exit 77 ;;
+      *gettext* ) test $gettext_found = yes || Exit 77 ;;
     esac
     # Libtool cannot cope with spaces in the build tree.  Our testsuite setup
     # cannot cope with spaces in the source tree name for Libtool and gettext
     # tests.
     case $srcdir,`pwd` in
-      *\ * | *\        *) exit 77 ;;
+      *\ * | *\        *) Exit 77 ;;
     esac
     ACLOCAL="$ACLOCAL -Wno-syntax -I $srcdir/../m4 $extra_includes -I 
$aclocaldir"
     ;;
@@ -372,7 +384,7 @@ AUTOMAKE_run ()
   $AUTOMAKE ${1+"$@"} >stdout 2>stderr || exitcode=$?
   cat stderr
   cat stdout
-  test $exitcode = $expected_exitcode || exit 1
+  test $exitcode = $expected_exitcode || Exit 1
 }
 
 # AUTOMAKE_fails [options...]