Re: [PATCH 3/5] Tests defs: some cleanup and minor fixes.

[Ralf Wildenhues]

* Stefano Lattarini wrote on Mon, Nov 15, 2010 at 06:24:52PM CET:
> * tests/defs: Exit with status 99 (hard error) rather than
> 1 (failure) on unexpected/internal errors, or when a signal
> is caught by the client script.
> Some minor changes in spacing and formatting.
> ($extra_includes): Variable renamed to ...
> ($acincludes): ... this one, and extended.

> From 3a37802eea8c78e915c2bb458179e1c4a17e2e74 Mon Sep 17 00:00:00 2001
[...]

> --- a/tests/defs
> +++ b/tests/defs
> @@ -23,7 +23,7 @@
>  # Ensure we are running from the right directory.
>  test -f ./defs-static || {
>     echo "$0: ./defs-static: not found in current directory" >&2
> -   exit 1
> +   exit 99
>  }
>  
>  # Source the shell sanitization and variables' definitions.
> @@ -40,13 +40,13 @@ me=`echo "$0" | sed -e 's,.*[\\/],,;s/\.test$//'`
>  # Ensure $testsrcdir is set correctly.
>  test -f "$testsrcdir/defs-static.in" || {
>     echo "$me: $testsrcdir/defs-static.in not found, check \$testsrcdir" >&2
> -   exit 1
> +   exit 99
>  }
>  
>  # Ensure $testbuilddir is set correctly.
>  test -f "$testbuilddir/defs-static" || {
>     echo "$me: $testbuilddir/defs-static not found, check \$testbuilddir" >&2
> -   exit 1
> +   exit 99
>  }
>  
>  # Unset some MAKE... variables that may cause $MAKE to act like a
> @@ -235,8 +235,8 @@ do
>        # Skip this test case if the user is root.
>        # We try to append to a read-only file to detect this.
>        priv_check_temp=priv-check.$$
> -      touch $priv_check_temp || exit 1
> -      chmod a-w $priv_check_temp || exit 1
> +      touch $priv_check_temp || exit 99
> +      chmod a-w $priv_check_temp || exit 99
>        (echo foo >> $priv_check_temp) >/dev/null 2>&1
>        overwrite_status=$?
>        rm -f $priv_check_temp
> @@ -259,8 +259,8 @@ do
>        # Skip this test case if read-only directories aren't supported
>        # (e.g., under DOS.)
>        ro_dir_temp=ro_dir.$$
> -      mkdir $ro_dir_temp || exit 1
> -      chmod a-w $ro_dir_temp || exit 1
> +      mkdir $ro_dir_temp || exit 99
> +      chmod a-w $ro_dir_temp || exit 99
>        (: > $ro_dir_temp/probe) >/dev/null 2>/dev/null
>        create_status=$?
>        rm -rf $ro_dir_temp

This is all OK.

> @@ -344,42 +344,37 @@ esac
>  case " $required " in
>    *' libtool '* | *' libtoolize '* | *' gettext '* )
>      aclocaldir=$testprefix/share/aclocal
> -    extra_includes=""
>      if test -f $aclocaldir/dirlist; then
> -       extra_includes=`
> +       acincludes=`
>         <$aclocaldir/dirlist \
>         sed  's/#.*//;s/[      ][      ]*$//g' \
>         | while read dir; do test ! -d "$dir" || echo "-I $dir"; done`
>      else :; fi
> -
> +    acincludes="-I $top_testsrcdir/m4 $acincludes -I $aclocaldir"
> +    unset aclocaldir
>      libtool_found=no
>      gettext_found=no
> -    for d in $extra_includes $aclocaldir ; do
> -      test "x$d" != x-I || continue
> -      if test -f "$d/libtool.m4"; then
> -        libtool_found=yes
> -      fi
> -      if test -f "$d/gettext.m4"; then
> -        gettext_found=yes
> -      fi
> +    for d in $acincludes; do
> +       test x"$d" = x"-I" && continue
> +       test -f "$d/libtool.m4" && libtool_found=yes
> +       test -f "$d/gettext.m4" && gettext_found=yes
>      done
>      case " $required " in
>        *' libtool '*|*' libtoolize '*)
>          if test $libtool_found != yes; then
>            skip "libtool is required, but libtool.m4 wasn't found in" \
> -               "directories $aclocaldir $extra_includes"
> +               "directories $acincludes"
>          fi
>          ;;
>        *' gettext '*)
>          if test $gettext_found != yes; then
>            skip "gettext is required, but gettext.m4 wasn't found in" \
> -               "directories $aclocaldir $extra_includes"
> +               "directories $acincludes"
>          fi
>          ;;
>      esac
> -    ACLOCAL="$ACLOCAL -Wno-syntax -I $top_testsrcdir/m4 $extra_includes -I 
> $aclocaldir"
> -    unset libtool_found gettext_found
> -    unset extra_includes aclocaldir
> +    ACLOCAL="$ACLOCAL -Wno-syntax $acincludes"
> +    unset libtool_found gettext_found acincludes
>      ;;
>  esac

Please either completely drop this hunk (if all it does it renaming but
no semantic changes), or split out and resubmit as separate patch with a
proper description (if there are intended semantic changes).  Thanks.

> @@ -418,14 +413,14 @@ if test "$sh_errexit_works" = yes; then
>      exit $exit_status
>    ' 0
>    for signal in 1 2 13 15; do
> -    trap 'signal='$signal'; { Exit 1; }' $signal
> +    trap 'signal='$signal'; { Exit 99; }' $signal
>    done
>    signal=0
>  fi
>  
>  # Copy in some files we need.
>  for file in install-sh missing depcomp; do
> -   cp "$top_testsrcdir/lib/$file" . || Exit 1
> +   cp "$top_testsrcdir/lib/$file" . || Exit 99
>  done
>  
>  # Build appropriate environment in test directory.  Eg create

These hunks are fine again.

Thanks,
Ralf