testrun.sh: Implement --tool=strace, --tool=valgrind

Message ID 20180630133359.B152643994575@oldenburg.str.redhat.com
State New
Headers show
Series
  • testrun.sh: Implement --tool=strace, --tool=valgrind
Related show

Commit Message

Florian Weimer June 30, 2018, 1:33 p.m.
$(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.

Comments

Joseph Myers June 30, 2018, 8:23 p.m. | #1
On Sat, 30 Jun 2018, 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.

An increase in make version required needs an entry in the "Changes to 
build and runtime requirements" section of NEWS for the release with the 
change.
Florian Weimer July 4, 2018, 11:32 a.m. | #2
On 06/30/2018 10:23 PM, Joseph Myers wrote:
> On Sat, 30 Jun 2018, 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.
> 
> An increase in make version required needs an entry in the "Changes to
> build and runtime requirements" section of NEWS for the release with the
> change.

Okay.  But first, we need to decide if we want this change, and if it is 
sufficient justification to bump the make requirement (although newer 
GNU make is also quite a bit faster at building glibc, it seems).

$(file …) could also be used to avoid passing down parameters to shell 
scripts in various tests, I expected.  The shell scripts themselves 
could be generated from makefile fragments instead, or a file which 
could be sourced by them.  This would avoid the confusion positional 
parameter refers to which makefile variable in the shell script.

Thanks,
Florian
Carlos O'Donell July 4, 2018, 1:22 p.m. | #3
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
>
Szabolcs Nagy July 5, 2018, 10:55 a.m. | #4
On 04/07/18 14:22, Carlos O'Donell wrote:
> 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.
> 

this broke my testing on various targets using ubuntu
14.04 lts and older debian systems.

i will build a new make, but on other projects i prefer
to stick to 3.82. make 4.0 introduced plugin support and
other regressions so its behaviour is much less reliable.
Carlos O'Donell July 5, 2018, 1:50 p.m. | #5
On 07/05/2018 06:55 AM, Szabolcs Nagy wrote:
> On 04/07/18 14:22, Carlos O'Donell wrote:
>> 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.
>>
> 
> this broke my testing on various targets using ubuntu
> 14.04 lts and older debian systems.

Ubuntu 14.04LTS will soon be EOL (10 months).

Ubuntu 16.04LTS or newer has make >= 4.1.

Debian Stretch 9.4 is stable and has make >= 4.1.

Fedora 27 is the oldest non-EOL and has make >= 4.2.

So I don't see this being an issue except with the oldest
Ubuntu 14.04LTS.

I don't think we should revert these changes based on
make availability, unless we see more issues where users
can't upgrade to a newer make for other architectures.

> i will build a new make, but on other projects i prefer
> to stick to 3.82. make 4.0 introduced plugin support and
> other regressions so its behaviour is much less reliable.

You face two issues here.

(a) Obsolescence of your development system.

I empathize with your pain, the base RHEL7 is no longer
capable of compiling glibc, but within RHEL we have a
Developer Toolset with newer binutils, gcc, make, etc. all
which can be used to compile glibc. It still doesn't fix the
kernel headers though, and I have to fetch and use the latest
upstream kernel headers. The glibc team for Red Hat builds on
RHEL7 for release testing and to ensure we could deliver any
new glibc feature if needed by backport (gives us some confidence).

In summary, there will always be some pain here in adopting
newer tooling, but make 4.0 is old enough that newer distributions
should have it.

(b) Reliability issues in make 4.0.

As toolchain developers we should be able to help iron out
robustness issues in GNU Make when we come across them. If
we need a new feature in make 4.0 we should not hesitate to
use it if it enables something newer or more useful, like
it does in this case. Though this kind of enablement should
be tempered against the problems it causes. I personally
haven't seen any issue with make 4.1, and I've been using
it for over 8 months.

Patch

diff --git a/INSTALL b/INSTALL
index 0a22aa7d01..3c656fb7a6 100644
--- a/INSTALL
+++ b/INSTALL
@@ -426,13 +426,7 @@  Recommended Tools for Compilation
 We recommend installing the following GNU tools before attempting to
 build the GNU C Library:
 
-   * GNU 'make' 3.79 or newer
-
-     You need the latest version of GNU 'make'.  Modifying the GNU C
-     Library to work with other 'make' programs would be so difficult
-     that we recommend you port GNU 'make' instead.  *Really.*  We
-     recommend GNU 'make' version 3.79.  All earlier versions have
-     severe bugs or lack features.
+   * GNU 'make' 4.0 or newer
 
    * GCC 4.9 or newer
 
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
+}
+
+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+"$$@"}
+    ;;
+  strace)
+    exec strace $(patsubst %, -E%, $(run-program-env)) \
+      $(test-via-rtld-prefix) $${1+"$$@"}
+    ;;
+  valgrind)
+    exec env $(run-program-env) valgrind $(test-via-rtld-prefix) $${1+"$$@"}
+    ;;
+  *)
+    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))
 	chmod a+x $@T
 	mv -f $@T $@
 postclean-generated += testrun.sh
diff --git a/configure b/configure
index ef18302215..eac7f292b4 100755
--- a/configure
+++ b/configure
@@ -4705,7 +4705,7 @@  $as_echo_n "checking version of $MAKE... " >&6; }
   ac_prog_version=`$MAKE --version 2>&1 | sed -n 's/^.*GNU Make[^0-9]*\([0-9][0-9.]*\).*$/\1/p'`
   case $ac_prog_version in
     '') ac_prog_version="v. ?.??, bad"; ac_verc_fail=yes;;
-    3.79* | 3.[89]* | [4-9].* | [1-9][0-9]*)
+    [4-9].* | [1-9][0-9]*)
        ac_prog_version="$ac_prog_version, ok"; ac_verc_fail=no;;
     *) ac_prog_version="$ac_prog_version, bad"; ac_verc_fail=yes;;
 
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")
 
 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
 
 @item
 GCC 4.9 or newer