Bug #105159 for Date-Manip: [Patch] Better heuristic (maybe) for Mac OS X in Brazil

Wed Jun 10 13:48:17 2015 wyant [...] cpan.org - Ticket created

Subject:

[Patch] Better heuristic (maybe) for Mac OS X in Brazil

Under Mac OS X (uname output appended) Date::Manip seems to end up deriving the Olson zone by a lookup on the results of 'date +%Z'. In Sao Paulo in June, this gives 'BRT'. Given the structure of the lookup table, this ends up as 'America/Araguaina'. This is fine as far as it goes, but Araguaina is up near the Equator, and does not do summer time. This is OK for times near the current time, but times in (say) January) parse an hour off. Now, it appears to me that the look-up table for 'BRST' has been juggled to put Sao Paulo at the top. The attached patch performs the same juggling act for 'BRT'. I am a GitHub neophyte, but will generate a pull request as soon as I can figure out how. Note that this does not truly fix the problem, it just makes the results consistent, and moves the locations where the default parse is incorrect from a more-populous area to a less-populous one. What is really needed is a way to derive the Olson zone directly. DateTime::TimeZone does it (at least, under Mac OS X) by looking for symbolic link /etc/localtime and deriving the name of the Olson zone from the name of the file the link points to. After I get the pull request for this ticket generated, I will try to come up with an enhancement to add this functionality to Date::Manip. $ uname -a Darwin Hairy-Toes.local 14.3.0 Darwin Kernel Version 14.3.0: Mon Mar 23 11:59:05 PDT 2015; root:xnu-2782.20.48~5/RELEASE_X86_64 x86_64 $ perl -v This is perl 5, version 22, subversion 0 (v5.22.0) built for darwin-2level ... $ perl -MDate::Manip -E 'say Date::Manip->VERSION' 6.50 The patch is against commit cdc16eeeb1e2688d6fb025490747da518caec27f

Subject:

Date-Manip.patch

diff --git a/lib/Date/Manip/Zones.pm b/lib/Date/Manip/Zones.pm index 707289f..433ca9f 100644 --- a/lib/Date/Manip/Zones.pm +++ b/lib/Date/Manip/Zones.pm @@ -1901,13 +1901,13 @@ $LastYear = '2065'; 'america/maceio', 'america/recife', 'america/belem' ], - 'brt' => [ 'america/araguaina', + 'brt' => [ 'america/sao_paulo', + 'america/araguaina', 'america/bahia', - 'america/belem', 'america/fortaleza', 'america/maceio', 'america/recife', - 'america/sao_paulo', + 'america/belem', 'america/santarem' ], 'bst' => [ 'europe/london', 'america/adak', diff --git a/t/tz.define_abbrev.t b/t/tz.define_abbrev.t index a38475b..4641c28 100755 --- a/t/tz.define_abbrev.t +++ b/t/tz.define_abbrev.t @@ -26,13 +26,13 @@ $obj->config("forcedate","now,America/New_York"); $tests=" BRT reset => + America/Sao_Paulo America/Araguaina America/Bahia - America/Belem America/Fortaleza America/Maceio America/Recife - America/Sao_Paulo + America/Belem America/Santarem BRT

Wed Jun 10 14:00:04 2015 wyant [...] cpan.org - Correspondence added

OK, the pull request is generated.

Wed Jun 10 14:12:35 2015 sbeck [...] cpan.org - Correspondence added

I will consider reordering the order of the timezones... but you are correct that no matter what the order is, the order will be incorrect for some subset of the population. The real fix is to be able to determine the real timezone in a more accurate way (using the abbreviation from the 'date' command is a last ditch attempt if everything else fails). So what I really need is the way (or ways) that I can better determine the timezone from OS X. You say that the /etc/localtime file is a link pointing to a file from which the timezone can be determined. Can you tell me the name of the file that /etc/localtime is linked to, and it's contents (unless the name of the zone is derived from the actual file name). I'll definitely build that method of determining the timezone into Date::Manip, and that should fix things for you. If there are alternate locations in OS X where the timezone can be determined, please send that information as well. Sorry, I'm not an OS X guy and don't have access to an OS X machine.

Wed Jun 10 14:12:36 2015 The RT System itself - Status changed from 'new' to 'open'

Wed Jun 10 17:07:42 2015 wyant [...] cpan.org - Correspondence added

I am not an OS X expert, nor do I play one on TV. What actually happened was that I noticed that DateTime::TimeZone got the zone correct, and tracked that functionality to DateTime::TimeZone::Local::Unix subroutine FromEtcLocalTime(). Unfortunately it appears from reading that code that /etc/localtime can be either a symbolic link or a copy of one of the files in /usr/share/zoneinfo. If it's a link (as it is under OS X) the link is to a file in /usr/share/zoneinfo that is named after the Olson zone, and the DateTime::TimeZone code just pulls the linked-to path apart and reassembles it right-to-left looking for a valid zone name. But if /etc/localtime is a copy (as the comments indicate it can be under FreeBSD) the code has to do a File::Find::find() on /usr/share/zoneinfo/, looking for a file that is the same as /etc/localtime. Once it finds the matching file it derives the zone name from the file's path name just as it does for the link. Now, I have no idea what the format of the /usr/share/zoneinfo files is. The relevant Wikipedia article seems to say they are text, but mine are binary. And 'hexdump -C' seems to say they do NOT contain the name of the zone they define. Which is too bad, because just reading the file would provide a much better implementation.

Sun Jun 14 10:43:37 2015 wyant [...] cpan.org - Correspondence added

It appears that FreeBSD 10.1 has the same behavior, which gives a way to work on the problem. The only difference for OS X appears to be that /etc/localtime is a link, so that the zone name can be derived by translating the link. Under FreeBSD it is a copy, so you have to grovel through /usr/share/zoneinfo/ looking for an identical file. I have attached three files: date_manip_tz is a Perl one-liner that traces Date::Manip::TZ as it determines the local zone. date_manip_tz.log is the output of date_manip_tz when run on FreeBSD 10.1 under Oracle VirtualBox on my machine. zoneinfo is a Perl script that determines the actual Olson zone under Mac OS 10.10.3, FreeBSD 10.1, and XUbuntu 13.04. I attempted to write this so that the relevant code could simply be lifted into Date::Manip::TZ if you want to go that way. I suspect that it needs to be made into a new zone determination mechanism (say, 'zoneinfo') that takes no parameters and which by default is used as the last mechanism before spawning 'date'.

Subject:

date_manip_tz

Download date_manip_tz
application/octet-stream 92b

Message body not shown because it is not plain text.

Subject:

date_manip_tz.log

Download date_manip_tz.log
application/octet-stream 371b

Message body not shown because it is not plain text.

Subject:

zoneinfo

Download zoneinfo
application/octet-stream 4.3k

Message body not shown because it is not plain text.

Mon Jun 15 15:19:57 2015 wyant [...] cpan.org - Correspondence added

I have made the code in yesterday's 'zoneinfo' script into a patch against Date::Manip::TZ version 6.50. The patch files are attached. The new code makes use of three core modules: Cwd, File::Find, and File::Spec. Because they are core I have not updated the requirements for the module. But because I feel a little guilty about not doing so, I have confessed.

Subject:

Date-Manip-TZ.pm.patch

--- lib/Date/Manip/TZ.pm.old 2015-06-01 10:31:50.000000000 -0300 +++ lib/Date/Manip/TZ.pm 2015-06-15 15:47:59.000000000 -0300 @@ -105,6 +105,7 @@ file /etc/timezone file /etc/sysconfig/clock file /etc/default/init + tzdata ), 'command', '/bin/date +%Z', 'command', '/usr/bin/date +%Z', @@ -558,6 +559,13 @@ } } + } elsif ($method eq 'tzdata' ) { + if ( defined( my $z = _get_zoneinfo_zone() ) ) { + push @zone, $z; + print "] $z\n" if ($debug); + } elsif ($debug) { + print "] no result\n"; + } } elsif ($method eq 'command') { if (! @methods) { print "]\n" if ($debug); @@ -731,6 +739,70 @@ } } +# The folloing is cribbed generally but not verbatim from +# DateTime::TimeZone::Local::Unix. +# + +sub _get_zoneinfo_zone { + my ( $localtime, $zoneinfo ) = @_; + + $localtime ||= '/etc/localtime'; + $zoneinfo ||= '/usr/share/zoneinfo'; + + -d $zoneinfo + and -f $localtime + or return; + + require Cwd; + -l $localtime + and return _zoneinfo_file_name_to_zone( + Cwd::abs_path( $localtime ), + $zoneinfo, + ); + + my $want_content; + my $want_size = -s $localtime; + + local $@ = undef; + eval { + require File::Find; + File::Find::find( sub { + my $zone; + defined( $zone = _zoneinfo_file_name_to_zone( + $File::Find::name, $zoneinfo ) ) + and -f $_ + and $want_size == -s _ + and ( $want_content ||= _zoneinfo_file_slurp( + $localtime ) ) + eq _zoneinfo_file_slurp( $_ ) + and die { zone => $zone }; + }, $zoneinfo, + ); + 1; + } and return; + ref $@ + and return $@->{zone}; + die $@; +} + +sub _zoneinfo_file_name_to_zone { + my ( $fn, $zoneinfo ) = @_; + require File::Spec; + my $zone = File::Spec->abs2rel( $fn, $zoneinfo ); + exists $Date::Manip::Zones::ZoneNames{ lc $zone } + and return $zone; + return; +} + +sub _zoneinfo_file_slurp { + my ( $fn ) = @_; + open my $fh, '<', $fn + or return; + binmode $fh; + local $/ = undef; + return <$fh>; +} + # We will be testing commands that don't exist on all architectures, # so disable warnings. #

Subject:

Date-Manip-TZ.pod.patch

--- lib/Date/Manip/TZ.pod.old 2015-06-01 10:31:50.000000000 -0300 +++ lib/Date/Manip/TZ.pod 2015-06-15 15:55:01.000000000 -0300 @@ -602,6 +602,11 @@ be the first non-blank non-comment line in the file. + tzdata Determine which /usr/share/zoneinfo + file /etc/localtime represents, and + derive the time zone from the name + of that file. + command COMMAND Runs a command which produces a time zone as the output. @@ -634,6 +639,7 @@ file /etc/timezone file /etc/sysconfig/clock file /etc/default/init + tzinfo command "/bin/date +%Z" command "/usr/bin/date +%Z" command "/usr/local/bin/date +%Z"

Sat Jul 04 15:04:10 2015 wyant [...] cpan.org - Correspondence added

It looks like I did not test my patch carefully enough. It determines the zone, but unfortunately the abort from File::Find leaves the process' working directory changed. The fix is to use no_chdir in File::Find::find(). Both patch files are attached even though only the .pm file changed. Sorry about that.

Subject:

Date-Manip-TZ.pm.patch

--- lib/Date/Manip/TZ.pm.old 2015-06-01 09:31:50.000000000 -0400 +++ lib/Date/Manip/TZ.pm 2015-07-04 14:43:52.000000000 -0400 @@ -105,6 +105,7 @@ file /etc/timezone file /etc/sysconfig/clock file /etc/default/init + tzdata ), 'command', '/bin/date +%Z', 'command', '/usr/bin/date +%Z', @@ -558,6 +559,13 @@ } } + } elsif ($method eq 'tzdata' ) { + if ( defined( my $z = _get_zoneinfo_zone() ) ) { + push @zone, $z; + print "] $z\n" if ($debug); + } elsif ($debug) { + print "] no result\n"; + } } elsif ($method eq 'command') { if (! @methods) { print "]\n" if ($debug); @@ -731,6 +739,73 @@ } } +# The folloing is cribbed generally but not verbatim from +# DateTime::TimeZone::Local::Unix. +# + +sub _get_zoneinfo_zone { + my ( $localtime, $zoneinfo ) = @_; + + $localtime ||= '/etc/localtime'; + $zoneinfo ||= '/usr/share/zoneinfo'; + + -d $zoneinfo + and -f $localtime + or return; + + require Cwd; + -l $localtime + and return _zoneinfo_file_name_to_zone( + Cwd::abs_path( $localtime ), + $zoneinfo, + ); + + my $want_content; + my $want_size = -s $localtime; + + local $@ = undef; + eval { + require File::Find; + File::Find::find( { + wanted => sub { + my $zone; + defined( $zone = _zoneinfo_file_name_to_zone( + $File::Find::name, $zoneinfo ) ) + and -f $_ + and $want_size == -s _ + and ( $want_content ||= _zoneinfo_file_slurp( + $localtime ) ) + eq _zoneinfo_file_slurp( $File::Find::name ) + and die { zone => $zone }; + }, + no_chdir => 1, + }, $zoneinfo, + ); + 1; + } and return; + ref $@ + and return $@->{zone}; + die $@; +} + +sub _zoneinfo_file_name_to_zone { + my ( $fn, $zoneinfo ) = @_; + require File::Spec; + my $zone = File::Spec->abs2rel( $fn, $zoneinfo ); + exists $Date::Manip::Zones::ZoneNames{ lc $zone } + and return $zone; + return; +} + +sub _zoneinfo_file_slurp { + my ( $fn ) = @_; + open my $fh, '<', $fn + or return; + binmode $fh; + local $/ = undef; + return <$fh>; +} + # We will be testing commands that don't exist on all architectures, # so disable warnings. #

Subject:

Date-Manip-TZ.pod.patch

--- lib/Date/Manip/TZ.pod.old 2015-06-01 10:31:50.000000000 -0300 +++ lib/Date/Manip/TZ.pod 2015-06-15 15:55:01.000000000 -0300 @@ -602,6 +602,11 @@ be the first non-blank non-comment line in the file. + tzdata Determine which /usr/share/zoneinfo + file /etc/localtime represents, and + derive the time zone from the name + of that file. + command COMMAND Runs a command which produces a time zone as the output. @@ -634,6 +639,7 @@ file /etc/timezone file /etc/sysconfig/clock file /etc/default/init + tzinfo command "/bin/date +%Z" command "/usr/bin/date +%Z" command "/usr/local/bin/date +%Z"

Tue Jul 07 12:43:44 2015 sbeck [...] cpan.org - Correspondence added

Sorry for the slow reply... I've been pretty swamped and am just now making time to get to this. It appears to me that your patch seems reasonable. I'm going to modify it slightly to take the directory where the tzdata files live. i.e. tzdata /usr/share/zoneinfo rather than assuming that they are in /usr/share/zoneinfo . Also, I may actually use two arguements: tzdata /etc/localtime /usr/share/zoneinfo I'm looking into that now...

Tue Jul 07 14:58:48 2015 sbeck [...] cpan.org - Correspondence added

Patch is applied (with some minor modifications to make the coding style a bit more consistent with my normal style). Thanks for the work.

Tue Jul 07 14:58:55 2015 sbeck [...] cpan.org - Status changed from 'open' to 'resolved'

Thu Jul 09 11:54:46 2015 wyant [...] cpan.org - Correspondence added

You are most welcome. Thank you for the opportunity to contribute to such a useful module. Sorry it took three iterations to get something actually usable. Under the circumstances it was a good thing it took you a while to get to it, since the second iteration was badly broken in a fairly subtle way that took me a while to find. I look forward to the release. If you intend a development release, I will be happy to exercise it, though I certainly anticipate no problems. Now if only RT will leave this ticket "resolved."

Thu Jul 09 12:15:53 2015 sbeck [...] cpan.org - Correspondence added

I don't do formal 'development releases', however, I am attaching a copy of the current code (which passes all of the tests), so you can test it out. Thanks for you help.

Subject:

Date-Manip-6.51.tar.gz

Download Date-Manip-6.51.tar.gz
application/gzip 1.7m

Message body not shown because it is not plain text.

Fri Jul 10 15:16:44 2015 wyant [...] cpan.org - Correspondence added

Thank you for the preview. I found this when I was actually in Sao Paulo and some tests in one of my modules failed. I am no longer physically there, but I told my computer it was there, installed Date-Manip 6.51, and the tests passed. Again, many thanks.