Re: bug#8365: 3 of 657 tests failed

[Stefano Lattarini]

On Friday 01 April 2011, Ralf Wildenhues wrote:
> * Stefano Lattarini wrote on Thu, Mar 31, 2011 at 02:54:36PM CEST:
> > At this point I'm not anymore sure this is just a testsuite-related issue
> > -- it seems like a genuine bug in Automake-generated remake rules.  WDYT?
> 
> No.  You were right originally.
>
I'll have to partly disagree with myself then...  The fact is that
the remake rules are quite unpredictable in this situation, and
what's worse, they might end up leaving the tree in an inconsistent
state (Makefile.in corresponds to both Makefile.am and configure.ac,
configure corresponds to configure.ac, but config.status doens't
correspond to configure and Makefile doesn't correspond to
Makefile.in; i.e., something gets updated in spite of the too-near
timestamp modification, but something else doesn't, all subject to
a race -- nasty!).  So this can be seen like an automake bug, or
at least a "limitation".

OTOH, such a bug is veeery unlikely to happen in real life, and
basically impossible to happen when make is being run by hand by
the developer (which I think is the only legitimate use case for
remake rules).  Also, I tried to fix the issue by modifiying
`lib/am/configure.am', but I realized that doing so would require
convoluted hacks with "touch" and "ls -t"/find" -- something IMHO
not worth doing to fix such a theoretical-only issue.

> The tests do need to $sleep.
> Please commit your patch with all the sleeps in it (to maint),
> but please also fix the wrong comments.
>
In the end, I'm OK with your proposal of adding a sleep to the
failing tests, but I'd also like to add an (xfailing) testcase
to expose the issue we have dig up with big efforts.  This is
what I've done in the attached patch.  OK for maint?

> My thinko was the following: given targets A -> B -> C, and A is out of
> date, after make updates A, it checks the time stamp again, even iff it
> knows it has decided to update A.  When that time stamp is the same as
> B, it won't update B.  This sequence shows the issue with high
> probability, the second 'make' often won't update B nor A again:
> 
> cat > Makefile <<EOF
> A: B; touch A
> B: C; touch B
> C: ; touch C
> EOF
> rm -f A B C; make; rm -f C; make
> 
> 
> Introducing sleeps in aclocal, autoconf, or automake would be a serious
> usability issue; I need to avoid falling into that misconception of mine
> again.
>
> Thanks for being persistent.
>
> Cheers,
> Ralf
> 

Thanks,
  Stefano

From fe2c36a9c87ce4a2bb3858a96f514cef3478792f Mon Sep 17 00:00:00 2001
From: Stefano Lattarini <address@hidden>
Date: Wed, 30 Mar 2011 18:06:29 +0200
Subject: [PATCH] tests: fix timestamp-related failures

Fixes automake bug#8365.

* tests/aclocal6.test: Sleep before modifying m4 files that should
trigger remake rules.  Remove incorrect/obsoleted comments.
* tests/subdir5.test: Likewise, and extend a bit.
* tests/subdir8.test: Likewise.

Report from Sam Steingold.
---
 ChangeLog                 |   10 +++++
 tests/Makefile.am         |    2 +
 tests/Makefile.in         |    2 +
 tests/aclocal6.test       |    4 +-
 tests/remake-bug8365.test |   99 +++++++++++++++++++++++++++++++++++++++++++++
 tests/subdir5.test        |   25 +++++++-----
 tests/subdir8.test        |   23 +++++-----
 7 files changed, 143 insertions(+), 22 deletions(-)
 create mode 100755 tests/remake-bug8365.test

diff --git a/ChangeLog b/ChangeLog
index dae2a48..265fd8c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2011-04-01  Stefano Lattarini  <address@hidden>
+
+       tests: fix timestamp-related failures
+       Fixes automake bug#8365.
+       * tests/aclocal6.test: Sleep before modifying m4 files that should
+       trigger remake rules.  Remove incorrect/obsoleted comments.
+       * tests/subdir5.test: Likewise, and extend a bit.
+       * tests/subdir8.test: Likewise.
+       Report from Sam Steingold.
+
 2011-03-21  Ralf Wildenhues  <address@hidden>
 
        tests: fix unindent to use printf not echo for script.
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 4becdbb..26420d4 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -21,6 +21,7 @@ all.test \
 auxdir2.test \
 cond17.test \
 gcj6.test \
+remake-bug8365.test \
 txinfo5.test
 
 include $(srcdir)/parallel-tests.am
@@ -639,6 +640,7 @@ remake4.test \
 remake5.test \
 remake6.test \
 remake7.test \
+remake-bug8365.test \
 regex.test \
 req.test \
 reqd.test \
diff --git a/tests/Makefile.in b/tests/Makefile.in
index b9b1f6e..4ddd123 100644
--- a/tests/Makefile.in
+++ b/tests/Makefile.in
@@ -277,6 +277,7 @@ all.test \
 auxdir2.test \
 cond17.test \
 gcj6.test \
+remake-bug8365.test \
 txinfo5.test
 
 parallel_tests = \
@@ -909,6 +910,7 @@ remake4.test \
 remake5.test \
 remake6.test \
 remake7.test \
+remake-bug8365.test \
 regex.test \
 req.test \
 reqd.test \
diff --git a/tests/aclocal6.test b/tests/aclocal6.test
index ea6bac3..c8ae21b 100755
--- a/tests/aclocal6.test
+++ b/tests/aclocal6.test
@@ -1,5 +1,5 @@
 #! /bin/sh
-# Copyright (C) 2003  Free Software Foundation, Inc.
+# Copyright (C) 2003, 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
@@ -57,6 +57,8 @@ cd build
 ../configure
 $MAKE
 
+# Modified configure dependencies must be newer than config.status.
+$sleep
 # Update an aclocal.m4 dependency, then make sure all Makefiles
 # are updated, even from a sub-directory.
 echo 'AC_DEFUN([SOME_DEFS], [MORE_DEFS])' > ../m4/somedefs.m4
diff --git a/tests/remake-bug8365.test b/tests/remake-bug8365.test
new file mode 100755
index 0000000..8fe00a8
--- /dev/null
+++ b/tests/remake-bug8365.test
@@ -0,0 +1,99 @@
+#! /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/>.
+
+# Test for automake bug#8365, related to Makefile remake rules.
+# The bug is due to subtle timestamp issues and limitations in
+# make's behaviour, and is very unlikely to be triggered (we have
+# to resort to timestamp edit hacks to consistently expose it); in
+# any account, it is nigh to impossible to trigger it by running
+# make by hand.  Thus, fixing it would not be worth the hassle, but
+# we prefer to keep it exposed anyway.
+
+. ./defs || Exit 1
+
+set -e
+
+cat >> configure.in << 'END'
+FOOBAR=zardoz
+AC_OUTPUT
+END
+
+: > Makefile.am
+
+$ACLOCAL
+# Run automake *before* autoconf, because we want to ensure that
+# Makefile.in is not newer than configure.
+$AUTOMAKE
+$AUTOCONF
+
+./configure
+$MAKE Makefile
+$EGREP 'FOOBAR|zardoz' Makefile && Exit 99 # Sanity check.
+
+echo 'AC_SUBST([FOOBAR])' >> configure.in
+
+# Modified configure dependencies must have the same timestamp of
+# config.status and Makefile in order to trigger the bug.
+touch -r config.status Makefile configure.in
+stat config.status Makefile configure.in || : # For debugging.
+
+# Also, the race condition is triggered only when aclocal, automake
+# and aclocal run fast enough to keep the timestamp of the generated
+# aclocal.m4, Makefile.in and configure equal to the timestamp of
+# Makefile & config.status.  To reproduce this race consistently, we
+# need the following hackish wrappers.
+
+save_AUTOCONF=$AUTOCONF
+
+cat > aclocal-wrap <<END
+#!/bin/sh
+set -ex
+# aclocal shouldn't use our autoconf wrapper when extracting
+# the races from configure.in.
+AUTOCONF='$save_AUTOCONF'; export AUTOCONF
+$ACLOCAL "\$@"
+touch -r config.status aclocal.m4
+stat aclocal.m4 || :
+END
+
+cat > automake-wrap <<END
+#!/bin/sh
+set -ex
+# automake shouldn't use our autoconf wrapper when extracting
+# the races from configure.in.
+AUTOCONF='$save_AUTOCONF'; export AUTOCONF
+$AUTOMAKE "\$@"
+touch -r config.status Makefile.in
+stat Makefile.in || :
+END
+
+cat > autoconf-wrap <<END
+#!/bin/sh
+set -ex
+$AUTOCONF "\$@"
+touch -r config.status configure
+stat configure || :
+END
+
+chmod a+x aclocal-wrap automake-wrap autoconf-wrap
+
+env \
+  ACLOCAL=./aclocal-wrap AUTOMAKE=./automake-wrap AUTOCONF=./autoconf-wrap \
+  $MAKE -e Makefile
+grep '^FOOBAR =' Makefile.in
+grep '^FOOBAR *= *zardoz *$' Makefile
+
+:
diff --git a/tests/subdir5.test b/tests/subdir5.test
index 5633ac6..3f4dfa3 100755
--- a/tests/subdir5.test
+++ b/tests/subdir5.test
@@ -1,5 +1,5 @@
 #! /bin/sh
-# Copyright (C) 2001, 2002, 2003, 2004, 2009, 2010 Free Software
+# Copyright (C) 2001, 2002, 2003, 2004, 2009, 2010, 2011 Free Software
 # Foundation, Inc.
 #
 # This program is free software; you can redistribute it and/or modify
@@ -55,18 +55,14 @@ $AUTOMAKE --include-deps --copy --add-missing
 $MAKE
 
 # Now add new directories.
-#
-# We shouldn't need to $sleep here: configure ensures that files
-# generated by it are newer than configure.  Thus, even if
-# Makefile.in is newer than configure but the updated Makefile.am
-# below has the same timestamp as Makefile.in, the latter should
-# be rebuilt due to its dependency on configure.in.
 
 # First we add a new directory by modifying configure.in directly.
 # We update configure.in *before* updating sub/Makefile.am; subdir8.test
 # does it in the other way: it updates confiles.m4 (which is m4_included
 # by configure.in there) after Makefile.am.
 
+# Modified configure dependencies must be newer than config.status.
+$sleep
 sed <configure.in >configure.tmp -e '/^AC_OUTPUT$/i\
 AC_CONFIG_FILES([maude/Makefile])\
 m4_include([confile.m4])\
@@ -89,11 +85,15 @@ echo 'SUBDIRS = maude' >> Makefile.am
 
 # We want a simple rebuild to create maude/Makefile automatically.
 $MAKE
+grep '^SUBDIRS = *maude *$' Makefile.in
+grep '^SUBDIRS = *maude *$' Makefile
 test -f maude/Makefile
 
 # Then we add a new directory by modifying a file included (through
 # `m4_include') by configure.in.
 mkdir maude2
+# Modified configure dependencies must be newer than config.status.
+$sleep
 cat >> confile.m4 << 'END'
 AC_CONFIG_FILES([maude2/Makefile])
 AC_SUBST([GREPME])
@@ -104,8 +104,13 @@ echo 'SUBDIRS += maude2' >> Makefile.am
 # We want a simple rebuild to create maude2/Makefile and update
 # all other Makefiles automatically.
 $MAKE
-grep '^GREPME =' Makefile
-grep '^GREPME =' maude/Makefile
-grep '^GREPME =' maude2/Makefile
+grep '^SUBDIRS =.* maude2' Makefile.in
+grep '^SUBDIRS =.* maude2' Makefile
+
+for ext in '.in' ''; do
+  for d in . maude maude2; do
+    grep '^GREPME =' $d/Makefile$ext
+  done
+done
 
 :
diff --git a/tests/subdir8.test b/tests/subdir8.test
index 093fac8..b39dfe4 100755
--- a/tests/subdir8.test
+++ b/tests/subdir8.test
@@ -1,5 +1,6 @@
 #! /bin/sh
-# Copyright (C) 2003, 2004, 2009, 2010 Free Software Foundation, Inc.
+# Copyright (C) 2003, 2004, 2009, 2010, 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
@@ -68,12 +69,6 @@ $AUTOMAKE --copy --add-missing
 $MAKE
 
 # Now add new directories.
-#
-# We shouldn't need to $sleep here: configure ensures that files
-# generated by it are newer than configure.  Thus, even if
-# Makefile.in is newer than configure but the updated Makefile.am
-# below has the same timestamp as Makefile.in, the latter should
-# be rebuilt due to its dependency on configure.in.
 
 # The first step users typically do when adding a new subdir is
 # editing configure.in.  That is already tested by subdir5.test,
@@ -94,6 +89,9 @@ mkdir maude
 
 # Update confiles.m4 *after* updating sub/Makefile.am; subdir5.test do
 # it in the other way: it updates configure.in before Makefile.am.
+# We sleep here because modified configure dependencies must be newer
+# than config.status.
+$sleep
 echo 'AC_CONFIG_FILES([maude/Makefile sub/maude/Makefile])' >> confiles.m4
 
 # We want a simple rebuild from sub/ to create sub/maude/Makefile
@@ -101,6 +99,8 @@ echo 'AC_CONFIG_FILES([maude/Makefile sub/maude/Makefile])' 
>> confiles.m4
 cd sub
 $MAKE
 cd ..
+grep '^SUBDIRS = *maude *$' sub/Makefile.in
+grep '^SUBDIRS = *maude *$' sub/Makefile
 test -f maude/Makefile
 test -f sub/maude/Makefile
 
@@ -109,9 +109,10 @@ test -f sub/maude/Makefile
 echo 'AC_DEFUN([MORE_DEFS], [AC_SUBST([GREPME])])' > m4/moredefs.m4
 $MAKE
 
-grep '^GREPME =' Makefile
-grep '^GREPME =' maude/Makefile
-grep '^GREPME =' sub/Makefile
-grep '^GREPME =' sub/maude/Makefile
+for ext in '.in' ''; do
+  for d in . maude sub sub/maude; do
+    grep '^GREPME =' $d/Makefile$ext
+  done
+done
 
 :
-- 
1.7.2.3