bug#9245: FreeBSD make in concurrent mode report spurious success in automake-generated tests harness

[Stefano Lattarini]

On Friday 12 August 2011, Stefano Lattarini wrote:
> OK, the attached hacky patch seems to fix the bug.
>
LOL no, there was an embarassingly stupid error in my patch:

 diff --git a/lib/am/check.am b/lib/am/check.am
 index 0b54312..75f8bba 100644
 --- a/lib/am/check.am
 +++ b/lib/am/check.am
 @@ -224,7 +224,9 @@ $(TEST_SUITE_LOG): $(TEST_LOGS)
           col="$$red";                                                  \
         fi;                                                             \
         echo "$$msg" | $(am__text_box) "col=$$col" "std=$$std";         \
 -       $$exit
 +## The apparently redundant and repeated "exit 1" works around a bug in
 +## FreeBSD make (present only when running in concurrent mode).
 +       $$exit || (exit 1); exit 1
 
This obviously causes the `check' target to exit unconditionally with a
non-zero exit status.  Hardly a fix for anything, sigh.

The "proper fix" is even easier BTW: just use "$$exit || exit 1" instead.

The updated patch should now work (and I've tested it properly this time).
This new patch hasn't been derived by "cargo-cult guessing", but from the
explanation of the relevant FreeBSD make bug provided by Jilles Tjoelker:
 <http://www.freebsd.org/cgi/query-pr.cgi?pr=159730>

Now that's something to be documented in the Autoconf manual I'd say ...

> I have no idea why
> exactly it is so yet, so we might want to wait to apply the patch until
> we have fully understood the reasons of the original failure,
> 
Glad I've waited this time!  And I will allow another couple of days for
comments and suggestions before pushing.

Regards,
  Stefano

From 3dfdeba20496399036825e321830e6079949e2ce Mon Sep 17 00:00:00 2001
Message-Id: <address@hidden>
From: Stefano Lattarini <address@hidden>
Date: Fri, 12 Aug 2011 18:29:28 +0200
Subject: [PATCH] parallel-tests: no more spurious successes for FreeBSD make

Work around a bug of FreeBSD make bug that was causing the
automake-generated "check" target to complete with success
even if some tests failed; this happened only when FreeBSD
make was run in concurrent mode (as in, e.g., "make -j2
check").  The bug is not present in NetBSD make.

This change fixes automake bug#9245:
 <http://debbugs.gnu.org/cgi/bugreport.cgi?bug=9245>

See also FreeBSD PR bin/159730:
 <http://www.freebsd.org/cgi/query-pr.cgi?pr=159730>

* lib/am/check.am [%?PARALLEL_TESTS%] $(TEST_SUITE_LOG): Use a
more "safe" (and apparently redundant) idiom to exit with error,
so that the non-zero exit status is picked up also by FreeBSD
make when it's running in concurrent mode.
* NEWS: Update.
* tests/check-concurrency-bug9245.test: New test.
* tests/Makefile.am (TESTS): Update.
---
 ChangeLog                            |   20 +++++++++++
 NEWS                                 |    4 ++-
 lib/am/check.am                      |    7 +++-
 tests/Makefile.am                    |    1 +
 tests/Makefile.in                    |    4 ++-
 tests/check-concurrency-bug9245.test |   62 ++++++++++++++++++++++++++++++++++
 6 files changed, 95 insertions(+), 3 deletions(-)
 create mode 100755 tests/check-concurrency-bug9245.test

diff --git a/ChangeLog b/ChangeLog
index af2556f..9be4bbf 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,23 @@
+2011-08-16  Stefano Lattarini  <address@hidden>
+
+       parallel-tests: no more spurious successes for FreeBSD make
+       Work around a bug of FreeBSD make bug that was causing the
+       automake-generated "check" target to complete with success
+       even if some tests failed; this happened only when FreeBSD
+       make was run in concurrent mode (as in, e.g., "make -j2
+       check").  The bug is not present in NetBSD make.
+       This change fixes automake bug#9245:
+        <http://debbugs.gnu.org/cgi/bugreport.cgi?bug=9245>
+       See also FreeBSD PR bin/159730:
+        <http://www.freebsd.org/cgi/query-pr.cgi?pr=159730>
+       * lib/am/check.am [%?PARALLEL_TESTS%] $(TEST_SUITE_LOG): Use a
+       more "safe" (and apparently redundant) idiom to exit with error,
+       so that the non-zero exit status is picked up also by FreeBSD
+       make when it's running in concurrent mode.
+       * NEWS: Update.
+       * tests/check-concurrency-bug9245.test: New test.
+       * tests/Makefile.am (TESTS): Update.
+
 2011-08-08  Stefano Lattarini  <address@hidden>
 
        test defs: more environment cleanup
diff --git a/NEWS b/NEWS
index 29c552b..5086f75 100644
--- a/NEWS
+++ b/NEWS
@@ -23,7 +23,9 @@ Bugs fixed in 1.11.0a:
 * Bugs introduced by 1.11:
 
   - The `parallel-tests' test driver works around a GNU make 3.80 bug with
-    trailing white space in the test list (`TESTS = foo $(EMPTY)').
+    trailing white space in the test list (`TESTS = foo $(EMPTY)'), and
+    does not report spurious successes when used with concurrent FreeBSD
+    make (e.g., "make check -j3").
 
   - The `silent-rules' option now also silences all compile rules if dependency
     tracking is disabled.  Also, when `silent-rules' is not used, the output 
from
diff --git a/lib/am/check.am b/lib/am/check.am
index 0b54312..3d0188d 100644
--- a/lib/am/check.am
+++ b/lib/am/check.am
@@ -224,7 +224,12 @@ $(TEST_SUITE_LOG): $(TEST_LOGS)
          col="$$red";                                                  \
        fi;                                                             \
        echo "$$msg" | $(am__text_box) "col=$$col" "std=$$std";         \
-       $$exit
+## The use of "exit 1" below is required to work around a FreeBSD make bug
+## (present only when running in concurrent mode).  See automake bug#9245:
+##  <http://debbugs.gnu.org/cgi/bugreport.cgi?bug=9245>
+## and FreeBSD PR bin/159730:
+##  <http://www.freebsd.org/cgi/query-pr.cgi?pr=159730>.
+       $$exit || exit 1
 
 RECHECK_LOGS = $(TEST_LOGS)
 
diff --git a/tests/Makefile.am b/tests/Makefile.am
index b545dda..c2a55fd 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -160,6 +160,7 @@ check11.test \
 check-exported-srcdir.test \
 check-tests-in-builddir.test \
 check-tests_environment.test \
+check-concurrency-bug9245.test \
 tests-environment-backcompat.test \
 checkall.test \
 clean.test \
diff --git a/tests/Makefile.in b/tests/Makefile.in
index fe44a40..5a3259e 100644
--- a/tests/Makefile.in
+++ b/tests/Makefile.in
@@ -291,6 +291,7 @@ yacc-dist-nobuild-subdir.test \
 txinfo5.test
 
 parallel_tests = \
+check-concurrency-bug9245-p.test \
 check-exported-srcdir-p.test \
 check-tests-in-builddir-p.test \
 check-tests_environment-p.test \
@@ -437,6 +438,7 @@ check11.test \
 check-exported-srcdir.test \
 check-tests-in-builddir.test \
 check-tests_environment.test \
+check-concurrency-bug9245.test \
 tests-environment-backcompat.test \
 checkall.test \
 clean.test \
@@ -1274,7 +1276,7 @@ $(TEST_SUITE_LOG): $(TEST_LOGS)
          col="$$red";                                                  \
        fi;                                                             \
        echo "$$msg" | $(am__text_box) "col=$$col" "std=$$std";         \
-       $$exit
+       $$exit || exit 1
 
 # Run all the tests.
 check-TESTS:
diff --git a/tests/check-concurrency-bug9245.test 
b/tests/check-concurrency-bug9245.test
new file mode 100755
index 0000000..e5dd153
--- /dev/null
+++ b/tests/check-concurrency-bug9245.test
@@ -0,0 +1,62 @@
+#! /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/>.
+
+# Look for a bug where FreeBSD make in concurrent mode reported success
+# even when the Automake-generated parallel testsuite harness failed.
+# See automake bug#9245.
+
+. ./defs || Exit 1
+
+cat >> configure.in << 'END'
+AC_OUTPUT
+END
+
+cat > Makefile.am << 'END'
+TESTS = foo.test bar.test
+END
+
+cat > foo.test <<'END'
+#!/bin/sh
+exit 1
+END
+chmod a+x foo.test
+
+cp foo.test bar.test
+
+$ACLOCAL
+$AUTOCONF
+$AUTOMAKE -a
+
+./configure
+
+# Some make implementations don't grok the `-j' option.
+$MAKE -j1 || Exit 77
+
+for j in '' -j1 -j2; do
+  $MAKE $j check && Exit 1
+  TESTS=foo.test $MAKE $j -e check && Exit 1
+  if test x"$parallel_tests" = x"yes"; then
+    $MAKE $j recheck && Exit 1
+    TEST_LOGS=foo.log $MAKE $j -e check && Exit 1
+    rm -f test-suite.log
+    $MAKE $j test-suite.log && Exit 1
+    test -f test-suite.log || Exit 1
+  else
+    : # For shells with buggy 'set -e'.
+  fi
+done
+
+:
-- 
1.7.2.3