bug#7868: [RFC] parallel-tests: avoid command-line length limit issue (was: Re: bug#7868: splitting up test suites)

[Stefano Lattarini]

On Thursday 20 January 2011, Stefano Lattarini wrote:
> Hello Ralf.
> 
> On Wednesday 19 January 2011, Ralf Wildenhues wrote:
> > The testsuite is too large for MSYS.
> >
> Ouch.
> 
> > We've finally reached the point where we have more than 1000
> > tests, $(TESTS) expands to 15k characters, and where 'make check' will
> > not work at all any more on MSYS, because it cannot spawn sh any more,
> > presumably in 'make check TESTS="..."'.  (MSYS make doesn't export
> > macros to the environment of spawned processes even without .NOEXPORT,
> > presumably otherwise lots of Makefiles would be really unusable here.)
> > This also clears up the spurious failure of sed a few days ago.
> >  
> > Here's a preliminary plan for multiple testsuites per Makefile.am.
> >
> Hmmm... while this feature might be worth having even indipendently
> from the issue at hand (but see below for small nits), I still think
> that in the long run it would be nicer to transparently work around
> such command-line length issues in the test driver, if possible.  Do
> you think your patch "parallel-tests: avoid command-line length limit
> issue" (from commit v1.11-191-g24e3b4e) could be resurrected in some
> way?
>
OK, I've done my homework and come up with the attached patch.

It's not yet very well polished, and certainly needs more testsuite
exposure [1] on various systems before being "ok to apply", but it
seems quite promising to me.

 [1] I'm posting it now anyway because I'm out of time for today.
     Sorry.

Also, I'd appreciate if anyone could test it on MSYS (and Cygwin?),
since I don't have access to those systems.

Thanks,
  Stefano

From f6f4dc5d2e6e3d174f696409fa5c07e207d377a4 Mon Sep 17 00:00:00 2001
From: Ralf Wildenhues <address@hidden>
Date: Tue, 7 Sep 2010 04:38:08 +0200
Subject: [PATCH] parallel-tests: avoid command-line length limit issue.

* automake.in (handle_tests): New argument $makefile, new
substitution %MAKEFILE%.
(generate_makefile): Adjust.
* lib/am/check.am [%?PARALLEL_TESTS%] (check-TESTS): Use a
temporary makefile to sanitize TEST_LOGS value passed to the
recursive $(MAKE) invocation, to avoid exceeding the command
line limit on w32 (MSYS).  Extend comments.
* tests/parallel-tests-linewrap.test: New test.
* tests/parallel-tests-cleanup.test: Likewise.
* tests/parallel-tests-gnumakefile.test: Likewise.
* tests/parallel-tests-long-cmdline.test: Likewise.
* tests/Makefile.am (TESTS): Updated.
* NEWS: Update.

Report by Bob Friesenhahn.
---
 ChangeLog                             |   19 +++++++
 NEWS                                  |    3 +
 automake.in                           |   11 +++-
 lib/Automake/tests/Makefile.in        |   21 ++++++--
 lib/am/check.am                       |   35 +++++++++++--
 tests/Makefile.am                     |    3 +
 tests/Makefile.in                     |   24 +++++++--
 tests/parallel-tests-cleanup.test     |   91 +++++++++++++++++++++++++++++++++
 tests/parallel-tests-gnumakefile.test |   49 ++++++++++++++++++
 tests/parallel-tests-linewrap.test    |   63 +++++++++++++++++++++++
 10 files changed, 301 insertions(+), 18 deletions(-)
 create mode 100755 tests/parallel-tests-cleanup.test
 create mode 100755 tests/parallel-tests-gnumakefile.test
 create mode 100755 tests/parallel-tests-linewrap.test

diff --git a/ChangeLog b/ChangeLog
index 31ff009..4a0e7e1 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,22 @@
+2011-01-20  Stefano Lattarini  <address@hidden>
+           Ralf Wildenhues  <address@hidden>
+
+       parallel-tests: avoid command-line length limit issue.
+       * automake.in (handle_tests): New argument $makefile, new
+       substitution %MAKEFILE%.
+       (generate_makefile): Adjust.
+       * lib/am/check.am [%?PARALLEL_TESTS%] (check-TESTS): Use a
+       temporary makefile to sanitize TEST_LOGS value passed to the
+       recursive $(MAKE) invocation, to avoid exceeding the command
+       line limit on w32 (MSYS).  Extend comments.
+       * tests/parallel-tests-linewrap.test: New test.
+       * tests/parallel-tests-cleanup.test: Likewise.
+       * tests/parallel-tests-gnumakefile.test: Likewise.
+       * tests/parallel-tests-long-cmdline.test: Likewise.
+       * tests/Makefile.am (TESTS): Updated.
+       * NEWS: Update.
+       Report by Bob Friesenhahn.
+
 2011-01-19  Stefano Lattarini  <address@hidden>
            Ralf Wildenhues  <address@hidden>
 
diff --git a/NEWS b/NEWS
index b5cb6e9..c5f28b5 100644
--- a/NEWS
+++ b/NEWS
@@ -22,6 +22,9 @@ Bugs fixed in 1.11.0a:
   - The AM_COND_IF macro also works if the shell expression for the conditional
     is no longer valid for the condition.
 
+  - The `parallel-tests' driver works around a problem with command-line
+    length limits with `make check' on w32 (MSYS).
+
 * Long standing bugs:
 
   - On Darwin 9, `pythondir' and `pyexecdir' pointed below `/Library/Python'
diff --git a/automake.in b/automake.in
index d56fbf7..bd9f453 100755
--- a/automake.in
+++ b/automake.in
@@ -4919,9 +4919,13 @@ sub handle_tests_dejagnu
 }
 
 
+# handle_tests ($MAKEFILE)
+# ------------------------
 # Handle TESTS variable and other checks.
-sub handle_tests
+sub handle_tests ($)
 {
+  my ($makefile) = @_;
+
   if (option 'dejagnu')
     {
       &handle_tests_dejagnu;
@@ -4940,7 +4944,8 @@ sub handle_tests
       push (@check_tests, 'check-TESTS');
       $output_rules .= &file_contents ('check', new Automake::Location,
                                       COLOR => !! option 'color-tests',
-                                      PARALLEL_TESTS => !! option 
'parallel-tests');
+                                      PARALLEL_TESTS => !! option 
'parallel-tests',
+                                      MAKEFILE => basename $makefile);
 
       # Tests that are known programs should have $(EXEEXT) appended.
       # For matching purposes, we need to adjust XFAIL_TESTS as well.
@@ -8220,7 +8225,7 @@ sub generate_makefile ($$)
   handle_tags;
   handle_minor_options;
   # Must come after handle_programs so that %known_programs is up-to-date.
-  handle_tests;
+  handle_tests ($makefile);
 
   # This must come after most other rules.
   handle_dist;
diff --git a/lib/Automake/tests/Makefile.in b/lib/Automake/tests/Makefile.in
index b4940db..d3846aa 100644
--- a/lib/Automake/tests/Makefile.in
+++ b/lib/Automake/tests/Makefile.in
@@ -403,11 +403,22 @@ $(TEST_SUITE_LOG): $(TEST_LOGS)
 check-TESTS:
        @list='$(RECHECK_LOGS)'; test -z "$$list" || rm -f $$list
        @test -z "$(TEST_SUITE_LOG)" || rm -f $(TEST_SUITE_LOG)
-       @list='$(TEST_LOGS)';                                           \
-       list=`for f in $$list; do                                       \
-         test .log = $$f || echo $$f;                                  \
-       done | tr '\012\015' '  '`;                                     \
-       $(MAKE) $(AM_MAKEFLAGS) $(TEST_SUITE_LOG) TEST_LOGS="$$list"
+       @am__tmpdir=am-tmp$$$$;                                         \
+       am__mkfile=$$am__tmpdir/Makefile;                               \
+       am__trap='rm -rf $$am__tmpdir; (exit $$st); exit $$st';         \
+       trap "st=129; $$am__trap" 1; trap "st=130; $$am__trap" 2;       \
+       trap "st=141; $$am__trap" 13; trap "st=143; $$am__trap" 15;     \
+       st=0;                                                           \
+       mkdir $$am__tmpdir                                              \
+         && { echo "TEST_LOGS = \\";                                   \
+               list='$(TEST_LOGS)'; for f in $$list; do                \
+                  test .log = $$f || echo "$$f \\";                    \
+               done;                                                   \
+            } | sed '$$s/ *\\$$//' > $$am__mkfile                      \
+         && sed '/^TEST_LOGS =/d' Makefile >> $$am__mkfile             \
+         && $(MAKE) -f $$am__mkfile $(AM_MAKEFLAGS) $(TEST_SUITE_LOG)  \
+         || st=$$?;                                                    \
+       rm -rf $$am__tmpdir; exit $$st
 
 .log.html:
        @list='$(RST2HTML) $$RST2HTML rst2html rst2html.py';            \
diff --git a/lib/am/check.am b/lib/am/check.am
index 5728081..837f1b4 100644
--- a/lib/am/check.am
+++ b/lib/am/check.am
@@ -236,11 +236,36 @@ check-TESTS:
 ## cannot use `$?' to compute the set of lazily rerun tests, lest
 ## we rely on .PHONY to work portably.
        @test -z "$(TEST_SUITE_LOG)" || rm -f $(TEST_SUITE_LOG)
-       @list='$(TEST_LOGS)';                                           \
-       list=`for f in $$list; do                                       \
-         test .log = $$f || echo $$f;                                  \
-       done | tr '\012\015' '  '`;                                     \
-       $(MAKE) $(AM_MAKEFLAGS) $(TEST_SUITE_LOG) TEST_LOGS="$$list"
+## We'll need a temporary file (see comments below for why), and the safer
+## way to create it portably is to use a temporary directory (as the mkdir
+## utility should be truly atomic everywhere).
+       @am__tmpdir=am-tmp$$$$;                                         \
+       am__mkfile=$$am__tmpdir/Makefile;                               \
+       am__trap='rm -rf $$am__tmpdir; (exit $$st); exit $$st';         \
+       trap "st=129; $$am__trap" 1; trap "st=130; $$am__trap" 2;       \
+       trap "st=141; $$am__trap" 13; trap "st=143; $$am__trap" 15;     \
+       st=0;                                                           \
+       mkdir $$am__tmpdir                                              \
+## Yes, this is hacky, but we cannot simply override the $(TEST_LOGS)
+## definition by appending to Makefile, since at that point it's original
+## value has been already used in a dependency declaration, i.e.
+##   $(TEST_SUITE_LOG): $(TEST_LOGS)
+## (see above in this same *.am fragment).
+         && { echo "TEST_LOGS = \\";                                   \
+               list='$(TEST_LOGS)'; for f in $$list; do                \
+## A bug in GNU make 3.80 can lead to bare `.log' occurrences.
+## Strip them out.
+                  test .log = $$f || echo "$$f \\";                    \
+               done;                                                   \
+            } | sed '$$s/ *\\$$//' > $$am__mkfile                      \
+         && sed '/^TEST_LOGS =/d' %MAKEFILE% >> $$am__mkfile           \
+## It would have been nice to be able to use something like:
+##   $(POST_PROCESSING_COMMAND) Makefile.in | $(MAKE) -f -
+## instead of a temporary file here, but unfortunately that doesn't
+## work with at least Solaris 10 dmake.
+         && $(MAKE) -f $$am__mkfile $(AM_MAKEFLAGS) $(TEST_SUITE_LOG)  \
+         || st=$$?;                                                    \
+       rm -rf $$am__tmpdir; exit $$st
 
 AM_RECURSIVE_TARGETS += check
 
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 713dd92..f263764 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -567,6 +567,9 @@ parallel-tests7.test \
 parallel-tests8.test \
 parallel-tests9.test \
 parallel-tests10.test \
+parallel-tests-cleanup.test \
+parallel-tests-gnumakefile.test \
+parallel-tests-linewrap.test \
 parallel-tests-unreadable-log.test \
 parse.test \
 percent.test \
diff --git a/tests/Makefile.in b/tests/Makefile.in
index 45adb04..7491f05 100644
--- a/tests/Makefile.in
+++ b/tests/Makefile.in
@@ -834,6 +834,9 @@ parallel-tests7.test \
 parallel-tests8.test \
 parallel-tests9.test \
 parallel-tests10.test \
+parallel-tests-cleanup.test \
+parallel-tests-gnumakefile.test \
+parallel-tests-linewrap.test \
 parallel-tests-unreadable-log.test \
 parse.test \
 percent.test \
@@ -1222,11 +1225,22 @@ $(TEST_SUITE_LOG): $(TEST_LOGS)
 check-TESTS:
        @list='$(RECHECK_LOGS)'; test -z "$$list" || rm -f $$list
        @test -z "$(TEST_SUITE_LOG)" || rm -f $(TEST_SUITE_LOG)
-       @list='$(TEST_LOGS)';                                           \
-       list=`for f in $$list; do                                       \
-         test .log = $$f || echo $$f;                                  \
-       done | tr '\012\015' '  '`;                                     \
-       $(MAKE) $(AM_MAKEFLAGS) $(TEST_SUITE_LOG) TEST_LOGS="$$list"
+       @am__tmpdir=am-tmp$$$$;                                         \
+       am__mkfile=$$am__tmpdir/Makefile;                               \
+       am__trap='rm -rf $$am__tmpdir; (exit $$st); exit $$st';         \
+       trap "st=129; $$am__trap" 1; trap "st=130; $$am__trap" 2;       \
+       trap "st=141; $$am__trap" 13; trap "st=143; $$am__trap" 15;     \
+       st=0;                                                           \
+       mkdir $$am__tmpdir                                              \
+         && { echo "TEST_LOGS = \\";                                   \
+               list='$(TEST_LOGS)'; for f in $$list; do                \
+                  test .log = $$f || echo "$$f \\";                    \
+               done;                                                   \
+            } | sed '$$s/ *\\$$//' > $$am__mkfile                      \
+         && sed '/^TEST_LOGS =/d' Makefile >> $$am__mkfile             \
+         && $(MAKE) -f $$am__mkfile $(AM_MAKEFLAGS) $(TEST_SUITE_LOG)  \
+         || st=$$?;                                                    \
+       rm -rf $$am__tmpdir; exit $$st
 
 .log.html:
        @list='$(RST2HTML) $$RST2HTML rst2html rst2html.py';            \
diff --git a/tests/parallel-tests-cleanup.test 
b/tests/parallel-tests-cleanup.test
new file mode 100755
index 0000000..26e9d5c
--- /dev/null
+++ b/tests/parallel-tests-cleanup.test
@@ -0,0 +1,91 @@
+#! /bin/sh
+# Copyright (C) 2011 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check that the temporary files/directories used by the Makefile
+# post-processing code in the parallel-tests testsuite driver are
+# duly cleaned up on success, on failure, and when well-known signals
+# are received.
+
+parallel_tests=yes
+. ./defs || Exit 1
+
+set -e
+
+cat >> configure.in << 'END'
+AC_OUTPUT
+END
+
+cat > Makefile.am << 'END'
+TEST_EXTENSIONS = .sh
+TESTS = foo.sh
+SH_LOG_COMPILER = sh
+EXTRA_DIST = foo.sh
+END
+
+check_filelist ()
+{
+  expect_filelist=$1
+  got_filelist=`ls`
+  if test x"$expect_filelist" != x"$got_filelist"; then
+    # Display the differences in a more user-friendly way.
+    # Useful for debugging and failure analysis.
+    set +x # Don't be overly verbose.
+    for f in $expect_filelist; do echo $f; done > exp
+    for f in $got_filelist; do echo $f; done > got
+    set -x
+    diff exp got || :
+    return 1
+  else
+    return 0
+  fi
+}
+
+$ACLOCAL
+$AUTOMAKE -a
+$AUTOCONF
+
+./configure
+
+aborted=`(ls && echo foo.sh ) | sort`
+completed=`(echo "$aborted" && echo test-suite.log && echo foo.log) | sort`
+
+echo 'exit 0' > foo.sh
+$MAKE check
+check_filelist "$completed"
+$MAKE clean
+
+echo 'exit 1' > foo.sh
+$MAKE check && Exit 1
+check_filelist "$completed"
+$MAKE clean
+
+echo 'sleep 10' > foo.sh
+
+# Yes, all the "sleeps" below sucks, but here is better to play "dumb
+# and safer" than having stray I/O or leaving zombies around.
+for signum in  1 2 13 15; do 
+  failed=false
+  $MAKE check &
+  kill -$signum $!
+  check_filelist "$aborted" || failed=:
+  sleep 12 # Wait for all the children to complete.
+  $failed && Exit 1
+  $MAKE clean
+done
+
+$MAKE distcheck
+
+:
diff --git a/tests/parallel-tests-gnumakefile.test 
b/tests/parallel-tests-gnumakefile.test
new file mode 100755
index 0000000..60d6fd7
--- /dev/null
+++ b/tests/parallel-tests-gnumakefile.test
@@ -0,0 +1,49 @@
+#! /bin/sh
+# Copyright (C) 2011 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check that recursive make calls triggered by parallel-tests testsuite
+# driver work also if the makefile is named GNUmakefile.
+
+required=GNUmake
+parallel_tests=yes
+. ./defs || Exit 1
+set -e
+
+cat configure.in - > t << 'END'
+AC_OUTPUT
+END
+sed '/^AC_CONFIG_FILES/s|Makefile|GNUmakefile|' t > configure.in
+rm -f t
+
+cat > GNUmakefile.am <<'END'
+TESTS = foo.test
+END
+
+cat > foo.test <<'END'
+#! /bin/sh
+exit 0
+END
+chmod +x foo.test
+
+$ACLOCAL
+$AUTOCONF
+$AUTOMAKE -a
+
+./configure
+
+$MAKE check
+
+:
diff --git a/tests/parallel-tests-linewrap.test 
b/tests/parallel-tests-linewrap.test
new file mode 100755
index 0000000..b3969f0
--- /dev/null
+++ b/tests/parallel-tests-linewrap.test
@@ -0,0 +1,63 @@
+#! /bin/sh
+# Copyright (C) 2011 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check that the definition of TEST_LOGS is not broken on multiple lines,
+# even if the list of tests, the list of test extensions, the test names
+# and the test extensions are all very long.
+# We need to be sure of this in order for the Makefile post-processing
+# code in the parallel-tests testsuite driver to work.
+
+parallel_tests=yes
+. ./defs || Exit 1
+
+set -e
+
+# Avoid forks if possible
+i=1
+if (i=$(($i+1)) && test $i -eq 2); then
+  incr_i() { i=$(($i+1)); }
+else
+  incr_i() { i=`expr $i + 1`; }
+fi
+
+cat >> configure.in << 'END'
+AC_OUTPUT
+END
+
+cat > Makefile.am << 'END'
+TEST_EXTENSIONS =
+TESTS =
+END
+
+lc_ext=a_very_very_very_long_test_extension_indeed_it_is_slightly_longer_than_81_characters
+uc_ext=A_VERY_VERY_VERY_LONG_TEST_EXTENSION_INDEED_IT_IS_SLIGHTLY_LONGER_THAN_81_CHARACTERS
+
+while test $i -lt 200; do
+  echo TEST_EXTENSIONS += .${lc_ext}_${i}
+  echo ${uc_ext}_${i}_LOG_COMPILER = sh
+  echo TESTS += 
wow-this-definitely-is-a-very-long-name-for-a-dummy-testcase.${lc_ext}_${i}
+  incr_i
+done >> Makefile.am
+
+$ACLOCAL
+$AUTOMAKE -a
+
+# For debugging.
+$FGREP 'TEST_LOGS' Makefile.in
+
+grep '^TEST_LOGS *=.*\\$' Makefile.in && Exit 1
+
+:
-- 
1.7.2.3