Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lib/File/Copy.t fails mtime test in perl-5.28 built with quadmath #16632

Closed
p5pRT opened this issue Jul 15, 2018 · 20 comments
Closed

lib/File/Copy.t fails mtime test in perl-5.28 built with quadmath #16632

p5pRT opened this issue Jul 15, 2018 · 20 comments

Comments

@p5pRT
Copy link

p5pRT commented Jul 15, 2018

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

Searchable as RT133377$

@p5pRT
Copy link
Author

p5pRT commented Jul 15, 2018

From @jkeenan

When perl is compiled with -Dusequadmath, we are prone to an error in
lib/File/Copy.t -- though the error may be more one of rounding than
anything else. This error appears to have emerged between 5.26 and 5.28.

#####
$ git show | head -1
commit d94e901

$ ./perl -Ilib -V | grep config_args
  config_args='-des -Dusedevel -Dusequadmath'

$ cd t;./perl harness -v ../lib/File/Copy.t; cd -
# Failed test 'mtime preserved by copy() while testing cross-partition'
# at ../lib/File/Copy.t line 128.
# got​: '1000000000.123449998'
# expected​: '1000000000.123449999'
# Looks like you failed 1 test of 466.
../lib/File/Copy.t ..
# Testing Time​::HiRes​::utime support
1..466
ok 1 - 'copy()' is a usage error
ok 2 - 'copy('arg')' is a usage error
ok 3 - 'copy('arg', 'arg', 'arg', 'arg')' is a usage error
...
ok 52 - contents preserved
not ok 53 - mtime preserved by copy() while testing cross-partition
ok 54 - copy(fn, dir)​: same contents
...
Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/466 subtests

Test Summary Report


../lib/File/Copy.t (Wstat​: 256 Tests​: 466 Failed​: 1)
  Failed test​: 53
  Non-zero exit status​: 1
Files=1, Tests=466, 0 wallclock secs ( 0.04 usr 0.00 sys + 0.13 cusr
0.02 csys = 0.19 CPU)
Result​: FAIL
#####

Per suggestion from khw on #p5p, I created a branch in which I reverted
the most recent commit to blead (that above), reconfigured, rebuilt and
re-tested -- with the same result.

I next checked out tag v5.26.0 and re-configured with '-Dusequadmath'.
But here lib/File/Copy.t PASSed.

I next checked out tag v5.28.0 and re-configured with '-Dusequadmath'.
Here the FAIL in lib/File/Copy.t was manifest​:

#####
/home/jkeenan/gitwork/perl2/lib/File/Copy.t ..
# Testing Time​::HiRes​::utime support
1..466
ok 1 - 'copy()' is a usage error
ok 2 - 'copy('arg')' is a usage error
ok 3 - 'copy('arg', 'arg', 'arg', 'arg')' is a usage error
...
ok 52 - contents preserved
not ok 53 - mtime preserved by copy() while testing cross-partition
# Failed test 'mtime preserved by copy() while testing cross-partition'
# at /home/jkeenan/gitwork/perl2/lib/File/Copy.t line 128.
# got​: '1000000000.123449998'
# expected​: '1000000000.123449999'
ok 54 - copy(fn, dir)​: same contents
...
ok 466 - copy with buffer above normal size
# Looks like you failed 1 test of 466.
Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/466 subtests

Test Summary Report


/home/jkeenan/gitwork/perl2/lib/File/Copy.t (Wstat​: 256 Tests​: 466
Failed​: 1)
  Failed test​: 53
  Non-zero exit status​: 1
Files=1, Tests=466, 1 wallclock secs ( 0.07 usr 0.00 sys + 0.17 cusr
0.02 csys = 0.26 CPU)
Result​: FAIL
#####

Thank you very much.
Jim Keenan

@p5pRT
Copy link
Author

p5pRT commented Jul 15, 2018

From @jkeenan

Summary of my perl5 (revision 5 version 28 subversion 0) configuration​:
  Commit id​: 6cb72a3
  Platform​:
  osname=linux
  osvers=4.4.0-130-generic
  archname=x86_64-linux-quadmath
  uname='linux zareason 4.4.0-130-generic #156-ubuntu smp thu jun 14 08​:53​:28 utc 2018 x86_64 x86_64 x86_64 gnulinux '
  config_args='-des -Dusedevel -Dusequadmath -Dprefix=/home/jkeenan/testing/5.28.0-quadmath -Uversiononly -Dman1dir=none -Dman3dir=none'
  hint=recommended
  useposix=true
  d_sigaction=define
  useithreads=undef
  usemultiplicity=undef
  use64bitint=define
  use64bitall=define
  uselongdouble=undef
  usemymalloc=n
  default_inc_excludes_dot=define
  bincompat5005=undef
  Compiler​:
  cc='cc'
  ccflags ='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64'
  optimize='-O2'
  cppflags='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
  ccversion=''
  gccversion='5.4.0 20160609'
  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='__float128'
  nvsize=16
  Off_t='off_t'
  lseeksize=8
  alignbytes=16
  prototype=define
  Linker and Libraries​:
  ld='cc'
  ldflags =' -fstack-protector-strong -L/usr/local/lib'
  libpth=/usr/local/lib /usr/lib/gcc/x86_64-linux-gnu/5/include-fixed /usr/include/x86_64-linux-gnu /usr/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib /lib64 /usr/lib64
  libs=-lpthread -lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc -lgdbm_compat -lquadmath
  perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc -lquadmath
  libc=libc-2.23.so
  so=so
  useshrplib=false
  libperl=libperl.a
  gnulibc_version='2.23'
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs
  dlext=so
  d_dlsymun=undef
  ccdlflags='-Wl,-E'
  cccdlflags='-fPIC'
  lddlflags='-shared -O2 -L/usr/local/lib -fstack-protector-strong'

Characteristics of this binary (from libperl)​:
  Compile-time options​:
  HAS_TIMES
  PERLIO_LAYERS
  PERL_COPY_ON_WRITE
  PERL_DONT_CREATE_GVSV
  PERL_MALLOC_WRAP
  PERL_OP_PARENT
  PERL_PRESERVE_IVUV
  PERL_USE_DEVEL
  USE_64_BIT_ALL
  USE_64_BIT_INT
  USE_LARGE_FILES
  USE_LOCALE
  USE_LOCALE_COLLATE
  USE_LOCALE_CTYPE
  USE_LOCALE_NUMERIC
  USE_LOCALE_TIME
  USE_PERLIO
  USE_PERL_ATOF
  USE_QUADMATH
  Built under linux
  Compiled at Jul 15 2018 17​:07​:00
  %ENV​:
  PERL2DIR="/home/jkeenan/gitwork/perl2"
  PERLBREW_BASHRC_VERSION="0.78"
  PERLBREW_HOME="/home/jkeenan/.perlbrew"
  PERLBREW_MANPATH="/home/jkeenan/perl5/perlbrew/perls/perl-5.28.0/man"
  PERLBREW_PATH="/home/jkeenan/perl5/perlbrew/bin​:/home/jkeenan/perl5/perlbrew/perls/perl-5.28.0/bin"
  PERLBREW_PERL="perl-5.28.0"
  PERLBREW_ROOT="/home/jkeenan/perl5/perlbrew"
  PERLBREW_VERSION="0.78"
  PERL_WORKDIR="/home/jkeenan/gitwork/perl"
  @​INC​:
  lib
  /home/jkeenan/testing/5.28.0-quadmath/lib/perl5/site_perl/5.28.0/x86_64-linux-quadmath
  /home/jkeenan/testing/5.28.0-quadmath/lib/perl5/site_perl/5.28.0
  /home/jkeenan/testing/5.28.0-quadmath/lib/perl5/5.28.0/x86_64-linux-quadmath
  /home/jkeenan/testing/5.28.0-quadmath/lib/perl5/5.28.0

@p5pRT
Copy link
Author

p5pRT commented Jul 16, 2018

From @sisyphus

On Sun, 15 Jul 2018 14​:15​:29 -0700, jkeenan@​pobox.com wrote​:

When perl is compiled with -Dusequadmath, we are prone to an error in
lib/File/Copy.t -- though the error may be more one of rounding than
anything else. This error appears to have emerged between 5.26 and 5.28.

There's some discussion about this at​:
https://www.nntp.perl.org/group/perl.perl5.porters/2018/04/msg250721.html

I placed the first release affected by the bug at 5.27.6. (That probably should be double checked, but.)

Cheers,
Rob

@p5pRT
Copy link
Author

p5pRT commented Jul 16, 2018

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

@p5pRT
Copy link
Author

p5pRT commented Jul 19, 2018

From @jkeenan

On Mon, 16 Jul 2018 00​:31​:08 GMT, sisyphus@​cpan.org wrote​:

On Sun, 15 Jul 2018 14​:15​:29 -0700, jkeenan@​pobox.com wrote​:

When perl is compiled with -Dusequadmath, we are prone to an error in
lib/File/Copy.t -- though the error may be more one of rounding than
anything else. This error appears to have emerged between 5.26 and
5.28.

There's some discussion about this at​:
https://www.nntp.perl.org/group/perl.perl5.porters/2018/04/msg250721.html

Reviewing that thread, I see that a fix was pushed to a smoke-me branch, but all the smoke tests failed due to a porting test failure.

I have corrected that error and pushed the result to a new branch​:

smoke-me/jkeenan/davem/133377-time-hires

Can we get that branch smoked on platforms that support quadmath? (And, does that include anything beyond Linux?)

I placed the first release affected by the bug at 5.27.6. (That
probably should be double checked, but.)

Cheers,
Rob

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

@p5pRT
Copy link
Author

p5pRT commented Jul 20, 2018

From @sisyphus

Can we get that branch smoked on platforms that support quadmath? (And,
does that include anything beyond Linux?)

"Yes" to both. In fact it's desirable.

However, I've just agreed (in #41202) to moving part of my patch into
perl.h - because I think Karl's suggestion to do that makes good sense.

When I've got that change sorted, how about I​:

1) update blead to current state
2) checkout the branch "smoke-me/jkeenan/davem/133377-time-hires"
3) create git format patches of perl.h and numeric.c against that branch
4) send those patches to you so that you can apply them to that branch

Is that the best way for me to proceed ?
Which files in that branch already differ from the files in the "blead"
branch ?

Cheers,
Rob

On Thu, Jul 19, 2018 at 10​:46 AM, James E Keenan via RT <
perlbug-followup@​perl.org> wrote​:

On Mon, 16 Jul 2018 00​:31​:08 GMT, sisyphus@​cpan.org wrote​:

On Sun, 15 Jul 2018 14​:15​:29 -0700, jkeenan@​pobox.com wrote​:

When perl is compiled with -Dusequadmath, we are prone to an error in
lib/File/Copy.t -- though the error may be more one of rounding than
anything else. This error appears to have emerged between 5.26 and
5.28.

There's some discussion about this at​:
https://www.nntp.perl.org/group/perl.perl5.porters/2018/
04/msg250721.html

Reviewing that thread, I see that a fix was pushed to a smoke-me branch,
but all the smoke tests failed due to a porting test failure.

I have corrected that error and pushed the result to a new branch​:

smoke-me/jkeenan/davem/133377-time-hires

Can we get that branch smoked on platforms that support quadmath? (And,
does that include anything beyond Linux?)

I placed the first release affected by the bug at 5.27.6. (That
probably should be double checked, but.)

Cheers,
Rob

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

---
via perlbug​: queue​: perl5 status​: open
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=133377

@p5pRT
Copy link
Author

p5pRT commented Jul 27, 2018

From @sisyphus

On Thu, 19 Jul 2018 18​:10​:23 -0700, sisyphus359@​gmail.com wrote​:

Can we get that branch smoked on platforms that support quadmath? (And,
does that include anything beyond Linux?)

"Yes" to both. In fact it's desirable.

Duh - ignore that previous post of mine as it has no relevance to this ticket. (Using gmail, I thought I was posting to a different thread.)

Apologies,
Rob

@p5pRT
Copy link
Author

p5pRT commented Aug 3, 2018

From @sisyphus

On Wed, 18 Jul 2018 17​:46​:19 -0700, jkeenan wrote​:

Can we get that branch smoked on platforms that support quadmath?

I see plenty of evidence that the problem still persists.
Was this something that Jarkko was working on prior to the release of perl-5.28.0, before he ran out of time and/or energy ?

I don't have a solution, but I do find the following rather interesting​:

Firstly, the test script does​:

  my $time = 1000000000.12345;
  utime( $time, $time, "copy-$$" );

If one then immediately checks the value returned by​:

(stat("copy-$$"))[9]

one finds that it is exactly equal to $time - 1e-9.

Following on from that, there's a call to​:

move("copy-$$", "file-$$")

and if we then immediately check the value returned by​:

(stat("file-$$"))[9]

we find that it is exactly equal to $time - 1e-9 - 1e-9

These aren't simple rounding errors, and I've no idea of (and even less interest in) what the OS is making of all this.
However, something is obviously fundamentally flawed.

One workaround would be to instead set​:

$time = 1000000000.123449998;

With that in place, all tests pass.
But, of course, we've then swept a problem under the carpet.

Maybe there should also be a TODO test using the original value of 1000000000.12345 ?

I was curious as to whether the (stat(...))[9] values would change every time the file was moved, but I found that, having arrived at the value of 1000000000.123449998, that's where it remained.

Cheers,
Rob

@p5pRT
Copy link
Author

p5pRT commented Aug 3, 2018

From @iabyn

On Fri, Aug 03, 2018 at 06​:12​:31AM -0700, sisyphus@​cpan.org via RT wrote​:

On Wed, 18 Jul 2018 17​:46​:19 -0700, jkeenan wrote​:
I see plenty of evidence that the problem still persists. Was this
something that Jarkko was working on prior to the release of
perl-5.28.0, before he ran out of time and/or energy ?

I haven't read this thread closely, but is this same issue that I produced
a patch for back in April (but didn't apply because we were in code
freeze)?

commit 3e15af2 (origin/smoke-me/davem/time_hires_nsec, time_hires_nsec)
Author​: David Mitchell <davem@​iabyn.com>
AuthorDate​: Fri Apr 27 12​:43​:44 2018 +0100
Commit​: David Mitchell <davem@​iabyn.com>
CommitDate​: Fri Apr 27 14​:21​:28 2018 +0100

  time​::HiRes​: don't truncate nanosec utime
 
  When passed a floating point atime/mtime value, T​::HR​::utime()
  was converting it into two longs​: secs and nsec. But the nanosec value
  was calculated using a final NV to long cast, which truncates any
  fractional part rather than rounding to nearest. Use a 0.5 addition to
  force rounding.
 
  This was manifesting as a test in lib/File/Copy.t failing to preserve
  the same mtime after a couple of round trips with utime() and stat().
 
  In particular, the test was attempting to set an mtime to the literal
  floating-point value
 
  1000000000.12345
 
  This value can't be represented exactly as an NV, so was actually
  (under -Dquadmath)
 
  1000000000.1234499999999999999999999568211720247320
 
  which was (using truncation) being converted into the two sec/nsec
  longs​:
 
  1000000000, 123449999
 
  After this commit, it instead correctly gets converted to
 
  1000000000, 123450000

M dist/Time-HiRes/HiRes.xs
M dist/Time-HiRes/t/utime.t

--
O Unicef Clearasil!
Gibberish and Drivel!
  -- "Bored of the Rings"

1 similar comment
@p5pRT
Copy link
Author

p5pRT commented Aug 3, 2018

From @iabyn

On Fri, Aug 03, 2018 at 06​:12​:31AM -0700, sisyphus@​cpan.org via RT wrote​:

On Wed, 18 Jul 2018 17​:46​:19 -0700, jkeenan wrote​:
I see plenty of evidence that the problem still persists. Was this
something that Jarkko was working on prior to the release of
perl-5.28.0, before he ran out of time and/or energy ?

I haven't read this thread closely, but is this same issue that I produced
a patch for back in April (but didn't apply because we were in code
freeze)?

commit 3e15af2 (origin/smoke-me/davem/time_hires_nsec, time_hires_nsec)
Author​: David Mitchell <davem@​iabyn.com>
AuthorDate​: Fri Apr 27 12​:43​:44 2018 +0100
Commit​: David Mitchell <davem@​iabyn.com>
CommitDate​: Fri Apr 27 14​:21​:28 2018 +0100

  time​::HiRes​: don't truncate nanosec utime
 
  When passed a floating point atime/mtime value, T​::HR​::utime()
  was converting it into two longs​: secs and nsec. But the nanosec value
  was calculated using a final NV to long cast, which truncates any
  fractional part rather than rounding to nearest. Use a 0.5 addition to
  force rounding.
 
  This was manifesting as a test in lib/File/Copy.t failing to preserve
  the same mtime after a couple of round trips with utime() and stat().
 
  In particular, the test was attempting to set an mtime to the literal
  floating-point value
 
  1000000000.12345
 
  This value can't be represented exactly as an NV, so was actually
  (under -Dquadmath)
 
  1000000000.1234499999999999999999999568211720247320
 
  which was (using truncation) being converted into the two sec/nsec
  longs​:
 
  1000000000, 123449999
 
  After this commit, it instead correctly gets converted to
 
  1000000000, 123450000

M dist/Time-HiRes/HiRes.xs
M dist/Time-HiRes/t/utime.t

--
O Unicef Clearasil!
Gibberish and Drivel!
  -- "Bored of the Rings"

@p5pRT
Copy link
Author

p5pRT commented Aug 3, 2018

From @sisyphus

< I haven't read this thread closely, but is this same issue that I
produced a patch for back in April (but didn't apply because we were in
code freeze)?

Seems very likely to me that the answer is "yes".

Could the patch be applied now, so that we can try it out ?

Cheers,
Rob

On Fri, Aug 3, 2018 at 11​:29 PM, Dave Mitchell <davem@​iabyn.com> wrote​:

On Fri, Aug 03, 2018 at 06​:12​:31AM -0700, sisyphus@​cpan.org via RT wrote​:

On Wed, 18 Jul 2018 17​:46​:19 -0700, jkeenan wrote​:
I see plenty of evidence that the problem still persists. Was this
something that Jarkko was working on prior to the release of
perl-5.28.0, before he ran out of time and/or energy ?

I haven't read this thread closely, but is this same issue that I produced
a patch for back in April (but didn't apply because we were in code
freeze)?

commit 3e15af2
(origin/smoke-me/davem/time_hires_nsec, time_hires_nsec)
Author​: David Mitchell <davem@​iabyn.com>
AuthorDate​: Fri Apr 27 12​:43​:44 2018 +0100
Commit​: David Mitchell <davem@​iabyn.com>
CommitDate​: Fri Apr 27 14​:21​:28 2018 +0100

time&#8203;::HiRes&#8203;: don't truncate nanosec utime

When passed a floating point atime/mtime value\, T&#8203;::HR&#8203;::utime\(\)
was converting it into two longs&#8203;: secs and nsec\. But the nanosec value
was calculated using a final NV to long cast\, which truncates any
fractional part rather than rounding to nearest\. Use a 0\.5 addition to
force rounding\.

This was manifesting as a test in lib/File/Copy\.t failing to preserve
the same mtime after a couple of round trips with utime\(\) and stat\(\)\.

In particular\, the test was attempting to set an mtime to the literal
floating\-point value

    1000000000\.12345

This value can't be represented exactly as an NV\, so was actually
\(under \-Dquadmath\)

1000000000\.1234499999999999999999999568211720247320

which was \(using truncation\) being converted into the two sec/nsec
longs&#8203;:

    1000000000\, 123449999

After this commit\, it instead correctly gets converted to

    1000000000\, 123450000

M dist/Time-HiRes/HiRes.xs
M dist/Time-HiRes/t/utime.t

--
O Unicef Clearasil!
Gibberish and Drivel!
-- "Bored of the Rings"

1 similar comment
@p5pRT
Copy link
Author

p5pRT commented Aug 3, 2018

From @sisyphus

< I haven't read this thread closely, but is this same issue that I
produced a patch for back in April (but didn't apply because we were in
code freeze)?

Seems very likely to me that the answer is "yes".

Could the patch be applied now, so that we can try it out ?

Cheers,
Rob

On Fri, Aug 3, 2018 at 11​:29 PM, Dave Mitchell <davem@​iabyn.com> wrote​:

On Fri, Aug 03, 2018 at 06​:12​:31AM -0700, sisyphus@​cpan.org via RT wrote​:

On Wed, 18 Jul 2018 17​:46​:19 -0700, jkeenan wrote​:
I see plenty of evidence that the problem still persists. Was this
something that Jarkko was working on prior to the release of
perl-5.28.0, before he ran out of time and/or energy ?

I haven't read this thread closely, but is this same issue that I produced
a patch for back in April (but didn't apply because we were in code
freeze)?

commit 3e15af2
(origin/smoke-me/davem/time_hires_nsec, time_hires_nsec)
Author​: David Mitchell <davem@​iabyn.com>
AuthorDate​: Fri Apr 27 12​:43​:44 2018 +0100
Commit​: David Mitchell <davem@​iabyn.com>
CommitDate​: Fri Apr 27 14​:21​:28 2018 +0100

time&#8203;::HiRes&#8203;: don't truncate nanosec utime

When passed a floating point atime/mtime value\, T&#8203;::HR&#8203;::utime\(\)
was converting it into two longs&#8203;: secs and nsec\. But the nanosec value
was calculated using a final NV to long cast\, which truncates any
fractional part rather than rounding to nearest\. Use a 0\.5 addition to
force rounding\.

This was manifesting as a test in lib/File/Copy\.t failing to preserve
the same mtime after a couple of round trips with utime\(\) and stat\(\)\.

In particular\, the test was attempting to set an mtime to the literal
floating\-point value

    1000000000\.12345

This value can't be represented exactly as an NV\, so was actually
\(under \-Dquadmath\)

1000000000\.1234499999999999999999999568211720247320

which was \(using truncation\) being converted into the two sec/nsec
longs&#8203;:

    1000000000\, 123449999

After this commit\, it instead correctly gets converted to

    1000000000\, 123450000

M dist/Time-HiRes/HiRes.xs
M dist/Time-HiRes/t/utime.t

--
O Unicef Clearasil!
Gibberish and Drivel!
-- "Bored of the Rings"

@p5pRT
Copy link
Author

p5pRT commented Aug 3, 2018

From @khwilliamson

On 08/03/2018 07​:29 AM, Dave Mitchell wrote​:

On Fri, Aug 03, 2018 at 06​:12​:31AM -0700, sisyphus@​cpan.org via RT wrote​:

On Wed, 18 Jul 2018 17​:46​:19 -0700, jkeenan wrote​:
I see plenty of evidence that the problem still persists. Was this
something that Jarkko was working on prior to the release of
perl-5.28.0, before he ran out of time and/or energy ?

I haven't read this thread closely, but is this same issue that I produced
a patch for back in April (but didn't apply because we were in code
freeze)?

commit 3e15af2 (origin/smoke-me/davem/time_hires_nsec, time_hires_nsec)
Author​: David Mitchell <davem@​iabyn.com>
AuthorDate​: Fri Apr 27 12​:43​:44 2018 +0100
Commit​: David Mitchell <davem@​iabyn.com>
CommitDate​: Fri Apr 27 14​:21​:28 2018 +0100

 time&#8203;::HiRes&#8203;: don't truncate nanosec utime

I have pushed this patch to the smoke report currently running. The
status can be viewed at

http​://perl.develop-help.com/?b=smoke-me%2Fkhw-locale

 When passed a floating point atime/mtime value\, T&#8203;::HR&#8203;::utime\(\)
 was converting it into two longs&#8203;: secs and nsec\. But the nanosec value
 was calculated using a final NV to long cast\, which truncates any
 fractional part rather than rounding to nearest\. Use a 0\.5 addition to
 force rounding\.
 
 This was manifesting as a test in lib/File/Copy\.t failing to preserve
 the same mtime after a couple of round trips with utime\(\) and stat\(\)\.
 
 In particular\, the test was attempting to set an mtime to the literal
 floating\-point value
 
     1000000000\.12345
 
 This value can't be represented exactly as an NV\, so was actually
 \(under \-Dquadmath\)
 
 1000000000\.1234499999999999999999999568211720247320
 
 which was \(using truncation\) being converted into the two sec/nsec
 longs&#8203;:
 
     1000000000\, 123449999
 
 After this commit\, it instead correctly gets converted to
 
     1000000000\, 123450000

M dist/Time-HiRes/HiRes.xs
M dist/Time-HiRes/t/utime.t

1 similar comment
@p5pRT
Copy link
Author

p5pRT commented Aug 3, 2018

From @khwilliamson

On 08/03/2018 07​:29 AM, Dave Mitchell wrote​:

On Fri, Aug 03, 2018 at 06​:12​:31AM -0700, sisyphus@​cpan.org via RT wrote​:

On Wed, 18 Jul 2018 17​:46​:19 -0700, jkeenan wrote​:
I see plenty of evidence that the problem still persists. Was this
something that Jarkko was working on prior to the release of
perl-5.28.0, before he ran out of time and/or energy ?

I haven't read this thread closely, but is this same issue that I produced
a patch for back in April (but didn't apply because we were in code
freeze)?

commit 3e15af2 (origin/smoke-me/davem/time_hires_nsec, time_hires_nsec)
Author​: David Mitchell <davem@​iabyn.com>
AuthorDate​: Fri Apr 27 12​:43​:44 2018 +0100
Commit​: David Mitchell <davem@​iabyn.com>
CommitDate​: Fri Apr 27 14​:21​:28 2018 +0100

 time&#8203;::HiRes&#8203;: don't truncate nanosec utime

I have pushed this patch to the smoke report currently running. The
status can be viewed at

http​://perl.develop-help.com/?b=smoke-me%2Fkhw-locale

 When passed a floating point atime/mtime value\, T&#8203;::HR&#8203;::utime\(\)
 was converting it into two longs&#8203;: secs and nsec\. But the nanosec value
 was calculated using a final NV to long cast\, which truncates any
 fractional part rather than rounding to nearest\. Use a 0\.5 addition to
 force rounding\.
 
 This was manifesting as a test in lib/File/Copy\.t failing to preserve
 the same mtime after a couple of round trips with utime\(\) and stat\(\)\.
 
 In particular\, the test was attempting to set an mtime to the literal
 floating\-point value
 
     1000000000\.12345
 
 This value can't be represented exactly as an NV\, so was actually
 \(under \-Dquadmath\)
 
 1000000000\.1234499999999999999999999568211720247320
 
 which was \(using truncation\) being converted into the two sec/nsec
 longs&#8203;:
 
     1000000000\, 123449999
 
 After this commit\, it instead correctly gets converted to
 
     1000000000\, 123450000

M dist/Time-HiRes/HiRes.xs
M dist/Time-HiRes/t/utime.t

@p5pRT
Copy link
Author

p5pRT commented Aug 4, 2018

From @khwilliamson

On 08/03/2018 09​:05 AM, Karl Williamson wrote​:

On 08/03/2018 07​:29 AM, Dave Mitchell wrote​:

On Fri, Aug 03, 2018 at 06​:12​:31AM -0700, sisyphus@​cpan.org via RT wrote​:

On Wed, 18 Jul 2018 17​:46​:19 -0700, jkeenan wrote​:
I see plenty of evidence that the problem still persists.  Was this
something that Jarkko was working on prior to the release of
perl-5.28.0, before he ran out of time and/or energy ?

I haven't read this thread closely, but is this same issue that I
produced
a patch for back in April (but didn't apply because we were in code
freeze)?

commit 3e15af2
(origin/smoke-me/davem/time_hires_nsec, time_hires_nsec)
Author​:     David Mitchell <davem@​iabyn.com>
AuthorDate​: Fri Apr 27 12​:43​:44 2018 +0100
Commit​:     David Mitchell <davem@​iabyn.com>
CommitDate​: Fri Apr 27 14​:21​:28 2018 +0100

     time​::HiRes​: don't truncate nanosec utime

I have pushed this patch to the smoke report currently running.  The
status can be viewed at

http​://perl.develop-help.com/?b=smoke-me%2Fkhw-locale

The smokes are coming out much better now. But there are still some
HiRes failures on some Linuxes, and some apparent unrelated failures
that I don't think there are tickets for.

And there's a small problem with Rob's patch,

proto.h​:4332​:11​: warning​: 'S_mulexp10' declared 'static' but never
defined [-Wunused-function]

There needs to be a #ifdef in embed.fnc to fix this warning

     When passed a floating point atime/mtime value, T​::HR​::utime()
     was converting it into two longs​: secs and nsec. But the nanosec
value
     was calculated using a final NV to long cast, which truncates any
     fractional part rather than rounding to nearest. Use a 0.5
addition to
     force rounding.
     This was manifesting as a test in lib/File/Copy.t failing to
preserve
     the same mtime after a couple of round trips with utime() and
stat().
     In particular, the test was attempting to set an mtime to the
literal
     floating-point value
         1000000000.12345
     This value can't be represented exactly as an NV, so was actually
     (under -Dquadmath)
     1000000000.1234499999999999999999999568211720247320
     which was (using truncation) being converted into the two sec/nsec
     longs​:
         1000000000, 123449999
     After this commit, it instead correctly gets converted to
         1000000000, 123450000

M       dist/Time-HiRes/HiRes.xs
M       dist/Time-HiRes/t/utime.t

1 similar comment
@p5pRT
Copy link
Author

p5pRT commented Aug 4, 2018

From @khwilliamson

On 08/03/2018 09​:05 AM, Karl Williamson wrote​:

On 08/03/2018 07​:29 AM, Dave Mitchell wrote​:

On Fri, Aug 03, 2018 at 06​:12​:31AM -0700, sisyphus@​cpan.org via RT wrote​:

On Wed, 18 Jul 2018 17​:46​:19 -0700, jkeenan wrote​:
I see plenty of evidence that the problem still persists.  Was this
something that Jarkko was working on prior to the release of
perl-5.28.0, before he ran out of time and/or energy ?

I haven't read this thread closely, but is this same issue that I
produced
a patch for back in April (but didn't apply because we were in code
freeze)?

commit 3e15af2
(origin/smoke-me/davem/time_hires_nsec, time_hires_nsec)
Author​:     David Mitchell <davem@​iabyn.com>
AuthorDate​: Fri Apr 27 12​:43​:44 2018 +0100
Commit​:     David Mitchell <davem@​iabyn.com>
CommitDate​: Fri Apr 27 14​:21​:28 2018 +0100

     time​::HiRes​: don't truncate nanosec utime

I have pushed this patch to the smoke report currently running.  The
status can be viewed at

http​://perl.develop-help.com/?b=smoke-me%2Fkhw-locale

The smokes are coming out much better now. But there are still some
HiRes failures on some Linuxes, and some apparent unrelated failures
that I don't think there are tickets for.

And there's a small problem with Rob's patch,

proto.h​:4332​:11​: warning​: 'S_mulexp10' declared 'static' but never
defined [-Wunused-function]

There needs to be a #ifdef in embed.fnc to fix this warning

     When passed a floating point atime/mtime value, T​::HR​::utime()
     was converting it into two longs​: secs and nsec. But the nanosec
value
     was calculated using a final NV to long cast, which truncates any
     fractional part rather than rounding to nearest. Use a 0.5
addition to
     force rounding.
     This was manifesting as a test in lib/File/Copy.t failing to
preserve
     the same mtime after a couple of round trips with utime() and
stat().
     In particular, the test was attempting to set an mtime to the
literal
     floating-point value
         1000000000.12345
     This value can't be represented exactly as an NV, so was actually
     (under -Dquadmath)
     1000000000.1234499999999999999999999568211720247320
     which was (using truncation) being converted into the two sec/nsec
     longs​:
         1000000000, 123449999
     After this commit, it instead correctly gets converted to
         1000000000, 123450000

M       dist/Time-HiRes/HiRes.xs
M       dist/Time-HiRes/t/utime.t

@p5pRT
Copy link
Author

p5pRT commented Apr 18, 2019

From @khwilliamson

On Fri, 03 Aug 2018 21​:56​:47 -0700, public@​khwilliamson.com wrote​:

On 08/03/2018 09​:05 AM, Karl Williamson wrote​:

On 08/03/2018 07​:29 AM, Dave Mitchell wrote​:

On Fri, Aug 03, 2018 at 06​:12​:31AM -0700, sisyphus@​cpan.org via RT wrote​:

On Wed, 18 Jul 2018 17​:46​:19 -0700, jkeenan wrote​:
I see plenty of evidence that the problem still persists.  Was this
something that Jarkko was working on prior to the release of
perl-5.28.0, before he ran out of time and/or energy ?

I haven't read this thread closely, but is this same issue that I
produced
a patch for back in April (but didn't apply because we were in code
freeze)?

commit 3e15af2
(origin/smoke-me/davem/time_hires_nsec, time_hires_nsec)
Author​:     David Mitchell <davem@​iabyn.com>
AuthorDate​: Fri Apr 27 12​:43​:44 2018 +0100
Commit​:     David Mitchell <davem@​iabyn.com>
CommitDate​: Fri Apr 27 14​:21​:28 2018 +0100

     time​::HiRes​: don't truncate nanosec utime

I have pushed this patch to the smoke report currently running.  The
status can be viewed at

http​://perl.develop-help.com/?b=smoke-me%2Fkhw-locale

The smokes are coming out much better now. But there are still some
HiRes failures on some Linuxes, and some apparent unrelated failures
that I don't think there are tickets for.

And there's a small problem with Rob's patch,

proto.h​:4332​:11​: warning​: 'S_mulexp10' declared 'static' but never
defined [-Wunused-function]

There needs to be a #ifdef in embed.fnc to fix this warning

     When passed a floating point atime/mtime value, T​::HR​::utime()
     was converting it into two longs​: secs and nsec. But the nanosec
value
     was calculated using a final NV to long cast, which truncates any
     fractional part rather than rounding to nearest. Use a 0.5
addition to
     force rounding.
     This was manifesting as a test in lib/File/Copy.t failing to
preserve
     the same mtime after a couple of round trips with utime() and
stat().
     In particular, the test was attempting to set an mtime to the
literal
     floating-point value
         1000000000.12345
     This value can't be represented exactly as an NV, so was actually
     (under -Dquadmath)
     1000000000.1234499999999999999999999568211720247320
     which was (using truncation) being converted into the two sec/nsec
     longs​:
         1000000000, 123449999
     After this commit, it instead correctly gets converted to
         1000000000, 123450000

M       dist/Time-HiRes/HiRes.xs
M       dist/Time-HiRes/t/utime.t

Is this ticket closable?
--
Karl Williamson

@p5pRT
Copy link
Author

p5pRT commented Apr 18, 2019

From @jkeenan

On Thu, 18 Apr 2019 14​:57​:13 GMT, khw wrote​:

On Fri, 03 Aug 2018 21​:56​:47 -0700, public@​khwilliamson.com wrote​:

On 08/03/2018 09​:05 AM, Karl Williamson wrote​:

On 08/03/2018 07​:29 AM, Dave Mitchell wrote​:

On Fri, Aug 03, 2018 at 06​:12​:31AM -0700, sisyphus@​cpan.org via RT
wrote​:

On Wed, 18 Jul 2018 17​:46​:19 -0700, jkeenan wrote​:
I see plenty of evidence that the problem still persists.  Was
this
something that Jarkko was working on prior to the release of
perl-5.28.0, before he ran out of time and/or energy ?

I haven't read this thread closely, but is this same issue that I
produced
a patch for back in April (but didn't apply because we were in
code
freeze)?

commit 3e15af2
(origin/smoke-me/davem/time_hires_nsec, time_hires_nsec)
Author​:     David Mitchell <davem@​iabyn.com>
AuthorDate​: Fri Apr 27 12​:43​:44 2018 +0100
Commit​:     David Mitchell <davem@​iabyn.com>
CommitDate​: Fri Apr 27 14​:21​:28 2018 +0100

     time​::HiRes​: don't truncate nanosec utime

I have pushed this patch to the smoke report currently running. 
The
status can be viewed at

http​://perl.develop-help.com/?b=smoke-me%2Fkhw-locale

The smokes are coming out much better now. But there are still some
HiRes failures on some Linuxes, and some apparent unrelated failures
that I don't think there are tickets for.

And there's a small problem with Rob's patch,

proto.h​:4332​:11​: warning​: 'S_mulexp10' declared 'static' but never
defined [-Wunused-function]

There needs to be a #ifdef in embed.fnc to fix this warning

     When passed a floating point atime/mtime value,
T​::HR​::utime()
     was converting it into two longs​: secs and nsec. But the
nanosec
value
     was calculated using a final NV to long cast, which truncates
any
     fractional part rather than rounding to nearest. Use a 0.5
addition to
     force rounding.
     This was manifesting as a test in lib/File/Copy.t failing to
preserve
     the same mtime after a couple of round trips with utime()
and
stat().
     In particular, the test was attempting to set an mtime to
the
literal
     floating-point value
         1000000000.12345
     This value can't be represented exactly as an NV, so was
actually
     (under -Dquadmath)
     1000000000.1234499999999999999999999568211720247320
     which was (using truncation) being converted into the two
sec/nsec
     longs​:
         1000000000, 123449999
     After this commit, it instead correctly gets converted to
         1000000000, 123450000

M       dist/Time-HiRes/HiRes.xs
M       dist/Time-HiRes/t/utime.t

Is this ticket closable?

Just now I built blead v5.29.9-151-g43049430d1 on Linux with -Dusequadmath. I got no errors in lib/File/Copy.t. So the original complaint has been satisfied. (A lot else got discussed in this ticket, though. And with quadmath there are known failures in t/re/uniprops02.t -- but that's outside the scope of this RT.) So, by me at least, it's closable.

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

@p5pRT
Copy link
Author

p5pRT commented Aug 20, 2019

From @jkeenan

On Thu, 18 Apr 2019 22​:11​:42 GMT, jkeenan wrote​:

On Thu, 18 Apr 2019 14​:57​:13 GMT, khw wrote​:

On Fri, 03 Aug 2018 21​:56​:47 -0700, public@​khwilliamson.com wrote​:

On 08/03/2018 09​:05 AM, Karl Williamson wrote​:

On 08/03/2018 07​:29 AM, Dave Mitchell wrote​:

On Fri, Aug 03, 2018 at 06​:12​:31AM -0700, sisyphus@​cpan.org via
RT
wrote​:

On Wed, 18 Jul 2018 17​:46​:19 -0700, jkeenan wrote​:
I see plenty of evidence that the problem still persists.  Was
this
something that Jarkko was working on prior to the release of
perl-5.28.0, before he ran out of time and/or energy ?

I haven't read this thread closely, but is this same issue that
I
produced
a patch for back in April (but didn't apply because we were in
code
freeze)?

commit 3e15af2
(origin/smoke-me/davem/time_hires_nsec, time_hires_nsec)
Author​:     David Mitchell <davem@​iabyn.com>
AuthorDate​: Fri Apr 27 12​:43​:44 2018 +0100
Commit​:     David Mitchell <davem@​iabyn.com>
CommitDate​: Fri Apr 27 14​:21​:28 2018 +0100

     time​::HiRes​: don't truncate nanosec utime

I have pushed this patch to the smoke report currently running.
The
status can be viewed at

http​://perl.develop-help.com/?b=smoke-me%2Fkhw-locale

The smokes are coming out much better now. But there are still
some
HiRes failures on some Linuxes, and some apparent unrelated
failures
that I don't think there are tickets for.

And there's a small problem with Rob's patch,

proto.h​:4332​:11​: warning​: 'S_mulexp10' declared 'static' but never
defined [-Wunused-function]

There needs to be a #ifdef in embed.fnc to fix this warning

     When passed a floating point atime/mtime value,
T​::HR​::utime()
     was converting it into two longs​: secs and nsec. But the
nanosec
value
     was calculated using a final NV to long cast, which
truncates
any
     fractional part rather than rounding to nearest. Use a 0.5
addition to
     force rounding.
     This was manifesting as a test in lib/File/Copy.t failing
to
preserve
     the same mtime after a couple of round trips with utime()
and
stat().
     In particular, the test was attempting to set an mtime to
the
literal
     floating-point value
         1000000000.12345
     This value can't be represented exactly as an NV, so was
actually
     (under -Dquadmath)
     1000000000.1234499999999999999999999568211720247320
     which was (using truncation) being converted into the two
sec/nsec
     longs​:
         1000000000, 123449999
     After this commit, it instead correctly gets converted to
         1000000000, 123450000

M       dist/Time-HiRes/HiRes.xs
M       dist/Time-HiRes/t/utime.t

Is this ticket closable?

Just now I built blead v5.29.9-151-g43049430d1 on Linux with
-Dusequadmath. I got no errors in lib/File/Copy.t. So the original
complaint has been satisfied. (A lot else got discussed in this
ticket, though. And with quadmath there are known failures in
t/re/uniprops02.t -- but that's outside the scope of this RT.) So, by
me at least, it's closable.

Thank you very much.

No one has spoken in favor of keeping this ticket open, so I'm closing it.

In the course of this ticket there was a branch created pertaining to Time​::HiRes. However, that module has moved on since that time and the branch will no longer cleanly merge into blead. So any problems with Time​::HiRes should go into a new RT.

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

@p5pRT
Copy link
Author

p5pRT commented Aug 20, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant