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

POSIX::strftime: modified behavior surprises users #22351

Closed
jkeenan opened this issue Jun 27, 2024 · 2 comments
Closed

POSIX::strftime: modified behavior surprises users #22351

jkeenan opened this issue Jun 27, 2024 · 2 comments

Comments

@jkeenan
Copy link
Contributor

jkeenan commented Jun 27, 2024

Our heavy-duty smoke-tester Carlos (@cjg-cguevara) has contacted me about an anomaly on blead perl with the following program:

#!/usr/bin/env perl

use strict;
use warnings;

use POSIX qw(strftime);

print "$^V\n";
print `date '+%s'`;
print strftime('%s', localtime), "\n";

Run it at perl-5.40.0, you get:

$ perl -v | head -2 | tail -1
This is perl 5, version 40, subversion 0 (v5.40.0) built for x86_64-linux

$ perl cjg-foo.pl 
v5.40.0
1719498710
1719498710

Run it at HEAD of blead, you get:

$ bleadperl -v | head -2 | tail -1
This is perl 5, version 41, subversion 1 (v5.41.1 (v5.41.0-106-g99e7291a16)) built for x86_64-linux

$ bleadperl cjg-foo.pl 
v5.41.1
1719498720
1719502320

I adapted his progam to be suitable for bisection:

$ cat strftime-problem.pl 
#!/usr/bin/env perl

use strict;
use warnings;

use POSIX qw(strftime);

my $linux_date = `date '+%s'`;
chomp $linux_date;
my $posix_strftime = strftime('%s', localtime);
my $perl_time = time;

die 'POSIX::strftime anomaly' if (
    ($posix_strftime != $linux_date) 
    ||
    ($posix_strftime != $perl_time) 
);

$ perl strftime-problem.pl 

$ bleadperl strftime-problem.pl 
POSIX::strftime anomaly at strftime-problem.pl line 13.

Bisection pointed to commit 86a9c18:

86a9c18b6fab1949a26de790418b8b897a71e4ac is the first bad commit
commit 86a9c18b6fab1949a26de790418b8b897a71e4ac
Author: Karl Williamson <khw@cpan.org>
Date:   Sun Jun 16 10:17:11 2024 -0600
Commit:     Karl Williamson <khw@cpan.org>
CommitDate: Wed Jun 19 19:50:39 2024 -0600

    Fix up and make sv_strftime_ints() public
    
    This enhanced function now allows you to specify if you want the system
    to consider the possibility of daylight savings time being in effect.
    Formerly, it was never considered.  As a result, the function is good
    enough to be made public.

If this is a change in the behavior of POSIX::strftime to which we are committed, then (a) we should prepare for a log of complaints starting with the release of perl-5.41.1 (which I hope will be soon); and (b) we will have to figure out how to educate the general perl-using public about that change in behavior.

@khwilliamson and others, please comment. Thanks!

@khwilliamson
Copy link
Contributor

khwilliamson commented Jun 27, 2024 via email

khwilliamson added a commit to khwilliamson/perl5 that referenced this issue Jul 1, 2024
This fixes GH Perl#22351

The new sv_strftime_ints() API function acts consistently with regards
to daylight savings time, with POSIX-mandated behavior, which takes the
current locale into account.

POSIX::strftime() is problematic in regard to this.  This commit adds a
backwards compatibility mode to preserve its behavior that inadvertently
was changed by 86a9c18.

These functions are intended to wrap libc strftime(), including
normalizing the input values.  For example, if you pass 63 seconds as a
value, they will typically change that to be 3 seconds into the next
minute up.  In C, the mktime() function is typically called first to do
that normalization.  (I don't know what happens if plain strftime() is
called with unnormalized values.).  mktime() looks at the current
locale, determines if daylight savings time is in effect, and adjusts
accordingly.  Perl calls its own mktime-equivalent for you, eliminating
the need for explicitly calling something that would need to always be
called anyway, hence saving an extra step.

mini_mktime() is the Perl mktime-equivalent.  It is unaffected by
locales, and does not consider the possibility of there being daylight
savings time.  The problem is that libc strftime() is affected by
locale, and does consider dst.  Perl uses these two functions together,
and this inconsistency between them is bound to cause problems.

The 'isdst' parameter to POSIX::strftime() is used to indicate if
daylight savings is in effect for the time and date given by the other
parameters.  If perl used mktime() to normalize those values, it would
calculate this for itself, using the passed-in value only as a hint.
But since it uses mini_mktime(), it has to take that value as-is.
Thus, daylight savings alone, out of all the input values, is not
normalized.  This is contrary to the documentation, and I didn't know
this when I wrote the blamed commit.

It turns out that typical uses of this function, like

    POSIX::strftime('%s', localtime)
    POSIX::strftime('%s', gmtime)

cause the correct 'isdst' to be passed.  But this is a defect in the
interface that can bite you; changing it would cause cpan breakage.

I wrote the API function sv_strftime_ints() to not have these issues.
And, to workaround a defect in the POSIX definition of mktime().  It
turns out you can't tell it you don't want to adjust for dayight time,
except by changing the locale to one that doesn't have dst, such as the
"C" locale.  But this isn't a Perl-level function.
jkeenan added a commit to jkeenan/perl5 that referenced this issue Jul 1, 2024
khwilliamson added a commit to khwilliamson/perl5 that referenced this issue Jul 1, 2024
This fixes GH Perl#22351

The new sv_strftime_ints() API function acts consistently with regards
to daylight savings time, with POSIX-mandated behavior, which takes the
current locale into account.

POSIX::strftime() is problematic in regard to this.  This commit adds a
backwards compatibility mode to preserve its behavior that inadvertently
was changed by 86a9c18.

These functions are intended to wrap libc strftime(), including
normalizing the input values.  For example, if you pass 63 seconds as a
value, they will typically change that to be 3 seconds into the next
minute up.  In C, the mktime() function is typically called first to do
that normalization.  (I don't know what happens if plain strftime() is
called with unnormalized values.).  mktime() looks at the current
locale, determines if daylight savings time is in effect, and adjusts
accordingly.  Perl calls its own mktime-equivalent for you, eliminating
the need for explicitly calling something that would need to always be
called anyway, hence saving an extra step.

mini_mktime() is the Perl mktime-equivalent.  It is unaffected by
locales, and does not consider the possibility of there being daylight
savings time.  The problem is that libc strftime() is affected by
locale, and does consider dst.  Perl uses these two functions together,
and this inconsistency between them is bound to cause problems.

The 'isdst' parameter to POSIX::strftime() is used to indicate if
daylight savings is in effect for the time and date given by the other
parameters.  If perl used mktime() to normalize those values, it would
calculate this for itself, using the passed-in value only as a hint.
But since it uses mini_mktime(), it has to take that value as-is.
Thus, daylight savings alone, out of all the input values, is not
normalized.  This is contrary to the documentation, and I didn't know
this when I wrote the blamed commit.

It turns out that typical uses of this function, like

    POSIX::strftime('%s', localtime)
    POSIX::strftime('%s', gmtime)

cause the correct 'isdst' to be passed.  But this is a defect in the
interface that can bite you; changing it would cause cpan breakage.

I wrote the API function sv_strftime_ints() to not have these issues.
And, to workaround a defect in the POSIX definition of mktime().  It
turns out you can't tell it you don't want to adjust for dayight time,
except by changing the locale to one that doesn't have dst, such as the
"C" locale.  But this isn't a Perl-level function.
khwilliamson added a commit to khwilliamson/perl5 that referenced this issue Jul 1, 2024
This fixes GH Perl#22351

The new sv_strftime_ints() API function acts consistently with regards
to daylight savings time, with POSIX-mandated behavior, which takes the
current locale into account.

POSIX::strftime() is problematic in regard to this.  This commit adds a
backwards compatibility mode to preserve its behavior that inadvertently
was changed by 86a9c18.

These functions are intended to wrap libc strftime(), including
normalizing the input values.  For example, if you pass 63 seconds as a
value, they will typically change that to be 3 seconds into the next
minute up.  In C, the mktime() function is typically called first to do
that normalization.  (I don't know what happens if plain strftime() is
called with unnormalized values.).  mktime() looks at the current
locale, determines if daylight savings time is in effect, and adjusts
accordingly.  Perl calls its own mktime-equivalent for you, eliminating
the need for explicitly calling something that would need to always be
called anyway, hence saving an extra step.

mini_mktime() is the Perl mktime-equivalent.  It is unaffected by
locales, and does not consider the possibility of there being daylight
savings time.  The problem is that libc strftime() is affected by
locale, and does consider dst.  Perl uses these two functions together,
and this inconsistency between them is bound to cause problems.

The 'isdst' parameter to POSIX::strftime() is used to indicate if
daylight savings is in effect for the time and date given by the other
parameters.  If perl used mktime() to normalize those values, it would
calculate this for itself, using the passed-in value only as a hint.
But since it uses mini_mktime(), it has to take that value as-is.
Thus, daylight savings alone, out of all the input values, is not
normalized.  This is contrary to the documentation, and I didn't know
this when I wrote the blamed commit.

It turns out that typical uses of this function, like

    POSIX::strftime('%s', localtime)
    POSIX::strftime('%s', gmtime)

cause the correct 'isdst' to be passed.  But this is a defect in the
interface that can bite you; changing it would cause cpan breakage.

I wrote the API function sv_strftime_ints() to not have these issues.
And, to workaround a defect in the POSIX definition of mktime().  It
turns out you can't tell it you don't want to adjust for dayight time,
except by changing the locale to one that doesn't have dst, such as the
"C" locale.  But this isn't a Perl-level function.
khwilliamson added a commit to khwilliamson/perl5 that referenced this issue Jul 1, 2024
This fixes GH Perl#22351

The new sv_strftime_ints() API function acts consistently with regards
to daylight savings time, with POSIX-mandated behavior, which takes the
current locale into account.

POSIX::strftime() is problematic in regard to this.  This commit adds a
backwards compatibility mode to preserve its behavior that inadvertently
was changed by 86a9c18.

These functions are intended to wrap libc strftime(), including
normalizing the input values.  For example, if you pass 63 seconds as a
value, they will typically change that to be 3 seconds into the next
minute up.  In C, the mktime() function is typically called first to do
that normalization.  (I don't know what happens if plain strftime() is
called with unnormalized values.).  mktime() looks at the current
locale, determines if daylight savings time is in effect, and adjusts
accordingly.  Perl calls its own mktime-equivalent for you, eliminating
the need for explicitly calling something that would need to always be
called anyway, hence saving an extra step.

mini_mktime() is the Perl mktime-equivalent.  It is unaffected by
locales, and does not consider the possibility of there being daylight
savings time.  The problem is that libc strftime() is affected by
locale, and does consider dst.  Perl uses these two functions together,
and this inconsistency between them is bound to cause problems.

The 'isdst' parameter to POSIX::strftime() is used to indicate if
daylight savings is in effect for the time and date given by the other
parameters.  If perl used mktime() to normalize those values, it would
calculate this for itself, using the passed-in value only as a hint.
But since it uses mini_mktime(), it has to take that value as-is.
Thus, daylight savings alone, out of all the input values, is not
normalized.  This is contrary to the documentation, and I didn't know
this when I wrote the blamed commit.

It turns out that typical uses of this function, like

    POSIX::strftime('%s', localtime)
    POSIX::strftime('%s', gmtime)

cause the correct 'isdst' to be passed.  But this is a defect in the
interface that can bite you; changing it would cause cpan breakage.

I wrote the API function sv_strftime_ints() to not have these issues.
And, to workaround a defect in the POSIX definition of mktime().  It
turns out you can't tell it you don't want to adjust for dayight time,
except by changing the locale to one that doesn't have dst, such as the
"C" locale.  But this isn't a Perl-level function.
khwilliamson added a commit that referenced this issue Jul 2, 2024
This fixes GH #22351

The new sv_strftime_ints() API function acts consistently with regards
to daylight savings time, with POSIX-mandated behavior, which takes the
current locale into account.

POSIX::strftime() is problematic in regard to this.  This commit adds a
backwards compatibility mode to preserve its behavior that inadvertently
was changed by 86a9c18.

These functions are intended to wrap libc strftime(), including
normalizing the input values.  For example, if you pass 63 seconds as a
value, they will typically change that to be 3 seconds into the next
minute up.  In C, the mktime() function is typically called first to do
that normalization.  (I don't know what happens if plain strftime() is
called with unnormalized values.).  mktime() looks at the current
locale, determines if daylight savings time is in effect, and adjusts
accordingly.  Perl calls its own mktime-equivalent for you, eliminating
the need for explicitly calling something that would need to always be
called anyway, hence saving an extra step.

mini_mktime() is the Perl mktime-equivalent.  It is unaffected by
locales, and does not consider the possibility of there being daylight
savings time.  The problem is that libc strftime() is affected by
locale, and does consider dst.  Perl uses these two functions together,
and this inconsistency between them is bound to cause problems.

The 'isdst' parameter to POSIX::strftime() is used to indicate if
daylight savings is in effect for the time and date given by the other
parameters.  If perl used mktime() to normalize those values, it would
calculate this for itself, using the passed-in value only as a hint.
But since it uses mini_mktime(), it has to take that value as-is.
Thus, daylight savings alone, out of all the input values, is not
normalized.  This is contrary to the documentation, and I didn't know
this when I wrote the blamed commit.

It turns out that typical uses of this function, like

    POSIX::strftime('%s', localtime)
    POSIX::strftime('%s', gmtime)

cause the correct 'isdst' to be passed.  But this is a defect in the
interface that can bite you; changing it would cause cpan breakage.

I wrote the API function sv_strftime_ints() to not have these issues.
And, to workaround a defect in the POSIX definition of mktime().  It
turns out you can't tell it you don't want to adjust for dayight time,
except by changing the locale to one that doesn't have dst, such as the
"C" locale.  But this isn't a Perl-level function.
@jkeenan
Copy link
Contributor Author

jkeenan commented Jul 6, 2024

Fixed by #22369, 4c99243

@jkeenan jkeenan closed this as completed Jul 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants