Re: [PATCH 1/3] {testsuite-work} tests defs: better requirements for XSI shells

[Peter Rosin]

Den 2011-06-06 21:48 skrev Stefano Lattarini:
> This change avoids potential spurious failures with tests using
> the requirement 'xsi-shell' to mean that they want */bin/sh* (not
> $SHELL) to be XSI-conforming.  This idiom used to work before
> commit `v1.11-874-g1321be7' (as back then the test scripts were
> unconditionally run with /bin/sh), but has become inconsistent
> now that the test scripts re-execute themselves with configure
> determined $SHELL.
> 
> The described spurious failures have already occurred in practice,
> for examples on Solaris systems which also had GNU Bash installed.
> 
> From a suggestion by Peter Rosin.  See discussion at:
> <http://lists.gnu.org/archive/html/automake-patches/2011-06/msg00016.html>
> 
> * tests/defs (xsi-shell): Now check that $SHELL, rather than the
> shell currently running the test script, is an XSI shell.
> (xsi-bin-sh): New requirement, checking that /bin/sh (which can
> differ from $SHELL) is an XSI shell.
> (xsi-lib-shell): New requirement, checking that the shell that
> should be used to test the Automake-provided scripts from `lib/'
> is an XSI shell.  For the moment, this is just an alias for
> `xsi-bin-sh'.
> (require_xsi): New subroutine, used to factor out code common to
> the requirements above.
> ($xsi_shell_code): New variable, contains shell code supposed to
> work only with XSI shells.  Used by the new subroutine above.
> * tests/ar-lib.test ($required): Require 'xsi-lib-shell' instead
> of 'xsi-shell', since the script we test here is run with /bin/sh,
> not with $SHELL.
> * tests/compile3.test: Likewise.
> * tests/compile6.test: Likewise.
> ---
>  ChangeLog           |   32 ++++++++++++++++++++++++++++++++
>  tests/ar-lib.test   |    2 +-
>  tests/compile3.test |    2 +-
>  tests/compile6.test |    2 +-
>  tests/defs          |   33 ++++++++++++++++++++++-----------
>  5 files changed, 57 insertions(+), 14 deletions(-)
> 
> diff --git a/ChangeLog b/ChangeLog
> index 569f44a..35e18d0 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,35 @@
> +2011-06-06  Stefano Lattarini  <address@hidden>
> +
> +     tests defs: better requirements for XSI shells
> +     This change avoids potential spurious failures with tests using
> +     the requirement 'xsi-shell' to mean that they want */bin/sh* (not
> +     $SHELL) to be XSI-conforming.  This idiom used to work before
> +     commit `v1.11-874-g1321be7' (as back then the test scripts were
> +     unconditionally run with /bin/sh), but has become inconsistent
> +     now that the test scripts re-execute themselves with configure
> +     determined $SHELL.
> +     The described spurious failures have already occurred in practice,
> +     for examples on Solaris systems which also had GNU Bash installed.
> +     From a suggestion by Peter Rosin.  See discussion at:
> +     
> <http://lists.gnu.org/archive/html/automake-patches/2011-06/msg00016.html>
> +     * tests/defs (xsi-shell): Now check that $SHELL, rather than the
> +     shell currently running the test script, is an XSI shell.
> +     (xsi-bin-sh): New requirement, checking that /bin/sh (which can
> +     differ from $SHELL) is an XSI shell.
> +     (xsi-lib-shell): New requirement, checking that the shell that
> +     should be used to test the Automake-provided scripts from `lib/'
> +     is an XSI shell.  For the moment, this is just an alias for
> +     `xsi-bin-sh'.
> +     (require_xsi): New subroutine, used to factor out code common to
> +     the requirements above.
> +     ($xsi_shell_code): New variable, contains shell code supposed to
> +     work only with XSI shells.  Used by the new subroutine above.
> +     * tests/ar-lib.test ($required): Require 'xsi-lib-shell' instead
> +     of 'xsi-shell', since the script we test here is run with /bin/sh,
> +     not with $SHELL.
> +     * tests/compile3.test: Likewise.
> +     * tests/compile6.test: Likewise.
> +
>  2011-06-02  Stefano Lattarini  <address@hidden>
>  
>       self tests: fix another spurious failure
> diff --git a/tests/ar-lib.test b/tests/ar-lib.test
> index 7168ecd..9936c0e 100755
> --- a/tests/ar-lib.test
> +++ b/tests/ar-lib.test
> @@ -16,7 +16,7 @@
>  
>  # Make sure `ar-lib' wraps the Microsoft Library Manager (lib) correctly
>  
> -required=xsi-shell
> +required=xsi-lib-shell
>  . ./defs || Exit 1
>  
>  cp "$top_testsrcdir/lib/ar-lib" .
> diff --git a/tests/compile3.test b/tests/compile3.test
> index f949d1c..61cde6b 100755
> --- a/tests/compile3.test
> +++ b/tests/compile3.test
> @@ -16,7 +16,7 @@
>  
>  # Make sure `compile' wraps the Microsoft C/C++ compiler (cl) correctly
>  
> -required=xsi-shell
> +required=xsi-lib-shell
>  . ./defs || Exit 1
>  
>  cp "$top_testsrcdir/lib/compile" .
> diff --git a/tests/compile6.test b/tests/compile6.test
> index 02feb4b..ca1c671 100755
> --- a/tests/compile6.test
> +++ b/tests/compile6.test
> @@ -16,7 +16,7 @@
>  
>  # Make sure `compile' searches libraries correctly
>  
> -required=xsi-shell
> +required=xsi-lib-shell
>  . ./defs || Exit 1
>  
>  cp "$top_testsrcdir/lib/compile" .
> diff --git a/tests/defs b/tests/defs
> index ea036f8..37b5baa 100644
> --- a/tests/defs
> +++ b/tests/defs
> @@ -283,6 +283,22 @@ unindent ()
>  }
>  sed_unindent_prog="" # Avoid interferences from the environment.

s/interferences/interference/

>  
> +# require_xsi SHELL
> +# -----------------
> +# Skip the test is the given shell fails to support common XSI constructs.

s/ is / if /

> +require_xsi ()
> +{
> +  test $# -eq 1 || framework_failure_ "require_xsi needs exactly one arg"
> +  echo "$me: trying some XSI constructs with $1"
> +  $1 -c "$xsi_shell_code" || skip_ "$1 lacks XSI features"
> +}
> +# Shell code supposed to work only with XSI shells.  Inspired to

s/Inspired to/Inspired by/

> +# libtool.m4:_LT_CHECK_SHELL_FEATURES.
> +xsi_shell_code='
> +    var="a/b/c" \
> +      && test "${var##*/},${var%/*},${var#??}"${var%"$var"}, = c,a/b,b/c, \
> +      && test $(( 1 + 1 )) -eq 2 && test "${#var}" -eq 5'
> +

Changing the variable names makes it harder to keep it in sync with Libtool.
I think this was mentioned the last time you tried to change it? Also, I
notice that the code is not completely equal (one extra && occurrence, and
one eval is removed), are you sure the code is equivalent? I'm not qualified
to tell.

>  
>  ## ----------------------------------------------------------- ##
>  ##  Checks for required tools, and additional setups (if any)  ##
> @@ -316,6 +332,12 @@ do
>        test "$FC"  = false && skip_ "no Fortran compiler available";;
>      fortran77)
>        test "$F77" = false && skip_ "no Fortran 77 compiler available";;
> +    xsi-shell)
> +      require_xsi "$SHELL";;
> +    xsi-bin-sh)
> +      require_xsi "/bin/sh";;
> +    xsi-lib-shell)
> +      require_xsi "/bin/sh";;
>      bzip2)
>        # Do not use --version, bzip2 still tries to compress stdin.
>        echo "$me: running bzip2 --help"
> @@ -509,17 +531,6 @@ do
>        texi2dvi -o /dev/null --version \
>          || skip_ "required program \`texi2dvi' not available"
>        ;;
> -    xsi-shell)
> -      # Try some XSI features.
> -      # Keep this in sync with libtool.m4:_LT_CHECK_SHELL_FEATURES.
> -      echo "$me: trying some XSI constructs"
> -      ( _am_dummy="a/b/c"
> -     test 
> "${_am_dummy##*/},${_am_dummy%/*},${_am_dummy#??}"${_am_dummy%"$_am_dummy"}, \
> -             = c,a/b,b/c, \
> -     && eval 'test $(( 1 + 1 )) -eq 2 \
> -     && test "${#_am_dummy}" -eq 5' ) \
> -          || skip_ "the shell lacks some required XSI features"
> -      ;;
>      flex)
>        # Since flex is required, we pick LEX for ./configure.
>        LEX=flex

Cheers,
Peter