GNU bug report logs - #8893
[PATCH 1/2] tests: make test runner a script, not a shell function (was: Automake patches for custom test drivers' support break coreutils testsuite)

Previous Next

Full log

Message #8 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: Jim Meyering <jim <at> meyering.net>
Cc: Ralf Wildenhues <Ralf.Wildenhues <at> gmx.de>, bug-coreutils <at> gnu.org,
	automake-patches <at> gnu.org
Subject: Re: [PATCH 1/2] tests: make test runner a script,
	not a shell function (was: Automake patches for custom test drivers'
	support break coreutils testsuite)
Date: Sat, 18 Jun 2011 20:18:10 +0200

[Message part 1 (text/plain, inline)]

On Saturday 18 June 2011, Stefano Lattarini wrote:
> On Saturday 18 June 2011, Jim Meyering wrote:
> > Stefano Lattarini wrote:
> > >> ...
> > >> >
> > >> > In order to work with the upcoming new Automake testsuite harness, coreutils
> > >> > have two possibilities:
> > >> >  1. move the `shell_or_perl_' subroutine's functionality into a real acript,
> > >> >     and define the LOG_COMPILER to point to it; or
> > >> >  2. add a `.pl' extension to the perl test scripts, and define PL_LOG_COMPILER
> > >> >     appropriately (might be a little tricky, considering the hops that the
> > >> >     `shell_or_perl_' subroutine goes through in order to get the flags and
> > >> >     imports right).
> > >>
> > >> 1) sounds preferable.
> > >>
> > > But it has a serious drawback: the redirection `9>&2' placed at the end
> > > of TESTS_ENVIRONMENT will be rendered useless by the final exec done
> > > in the new `shell_or_perl' script (at least for with shells using the
> > > `cloexec' flag on fds > 2); this will bring back the problems fixed by
> > > commit `v8.12-82-g6b68745' :-(
> > >
> Well, no, that's not true; the "problematic" shells only set the 'close-on-exec'
> flag on the file descriptors they themselves create, not on the ones they inherit
> from their parents:
> 
>   $ ksh -c 'exec 9>&1; sh -c "echo foo >&9"'
>   sh: 9: Bad file descriptor
>   $ ksh -c 'sh -c "echo foo >&9"' 9>&1
>   foo
>   $ (exec 9>&1; ksh -c 'sh -c "echo foo >&9"')
>   foo
> 
> I should have tested better before reporting an imaginary problem.
> 
> > > So I now think we should go with solution (2).
> > 
> > Ok.
> > 
> I've gone with the less invasive solution (1) instead.  See the attached
> patch.  I will soon post a follow up that tries to compensate for the
> extra forks I've introduced here by removing a couple of grep calls from
> the `shell-or-perl' script
>
Here it is.

Regards,
  Stefano

[0002-tests-avoid-extra-forks-in-the-testsuite.patch (text/x-patch, inline)]

From bb3cbef9eece619dd694a60e5a738e71e939f9aa Mon Sep 17 00:00:00 2001
Message-Id: <bb3cbef9eece619dd694a60e5a738e71e939f9aa.1308420965.git.stefano.lattarini <at> gmail.com>
In-Reply-To: <e4dd05df8ea1f0fc99649627f895a76ec776279a.1308420965.git.stefano.lattarini <at> gmail.com>
References: <e4dd05df8ea1f0fc99649627f895a76ec776279a.1308420965.git.stefano.lattarini <at> gmail.com>
From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
Date: Sat, 18 Jun 2011 17:57:53 +0200
Subject: [PATCH 2/2] tests: avoid extra forks in the testsuite

* tests/shell-or-perl: Prefer the `read' builtin over `grep' to
look at the shebang line of test scripts.  Since `read' is a
special builtin, it might abort the whole program upon failures,
so add extra sanity checks, verifying that the test script exists
and is readable, before trying to read from it.
---
 tests/shell-or-perl |   16 ++++++++++++----
 1 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/tests/shell-or-perl b/tests/shell-or-perl
index ff92009..8c92f87 100644
--- a/tests/shell-or-perl
+++ b/tests/shell-or-perl
@@ -84,12 +84,19 @@ test -z "$test_name" && test_name=$test_script
 #  Run the test script  #
 # --------------------- #
 
-if grep '^#!/usr/bin/perl' "$test_script" >/dev/null; then
+test -f "$test_script" && test -r "$test_script" \
+  || error_ "test script '$test_script' does not exist, or isn't readable"
+
+read shebang_line < "$test_script" \
+  || error_ "cannot read from the test script '$test_script'"
+
+case $shebang_line in
+'#!/usr/bin/perl'*)
   # The test is a perl script.
   if $cu_PERL -e 'use warnings' > /dev/null 2>&1; then
     # Perl is available, see if we must run the test with taint
     # mode on or not.
-    grep '^#!/usr/bin/perl -T' "$test_script" >/dev/null && T_=T || T_=
+    case $shebang_line in *\ -T*) T_=T;; *) T_=;; esac
     # Now run it.
     exec $cu_PERL -w$T_ -I"$srcdir" -MCoreutils -MCuSkip \
                   -M"CuTmpdir qw($test_name)" \
@@ -99,10 +106,11 @@ if grep '^#!/usr/bin/perl' "$test_script" >/dev/null; then
     echo "$test_name: skip: no usable version of Perl found"
     exit 77
   fi
-else
+  ;;
+*)
   # Assume the test is a shell script.
   exec $cu_SHELL "$test_script" ${1+"$@"}
-fi
+esac
 
 # ------------- #
 #  Not reached  #
-- 
1.7.2.3

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.