Patchwork make recheck?

login
register
mail settings
Submitter Ralf Wildenhues
Date Oct. 10, 2010, 4:06 p.m.
Message ID <20101010160601.GE14372@gmx.de>
Download mbox | patch
Permalink /patch/67353/
State New
Headers show

Comments

Ralf Wildenhues - Oct. 10, 2010, 4:06 p.m.
[ moving from gcc@ ]

* Diego Novillo wrote on Sat, Oct 02, 2010 at 06:44:09PM CEST:
> On Sat, Oct 2, 2010 at 05:54, Ralf Wildenhues wrote:
> 
> > Asking because it could help speed up patch development:
> > 1) hack hack hack
> > 2) make -k check-$whatever
> > 3) go back to (1) until satisfactory
> > 4) git commit patch, undo patch in work tree, rebuild
> > 5) run 'make recheck' to ensure all new failures were already old.
> 
> This sounds like a great idea.  You'd need to extract the FAIL lines
> from the .log file and backtrack to figure out which .exp file
> produced them.  This would give you the input for
> RUNTESTFLAGS=f.exp=...
> 
> I don't recall if you can specify more than one file in the
> RUNTESTFLAGS argument, though.

You can specify more than one file, but the parallel check rules broke
passing quoted content, which is needed for passing more than one test
name per .exp file.  The first patch below fixes that, but ...

it turned out that it was still ugly to get things in a Makefile rule,
so I ended up with an external script that doesn't yet use the make
rules (so strictly, the first patch is not yet needed, but will be at
some point).  It also has the advantage of working outside of the gcc
subdir.

Longer term, I'd like to overhaul the Automake rules for dejagnu
support, add the functionality there and in gcc/Makefile.in.

The script has been tested lightly on a full build tree, so beware and
use -n to see what would happen.

OK for both patches?

I suppose I should add code to backup old .sum files before rechecking
next.

Hmm, the parallel check rules also broke in-tree dejagnu (i.e.,
extracting the dejagnu package sources below toplevel), I wonder if
anybody uses that.

Thanks,
Ralf

Fix quoting for RUNTESTFLAGS in gcc/.

gcc/ChangeLog:
2010-10-10  Ralf Wildenhues  <Ralf.Wildenhues@gmx.de>

	* Makefile.in ($(lang_checks_parallel))
	($(lang_checks_parallelized)): Use single quotes for
	$(RUNTESTFLAGS), to allow passing quoted content.
Paolo Bonzini - Oct. 11, 2010, 11:32 a.m.
On 10/10/2010 06:06 PM, Ralf Wildenhues wrote:
> [ moving from gcc@ ]
>
> * Diego Novillo wrote on Sat, Oct 02, 2010 at 06:44:09PM CEST:
>> On Sat, Oct 2, 2010 at 05:54, Ralf Wildenhues wrote:
>>
>>> Asking because it could help speed up patch development:
>>> 1) hack hack hack
>>> 2) make -k check-$whatever
>>> 3) go back to (1) until satisfactory
>>> 4) git commit patch, undo patch in work tree, rebuild
>>> 5) run 'make recheck' to ensure all new failures were already old.
>>
>> This sounds like a great idea.  You'd need to extract the FAIL lines
>> from the .log file and backtrack to figure out which .exp file
>> produced them.  This would give you the input for
>> RUNTESTFLAGS=f.exp=...
>>
>> I don't recall if you can specify more than one file in the
>> RUNTESTFLAGS argument, though.
>
> You can specify more than one file, but the parallel check rules broke
> passing quoted content, which is needed for passing more than one test
> name per .exp file.

This is useful anyway, so please apply.

> it turned out that it was still ugly to get things in a Makefile rule,
> so I ended up with an external script that doesn't yet use the make
> rules (so strictly, the first patch is not yet needed, but will be at
> some point).  It also has the advantage of working outside of the gcc
> subdir.

You don't really need formal approval for changes in contrib/, so go 
ahead and commit it.  You also won't need approval for further changes 
to test_recheck.

Paolo
H.J. Lu - Oct. 12, 2010, 2:08 a.m.
On Sun, Oct 10, 2010 at 9:06 AM, Ralf Wildenhues <Ralf.Wildenhues@gmx.de> wrote:
> [ moving from gcc@ ]
>
> * Diego Novillo wrote on Sat, Oct 02, 2010 at 06:44:09PM CEST:
>> On Sat, Oct 2, 2010 at 05:54, Ralf Wildenhues wrote:
>>
>> > Asking because it could help speed up patch development:
>> > 1) hack hack hack
>> > 2) make -k check-$whatever
>> > 3) go back to (1) until satisfactory
>> > 4) git commit patch, undo patch in work tree, rebuild
>> > 5) run 'make recheck' to ensure all new failures were already old.
>>
>> This sounds like a great idea.  You'd need to extract the FAIL lines
>> from the .log file and backtrack to figure out which .exp file
>> produced them.  This would give you the input for
>> RUNTESTFLAGS=f.exp=...
>>
>> I don't recall if you can specify more than one file in the
>> RUNTESTFLAGS argument, though.
>
> You can specify more than one file, but the parallel check rules broke
> passing quoted content, which is needed for passing more than one test
> name per .exp file.  The first patch below fixes that, but ...
>
> it turned out that it was still ugly to get things in a Makefile rule,
> so I ended up with an external script that doesn't yet use the make
> rules (so strictly, the first patch is not yet needed, but will be at
> some point).  It also has the advantage of working outside of the gcc
> subdir.
>
> Longer term, I'd like to overhaul the Automake rules for dejagnu
> support, add the functionality there and in gcc/Makefile.in.
>
> The script has been tested lightly on a full build tree, so beware and
> use -n to see what would happen.
>
> OK for both patches?
>
> I suppose I should add code to backup old .sum files before rechecking
> next.
>
> Hmm, the parallel check rules also broke in-tree dejagnu (i.e.,
> extracting the dejagnu package sources below toplevel), I wonder if
> anybody uses that.
>
> Thanks,
> Ralf
>
> Fix quoting for RUNTESTFLAGS in gcc/.
>
> gcc/ChangeLog:
> 2010-10-10  Ralf Wildenhues  <Ralf.Wildenhues@gmx.de>
>
>        * Makefile.in ($(lang_checks_parallel))
>        ($(lang_checks_parallelized)): Use single quotes for
>        $(RUNTESTFLAGS), to allow passing quoted content.
>

This patch is bogus and caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45974
Ralf Wildenhues - Oct. 12, 2010, 5:08 a.m.
* H.J. Lu wrote on Tue, Oct 12, 2010 at 04:08:07AM CEST:
> On Sun, Oct 10, 2010 at 9:06 AM, Ralf Wildenhues wrote:
> > Fix quoting for RUNTESTFLAGS in gcc/.
> >
> > gcc/ChangeLog:
> > 2010-10-10  Ralf Wildenhues  <Ralf.Wildenhues@gmx.de>
> >
> >        * Makefile.in ($(lang_checks_parallel))
> >        ($(lang_checks_parallelized)): Use single quotes for
> >        $(RUNTESTFLAGS), to allow passing quoted content.
> >
> 
> This patch is bogus and caused:
> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45974

Thanks, reverted.  A working quoting style that allows to pass multiple
tests needs to be documented in the manual.

Cheers,
Ralf

Patch

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 962b310..6fda533 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -4888,7 +4888,7 @@  $(lang_checks_parallel): site.exp
 	variant=`echo "$@" | sed 's,^[^/]*//,,'`; \
 	vardots=`echo "$$variant" | sed 's,/,.,g'`; \
 	$(MAKE) TESTSUITEDIR="testsuite.$$vardots" \
-	  RUNTESTFLAGS="--target_board=$$variant $(RUNTESTFLAGS)" \
+	  RUNTESTFLAGS="--target_board=$$variant "'$(RUNTESTFLAGS)' \
 	  "$$target"
 
 TESTSUITEDIR = testsuite
@@ -4947,9 +4947,9 @@  check_p_subdirs=$(wordlist 1,$(words $(check_$*_parallelize)),$(check_p_numbers)
 # to lang_checks_parallelized variable and define check_$lang_parallelize
 # variable (see above check_gcc_parallelize description).
 $(lang_checks_parallelized): check-% : site.exp
-	@if [ -z "$(filter-out --target_board=%, $(RUNTESTFLAGS))" ] \
+	@if [ -z '$(filter-out --target_board=%, $(RUNTESTFLAGS))' ] \
 	    && [ "$(filter -j, $(MFLAGS))" = "-j" ]; then \
-	  $(MAKE) TESTSUITEDIR="$(TESTSUITEDIR)" RUNTESTFLAGS="$(RUNTESTFLAGS)" \
+	  $(MAKE) TESTSUITEDIR="$(TESTSUITEDIR)" RUNTESTFLAGS='$(RUNTESTFLAGS)' \
 	    check-parallel-$* \
 	    $(patsubst %,check-parallel-$*_%, $(check_p_subdirs)); \
 	  for file in $(TESTSUITEDIR)/$*/$* \
@@ -4966,7 +4966,7 @@  $(lang_checks_parallelized): check-% : site.exp
 	    $(patsubst %,$(TESTSUITEDIR)/$*%/$*.log.sep,$(check_p_subdirs)) \
 	    > $(TESTSUITEDIR)/$*/$*.log; \
 	else \
-	  $(MAKE) TESTSUITEDIR="$(TESTSUITEDIR)" RUNTESTFLAGS="$(RUNTESTFLAGS)" \
+	  $(MAKE) TESTSUITEDIR="$(TESTSUITEDIR)" RUNTESTFLAGS='$(RUNTESTFLAGS)' \
 	    check_$*_parallelize= check-parallel-$*; \
 	fi
 


New contrib/test_recheck script to rerun unsuccessful tests.

contrib/ChangeLog:
2010-10-10  Ralf Wildenhues  <Ralf.Wildenhues@gmx.de>

	* test_recheck: New script.

diff --git a/contrib/test_recheck b/contrib/test_recheck
new file mode 100755
index 0000000..193cd3d
--- /dev/null
+++ b/contrib/test_recheck
@@ -0,0 +1,98 @@ 
+#! /bin/sh
+
+# (C) 2010 Free Software Foundation
+# Written by Ralf Wildenhues <Ralf.Wildenhues@gmx.de>.
+
+# This script is Free Software, and it can be copied, distributed and
+# modified as defined in the GNU General Public License.  A copy of
+# its license can be downloaded from http://www.gnu.org/copyleft/gpl.html
+
+PROGNAME=test_recheck
+
+usage ()
+{
+  cat <<EOF
+Usage: $PROGNAME [-h] [-n] DIR|FILE.sum...
+
+Rerun unsuccessful tests for testsuites below DIR or for FILE.sum.
+
+  -h     display this help and exit
+  -n     dry run, only show what would be run
+EOF
+  exit $?
+}
+
+error ()
+{
+  echo "$@" >&2
+  exit 1
+}
+
+dry=
+for arg
+do
+  case $arg in
+    -h | \?) usage ;;
+    -n) dry=:; shift ;;
+    -*) error "unknown argument $arg" ;;
+    *)  break ;;
+  esac
+done
+test $# -gt 0 || usage
+
+# Find a good awk.
+if test -z "$AWK" ; then
+  for AWK in gawk nawk awk
+  do
+    if type $AWK 2>&1 | grep 'not found' > /dev/null 2>&1 ; then
+      :
+    else
+      break
+    fi
+  done
+fi
+
+: ${MAKE=make}
+: ${filesuffix=}
+cwd=`pwd`
+files=`find "$@" -name \*.sum$filesuffix -print | grep testsuite | sort`
+st=0
+
+for file in $files; do
+  dir=`echo $file | sed 's,/[^/]*$,,'`
+  base=`echo $file | sed 's,.*/,,; s,\.sum$,,'`
+  flags=`$AWK '
+/^Running .*\.exp \.\.\./ {
+  if (expfile != "" && tests != "")
+    printf (" %s=\"%s\"", expfile, tests)
+  expfile = $2
+  sub (/^[^ ]*\//, "", expfile)
+  sep = ""
+  tests = ""
+}
+/^(FAIL|XPASS|UNRESOLVED|WARNING|ERROR): / {
+  if (test != $2 "" && $2 != "" ) {
+    test = $2
+    tests = tests sep test
+    sep = " "
+  }
+}
+END {
+  if (expfile != "" && tests != "")
+    printf (" %s=\"%s\"", expfile, tests)
+}' $file`
+  if test -n "$flags"; then
+    cd $dir
+    amflags=
+    if grep '^AM_RUNTESTFLAGS =' Makefile >/dev/null 2>&1; then
+      amflags=`echo 'print-runtestflags: ; @echo $(AM_RUNTESTFLAGS)' \
+		 | ${MAKE} -s -f Makefile -f - print-runtestflags`
+    fi
+    echo "(cd $dir && runtest $amflags --tool $base $flags)"
+    if test -z "$dry"; then
+      eval runtest --tool $base $flags || st=$?
+    fi
+    cd "$cwd"
+  fi
+done
+exit $st