Patchwork test-parse-date: fix timezone name used to test local time with DST

login
register
mail settings
Submitter Sébastien Brissaud
Date Feb. 7, 2016, 8:31 a.m.
Message ID <cfeb014c4549604a99d4.1454833882@descartes.seb.brissaud.name>
Download mbox | patch
Permalink /patch/13071/
State Changes Requested
Delegated to: Yuya Nishihara
Headers show

Comments

Sébastien Brissaud - Feb. 7, 2016, 8:31 a.m.
# HG changeset patch
# User Sébastien Brissaud <sebastien@brissaud.name>
# Date 1454833607 -3600
#      Sun Feb 07 09:26:47 2016 +0100
# Node ID cfeb014c4549604a99d4a40e29d0773ee6b010a4
# Parent  a036e1ae1fbe88ab99cb861ebfc2e4da7a3912ca
test-parse-date: fix timezone name used to test local time with DST

According to ftp://ftp.iana.org/tz/data/northamerica, the correct name of
Pacific Standard Time with Daylight Saving Time is 'PST8PDT'.
Yuya Nishihara - Feb. 11, 2016, 6:29 a.m.
On Sun, 07 Feb 2016 09:31:22 +0100, Sébastien Brissaud wrote:
> # HG changeset patch
> # User Sébastien Brissaud <sebastien@brissaud.name>
> # Date 1454833607 -3600
> #      Sun Feb 07 09:26:47 2016 +0100
> # Node ID cfeb014c4549604a99d4a40e29d0773ee6b010a4
> # Parent  a036e1ae1fbe88ab99cb861ebfc2e4da7a3912ca
> test-parse-date: fix timezone name used to test local time with DST
> 
> According to ftp://ftp.iana.org/tz/data/northamerica, the correct name of
> Pacific Standard Time with Daylight Saving Time is 'PST8PDT'.
> 
> diff --git a/tests/test-parse-date.t b/tests/test-parse-date.t
> --- a/tests/test-parse-date.t
> +++ b/tests/test-parse-date.t
> @@ -25,13 +25,13 @@ This runs with TZ="GMT"
>    $ hg ci -d "100000 1400000" -m "fail"
>    abort: impossible time zone offset: 1400000
>    [255]
>  
>  Check with local timezone other than GMT and with DST
>  
> -  $ TZ="PST+8PDT"
> +  $ TZ="PST8PDT"
>    $ export TZ

It seems "PST+8PDT" is valid.

http://www.gnu.org/software/libc/manual/html_node/TZ-Variable.html

  PST +8     PDT
  --- ------ ---
  std offset dst

Do you have any problem with "PST+8PDT" ?
Sébastien Brissaud - Feb. 13, 2016, 6:24 a.m.
On 2016-02-11 07:29, Yuya Nishihara wrote:
> On Sun, 07 Feb 2016 09:31:22 +0100, Sébastien Brissaud wrote:
>> -  $ TZ="PST+8PDT"
>> +  $ TZ="PST8PDT"
>>    $ export TZ
> 
> It seems "PST+8PDT" is valid.
> 
> http://www.gnu.org/software/libc/manual/html_node/TZ-Variable.html

> Do you have any problem with "PST+8PDT" ?

I'm running Debian testing and test-parse-date.t failed with incorrect 
DST offsets:
-  Sat Jul 15 13:30:00 2006 -0700
+  Sat Jul 15 13:30:00 2006 -0800

Setting TZ to PST8PDT make the test pass.
But as you said earlier 'PST+8PDT' is a valid TZ value.

In fact it seems to be related as how TZ is managed on my system.

Testing it directly in a python shell exhibit the same problem:
Python 2.7.11 (default, Jan 11 2016, 21:04:40)
[GCC 5.3.1 20160101] on linux2
>>> os.environ['TZ'] = 'PST+8PDT'
>>> time.tzset()
>>> time.tzname
('PST', 'PST')
>>> 
>>> os.environ['TZ'] = 'PST8PDT'
>>> time.tzset()
>>> time.tzname
('PST', 'PDT')

PST8PDT works for me because it's resolved to a timezone file in 
/usr/share/zoneinfo.
I'll investigate the posix compliancy/configuration of TZ in my system.

test-parse-date.t is thus correct and my patch is irrelevant.
Yuya Nishihara - Feb. 13, 2016, 8:19 a.m.
On Sat, 13 Feb 2016 07:24:11 +0100, Sébastien Brissaud wrote:
> On 2016-02-11 07:29, Yuya Nishihara wrote:
> > On Sun, 07 Feb 2016 09:31:22 +0100, Sébastien Brissaud wrote:
> >> -  $ TZ="PST+8PDT"
> >> +  $ TZ="PST8PDT"
> >>    $ export TZ
> > 
> > It seems "PST+8PDT" is valid.
> > 
> > http://www.gnu.org/software/libc/manual/html_node/TZ-Variable.html
> 
> > Do you have any problem with "PST+8PDT" ?
> 
> I'm running Debian testing and test-parse-date.t failed with incorrect 
> DST offsets:
> -  Sat Jul 15 13:30:00 2006 -0700
> +  Sat Jul 15 13:30:00 2006 -0800

Hmm, I'm on Debian sid, but I couldn't reproduce the problem.

ii  libc6:amd64    2.21-7       amd64        GNU C Library: Shared libraries
ii  python         2.7.11-1     amd64        interactive high-level object-ori
ii  tzdata         2016a-1      all          time zone and daylight-saving tim

> Setting TZ to PST8PDT make the test pass.
> But as you said earlier 'PST+8PDT' is a valid TZ value.
> 
> In fact it seems to be related as how TZ is managed on my system.
> 
> Testing it directly in a python shell exhibit the same problem:
> Python 2.7.11 (default, Jan 11 2016, 21:04:40)
> [GCC 5.3.1 20160101] on linux2
> >>> os.environ['TZ'] = 'PST+8PDT'
> >>> time.tzset()
> >>> time.tzname
> ('PST', 'PST')
> >>> 
> >>> os.environ['TZ'] = 'PST8PDT'
> >>> time.tzset()
> >>> time.tzname
> ('PST', 'PDT')
> 
> PST8PDT works for me because it's resolved to a timezone file in
> /usr/share/zoneinfo.
> I'll investigate the posix compliancy/configuration of TZ in my system.
> 
> test-parse-date.t is thus correct and my patch is irrelevant.

That said, since "PST+8PDT" does not specify dst start/end, it would depend
on the system zoneinfo files anyway.

  % TZ=PST+8PDT strace date 2>&1 | grep zoneinfo
  open("/usr/share/zoneinfo/PST+8PDT", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
  open("/usr/share/zoneinfo/posixrules", O_RDONLY|O_CLOEXEC) = 3
  % TZ=PST8PDT strace date 2>&1 | grep zoneinfo
  open("/usr/share/zoneinfo/PST8PDT", O_RDONLY|O_CLOEXEC) = 3

https://sourceware.org/git/?p=glibc.git;a=blob;f=time/tzset.c;h=8bc7a2e05bb83e9e6e37d6f2b6db2d6ad49b9b8b;hb=4e42b5b8f89f0e288e68be7ad70f9525aebc2cff#l278

If PST8PDT can provide more stable result, it will be fine to use that TZ
value. Can you resend the patch with updated commit message?
Sébastien Brissaud - Feb. 15, 2016, 7:04 a.m.
On 2016-02-13 09:19, Yuya Nishihara wrote:
> On Sat, 13 Feb 2016 07:24:11 +0100, Sébastien Brissaud wrote:
>> On 2016-02-11 07:29, Yuya Nishihara wrote:

>> > http://www.gnu.org/software/libc/manual/html_node/TZ-Variable.html

Re-reading this page, DST start/end aren't optional and need to be 
specified.

$ zdump -v /usr/share/zoneinfo/PST8PDT | cut -d ' ' -f 3- | grep 2006
Sun Apr  2 09:59:59 2006 UT = Sun Apr  2 01:59:59 2006 PST isdst=0 
gmtoff=-28800
Sun Apr  2 10:00:00 2006 UT = Sun Apr  2 03:00:00 2006 PDT isdst=1 
gmtoff=-25200
Sun Oct 29 08:59:59 2006 UT = Sun Oct 29 01:59:59 2006 PDT isdst=1 
gmtoff=-25200
Sun Oct 29 09:00:00 2006 UT = Sun Oct 29 01:00:00 2006 PST isdst=0 
gmtoff=-28800

Thus 'PST+8PDT' is invalid and should be, for 2006, 
'PST+8PDT,M4.1.0,M10.5.0'.

> That said, since "PST+8PDT" does not specify dst start/end, it would 
> depend
> on the system zoneinfo files anyway.
> 
>   % TZ=PST+8PDT strace date 2>&1 | grep zoneinfo
>   open("/usr/share/zoneinfo/PST+8PDT", O_RDONLY|O_CLOEXEC) = -1 ENOENT
> (No such file or directory)
>   open("/usr/share/zoneinfo/posixrules", O_RDONLY|O_CLOEXEC) = 3
>   % TZ=PST8PDT strace date 2>&1 | grep zoneinfo
>   open("/usr/share/zoneinfo/PST8PDT", O_RDONLY|O_CLOEXEC) = 3
> 
> https://sourceware.org/git/?p=glibc.git;a=blob;f=time/tzset.c;h=8bc7a2e05bb83e9e6e37d6f2b6db2d6ad49b9b8b;hb=4e42b5b8f89f0e288e68be7ad70f9525aebc2cff#l278

On my system, '/usr/share/zoneinfo/posixrules' defines time in the 
EST/EDT timezone.

$ zdump -v /usr/share/zoneinfo/posixrules | cut -d ' ' -f 3- | grep 2006
Sun Apr  2 06:59:59 2006 UT = Sun Apr  2 01:59:59 2006 EST isdst=0 
gmtoff=-18000
Sun Apr  2 07:00:00 2006 UT = Sun Apr  2 03:00:00 2006 EDT isdst=1 
gmtoff=-14400
Sun Oct 29 05:59:59 2006 UT = Sun Oct 29 01:59:59 2006 EDT isdst=1 
gmtoff=-14400
Sun Oct 29 06:00:00 2006 UT = Sun Oct 29 01:00:00 2006 EST isdst=0 
gmtoff=-18000

And that is certainly why my system correctly applied DST with e.g. 
TZ='PST+5PDT' but not with TZ='PST+8PDT'.

> If PST8PDT can provide more stable result, it will be fine to use that 
> TZ
> value. Can you resend the patch with updated commit message?

TZ can be defined to a timezone file (TZ=":PST8PDT") but this file is 
now expected to be present on the system running the test.
(https://www.gnu.org/software/libc/manual/html_node/TZ-Variable.html#TZ-Variable 
defines the third format as ':characters' but the implementation 
silently ignore it: 
https://sourceware.org/git/?p=glibc.git;a=blob;f=time/tzset.c;h=8bc7a2e05bb83e9e6e37d6f2b6db2d6ad49b9b8b;hb=4e42b5b8f89f0e288e68be7ad70f9525aebc2cff#l424))

We can also define TZ by using the second format 
(TZ="PST+8PDT+7,M4.1.0/02:00:00,M10.5.0/02:00:00") and thus only 
requiring the system running the test to be POSIX compliant.

I prefer the second option which I found more explicit about the offset 
definition and DST transitions.

What did you think?
Yuya Nishihara - Feb. 15, 2016, 12:52 p.m.
On Mon, 15 Feb 2016 08:04:40 +0100, Sébastien Brissaud wrote:
> On 2016-02-13 09:19, Yuya Nishihara wrote:
> > On Sat, 13 Feb 2016 07:24:11 +0100, Sébastien Brissaud wrote:
> >> On 2016-02-11 07:29, Yuya Nishihara wrote:
> 
> >> > http://www.gnu.org/software/libc/manual/html_node/TZ-Variable.html
> 
> Re-reading this page, DST start/end aren't optional and need to be
> specified.

Indeed. It seems glibc is permissive, but the doc doesn't state it.

> On my system, '/usr/share/zoneinfo/posixrules' defines time in the 
> EST/EDT timezone.
> 
> $ zdump -v /usr/share/zoneinfo/posixrules | cut -d ' ' -f 3- | grep 2006
> Sun Apr  2 06:59:59 2006 UT = Sun Apr  2 01:59:59 2006 EST isdst=0 gmtoff=-18000
> Sun Apr  2 07:00:00 2006 UT = Sun Apr  2 03:00:00 2006 EDT isdst=1 gmtoff=-14400
> Sun Oct 29 05:59:59 2006 UT = Sun Oct 29 01:59:59 2006 EDT isdst=1 gmtoff=-14400
> Sun Oct 29 06:00:00 2006 UT = Sun Oct 29 01:00:00 2006 EST isdst=0 gmtoff=-18000

FYI, I got exactly the same result om my machine.

> And that is certainly why my system correctly applied DST with e.g. 
> TZ='PST+5PDT' but not with TZ='PST+8PDT'.
> 
> > If PST8PDT can provide more stable result, it will be fine to use that 
> > TZ
> > value. Can you resend the patch with updated commit message?
> 
> TZ can be defined to a timezone file (TZ=":PST8PDT") but this file is 
> now expected to be present on the system running the test.
> (https://www.gnu.org/software/libc/manual/html_node/TZ-Variable.html#TZ-Variable 
> defines the third format as ':characters' but the implementation 
> silently ignore it: 
> https://sourceware.org/git/?p=glibc.git;a=blob;f=time/tzset.c;h=8bc7a2e05bb83e9e6e37d6f2b6db2d6ad49b9b8b;hb=4e42b5b8f89f0e288e68be7ad70f9525aebc2cff#l424))
> 
> We can also define TZ by using the second format 
> (TZ="PST+8PDT+7,M4.1.0/02:00:00,M10.5.0/02:00:00") and thus only 
> requiring the system running the test to be POSIX compliant.
> 
> I prefer the second option which I found more explicit about the offset 
> definition and DST transitions.

I like the explicit version if it is cross-platform enough. It worked on
Windows, so perhaps it is okay.
Sébastien Brissaud - March 10, 2016, 7:10 a.m.
On 2016-02-15 13:52, Yuya Nishihara wrote:
> On Mon, 15 Feb 2016 08:04:40 +0100, Sébastien Brissaud wrote:
>> On 2016-02-13 09:19, Yuya Nishihara wrote:
>> > On Sat, 13 Feb 2016 07:24:11 +0100, Sébastien Brissaud wrote:
>> >> On 2016-02-11 07:29, Yuya Nishihara wrote:
>> 
>> >> > http://www.gnu.org/software/libc/manual/html_node/TZ-Variable.html
>> 
>> Re-reading this page, DST start/end aren't optional and need to be
>> specified.
> 
> Indeed. It seems glibc is permissive, but the doc doesn't state it.

The POSIX definition allow for optional DST start/end:
http://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap08.html

Glibc conform to the specification even if its documentation mandate DST 
transition times.


But I found a bug in glibc that prevent it to accurately guess DST 
transition times on 32bit platforms with timezone files generated with 
zic from tzcode >= 2014c.

See https://sourceware.org/bugzilla/show_bug.cgi?id=19738

>> We can also define TZ by using the second format
>> (TZ="PST+8PDT+7,M4.1.0/02:00:00,M10.5.0/02:00:00") and thus only
>> requiring the system running the test to be POSIX compliant.
>> 
>> I prefer the second option which I found more explicit about the 
>> offset
>> definition and DST transitions.
> 
> I like the explicit version if it is cross-platform enough. It worked 
> on
> Windows, so perhaps it is okay.

Patch follow.

Thank you for taking time to help me with this issue.

Patch

diff --git a/tests/test-parse-date.t b/tests/test-parse-date.t
--- a/tests/test-parse-date.t
+++ b/tests/test-parse-date.t
@@ -25,13 +25,13 @@  This runs with TZ="GMT"
   $ hg ci -d "100000 1400000" -m "fail"
   abort: impossible time zone offset: 1400000
   [255]
 
 Check with local timezone other than GMT and with DST
 
-  $ TZ="PST+8PDT"
+  $ TZ="PST8PDT"
   $ export TZ
 
 PST=UTC-8 / PDT=UTC-7
 
   $ hg debugrebuildstate
   $ echo "a" > a