tags 16302 + patch
severity 16302 minor
stop
On 12/30/2013 03:48 PM, Akim Demaille wrote:
> Hi all,
>
Hi Akim.
> I have this piece of software with several APIs, organized in clear
> layers. Building the whole package is costly, especially because of
> the top-level layers (dozens of binaries), and the whole test suite
> is even costlier (because it requires to build the whole set of binaries,
> and then it runs all the tests, including the costly top-level tests).
>
> So, « of course », when developing this piece of software, I seldom run
> the full test-suite, and run selected bits. In particular I avoid
> « make all », so, of course, I also avoid « make check » since it depends
> on the former. Rather I use check-TESTS and deal with the dependencies
> myself, like a grownup.
>
> Alas, for some reason, while it works well to generate individual test logs,
> but when generating (the partial) test-suite.log, « make all-am » is
> invoked.
>
> Please try to attached tarball. It features a lazy test suite with two
> test scripts.
>
>> AM_TESTS_ENVIRONMENT = export PATH=$(abs_top_builddir):$PATH;
>> RECHECK_LOGS =
>> TESTS = fast slow
>
> « slow » runs a binary built by the package, so
> of course, its log depends on this binary:
>
>> slow.log: foo
>
> but ‘fast’ depends on nothing, so to run it and only it, I have
> this piece of syntactic sugar:
>
>> check-fast:
>> $(MAKE) $(AM_MAKEFLAGS) check-TESTS TESTS='fast'
>
> As expected ‘make check’ is lazy. However, if I touch
> fast and foo.c (the dependency of slow), then observe:
>
>> $ touch ~/src/gnu/test-suite/{foo.c,fast}
>> $ make check-fast
>> /usr/bin/make check-TESTS TESTS='fast'
>> PASS: fast
>> depbase=`echo foo.o | sed 's|[^/]*$|.deps/&|;s|\.o$||'`;\
>> ccache gcc-mp-4.8 -DPACKAGE_NAME=\"test-suite\"
>> -DPACKAGE_TARNAME=\"test-suite\" -DPACKAGE_VERSION=\"1.0\"
>> -DPACKAGE_STRING=\"test-suite\ 1.0\" -DPACKAGE_BUGREPORT=\"\"
>> -DPACKAGE_URL=\"\" -DPACKAGE=\"test-suite\" -DVERSION=\"1.0\" -I.
>> -I/Users/akim/src/gnu/test-suite -DNDEBUG -isystem /opt/local/include -O3
>> -MT foo.o -MD -MP -MF $depbase.Tpo -c -o foo.o
>> /Users/akim/src/gnu/test-suite/foo.c &&\
>> mv -f $depbase.Tpo $depbase.Po
>> ccache gcc-mp-4.8 -O3 -L/opt/local/lib -o foo foo.o
>> ============================================================================
>> Testsuite summary for test-suite 1.0
>> ============================================================================
>> # TOTAL: 1
>> # PASS: 1
>> # SKIP: 0
>> # XFAIL: 0
>> # FAIL: 0
>> # XPASS: 0
>> # ERROR: 0
>> ============================================================================
>
> As you can see, « fast.log » was recreated appropriately, but then,
> to build test-suite.log, something fires « all-am » (running make
> with -d shows this very well). It appears to be due to this:
>
>> $(TEST_SUITE_LOG): $(TEST_LOGS)
>> @$(am__set_TESTS_bases); \
>> am__f_ok () { test -f "$$1" && test -r "$$1"; }; \
>> redo_bases=`for i in $$bases; do \
>> am__f_ok $$i.trs && am__f_ok $$i.log || echo $$i; \
>> done`; \
>> if test -n "$$redo_bases"; then \
>> redo_logs=`for i in $$redo_bases; do echo $$i.log; done`; \
>> redo_results=`for i in $$redo_bases; do echo $$i.trs; done`; \
>> if $(am__make_dryrun); then :; else \
>> rm -f $$redo_logs && rm -f $$redo_results || exit 1; \
>> fi; \
>> fi; \
>> if test -n "$$am__remaking_logs"; then \
>> echo "fatal: making $(TEST_SUITE_LOG): possible infinite" \
>> "recursion detected" >&2; \
>> else \
>> am__remaking_logs=yes $(MAKE) $(AM_MAKEFLAGS) $$redo_logs; \
>> fi; \
>
> in this last bit, « $$redo_logs » is empty, so, blam, a full cost
> make all. What was the point of this piece of code?
>
See the '##'-style comments in lib/am/check.am for detailed explanations
(such comments are stripped from the generated Makefiles, as I'm sure
you know).
> At first sight it seems that it should be guarder by ‘test -n $$redo_log’.
>
Indeed.
> This is *really* costly, I’d be happy to have nice workarounds.
>
Or eve better, to fix the bug :-) See attached patch. Does it work
for you?
> Thanks, and happy new year!
>
Thanks,
Stefano
>From ad9804e1b825ddb304bfaf62b7108f2b6e08dc81 Mon Sep 17 00:00:00 2001
Message-Id: <ad9804e1b825ddb304bfaf62b7108f2b6e08dc81.1388444976.git.stefano.lattar...@gmail.com>
From: Stefano Lattarini <stefano.lattar...@gmail.com>
Date: Mon, 30 Dec 2013 23:21:03 +0100
Subject: [PATCH] parallel-tests: avoid possible implicit "make all" in
test-suite.log rule
This change fixes automake bug#16302.
* lib/am/check.am ($(TEST_SUITE_LOG)): Avoid running "make $redo_logs"
when $redo_logs expands to empty, since in that case we are actually
ending up invoking a full "make all". That shouldn't be required, and
can cause slowdowns for people implementing their extra "laziness
wrappers" around check-TESTS (automake bug#16302).
* NEWS: Update.
Signed-off-by: Stefano Lattarini <stefano.lattar...@gmail.com>
---
NEWS | 4 ++++
lib/am/check.am | 4 +++-
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/NEWS b/NEWS
index dd2812a..a73804a 100644
--- a/NEWS
+++ b/NEWS
@@ -93,6 +93,10 @@ New in 1.15:
implementation of the TAP driver (that is documented in the manual)
is more portable and has feature parity with the perl implementation.
+ - The rule generating 'test-suite.log' no longer risk incurring in an
+ extra useless "make all" recursive invocation in some corner cases
+ (automake bug#16302).
+
* Bug fixes:
- The user can now extend the special .PRECIOUS target, the same way
diff --git a/lib/am/check.am b/lib/am/check.am
index 7012d5a..4d2a498 100644
--- a/lib/am/check.am
+++ b/lib/am/check.am
@@ -274,7 +274,9 @@ $(TEST_SUITE_LOG): $(TEST_LOGS)
if test -n "$$am__remaking_logs"; then \
echo "fatal: making $(TEST_SUITE_LOG): possible infinite" \
"recursion detected" >&2; \
- else \
+## Invoking this unconditionally could cause a useless "make all" to
+## be invoked when '$redo_logs' expands to empty (automake bug#16302).
+ elif test -n "$$redo_logs"; then \
am__remaking_logs=yes $(MAKE) $(AM_MAKEFLAGS) $$redo_logs; \
fi; \
if $(am__make_dryrun); then :; else \
--
1.8.5.rc0.335.g7794a68
