Re: [RFC PATCH]: autom4te: report subsecond timestamp support in --version

[Jacob Bachmeyer]

Zack Weinberg wrote:

The Automake test suite wants this in order to know if it’s safe to
reduce the length of various delays for the purpose of ensuring files
in autom4te.cache are newer than the corresponding source files.

* lib/Autom4te/FileUtils.pm: Provide (but do not export) a flag
  $subsecond_mtime, indicating whether the ‘mtime’ function reports
  modification time with precision greater than one second.
  Reorganize commentary and import logic for clarity.  Add
  configuration for emacs’ perl-mode to the bottom of the file.
  


Now that I have seen the actual patch, yes, this test should be 
accurate.  The test in the main autom4te script will also work, even if 
there is a mismatch between the script and its library, since Perl 
accepts a fully-qualified variable name even if that variable has never 
been declared; its value is undef, which is falsish in Boolean context.



* bin/autom4te.in ($version): If $Autom4te::FileUtils::subsecond_mtime
  is true, add the text “ (subsecond timestamps supported)” to the
  first line of --version output.
  


This appears to be misaligned with the GNU Coding Standards, which 
states:  "The first line is meant to be easy for a program to parse; the 
version number proper starts after the last space."


Perhaps the best option would be to conditionally add a line "This 
autom4te supports subsecond timestamps." after the license notice?



-- Jacob

Re: rhel8 test failure confirmation?

[Jacob Bachmeyer]

Zack Weinberg wrote:

On Sun, Dec 3, 2023, at 4:49 PM, Karl Berry wrote:
  
There would not need to be much parsing, just "automake --version | grep 
  
> HiRes" in that case, or "case `automake --version` in *HiRes*) ...;; 
> easc" to avoid running grep if you want.


I specifically want to hear what Karl thinks.

I lean towards Jacob's view that automake --version | grep HiRes will
suffice. Not having a new option seems simpler/better in terms of later
understanding, too. --thanks, karl.



Did I misunderstand which program's --version output we are talking about?
I thought we were talking about automake's testsuite probing the behavior
of *autom4te*, but all the quoted text seems to be imagining a probe of
*automake* instead.


Yes, there was a bit of confusion here; not only is the FileUtils module 
synchronized between autom4te and automake, but those two are near 
"sound-alikes" as I read them.  Oops.


The issue here seems to be determining if a fix that (I think) 
originated in automake has been applied to the active autom4te.



[...]

I'm not using the identifier "HiRes" because the use of Time::HiRes is an
implementation detail that could change.  For instance, if there's a third
party CPAN module that lets us get at nanosecond-resolution timestamps
*without* loss of precision due to conversion to an NV (aka IEEE double)
we could, in the future, look for that first.
  


That is fine, but "[HiRes]" or "[HiResTime]" is much shorter and we 
could use it as the name of the feature regardless of the underlying 
implementation.  Characters in the first line of `autom4te --version` 
are a finite resource if we want it to fit on a standard 80-column 
terminal without wrapping.  If we need to distinguish, "[HiRes] 
[HiRes-ns]" could be used to indicate your hypothetical integer 
nanosecond-resolution timestamp support, which would indicate also 
having sub-second timestamp support.


I also suggest changing the tag, since the GNU Coding Standards call for 
the version number to be indicated by the last space, but parenthesized 
text between the name and version is supposed to be the package, so this 
would lead to:


$ ./tests/autom4te --version
autom4te [HiResTime] (GNU Autoconf) 2.72d.6-49ab3-dirty
Copyright (C) 2023 Free Software Foundation, Inc.
[...]


Is this workable all the way around, everyone?  Or should the feature be 
indicated with another line after the license notice?  ("This autom4te 
has subsecond timestamp resolution.")  My apologies for neglecting to 
check this before suggesting a tag in the --version output.



The implementation is just

BEGIN
{
  our $subsecond_timestamps = 0;
  eval
{
  require Time::HiRes;
  import Time::HiRes qw(stat);
  $subsecond_timestamps = 1;
}
}

Jacob, can you confirm that's an accurate test, given all the things you
said earlier about ways that grepping the source code might get it wrong?


That will determine if (a) Time::HiRes could be loaded and (b) 
Time::HiRes::stat could be imported.  This is the same test that 
Autom{ak,4t}e::FileUtils effectively uses to use Time::HiRes::stat.  I 
believe that the import is not actually necessary (i.e. Time::HiRes 
always exported Time::HiRes::stat) but it should do no harm as long as 
any use of stat in the code is prepared to handle floating-point 
timestamps.  As long as the autom4te script and its library are 
consistent (which is the distribution's problem if they screw that up), 
this test should be accurate.



-- Jacob

Re: rhel8 test failure confirmation?

[Karl Berry]

I thought we were talking about automake's testsuite probing the behavior
of *autom4te*

Yes, exactly. Sorry for the confusion.

$ ./tests/autom4te --version
autom4te (GNU Autoconf) 2.72d.6-49ab3-dirty (subsecond timestamps supported)

Looks good to me. Thanks.

 a third party CPAN module

Good point.

The implementation is just

Your patch sent to the autoconf list looked fine to me, FWIW.

Perhaps worth mentioning somewhere that Automake is the consumer of this
information that caused the change. You could point to
https://bugs.gnu.org/64756 if you want something more convenient than
the long multi-month thread on the automake discussion list (which I
pointed to from that bug).

Jacob, can you confirm that's an accurate test, 

+1 for Jacob's confirmation being welcome :).

[RFC PATCH]: autom4te: report subsecond timestamp support in --version

[Zack Weinberg]

The Automake test suite wants this in order to know if it’s safe to
reduce the length of various delays for the purpose of ensuring files
in autom4te.cache are newer than the corresponding source files.

* lib/Autom4te/FileUtils.pm: Provide (but do not export) a flag
  $subsecond_mtime, indicating whether the ‘mtime’ function reports
  modification time with precision greater than one second.
  Reorganize commentary and import logic for clarity.  Add
  configuration for emacs’ perl-mode to the bottom of the file.

* bin/autom4te.in ($version): If $Autom4te::FileUtils::subsecond_mtime
  is true, add the text “ (subsecond timestamps supported)” to the
  first line of --version output.
---
 bin/autom4te.in   |  6 ++--
 lib/Autom4te/FileUtils.pm | 71 ---
 2 files changed, 56 insertions(+), 21 deletions(-)

diff --git a/bin/autom4te.in b/bin/autom4te.in
index 38a61ac9..9a2e5f12 100644
--- a/bin/autom4te.in
+++ b/bin/autom4te.in
@@ -207,8 +207,10 @@ General help using GNU software: 
.
 
 # $VERSION
 # 
-$version = "autom4te (@PACKAGE_NAME@) @VERSION@
-Copyright (C) @RELEASE_YEAR@ Free Software Foundation, Inc.
+$version = "autom4te (@PACKAGE_NAME@) @VERSION@"
+  . ($Autom4te::FileUtils::subsecond_mtime
+ ? " (subsecond timestamps supported)\n" : "\n")
+  . "Copyright (C) @RELEASE_YEAR@ Free Software Foundation, Inc.
 License GPLv3+/Autoconf: GNU GPL version 3 or later
 , 
 This is free software: you are free to change and redistribute it.
diff --git a/lib/Autom4te/FileUtils.pm b/lib/Autom4te/FileUtils.pm
index c1e8e8c3..06f87c31 100644
--- a/lib/Autom4te/FileUtils.pm
+++ b/lib/Autom4te/FileUtils.pm
@@ -38,24 +38,45 @@ use 5.006;
 use strict;
 use warnings FATAL => 'all';
 
-use Exporter;
+BEGIN
+{
+  require Exporter;
+  our @ISA = qw (Exporter);
+  our @EXPORT = qw (
+   _file 
+   _file
+_hint 
+   _has_case_matching_file _dir_cache
+   _dir_cache_file);
+}
+
+# Use sub-second resolution file timestamps if available, carry on
+# with one-second resolution timestamps if Time::HiRes is not available.
+#
+# Unfortunately, even if Time::HiRes is available, we don't get
+# timestamps to the full precision recorded by the operating system,
+# because Time::HiRes converts timestamps to floating-point, and the
+# rounding error is hundreds of nanoseconds for circa-2023 timestamps
+# in IEEE double precision.  But this is the best we can do without
+# dropping down to C.
+#
+# $subsecond_mtime is not exported, but is intended for external
+# consumption, as $Autom4te::FileUtils::subsecond_mtime.
+BEGIN
+{
+  our $subsecond_mtime = 0;
+  eval
+{
+  require Time::HiRes;
+  import Time::HiRes qw(stat);
+  $subsecond_mtime = 1;
+}
+}
+
 use IO::File;
-
-# use sub-second resolution timestamps if available,
-# carry on with one-second resolution timestamps if that is all we have
-BEGIN { eval { require Time::HiRes; import Time::HiRes qw(stat) } }
-
 use Autom4te::Channels;
 use Autom4te::ChannelDefs;
 
-our @ISA = qw (Exporter);
-our @EXPORT = qw (
- _file 
- _file
-  _hint 
- _has_case_matching_file _dir_cache
- _dir_cache_file);
-
 =over 4
 
 =item C
@@ -122,11 +143,6 @@ sub mtime ($)
 $atime,$mtime,$ctime,$blksize,$blocks) = stat ($file)
 or fatal "cannot stat $file: $!";
 
-  # Unfortunately Time::HiRes converts timestamps to floating-point, and the
-  # rounding error can be hundreds of nanoseconds for circa-2023 timestamps.
-  # Perhaps some day Perl will support accurate file timestamps.
-  # For now, do the best we can without going outside Perl.
-
   return $mtime;
 }
 
@@ -394,3 +410,20 @@ sub set_dir_cache_file ($$)
 =cut
 
 1; # for require
+
+### Setup "GNU" style for perl-mode and cperl-mode.
+## Local Variables:
+## perl-indent-level: 2
+## perl-continued-statement-offset: 2
+## perl-continued-brace-offset: 0
+## perl-brace-offset: 0
+## perl-brace-imaginary-offset: 0
+## perl-label-offset: -2
+## cperl-indent-level: 2
+## cperl-brace-offset: 0
+## cperl-continued-brace-offset: 0
+## cperl-label-offset: -2
+## cperl-extra-newline-before-brace: t
+## cperl-merge-trailing-else: nil
+## cperl-continued-statement-offset: 2
+## End:
-- 
2.41.0

Re: rhel8 test failure confirmation?

[Zack Weinberg]

On Sun, Dec 3, 2023, at 4:49 PM, Karl Berry wrote:
>> There would not need to be much parsing, just "automake --version | grep 
> > HiRes" in that case, or "case `automake --version` in *HiRes*) ...;; 
> > easc" to avoid running grep if you want.
>
> I specifically want to hear what Karl thinks.
>
> I lean towards Jacob's view that automake --version | grep HiRes will
> suffice. Not having a new option seems simpler/better in terms of later
> understanding, too. --thanks, karl.

Did I misunderstand which program's --version output we are talking about?
I thought we were talking about automake's testsuite probing the behavior
of *autom4te*, but all the quoted text seems to be imagining a probe of
*automake* instead.  Yinz can change automake yourselves, you don't need
me to jump in :-)

Assuming this is really about autom4te, how does this look?

$ ./tests/autom4te --version
autom4te (GNU Autoconf) 2.72d.6-49ab3-dirty (subsecond timestamps supported)
Copyright (C) 2023 Free Software Foundation, Inc.
[etc]

If subsecond timestamps are *not* supported, you'll get instead

autom4te (GNU Autoconf) 2.72d.6-49ab3-dirty
Copyright (C) 2023 Free Software Foundation, Inc.
[etc]

I'm not using the identifier "HiRes" because the use of Time::HiRes is an
implementation detail that could change.  For instance, if there's a third
party CPAN module that lets us get at nanosecond-resolution timestamps
*without* loss of precision due to conversion to an NV (aka IEEE double)
we could, in the future, look for that first.

The implementation is just

BEGIN
{
  our $subsecond_timestamps = 0;
  eval
{
  require Time::HiRes;
  import Time::HiRes qw(stat);
  $subsecond_timestamps = 1;
}
}

Jacob, can you confirm that's an accurate test, given all the things you
said earlier about ways that grepping the source code might get it wrong?

zw