automake-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: bug#8365: 3 of 657 tests failed


From: Stefano Lattarini
Subject: Re: bug#8365: 3 of 657 tests failed
Date: Fri, 1 Apr 2011 18:50:12 +0200
User-agent: KMail/1.13.3 (Linux/2.6.30-2-686; KDE/4.4.4; i686; ; )

On Friday 01 April 2011, Ralf Wildenhues wrote:
> * Stefano Lattarini wrote on Fri, Apr 01, 2011 at 01:09:41PM CEST:
> > 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?
> 
> OK but ...
> 
> > 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.
> 
> the log fails to mention the new test,
>
Oops, sorry. Fixed.

> >  tests/remake-bug8365.test |   99 
> > +++++++++++++++++++++++++++++++++++++++++++++
> 
> and the name of the new test is inconsistent.  I suggest either
> pr8365.test (yeah, can see you cringe already) or remake-timing.test
> or so.
>
[Disclaimer: BIKESHEDDING follows]
I have to object to this.  First, IMHO the "correct" way to resolve
this inconsistency would be to give more expressive names to the
existing `pr*.test' tests (so that `pr9.test' would become, e.g.,
`config-aux-dir-pr9.test'), not to "dumb down" the names of new tests.
Second, I really do believe that a name like `remake-bug8365.test' is
better than your proposed alternatives, because it's a good compromise
between length and expressivenes: it's pretty short, but it manages to
tell that the test is related to remake rules (which `pr8365.test'
fails to communicate), and that it exercises a reported bug that is
subtle/complex enough not to allow a more expressive short name
(`remake-timing.test' is not really expressive).
[End of bikeshedding]

All the above being bikeshedding, I will go with `remake-timing.test'
if you still think the advantages of the name `remake-bug8365.test'
don't compansate for its ugliness/inconsistency.

> Will the new test work on a file system with sub-second granularity
> where 'touch' has the issues described at 'info Autoconf --index touch'?
>
Uh-oh, this could be a problem.  In order to trigger the bug, various
files need to have the *same* timestamp.  So what about using the `-t'
option of touch instead?  See the attached squash-in.

Regards,
  Stefano
diff --git a/ChangeLog b/ChangeLog
index 265fd8c..063215f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -6,6 +6,8 @@
        trigger remake rules.  Remove incorrect/obsoleted comments.
        * tests/subdir5.test: Likewise, and extend a bit.
        * tests/subdir8.test: Likewise.
+       * tests/remake-bug8365.test: New xfailing test.
+       * tests/Makefile.am (TESTS): Update.
        Report from Sam Steingold.
 
 2011-03-21  Ralf Wildenhues  <address@hidden>
diff --git a/tests/remake-bug8365.test b/tests/remake-bug8365.test
index 8fe00a8..a247c21 100755
--- a/tests/remake-bug8365.test
+++ b/tests/remake-bug8365.test
@@ -46,8 +46,17 @@ $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
+# config.status and Makefile in order to trigger the bug.  Also,
+# `touch -r FILE' can truncate timestamps on some systems (see
+# Autoconf manual), thus we need to use explicit timestamps.
+timestamp=`date '+%Y%m%d%H%M.%S'` && test -n "$timestamp" || {
+  echo "$me: cannot get current time in required format" >&2
+  Exit 77
+}
+touch -t "$timestamp" Makefile config.status configure.in || {
+  echo "$me: couldn't set timestamp of files to current time" >&2
+  Exit 77
+}
 stat config.status Makefile configure.in || : # For debugging.
 
 # Also, the race condition is triggered only when aclocal, automake
@@ -65,7 +74,7 @@ set -ex
 # the races from configure.in.
 AUTOCONF='$save_AUTOCONF'; export AUTOCONF
 $ACLOCAL "\$@"
-touch -r config.status aclocal.m4
+touch -t '$timestamp' aclocal.m4
 stat aclocal.m4 || :
 END
 
@@ -76,7 +85,7 @@ set -ex
 # the races from configure.in.
 AUTOCONF='$save_AUTOCONF'; export AUTOCONF
 $AUTOMAKE "\$@"
-touch -r config.status Makefile.in
+touch -t '$timestamp' Makefile.in
 stat Makefile.in || :
 END
 
@@ -84,7 +93,7 @@ cat > autoconf-wrap <<END
 #!/bin/sh
 set -ex
 $AUTOCONF "\$@"
-touch -r config.status configure
+touch -t '$timestamp' configure
 stat configure || :
 END
 

reply via email to

[Prev in Thread] Current Thread [Next in Thread]