Re: [PATCH] testrun.sh: Implement --tool=strace, --tool=valgrind

[Carlos O'Donell <carlos at redhat dot com>]

On 06/30/2018 09:33 AM, Florian Weimer wrote:
> $(file …) appears to be the only convenient way to create files
> with newlines and make substitution variables.  This needs make 4.0
> (released in 2013), so update the requirement to match.
> 
> 2018-06-30  Florian Weimer  <fweimer@redhat.com>
> 
> 	testrun.sh: Implement --tool=strace, --tool=valgrind
> 	* Makefile (testrun-script): Define variable.
> 	(testrun.sh): Use variable.
> 	* manual/install.texi (Tools for Compilation): make 4.0 or later
> 	is required.
> 	* configure.ac: Check for make 4.0 or later.
> 	* INSTALL: Regenerate.
> 	* configure: Likewise.

I'm OK with this. It further enhances the testsuite, and we already
require a new enough compiler that make 4.0 should be a NOP for most
distributions, with make 4.2.x being used by many.

It also adds a nice feature for strace/valgrind to run the test.
Normally I use --hardcoded-path-in-tests, but that perturbs the
binary in some cases. This is a good addition.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>


> diff --git a/Makefile b/Makefile
> index bea4e27f8d..d3f25a525a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -128,17 +128,60 @@ ifeq (yes,$(build-shared))
>  lib: $(common-objpfx)libc.so $(common-objpfx)linkobj/libc.so
>  endif # $(build-shared)
>  
> +# Used to build testrun.sh.
> +define testrun-script
> +#!/bin/bash
> +builddir=`dirname "$$0"`
> +GCONV_PATH="$${builddir}/iconvdata"
> +
> +usage () {
> +  echo "usage: $$0 [--tool=strace] PROGRAM [ARGUMENTS...]" 2>&1
> +  echo "       $$0 --tool=valgrind PROGRAM [ARGUMENTS...]" 2>&1

OK.

> +}
> +
> +toolname=default
> +while test $$# -gt 0 ; do
> +  case "$$1" in
> +    --tool=*)
> +      toolname="$${1:7}"
> +      shift
> +      ;;
> +    --*)
> +      usage
> +      ;;
> +    *)
> +      break
> +      ;;
> +  esac
> +done
> +
> +if test $$# -eq 0 ; then
> +  usage
> +fi
> +
> +case "$$toolname" in
> +  default)
> +    exec $(subst $(common-objdir),"$${builddir}", $(test-program-prefix)) \
> +      $${1+"$$@"}

OK.

> +    ;;
> +  strace)
> +    exec strace $(patsubst %, -E%, $(run-program-env)) \
> +      $(test-via-rtld-prefix) $${1+"$$@"}

OK.

> +    ;;
> +  valgrind)
> +    exec env $(run-program-env) valgrind $(test-via-rtld-prefix) $${1+"$$@"}

OK.

> +    ;;
> +  *)
> +    usage
> +    ;;
> +esac
> +endef
>  
>  # This is a handy script for running any dynamically linked program against
>  # the current libc build for testing.
>  $(common-objpfx)testrun.sh: $(common-objpfx)config.make \
>  			    $(..)Makeconfig $(..)Makefile
> -	(echo '#!/bin/sh'; \
> -	 echo 'builddir=`dirname "$$0"`'; \
> -	 echo 'GCONV_PATH="$${builddir}/iconvdata" \'; \
> -	 echo 'exec $(subst $(common-objdir),"$${builddir}",\
> -			    $(test-program-prefix)) $${1+"$$@"}'; \
> -	) > $@T
> +	$(file >$@T, $(testrun-script))

OK.

>  	chmod a+x $@T
>  	mv -f $@T $@
>  postclean-generated += testrun.sh

> diff --git a/configure.ac b/configure.ac
> index dc517017f5..f41ed6decb 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -945,7 +945,7 @@ fi
>  AC_CHECK_TOOL_PREFIX
>  AC_CHECK_PROG_VER(MAKE, gnumake gmake make, --version,
>    [GNU Make[^0-9]*\([0-9][0-9.]*\)],
> -  [3.79* | 3.[89]* | [4-9].* | [1-9][0-9]*], critic_missing="$critic_missing make")
> +  [[4-9].* | [1-9][0-9]*], critic_missing="$critic_missing make")

OK.

>  
>  AC_CHECK_PROG_VER(MSGFMT, gnumsgfmt gmsgfmt msgfmt, --version,
>    [GNU gettext.* \([0-9]*\.[0-9.]*\)],
> diff --git a/manual/install.texi b/manual/install.texi
> index 422da1447e..42e9954199 100644
> --- a/manual/install.texi
> +++ b/manual/install.texi
> @@ -473,13 +473,7 @@ build @theglibc{}:
>  
>  @itemize @bullet
>  @item
> -GNU @code{make} 3.79 or newer
> -
> -You need the latest version of GNU @code{make}.  Modifying @theglibc{}
> -to work with other @code{make} programs would be so difficult that
> -we recommend you port GNU @code{make} instead.  @strong{Really.}  We
> -recommend GNU @code{make} version 3.79.  All earlier versions have severe
> -bugs or lack features.
> +GNU @code{make} 4.0 or newer

OK. Why didn't we end these with a period?

>  
>  @item
>  GCC 4.9 or newer
> 


-- 
Cheers,
Carlos.