[PATCH] {testsuite-work} test defs: fix ksh-related portability bug in warning messages (was: Re: bug#8846: coreutils-8.12 on HP-UX 11.31: 3 of 365 tests failed)

[Stefano Lattarini]

[adding automake-patches, dropping bug-coreutils and bug-autoconf]

On Tuesday 14 June 2011, Jim Meyering wrote:
> 
> From 67811abfda04dafdde114a50556f0e1d2e9b6270 Mon Sep 17 00:00:00 2001
> From: Jim Meyering <address@hidden>
> Date: Tue, 14 Jun 2011 09:59:14 +0200
> Subject: [PATCH] tests: accommodate HP-UX and Solaris shells
> 
> Running "make check" normally prints a diagnostic to the outermost
> stderr (usually a tty) to explain why a test is skipped.  It did this
> by redirecting FD 9 to stderr (via "exec 9>&2") before invoking the
> shell script.  Shell scripts write skip-explanation to FD 9 via
> init.sh's skip_ function.  However, with Solaris 10's ksh and HP-UX,
> the effects of "exec 9>&2" are canceled upon fork-and-exec, so we
> would get a "Bad file number" diagnostic and no skip explanation on
> those systems.
> * tests/check.mk (TESTS_ENVIRONMENT): Redirect more portably, via
> "$(SHELL) 9>&2", rather than the prior "exec 9>&2; $(SHELL) ..."
> Actually, we use "shell_or_perl_ 9>&2", to make this effective
> also for the perl-based tests.
> * tests/init.sh (stderr_fileno_): Update the advice in comments.
> See http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/22488
> for lots of discussion.  Stefano Lattarini suggested the solution
> of putting "9>&2" after the command.  Reported by Bruno Haible.
> ---
>  tests/check.mk |    3 +--
>  tests/init.sh  |    4 ++--
>  2 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/check.mk b/tests/check.mk
> index db7f067..9db96af 100644
> --- a/tests/check.mk
> +++ b/tests/check.mk
> @@ -58,7 +58,6 @@ TESTS_ENVIRONMENT =                         \
>    test -d "$$tmp__" && test -w "$$tmp__" || tmp__=.; \
>    . $(srcdir)/envvar-check;                  \
>    TMPDIR=$$tmp__; export TMPDIR;             \
> -  exec 9>&2;                                 \
>    shell_or_perl_() {                         \
>      if grep '^\#!/usr/bin/perl' "$$1" > /dev/null; then                      
> \
>        if $(PERL) -e 'use warnings' > /dev/null 2>&1; then            \
> @@ -100,6 +99,6 @@ TESTS_ENVIRONMENT =                                \
>    REPLACE_GETCWD=$(REPLACE_GETCWD)           \
>    ; test -d /usr/xpg4/bin && PATH='/usr/xpg4/bin$(PATH_SEPARATOR)'"$$PATH"; \
>    PATH='$(abs_top_builddir)/src$(PATH_SEPARATOR)'"$$PATH" \
> -  ; shell_or_perl_
> +  ; shell_or_perl_ 9>&2
> 
>  VERBOSE = yes
> diff --git a/tests/init.sh b/tests/init.sh
> index 60d1bc1..4a52626 100644
> --- a/tests/init.sh
> +++ b/tests/init.sh
> @@ -68,8 +68,8 @@ 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.
> +#   export ...ENVVAR_SETTINGS...; $(SHELL) 9>&2
> +# 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}
> --
> 1.7.6.rc0.293.g40857
> 
Thanks, I'll soonish apply a similar patch (see attachement) for Automake's
`tests/defs' if there is no objection.

Regards,
  Stefano

From 29ca903b799b0a319f881701788f9b350ac50cc8 Mon Sep 17 00:00:00 2001
Message-Id: <address@hidden>
From: Stefano Lattarini <address@hidden>
Date: Mon, 13 Jun 2011 22:42:25 +0200
Subject: [PATCH] test defs: fix ksh-related portability bug in warning messages

Running "make check" normally prints a diagnostic to the outermost
stderr (usually a tty) to explain why a test is skipped, thus
giving better and faster feedback to the user.  It used to do
so by redirecting file descriptor 9 to stderr (via "exec 9>&2")
before invoking the test scripts, which then would write any skip
explanation to file descriptor 9 via the `skip_' function defined
in `tests/defs'.

However, various Korn Shells (at least Solaris 10's /bin/ksh and
Debian GNU/Linux's /bin/ksh) and the HP-UX's /bin/sh close open
file descriptors > 2 upon an `exec' system call; thus the effects
of "exec 9>&2" are cancelled upon fork-and-exec, so we would get
a "Bad file number" diagnostic and no skip explanation with those
shells.

The present change remedies this situation.

* tests/Makefile.am (AM_TESTS_ENVIRONMENT): Redirect more portably,
via a trailing "9>&2", rather than the prior "exec 9>&2; ...".  Add
explanatory comments.
* tests/defs (stderr_fileno_): Update the advice in comments.

Based on commit v8.12-82-g6b68745 "tests: accommodate HP-UX and
ksh-derived shells" in GNU coreutils.

Further references, with lots of discussion:
 <http://lists.gnu.org/archive/html/bug-autoconf/2011-06/msg00002.html>
 <http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/22488>
 <http://debbugs.gnu.org/cgi/bugreport.cgi?bug=8846>
---
 ChangeLog         |   29 +++++++++++++++++++++++++++++
 tests/Makefile.am |   13 ++++++++++---
 tests/Makefile.in |   13 ++++++++++---
 tests/defs        |    5 +++--
 4 files changed, 52 insertions(+), 8 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 37c0a69..751410a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,32 @@
+2011-06-14  Stefano Lattarini  <address@hidden>
+           Jim Meyering  <address@hidden>
+
+       test defs: fix ksh-related portability bug in warning messages
+       Running "make check" normally prints a diagnostic to the outermost
+       stderr (usually a tty) to explain why a test is skipped, thus
+       giving better and faster feedback to the user.  It used to do
+       so by redirecting file descriptor 9 to stderr (via "exec 9>&2")
+       before invoking the test scripts, which then would write any skip
+       explanation to file descriptor 9 via the `skip_' function defined
+       in `tests/defs'.
+       However, various Korn Shells (at least Solaris 10's /bin/ksh and
+       Debian GNU/Linux's /bin/ksh) and the HP-UX's /bin/sh close open
+       file descriptors > 2 upon an `exec' system call; thus the effects
+       of "exec 9>&2" are cancelled upon fork-and-exec, so we would get
+       a "Bad file number" diagnostic and no skip explanation with those
+       shells.
+       The present change remedies this situation.
+       * tests/Makefile.am (AM_TESTS_ENVIRONMENT): Redirect more portably,
+       via a trailing "9>&2", rather than the prior "exec 9>&2; ...".  Add
+       explanatory comments.
+       * tests/defs (stderr_fileno_): Update the advice in comments.
+       Based on commit v8.12-82-g6b68745 "tests: accommodate HP-UX and
+       ksh-derived shells" in GNU coreutils.
+       Further references, with lots of discussion:
+        <http://lists.gnu.org/archive/html/bug-autoconf/2011-06/msg00002.html>
+        <http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/22488>
+        <http://debbugs.gnu.org/cgi/bugreport.cgi?bug=8846>
+
 2011-06-13  Stefano Lattarini  <address@hidden>
 
        tests: don't hard-code test name in txinfo21.test
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 9b979d4..e9172d2 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -124,12 +124,19 @@ $(config_shell_tests):
 
 # Some testsuite-influential variables should be overridable from the
 # test scripts, but not from the environment.
-# We want warning messages and explanations for skipped tests to go to
-# the console if possible, so set up `stderr_fileno_' properly.
 # The `AM_TESTS_REEXEC=no' setting tells the tests not to needlessly
 # re-execute themselves with the shell detected at configure time, since
 # we are already running them under it explicitly in our setup (see e.g.
 # the definition of TEST_LOG_COMPILER above).
+# We want warning messages and explanations for skipped tests to go to
+# the console if possible, so set up `stderr_fileno_' properly.
+# The `9>&2' redirection *must* be placed at the end, and without a
+# following semicolon, because some shells (e.g., various Korn Shells
+# or HP-HX /bin/sh) close any file descriptor > 2 upon `exec' sycall.
+# For more references, with lots of discussion, see:
+#  - http://lists.gnu.org/archive/html/bug-autoconf/2011-06/msg00002.html
+#  - http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/22488
+#  - http://debbugs.gnu.org/cgi/bugreport.cgi?bug=8846
 AM_TESTS_ENVIRONMENT = \
   test x"$$me" = x || unset me; \
   test x"$$required" = x || unset required; \
@@ -137,7 +144,7 @@ AM_TESTS_ENVIRONMENT = \
   test x"$$original_AUTOMAKE" = x || unset original_AUTOMAKE; \
   test x"$$original_ACLOCAL" = x || unset original_ACLOCAL; \
   AM_TESTS_REEXEC=no; export AM_TESTS_REEXEC; \
-  exec 9>&2; stderr_fileno_=9; export stderr_fileno_;
+  stderr_fileno_=9; export stderr_fileno_; 9>&2
 
 TESTS = \
   $(handwritten_tests) \
diff --git a/tests/Makefile.in b/tests/Makefile.in
index 92daaf3..57791f3 100644
--- a/tests/Makefile.in
+++ b/tests/Makefile.in
@@ -409,12 +409,19 @@ config_shell_tests = \
 
 # Some testsuite-influential variables should be overridable from the
 # test scripts, but not from the environment.
-# We want warning messages and explanations for skipped tests to go to
-# the console if possible, so set up `stderr_fileno_' properly.
 # The `AM_TESTS_REEXEC=no' setting tells the tests not to needlessly
 # re-execute themselves with the shell detected at configure time, since
 # we are already running them under it explicitly in our setup (see e.g.
 # the definition of TEST_LOG_COMPILER above).
+# We want warning messages and explanations for skipped tests to go to
+# the console if possible, so set up `stderr_fileno_' properly.
+# The `9>&2' redirection *must* be placed at the end, and without a
+# following semicolon, because some shells (e.g., various Korn Shells
+# or HP-HX /bin/sh) close any file descriptor > 2 upon `exec' sycall.
+# For more references, with lots of discussion, see:
+#  - http://lists.gnu.org/archive/html/bug-autoconf/2011-06/msg00002.html
+#  - http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/22488
+#  - http://debbugs.gnu.org/cgi/bugreport.cgi?bug=8846
 AM_TESTS_ENVIRONMENT = \
   test x"$$me" = x || unset me; \
   test x"$$required" = x || unset required; \
@@ -422,7 +429,7 @@ AM_TESTS_ENVIRONMENT = \
   test x"$$original_AUTOMAKE" = x || unset original_AUTOMAKE; \
   test x"$$original_ACLOCAL" = x || unset original_ACLOCAL; \
   AM_TESTS_REEXEC=no; export AM_TESTS_REEXEC; \
-  exec 9>&2; stderr_fileno_=9; export stderr_fileno_;
+  stderr_fileno_=9; export stderr_fileno_; 9>&2
 
 TESTS = \
   $(handwritten_tests) \
diff --git a/tests/defs b/tests/defs
index df62751..4f6f3b2 100644
--- a/tests/defs
+++ b/tests/defs
@@ -169,8 +169,9 @@ Exit ()
 
 # Print warnings (e.g., about skipped and failed tests) to this file
 # number.  Override by putting, say:
-#   stderr_fileno_=9; export stderr_fileno_; exec 9>&2;
-# in the definition of AM_TESTS_ENVIRONMENT.
+#   stderr_fileno_=9; export stderr_fileno_; 9>&2
+# at the *end* (yes, this is mandatory; see comments in Makefile.am) of
+# the definition of AM_TESTS_ENVIRONMENT.
 # 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}
-- 
1.7.2.3