Support Time::HiRes in File::Copy

[p5pRT]

Migrated from rt.perl.org#132401 (status was 'resolved')

Searchable as RT132401$

[p5pRT]

From afresh1@openbsd.org

Created by afresh1@openbsd.org

Autoconf uses File​::Copy to move files between filesystems, and because
File​::Copy​::move currently falls back to copying the file and then using
`utime` to set the time, without this patch the time is only second
resolution and therefore the file age can travel back in time causing
dependencies to appear out of date and build failures to occour. Now
that Time​::HiRes has support for setting hires timestamps with futimens,
File​::Copy should use that to do a better job.

https://marc.info/?t=150963727200003&r=1&w=2

Attached is a patch that attempts to import stat and utime from
File​::Copy and falls back if it fails, including test modifications
to verify that it works.

I have only tested this on OpenBSD, so not sure what fallout there may
be elsewhere.

Perl Info

Flags:
    category=library
    severity=medium
    module=File::Copy

Site configuration information for perl 5.24.3:

Configured by root at Thu Jan  1  0:00:00 UTC 1970.

Summary of my perl5 (revision 5 version 24 subversion 3) configuration:
   
  Platform:
    osname=openbsd, osvers=6.2, archname=amd64-openbsd
    uname='openbsd'
    config_args='-dsE -Dopenbsd_distribution=defined -Dccflags=-DNO_LOCALE_NUMERIC -DNO_LOCALE_COLLATE -Dmksymlinks'
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=undef, usemultiplicity=undef
    use64bitint=define, use64bitall=define, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='cc', ccflags ='-DNO_LOCALE_NUMERIC -DNO_LOCALE_COLLATE -fno-strict-aliasing -pipe -fstack-protector-strong -D_FORTIFY_SOURCE=2 -I/usr/local/include',
    optimize='-O2',
    cppflags='-DNO_LOCALE_NUMERIC -DNO_LOCALE_COLLATE -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
    ccversion='', gccversion='4.2.1 Compatible OpenBSD Clang 5.0.0 (tags/RELEASE_500/final)', gccosandvers=''
    intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678, doublekind=3
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16, longdblkind=3
    ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
    alignbytes=8, prototype=define
  Linker and Libraries:
    ld='cc', ldflags ='-Wl,-E  -fstack-protector-strong -L/usr/local/lib'
    libpth=/usr/lib /usr/lib
    libs=-lm -lc
    perllibs=-lm -lc
    libc=/usr/lib/libc.a, so=so, useshrplib=true, libperl=libperl.so.18.1
    gnulibc_version=''
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-R/usr/libdata/perl5/amd64-openbsd/CORE'
    cccdlflags='-DPIC -fPIC ', lddlflags='-shared -fPIC  -fstack-protector-strong -L/usr/local/lib'



@INC for perl 5.24.3:
    /usr/local/libdata/perl5/site_perl/amd64-openbsd
    /usr/local/libdata/perl5/site_perl
    /usr/libdata/perl5/amd64-openbsd
    /usr/libdata/perl5


Environment for perl 5.24.3:
    HOME=/home/afresh1
    LANG (unset)
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/home/afresh1/.plenv/bin:/home/afresh1/bin:/home/afresh1/bin:/bin:/sbin:/usr/bin:/usr/sbin:/usr/X11R6/bin:/usr/local/bin:/usr/local/sbin:/usr/games:.
    PERL_BADLANG (unset)
    SHELL=/bin/ksh

[p5pRT]

From afresh1@openbsd.org

0001-Support-Time-HiRes-utime-in-File-Copy.patch

From 07e85662fce844bb8ff74dec63f9e27058c69682 Mon Sep 17 00:00:00 2001
From: Andrew Hewus Fresh <afresh1@openbsd.org>
Date: Sun, 5 Nov 2017 12:31:12 -0800
Subject: [PATCH] Support Time::HiRes::utime in File::Copy

If Time::HiRes exists and has utime support for setting hires
utime, use that so cross-device moves can keep time accurately.

Used by autoconf.
---
 lib/File/Copy.pm | 2 ++
 lib/File/Copy.t  | 4 ++++
 2 files changed, 6 insertions(+)

diff --git a/lib/File/Copy.pm b/lib/File/Copy.pm
index 47e6429771..c36ff11c0a 100644
--- a/lib/File/Copy.pm
+++ b/lib/File/Copy.pm
@@ -16,6 +16,8 @@ use Config;
 # And then we need these games to avoid loading overload, as that will
 # confuse miniperl during the bootstrap of perl.
 my $Scalar_Util_loaded = eval q{ require Scalar::Util; require overload; 1 };
+# We want HiRes stat and utime if available
+eval q{ require Time::HiRes; Time::HiRes->import(qw( stat utime )); 1 };
 our(@ISA, @EXPORT, @EXPORT_OK, $VERSION, $Too_Big, $Syscopy_is_copy);
 sub copy;
 sub syscopy;
diff --git a/lib/File/Copy.t b/lib/File/Copy.t
index 25f340d1c0..b54ebf4c25 100644
--- a/lib/File/Copy.t
+++ b/lib/File/Copy.t
@@ -24,6 +24,9 @@ BEGIN { *CORE::GLOBAL::rename = sub { CORE::rename($_[0], $_[1]) }; }
 use File::Copy qw(copy move cp);
 use Config;
 
+# If we have Time::HiRes, File::Copy loaded it for us.
+my $have_hires_utime = eval {Time::HiRes::d_hires_utime()};
+note "Testing Time::HiRes::utime support" if $have_hires_utime;
 
 foreach my $code ("copy()", "copy('arg')", "copy('arg', 'arg', 'arg', 'arg')",
                   "move()", "move('arg')", "move('arg', 'arg', 'arg')"
@@ -103,6 +106,7 @@ for my $cross_partition_test (0..1) {
 
   # Doesn't really matter what time it is as long as its not now.
   my $time = 1000000000;
+  $time += 0.1234 if $have_hires_utime;
   utime( $time, $time, "copy-$$" );
 
   # Recheck the mtime rather than rely on utime in case we're on a
-- 
2.14.2

[p5pRT]

From andrew@cpan.org

As noticed by others, I did a poor job with the test and this was not actually working, as overriding utime needs to happen earlier in a BEGIN block. This patch should work better.

[p5pRT]

From andrew@cpan.org

0001-Support-Time-HiRes-utime-in-File-Copy.patch

From 0cc4a732b6a92c24b6cc7881676d37a1717ab21c Mon Sep 17 00:00:00 2001
From: Andrew Fresh <afresh1@openbsd.org>
Date: Sun, 5 Nov 2017 12:31:12 -0800
Subject: [PATCH] Support Time::HiRes::utime in File::Copy

If Time::HiRes exists and has utime support for setting hires
utime, use that so cross-device moves can keep time accurately.

Used by autoconf.
---
 lib/File/Copy.pm | 2 ++
 lib/File/Copy.t  | 7 ++++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/lib/File/Copy.pm b/lib/File/Copy.pm
index 47e6429771..7656cb7204 100644
--- a/lib/File/Copy.pm
+++ b/lib/File/Copy.pm
@@ -16,6 +16,8 @@ use Config;
 # And then we need these games to avoid loading overload, as that will
 # confuse miniperl during the bootstrap of perl.
 my $Scalar_Util_loaded = eval q{ require Scalar::Util; require overload; 1 };
+# We want HiRes stat and utime if available
+BEGIN { eval q{ use Time::HiRes qw( stat utime ) } };
 our(@ISA, @EXPORT, @EXPORT_OK, $VERSION, $Too_Big, $Syscopy_is_copy);
 sub copy;
 sub syscopy;
diff --git a/lib/File/Copy.t b/lib/File/Copy.t
index 25f340d1c0..57d9478a68 100644
--- a/lib/File/Copy.t
+++ b/lib/File/Copy.t
@@ -24,6 +24,11 @@ BEGIN { *CORE::GLOBAL::rename = sub { CORE::rename($_[0], $_[1]) }; }
 use File::Copy qw(copy move cp);
 use Config;
 
+# If we have Time::HiRes, File::Copy loaded it for us.
+BEGIN {
+  eval { Time::HiRes->import(qw( stat utime )) };
+  note "Testing Time::HiRes::utime support" unless $@;
+}
 
 foreach my $code ("copy()", "copy('arg')", "copy('arg', 'arg', 'arg', 'arg')",
                   "move()", "move('arg')", "move('arg', 'arg', 'arg')"
@@ -102,7 +107,7 @@ for my $cross_partition_test (0..1) {
   ok -e "copy-$$",                '  target still there';
 
   # Doesn't really matter what time it is as long as its not now.
-  my $time = 1000000000;
+  my $time = 1000000000.12345;
   utime( $time, $time, "copy-$$" );
 
   # Recheck the mtime rather than rely on utime in case we're on a
-- 
2.14.2

[p5pRT]

The RT System itself - Status changed from 'new' to 'open'

[p5pRT]

From @jkeenan

On Sun, 05 Nov 2017 22​:27​:48 GMT, andrew@​cpan.org wrote​:

As noticed by others, I did a poor job with the test and this was not
actually working, as overriding utime needs to happen earlier in a
BEGIN block. This patch should work better.

I have made this code available for smoke-testing (updating $VERSION) in this branch​:

smoke-me/jkeenan/afresh/132401-file-copy

I will apply this code to blead in 7 days unless there are unfavorable smoke results or someone objects.

Thank you very much.
--
James E Keenan (jkeenan@​cpan.org)

[p5pRT]

From @jkeenan

On Mon, 06 Nov 2017 14​:55​:25 GMT, jkeenan wrote​:

On Sun, 05 Nov 2017 22​:27​:48 GMT, andrew@​cpan.org wrote​:

As noticed by others, I did a poor job with the test and this was not
actually working, as overriding utime needs to happen earlier in a
BEGIN block. This patch should work better.

I have made this code available for smoke-testing (updating $VERSION)
in this branch​:

smoke-me/jkeenan/afresh/132401-file-copy

I will apply this code to blead in 7 days unless there are unfavorable
smoke results or someone objects.

Thank you very much.

Tony, thank you for smoke-testing the branch on Cygwin. Unfortunately I see that we have some test failures​:

#####
http​://perl.develop-help.com/raw/?id=202743 [1]

[stdio] -Uusethreads -Uusemymalloc
../dist/Time-HiRes/t/alarm.t................................FAILED
  5, 9
  Non-zero exit status​: 2

[1] For some reason this report is not showing up at http​://perl5.test-smoke.org/search
#####

This is the first smoke-test report on Cygwin in over 6 months. When I look at older reports, I notice failures in time-related tests. Example

#####
http​://perl5.test-smoke.org/report/55467

~~ ../dist/Time-HiRes/t/utime.t ................................ FAILED 2-3 Non-zero exit status​: 8
  [stdio/perlio] -Duselongdouble -Dcc='ccache gcc' -Accflags=-D_GNU_SOURCE -Alibswanted=cygwin
  [stdio/perlio] -Dusethreads -Duselongdouble -Dcc='ccache gcc' -Accflags=-D_GNU_SOURCE -Alibswanted=cygwin DEBUGGING
  [stdio/perlio] -Duselongdouble -Dcc='ccache gcc' -Accflags=-D_GNU_SOURCE -Alibswanted=cygwin DEBUGGING
  [stdio/perlio] -Dusethreads -Duselongdouble -Dcc='ccache gcc' -Accflags=-D_GNU_SOURCE -Alibswanted=cygwin
#####

My hunch is that we have long-standing problems with perl on Cygwin that need addressing -- but that there's no real obstacle to the application of the patch/branch discussed in this ticket.

Do you concur?

Thank you very much.
--
James E Keenan (jkeenan@​cpan.org)

[p5pRT]

From @jkeenan

On Tue, 07 Nov 2017 18​:02​:12 GMT, jkeenan wrote​:

On Mon, 06 Nov 2017 14​:55​:25 GMT, jkeenan wrote​:

On Sun, 05 Nov 2017 22​:27​:48 GMT, andrew@​cpan.org wrote​:

As noticed by others, I did a poor job with the test and this was
not
actually working, as overriding utime needs to happen earlier in a
BEGIN block. This patch should work better.

I have made this code available for smoke-testing (updating $VERSION)
in this branch​:

smoke-me/jkeenan/afresh/132401-file-copy

I will apply this code to blead in 7 days unless there are
unfavorable
smoke results or someone objects.

Thank you very much.

Tony, thank you for smoke-testing the branch on Cygwin. Unfortunately
I see that we have some test failures​:

#####
http​://perl.develop-help.com/raw/?id=202743 [1]

[stdio] -Uusethreads -Uusemymalloc
../dist/Time-HiRes/t/alarm.t................................FAILED
5, 9
Non-zero exit status​: 2

[1] For some reason this report is not showing up at
http​://perl5.test-smoke.org/search
#####

This is the first smoke-test report on Cygwin in over 6 months. When
I look at older reports, I notice failures in time-related tests.
Example

#####
http​://perl5.test-smoke.org/report/55467

~~ ../dist/Time-HiRes/t/utime.t ................................
FAILED 2-3 Non-zero exit status​: 8
[stdio/perlio] -Duselongdouble -Dcc='ccache gcc' -Accflags=-
D_GNU_SOURCE -Alibswanted=cygwin
[stdio/perlio] -Dusethreads -Duselongdouble -Dcc='ccache gcc'
-Accflags=-D_GNU_SOURCE -Alibswanted=cygwin DEBUGGING
[stdio/perlio] -Duselongdouble -Dcc='ccache gcc' -Accflags=-
D_GNU_SOURCE -Alibswanted=cygwin DEBUGGING
[stdio/perlio] -Dusethreads -Duselongdouble -Dcc='ccache gcc'
-Accflags=-D_GNU_SOURCE -Alibswanted=cygwin
#####

My hunch is that we have long-standing problems with perl on Cygwin
that need addressing -- but that there's no real obstacle to the
application of the patch/branch discussed in this ticket.

Do you concur?

Ah, I see that I can answer my own question with your data. At http​://perl.develop-help.com/raw/?id=202757 you have posted a smoke-test report on blead on Cygwin which shows the same test failure in branch. So I infer that those test failures are not associated with the patch/branch.

Thanks.
--
James E Keenan (jkeenan@​cpan.org)

[p5pRT]

From @jkeenan

commit 83aebc9
Merge​: 7074755 d7408e7
Author​: James E Keenan <jkeenan@​cpan.org>
Date​: Sun Nov 12 18​:10​:28 2017 -0500

  Merge branch 'smoke-me/jkeenan/afresh/132401-file-copy' into blead
--
James E Keenan (jkeenan@​cpan.org)

[p5pRT]

@jkeenan - Status changed from 'open' to 'resolved'

[p5pRT]

From @iabyn

On Sun, Nov 12, 2017 at 03​:34​:25PM -0800, James E Keenan via RT wrote​:

commit 83aebc9
Merge​: 7074755 d7408e7
Author​: James E Keenan <jkeenan@​cpan.org>
Date​: Sun Nov 12 18​:10​:28 2017 -0500

Merge branch 'smoke\-me/jkeenan/afresh/132401\-file\-copy' into blead

--
James E Keenan (jkeenan@​cpan.org)

That merge was a bit messy. We prefer merges which only have commits on
one leg, and which have a description in the merge commit which describes
overall the commits in the merge. See '=head2 On merging and rebasing' in
pod/perlgit.pod

--
The Enterprise is captured by a vastly superior alien intelligence which
does not put them on trial.
  -- Things That Never Happen in "Star Trek" #10