Re: rhel8 test failure confirmation? [PATCH for problem affecting Automake testsuite]

2023-04-01 Thread Paul Eggert 
Thanks for reporting that. I installed the attached slightly different 
patch into Autoconf on Savannah. This patch also changes one other 
instance of file timestamp comparison from < to <=. Something like this 
should appear in the next Autoconf release.From 713d9822bbfb2923115065efaefed34a0113f8a1 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Sat, 1 Apr 2023 16:44:03 -0700
Subject: [PATCH] Fix timing bug on high-speed builds
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Problem reported by Bogdan via Jacob Bachmeyer in:
https://lists.gnu.org/r/autoconf/2023-04/msg2.html
* bin/autom4te.in: If a file timestamp equals a dependency’s
timestamp, consider the file to be out of date.  Although this may
result in extra work, it fixes some rare timing bugs.
---
 bin/autom4te.in | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/bin/autom4te.in b/bin/autom4te.in
index 4b61f0a8..71d7e6a6 100644
--- a/bin/autom4te.in
+++ b/bin/autom4te.in
@@ -910,10 +910,8 @@ sub up_to_date ($)
   return 0
 if ! -f $tfile || ! -f $ofile;
 
-  # The youngest of the cache files must be older than the oldest of
+  # The younger of the cache files must be older than the oldest of
   # the dependencies.
-  # FIXME: These timestamps have only 1-second resolution.
-  # Time::HiRes fixes this, but assumes Perl 5.8 or later.
   my $tmtime = mtime ($tfile);
   my $omtime = mtime ($ofile);
   my ($file, $mtime) = ($tmtime < $omtime
@@ -926,7 +924,7 @@ sub up_to_date ($)
   # We depend at least upon the arguments.
   foreach my $dep (@ARGV)
 {
-  if ($mtime < mtime ($dep))
+  if ($mtime <= mtime ($dep))
 	{
 	  verb "up_to_date ($file): outdated: $dep";
 	  return 0;
@@ -949,7 +947,7 @@ sub up_to_date ($)
   # timestamp of that missing file was newer).
   return 0
 	if ! $dep;
-  if ($mtime < mtime ($dep))
+  if ($mtime <= mtime ($dep))
 	{
 	  verb "up_to_date ($file): outdated: $dep";
 	  return 0;
@@ -1038,7 +1036,7 @@ $icache_file = new Autom4te::XFile $icache, O_RDWR|O_CREAT;
 $icache_file->lock (LOCK_EX)
   if ($flock_implemented eq "yes");
 
-# Read the cache index if available and older than autom4te itself.
+# Read the cache index if available and younger than autom4te itself.
 # If autom4te is younger, then some structures such as C4che might
 # have changed, which would corrupt its processing.
 Autom4te::C4che->load ($icache_file)
@@ -1105,7 +1103,7 @@ else
 # Actual M4 expansion, if the user wants it, or if $output is old
 # (STDOUT is pretty old).
 handle_output ($req, $output)
-  if $force || mtime ($output) < mtime ($ocache . $req->id);
+  if $force || mtime ($output) <= mtime ($ocache . $req->id);
   }
 
 # If we ran up to here, the cache is valid.
-- 
2.37.2

Re: rhel8 test failure confirmation? [PATCH for problem affecting Automake testsuite]

2023-04-01 Thread Bogdan 
Jacob Bachmeyer , Sat Apr 01 2023 04:54:22 
GMT+0200 (Central European Summer Time)

A quick introduction to the situation for the Autoconf list:

The Automake maintainers have encountered a bizarre issue with 
sporadic random test failures, seemingly due to "disk writes not 
taking effect" (as Karl Berry mentioned when starting the thread).  
Bogdan appears to have traced the issue to autom4te caching and 
offered a patch.  I have attached a copy of Bogdan's patch.


Bogdan's patch is a subtle change:  the cache is now considered stale 
unless it is /newer/ than the source files, rather than being 
considered stale only if the source files are newer.  In short, this 
patch causes the cache to be considered stale if its timestamp 
/matches/ the source file, while it is currently considered valid if 
the timestamps match.  I am forwarding the patch to the Autoconf list 
now because I concur with the change, noting that Time:HiRes is also 
limited by the underlying filesystem and therefore is not a "magic 
bullet" solution.  Assuming the cache files are stale unless proven 
otherwise is therefore correct.



 Thank you :)


Note again that this is _Bogdan's_ patch I am forwarding unchanged.  I 
did not write it (but I agree with it).


[further comments inline below]

Bogdan wrote:
Bogdan , Sun Mar 05 2023 22:31:55 GMT+0100 
(Central European Standard Time)
Karl Berry , Sat Mar 04 2023 00:00:56 
GMT+0100 (Central European Standard Time)
 Note that 'config.h' is older (4 seconds) than './configure', 
which

 shouldn't be the case as it should get updated with new values.

Indeed. That is the same sort of thing as I was observing with nodef.
But what (at any level) could be causing that to happen?
Files just aren't getting updated as they should be.

I haven't yet tried older releases of automake to see if their tests
succeed on the systems that are failing now. That's next on my list.


[...]


  Another tip, maybe: cache again. When I compare which files are 
newer than the only trace file I get in the failing 'backcompat2' 
test ('autom4te.cache/traces.0'), I see that 'configure.ac' is 
older than this file in the succeeding run, but it's newer in the 
failing run. This could explain why 'configure' doesn't get updated 
to put new values in config.h (in my case) - 'autom4te' thinks it's 
up-to-date.

  The root cause may be in 'autom4te', sub 'up_to_date':

   # The youngest of the cache files must be older than the oldest of
   # the dependencies.
   # FIXME: These timestamps have only 1-second resolution.
   # Time::HiRes fixes this, but assumes Perl 5.8 or later.

(lines 913-916 in my version).


This comment Bogdan cites is not correct:  Time::HiRes could be 
installed from CPAN on Perls older than 5.8, and might be missing from 
a 5.8 or later installation if the distribution packager separated it 
into another package.  Nor is Time::HiRes guaranteed to fix the issue; 
the infamous example is the FAT filesystem, where timestamps only have 
2-second resolution.  Either way, Time::HiRes is now used if 
available, so this "FIXME" is fixed now.  :-)



 Good to hear :).
 I didn't comment on the comment itself ;). Time::HiRes could have 
been installed on Perl < 5.8, but since then it was in the core 
modules, right? So, it *should* work for users by default then, and 
Autoconf wouldn't require additional installations. That was the core 
message of the comment, I think.



  Perhaps 'configure.ac' in the case that fails is created "not 
late enough" (still within 1 second) when compared to the cache, 
and the cached values are taken, generating the old version of 
'configure' which, in turn, generates old versions of the output 
files.


  Still a guess, but maybe a bit more probable now.

  Does it work when you add '-f' to '$AUTOCONF'? It does for me - 
again, about 20 sequential runs of the same set of tests and about 
5 parallel with 4 threads. Zero failures.
  I'd probably get the same result if I did a 'rm -fr 
autom4te.cache' before each '$AUTOCONF' invocation.

[...]

  More input (or noise):

1) The t/backcompat2.sh test (the only test which fails for me) is a 
test which modifies configure.ac and calls $AUTOCONF several times.


2) Autom4te (part of Autoconf) has a 1-second resolution in checking 
if the input files are newer than the cache.


Maybe.  That comment could be wrong; the actual "sub mtime" is in 
Autom4te::FileUtils.  Does your version of that module use 
Time::HiRes? Git indicates that use of Time::HiRes was added to 
Autoconf at commit 3a9802d60156809c139e9b4620bf04917e143ee2 which is 
between the 2.72a and 2.72c snapshot tags.



 I'm using Autoconf provided by my system and it's version 2.71 
(official package, I assume). Autom4te::FileUtils is using the 
built-in stat() function.



3) Thus, a sequence: 'autoconf' + quickly modify configure.ac + 
quickly run 'autoconf' may cause autom4te to use the old values from 
the cache instead of processing the new configure.ac.