On 12/29/2011 12:26 AM, Stefano Lattarini wrote:
> On 12/28/2011 08:27 PM, Bob Friesenhahn wrote:
>> Sorry for top-posting. I don't want to lose any text of the original mail.
>>
>> I totally forgot that I had applied Ralf's patch to my 1.11.1 install. Now
>> that I have updated GraphicsMagick to using Automake 1.11.2, the patch is no
>> longer present and so 'make check' on MinGW/MSYS leads to this:
>>
>> /usr/bin/csmake check-TESTS check-local
>> csmake[2]: Entering directory `/home/bfriesen/mingw/GM-16-static'
>> csmake[3]: Entering directory `/home/bfriesen/mingw/GM-16-static'
>> csmake[3]: execvp: /bin/sh: Invalid argument
>> csmake[3]: *** [tests/constitute_char_bgr.log] Error 127
>> csmake[3]: Leaving directory `/home/bfriesen/mingw/GM-16-static'
>> csmake[2]: *** [check-TESTS] Error 2
>> csmake[2]: Leaving directory `/home/bfriesen/mingw/GM-16-static'
>> csmake[1]: *** [check-am] Error 2
>> csmake[1]: Leaving directory `/home/bfriesen/mingw/GM-16-static'
>> csmake: *** [check] Error 2
>>
>> Today I lost more hair because I had totally forgotten about this issue and
>> the problem was first noticed after making some changes to configure.ac.
>>
>> Hopefully this problem can be resolved before Automake 1.12 is released.
>>
> Let's start by exposing the problem once and for all.
>
> [SNIP]
>
OK, I hope I've finally managed to partially fix this incredibly annoying bug
-- I say "partially" because the fix is sadly *for GNU make*. Even more sadly,
the solution is pretty hacky and somewhat brittle. Still, it should cause no
regression with the non-GNU makes (as they will continue to use the old
implementation), and I have verified that it works with at least all the GNU
make versions >= 3.78.
I'm posting the patches here for early feedback (note that the second one still
needs a commit message and possibly some polishing), with the hope that someone
can suggest a less hacky solution.
Regards,
Stefano
>From e9d7d23529b21511e4d3d7c71bd239a45288e4bc Mon Sep 17 00:00:00 2001
Message-Id: <e9d7d23529b21511e4d3d7c71bd239a45288e4bc.1325246573.git.stefano.lattar...@gmail.com>
From: Stefano Lattarini <stefano.lattar...@gmail.com>
Date: Wed, 28 Dec 2011 22:37:44 +0100
Subject: [PATCH 1/2] coverage: expose automake bug#7868
Expose the command-line length limit issue that can affect the
Automake-generated parallel-tests harness, especially on systems
where this limit is smaller (e.g., MinGW/MSYS).
Suggestion by Bob Friesenhahn.
* tests/parallel-tests-many.test: New test. We have verified that
it actually exposes the bug#7868, as it passes when we opportunely
reduce the number of test cases in $(TESTS). Checked on NetBSD 5.1
64bit, Debian unstable 32bit, Solaris 10 64bit and Cygwin 1.5 32bit.
* tests/Makefile.am (TESTS, XFAIL_TESTS): Add it.
---
tests/Makefile.am | 2 +
tests/parallel-tests-many.test | 176 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 178 insertions(+), 0 deletions(-)
create mode 100755 tests/parallel-tests-many.test
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 6ce7c53..e14d57e 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -26,6 +26,7 @@ gcj6.test \
java-nobase.test \
pr8365-remake-timing.test \
lex-subobj-nodep.test \
+parallel-tests-many.test \
remake-am-pr10111.test \
remake-m4-pr10111.test \
vala-vpath.test \
@@ -648,6 +649,7 @@ parallel-tests-log-override-1.test \
parallel-tests-log-override-2.test \
parallel-tests-log-override-recheck.test \
parallel-tests-log-compiler-example.test \
+parallel-tests-many.test \
test-extensions.test \
test-extensions-cond.test \
parse.test \
diff --git a/tests/parallel-tests-many.test b/tests/parallel-tests-many.test
new file mode 100755
index 0000000..ef01db1
--- /dev/null
+++ b/tests/parallel-tests-many.test
@@ -0,0 +1,176 @@
+#! /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 parallel-tests harness does not hit errors due to
+# an exceeded command line length when there are many tests.
+# For automake bug#7868. This test is currently expected to fail.
+
+parallel_tests=yes
+. ./defs || Exit 1
+
+set -e
+
+cat >> configure.in << 'END'
+AC_OUTPUT
+END
+
+cat > Makefile.am <<'END'
+# Sanity check that the $(TESTS) is going to exceed the system
+# command line length.
+# Extra quoting and indirections below are required to ensure the
+# various make implementations (e.g, GNU make or Sun Distributed Make)
+# will truly spawn a shell to execute this command, instead of relying
+# on optimizations that might mask the "Argument list too long" error
+# we expect.
+this-will-fail:
+ @":" && ":" $(TEST_LOGS)
+TEST_LOG_COMPILER = true
+include list-of-tests.am
+# So that we won't have to create a ton of dummy test cases.
+$(TESTS):
+END
+
+# The real instance will be dynamically created later.
+echo TESTS = foo.test > list-of-tests.am
+
+$ACLOCAL && $AUTOCONF && $AUTOMAKE -a \
+ || framework_failure_ "unexpected autotools failure"
+./configure \
+ || framework_failure_ "unexpected configure failure"
+
+# We want to hit the system command-line length limit without hitting
+# the filename length limit or the PATHMAX limit; so we use longish
+# (but not too long) names for the testcase, and place them in a nested
+# (but not too deeply) directory.
+# We also prefer to use the minimal(ish) number of test cases that can
+# make us hit the command-line length limit, since the more the test
+# cases are, the more time "automake" and "make check" will take to run
+# (especially on Cygwin and MinGW/MSYS).
+
+tname="wow-this-is-a-very-long-name-for-a-simple-dummy-test-case"
+dname="and-this-too-is-a-very-long-name-for-a-dummy-directory"
+
+deepdir=.
+depth=0
+for i in 1 2 3 4 5 6 7 8 9 10 12 13 14 15 16 17 18 19 29 21 22 23 24; do
+ new_deepdir=$deepdir/$dname.d$i
+ mkdir $new_deepdir || break
+ tmpfile=$new_deepdir/$tname-some-more-chars-for-good-measure
+ if touch $tmpfile; then
+ rm -f $tmpfile
+ else
+ rmdir $new_deepdir
+ fi
+ deepdir=$new_deepdir
+ unset tmpfile new_deepdir
+ depth=$i
+done
+
+cat <<END
+*********************************************************************
+Our tests will be in the following directory (depth = $depth)
+*********************************************************************
+$deepdir
+*********************************************************************
+END
+
+setup_data ()
+{
+ awk "
+ BEGIN {
+ print \"TESTS = \\\\\"
+ for (i = 1; i < $count; i = i + 1)
+ print \" $deepdir/$tname-\" i \".test \\\\\"
+ print \" $deepdir/$tname-\" i \".test\"
+ }
+ " > list-of-tests.am || Exit 99
+ $AUTOMAKE Makefile \
+ || framework_failure_ "unexpected automake failure"
+ ./config.status Makefile \
+ || framework_failure_ "unexpected config.status failure"
+}
+
+for count in 1 2 4 8 12 16 20 24 28 32 48 64 96 128 E_HUGE; do
+ test $count = E_HUGE && break
+ count=`expr $count '*' 100`
+ setup_data
+ if $MAKE this-will-fail; then
+ continue
+ else
+ # We have managed to find a number of test cases large enough to
+ # hit the system command-line limits; we can stop. But first, for
+ # good measure, increase the number of tests of some 20%, to be
+ # "even more sure" of really tickling command line length limits.
+ count=`expr '(' $count '*' 12 ')' / 10` || Exit 99
+ setup_data
+ break
+ fi
+done
+
+if test $count = E_HUGE; then
+ framework_failure_ "system has a too-high limit on command line length"
+else
+ cat <<END
+*********************************************************************
+ Number of tests we will use: $count
+*********************************************************************
+END
+fi
+
+env TESTS=$deepdir/$tname-1.test $MAKE -e check \
+ && test -f $deepdir/$tname-1.log \
+ || framework_failure_ "\"make check\" with one single tests"
+
+rm -f $deepdir/* || Exit 99
+
+$MAKE check > stdout || { cat stdout; Exit 1; }
+cat stdout
+
+grep "All $count tests" stdout
+
+grep "^PASS: .*$tname-[0-9][0-9]*\.test" stdout > grp
+ls -1 $deepdir | grep '\.log$' > lst
+
+sed 20q lst # For debugging.
+sed 20q grp # Likewise.
+
+test `cat <grp | wc -l` -eq $count
+test `cat <lst | wc -l` -eq $count
+
+# We need to simulate a failure of two tests.
+st=0
+env TESTS="$deepdir/$tname-1.test $deepdir/$tname-2.test" \
+ TEST_LOG_COMPILER=false $MAKE -e check > stdout && st=1
+cat stdout
+test `grep -c '^FAIL:' stdout` -eq 2 || st=1
+test $st -eq 0 || fatal_ "couldn't simulate failure of two tests"
+unset st
+
+$MAKE recheck > stdout || { cat stdout; Exit 1; }
+cat stdout
+grep "^PASS: .*$tname-1\.test" stdout
+grep "^PASS: .*$tname-2\.test" stdout
+test `LC_ALL=C grep -c "^[A-Z][A-Z]*:" stdout` -eq 2
+grep "All 2 tests" stdout
+
+# "make clean" might ignore some failures, so we prefer to also grep its
+# output to ensure that no "Argument list too long" error was encountered.
+$MAKE clean >output 2>&1 || { cat output; Exit 1; }
+cat output
+grep -i 'list.* too long' output && Exit 1
+ls -1 $deepdir | grep '\.log$' && Exit 1
+
+:
--
1.7.7.3
>From ca6000f9cf6c84c516230dc86eba8b9d920dfb20 Mon Sep 17 00:00:00 2001
Message-Id: <ca6000f9cf6c84c516230dc86eba8b9d920dfb20.1325246574.git.stefano.lattar...@gmail.com>
In-Reply-To: <e9d7d23529b21511e4d3d7c71bd239a45288e4bc.1325246573.git.stefano.lattar...@gmail.com>
References: <e9d7d23529b21511e4d3d7c71bd239a45288e4bc.1325246573.git.stefano.lattar...@gmail.com>
From: Stefano Lattarini <stefano.lattar...@gmail.com>
Date: Thu, 29 Dec 2011 10:46:48 +0100
Subject: [PATCH 2/2] fix the bug for GNU make
---
NEWS | 6 +++
automake.in | 22 +++++-----
lib/am/check.am | 86 ++++++++++++++++++++++++++++++++++------
m4/init.m4 | 17 ++++++++
tests/Makefile.am | 1 -
tests/parallel-tests-many.test | 4 +-
6 files changed, 111 insertions(+), 25 deletions(-)
diff --git a/NEWS b/NEWS
index f6eb29f..34948f3 100644
--- a/NEWS
+++ b/NEWS
@@ -73,6 +73,12 @@ Bugs fixed in 1.11.0a:
* Bugs introduced by 1.11:
+ - When GNU make is used, the parallel testsuite harness can now work
+ around problems due to exceeded command-line length limits,
+ problems that that could be encountered when issuing "make check"
+ and "make clean". This is especially important for system with
+ low command-line length limits, like MinGW/MSYS.
+
- The `parallel-tests' test driver works around a GNU make 3.80 bug with
trailing white space in the test list (`TESTS = foo $(EMPTY)'), and
does not report spurious successes when used with concurrent FreeBSD
diff --git a/automake.in b/automake.in
index 309eade..a25c5c9 100644
--- a/automake.in
+++ b/automake.in
@@ -4933,9 +4933,13 @@ sub is_valid_test_extension ($)
return 0;
}
+# handle_tests ($MAKEFILE)
+# ------------------------
# Handle TESTS variable and other checks.
-sub handle_tests
+sub handle_tests ($)
{
+ my ($makefile) = @_;
+
if (option 'dejagnu')
{
&handle_tests_dejagnu;
@@ -4954,7 +4958,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.
@@ -5046,7 +5051,7 @@ sub handle_tests
{
if ($test_suffix eq $last_suffix)
{
- $cur = 'TEST_LOGS';
+ $cur = 'am__test_logs';
}
else
{
@@ -5075,13 +5080,8 @@ sub handle_tests
am__EXEEXT => $am_exeext);
}
}
-
- define_variable ('TEST_LOGS_TMP', '$(TEST_LOGS:.log=.log-t)', INTERNAL);
-
- $clean_files{'$(TEST_LOGS_TMP)'} = MOSTLY_CLEAN;
- $clean_files{'$(TEST_LOGS)'} = MOSTLY_CLEAN;
- $clean_files{'$(TEST_SUITE_LOG)'} = MOSTLY_CLEAN;
- $clean_files{'$(TEST_SUITE_HTML)'} = MOSTLY_CLEAN;
+ # This needs to be further postprocessed in lib:am/check.am.
+ define_variable ('am__test_logs', "\$($cur)", INTERNAL);
}
}
}
@@ -8269,7 +8269,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/am/check.am b/lib/am/check.am
index 3d0188d..82d9fe2 100644
--- a/lib/am/check.am
+++ b/lib/am/check.am
@@ -148,9 +148,46 @@ echo "$$res: $$f (exit: $$estatus)" | \
cat $@-t >>$@; \
rm -f $@-t
+if am__MAKE_IS_GNU
+## FIXME: This is heineous. There must be a better way to obtain what we
+## want from GNU make! But still, this at least works for the moment.
+am__start = <--am--start--here-->
+am__end= <--am--end--here-->
+am__get_list_from_var = \
+ { echo 'am--nil:' \
+ && echo '.PHONY: am--nil' \
+## We use $(warning ...), not $(info ...), since the latter has only been
+## introduced in GNU make 3.81.
+ && echo '$$(warning $(am__start) $$('$$am__varname') $(am__end))'; } \
+ | $(MAKE) $(AM_MAKEFLAGS) -f %MAKEFILE% -f - am--nil 2>&1 \
+ | tr ' ' '\012\012' | grep . \
+ | sed -n '/^$(am__start)$$/,/^$(am__end)$$/p' | sed -e '1d' -e '$$d'
+am__delete_files_in_var = \
+ $(am__get_list_from_var) | $(am__base_list) | ( \
+ st=0; \
+ while read files; do \
+ test -z "$$files" || rm -f $$files || st=1; \
+ done; \
+ exit $$st)
+am__get_test_logs_list = \
+ am__varname=TEST_LOGS && list=`$(am__get_list_from_var)` || exit 1
+else ! am__MAKE_IS_GNU
+am__get_test_logs_list = list=' $(TEST_LOGS) '
+endif
+
+# Trailing whitespace in "TESTS = foo.test $(empty)" causes at least
+# GNU make 3.80 to erroneously expand $(TESTS_LOGS) to "foo.log .log".
+# Work around this bug.
+if am__MAKE_IS_GNU
+TEST_LOGS = @am__dollar@(filter-out .log, $(am__test_logs))
+else
+TEST_LOGS = $(am__test_logs)
+endif
+TEST_LOGS_TMP = $(TEST_LOGS:.log=.log-t)
+
$(TEST_SUITE_LOG): $(TEST_LOGS)
@$(am__sh_e_setup); \
- list='$(TEST_LOGS)'; \
+ $(am__get_test_logs_list); \
results=`for f in $$list; do \
test -r $$f && read line < $$f && echo "$$line" \
|| echo FAIL; \
@@ -235,8 +272,11 @@ RECHECK_LOGS = $(TEST_LOGS)
# Run all the tests.
check-TESTS:
-## Expand $(RECHECK_LOGS) only once, to avoid exceeding line length limits.
+if am__MAKE_IS_GNU
+ @am__varname=RECHECK_LOGS && $(am__delete_files_in_var)
+else
@list='$(RECHECK_LOGS)'; test -z "$$list" || rm -f $$list
+endif
## We always have to remove TEST_SUITE_LOG, to ensure its rule is run
## in any case even in lazy mode: otherwise, if no test needs rerunning,
## or a prior run plus reruns all happen within the same timestamp
@@ -245,16 +285,15 @@ check-TESTS:
## OTOH, this means that, in the rule for `$(TEST_SUITE_LOG)', we
## cannot use `$?' to compute the set of lazily rerun tests, lest
## we rely on .PHONY to work portably.
-##
-## Trailing whitespace in `TESTS = foo.test $(empty)' causes GNU make
-## 3.80 to erroneously expand $(TESTS_LOGS) to `foo.log .log'.
-## Work around this bug.
@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"
+if am__MAKE_IS_GNU
+## No need to pass on '$(TEST_LOGS)' explicitly. GNU make will take
+## care of that automatically, even if the user has overridden $(TESTS)
+## or $(TEST_LOGS).
+ $(MAKE) $(AM_MAKEFLAGS) $(TEST_SUITE_LOG)
+else
+ $(MAKE) $(AM_MAKEFLAGS) $(TEST_SUITE_LOG) TEST_LOGS='$(TEST_LOGS)'
+endif
AM_RECURSIVE_TARGETS += check
@@ -299,7 +338,7 @@ AM_RECURSIVE_TARGETS += check-html
## Rerun all FAILed or XPASSed tests.
recheck recheck-html:
@target=`echo $@ | sed 's,^re,,'`; \
- list='$(TEST_LOGS)'; \
+ $(am__get_test_logs_list); \
list=`for f in $$list; do \
test -f $$f || continue; \
if test -r $$f && read line < $$f; then \
@@ -309,6 +348,8 @@ recheck recheck-html:
## This apparently useless munging helps to avoid a nasty bug (a
## segmentation fault!) on Solaris XPG4 make.
list=`echo "$$list" | sed 's/ *$$//'`; \
+## FIXME: this might fail on systems with low command-line length limits
+## FIXME: in case there are many failed tests. Is it worth worrying about?
$(MAKE) $(AM_MAKEFLAGS) $$target AM_MAKEFLAGS='$(AM_MAKEFLAGS) TEST_LOGS="'"$$list"'"'
.PHONY: recheck recheck-html
@@ -316,6 +357,27 @@ recheck recheck-html:
AM_RECURSIVE_TARGETS += recheck recheck-html
+## ------------- ##
+## Cleaning up. ##
+## ------------- ##
+
+## FIXME: the support for variables with contents exceeding the
+## FIXME: command line length limit should be propagated to
+## FIXME: `lib/am/clean.am'.
+mostlyclean-am: am--parallel-tests-mostlyclean
+am--parallel-tests-mostlyclean:
+ @echo ' rm -f $$(TEST_LOGS)'
+if am__MAKE_IS_GNU
+ @am__varname=TEST_LOGS_TMP && $(am__delete_files_in_var)
+ @am__varname=TEST_LOGS && $(am__delete_files_in_var)
+else
+ @list='$(TEST_LOGS_TMP)'; test -z "$$list" || rm -f $$list
+ @list='$(TEST_LOGS)'; test -z "$$list" || rm -f $$list
+endif
+ @echo " rm -f $(TEST_SUITE_LOG) $(TEST_SUITE_HTML)"
+ @rm -f $(TEST_SUITE_LOG) $(TEST_SUITE_HTML)
+.PHONY: am--parallel-tests-mostlyclean
+
else !%?PARALLEL_TESTS%
check-TESTS: $(TESTS)
diff --git a/m4/init.m4 b/m4/init.m4
index 365c9ac..84ada72 100644
--- a/m4/init.m4
+++ b/m4/init.m4
@@ -83,6 +83,23 @@ AC_REQUIRE([AM_PROG_MKDIR_P])dnl
AC_REQUIRE([AC_PROG_AWK])dnl
AC_REQUIRE([AC_PROG_MAKE_SET])dnl
AC_REQUIRE([AM_SET_LEADING_DOT])dnl
+dnl
+dnl FIXME: is it worth to extract this in a new macro of its own?
+am_make=${MAKE-make}
+AC_MSG_CHECKING([whether $am_make is GNU make])
+if ($am_make -n -f /dev/null --version) >/dev/null 2>&1; then
+ am_using_gmake=yes
+else
+ am_using_gmake=no
+fi
+AC_MSG_RESULT([$am_using_gmake])
+AM_CONDITIONAL([am__MAKE_IS_GNU], [test $am_using_gmake = yes])dnl
+dnl
+dnl Hack useful to write Makefile fragments that might otherwise
+dnl trigger spurious portability warning w.r.t. non-POSIX variable
+dnl names.
+AC_SUBST([am__dollar], [$])_AM_SUBST_NOTMAKE([am__dollar])dnl
+dnl
_AM_IF_OPTION([tar-ustar], [_AM_PROG_TAR([ustar])],
[_AM_IF_OPTION([tar-pax], [_AM_PROG_TAR([pax])],
[_AM_PROG_TAR([v7])])])
diff --git a/tests/Makefile.am b/tests/Makefile.am
index e14d57e..27305d0 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -26,7 +26,6 @@ gcj6.test \
java-nobase.test \
pr8365-remake-timing.test \
lex-subobj-nodep.test \
-parallel-tests-many.test \
remake-am-pr10111.test \
remake-m4-pr10111.test \
vala-vpath.test \
diff --git a/tests/parallel-tests-many.test b/tests/parallel-tests-many.test
index ef01db1..c470745 100755
--- a/tests/parallel-tests-many.test
+++ b/tests/parallel-tests-many.test
@@ -16,9 +16,11 @@
# Check that the parallel-tests harness does not hit errors due to
# an exceeded command line length when there are many tests.
-# For automake bug#7868. This test is currently expected to fail.
+# For automake bug#7868. This test is currently expected to pass
+# only with GNU make.
parallel_tests=yes
+required=GNUmake
. ./defs || Exit 1
set -e
--
1.7.7.3
