Subject: Re: [bug#61240] [PATCH 2/2] Gracefully degrade if Time::HiRes is not available

From: Jacob Bachmeyer <[email-masked]>
Date: Tue, 28 Mar 2023 21:06:40 -0500

[expanding CC list to cover later message]

Paul Eggert wrote:
> On 2023-02-07 21:29, Jacob Bachmeyer wrote:
>> use Exporter;
>> -use Time::HiRes qw(stat);
>> 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) } }
>
> Thanks for looking into this. Sorry about the long delay.

I was about to send another ping this weekend.

> My memory was jogged by the recent Autoconf release candidate, which 
> requires Perl 5.10 for what I think is the same reason 
> <https://lists.gnu.org/r/autoconf/2023-03/msg00020.html>. This release 
> candidate didn't build on my Solaris 10 server because Solaris 10 has 
> only Perl 5.8.4. Of course I can work around this by also installing a 
> recent Perl but that is a bit of a pain. I'll cc this email to 
> [email-masked] to give them a heads-up about 
> <https://bugs.gnu.org/61240>.
>
> It'd be nice (though not crucial) if we could get to the bottom of 
> this for Automake and to sync the result to Autoconf before the new 
> Autoconf release comes out, so that Solaris 10 users of the new 
> Autoconf need to install only recent GNU M4, and not also a recent Perl.
>
> To get back to the proposed patch quoted above:
>
> Why change from "use Time::HiRes qw(stat);" to "require Time::HiRes; 
> import Time::HiRes qw(stat)"? (Again, please bear in mind that my Perl 
> is quite rusty.)

In Perl, "use MODULE LIST;" is shorthand for "BEGIN { require Module; 
import Module LIST; }".  The patch must expand that shorthand in order 
to insert an "eval BLOCK" to catch the error "require" will throw if 
Time::HiRes is not available.  In this case, we do not worry about 
actually handling the error, since the fallback is to use the stat 
builtin instead of replacing it with Time::HiRes::stat.

Putting them close together, we have three forms:

(1) use Time::HiRes qw(stat);
(2) BEGIN { require Time::HiRes; import Time::HiRes qw(stat); }
(3) BEGIN { eval { require Time::HiRes; import Time::HiRes qw(stat); } }

According to the Perl manual ("use" in perlfunc), (1) and (2) are 
"exactly equivalent" forms, with a minor syntactic difference not 
relevant here.  (Form (2) is slightly more general.)  The patch replaces 
(1) with (3), which allows the error that occurs if Time::HiRes cannot 
be loaded to be ignored.  Notably, Time::HiRes could be installed from 
CPAN before it became a core module, so this patch also allows 
sub-second timestamps with Perl 5.6 if Time::HiRes has been installed.

Simply saying "eval { use Time::HiRes qw(stat) };" will not work because 
it would expand to "eval { BEGIN { require Time::HiRes; ...} };" which 
would execute the "require" while compiling the "eval" block, and thus 
fail to catch the error if Time::HiRes is not available because the 
error is thrown before the "eval" is actually in force.  Conversely, 
"eval 'use Time::HiRes qw(stat)';" would be too *late*:  the rest of the 
program would have already been compiled to use the stat builtin.  The 
import must be executed before the rest of the program is compiled, and 
an eval must be in force when it is executed to catch the error if 
Time::HiRes is not available.  This requires expanding "use" to its 
equivalent "BEGIN { require ... }" in order to put the "eval" in the 
right place.

> The code formerly had "use File::stat;" before it changed to "use 
> Time::HiRes qw(stat);". Why doesn't the proposed patch need to fall 
> back to "use File::stat;" on older Perls lacking Time::HiRes?

File::stat is a convenience wrapper around the stat builtin that 
modifies it to return an object instead of the 13-element list Perl's 
core stat produces.  File::stat and Time::HiRes::stat are incompatible, 
so the program had to be modified to remove the use of File::stat before 
it could use Time::HiRes::stat.

The reason that the fallback is to do nothing if requiring Time::HiRes 
and importing Time::HiRes::stat fails is that Time::HiRes::stat is a 
drop-in replacement for the stat builtin, which remains available if the 
import fails.  So we attempt to import Time::HiRes::stat (which will 
transparently replace the stat builtin if it succeeds) and proceed with 
the stat builtin (that Perl 5 always has) if the import fails.

> Thanks again for any advice you can provide.

You are welcome.  Also note patches at bug#61670 and bug#61671 which 
resolve some minor testsuite issues with Perl 5.6.2.

-- Jacob