Yuri6037 / time-tz

Implementation of tz database (IANA) for time Rust crate.
BSD 3-Clause "New" or "Revised" License
22 stars 7 forks source link

Parser for TZ #4

Closed Yuri6037 closed 2 years ago

Yuri6037 commented 2 years ago

This provides an implementation of the TZ POSIX environment variable specification for timezone information.

This exposes a new API function time_tz::parse_tz::parse(input: &str) which returns an owned type implementing the TimeZone trait.

This currently needs a lot of testing to ensure the hacks works and the many "unwrap" also passes.

Link to the spec: https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html#tag_08_03

Yuri6037 commented 2 years ago

I've locally added a main function that compares the conversion result against what TZ=... date does. https://github.com/MiSawa/time-tz/blob/smoke-test/src/main.rs

I've tried adding this as unit tests but it doesn't seem cross platform. On Mac I get "date: settimeofday (timezone): Operation not permitted".

MiSawa commented 2 years ago

It works great! Thank you so much!

I've locally added a main function that compares the conversion result against what TZ=... date does. https://github.com/MiSawa/time-tz/blob/smoke-test/src/main.rs

I've tried adding this as unit tests but it doesn't seem cross platform. On Mac I get "date: settimeofday (timezone): Operation not permitted".

Yeah it's very much non-portable, as (I don't know much but) it calls an external command date which probably doesn't exists in Windows machine in the first place. And also the command line args of OSX is different from that of gnu ones. And this is one of the reasons why I didn't make it a cargo test (another reason is it's very slow). To run this on OSX, installing coreutils plus use gdate instead of date should work I believe. The output currently differs for an edge case where the tz rules are very near to the year boundary like ABC4DEF,J1/0,J365/23, but practically that not going to happen, so I guess it's fine.

Yuri6037 commented 2 years ago

It works great! Thank you so much!

I've locally added a main function that compares the conversion result against what TZ=... date does. https://github.com/MiSawa/time-tz/blob/smoke-test/src/main.rs

I've tried adding this as unit tests but it doesn't seem cross platform. On Mac I get "date: settimeofday (timezone): Operation not permitted".

Yeah it's very much non-portable, as (I don't know much but) it calls an external command date which probably doesn't exists in Windows machine in the first place. And also the command line args of OSX is different from that of gnu ones. And this is one of the reasons why I didn't make it a cargo test (another reason is it's very slow). To run this on OSX, installing coreutils plus use gdate instead of date should work I believe. The output currently differs for an edge case where the tz rules are very near to the year boundary like ABC4DEF,J1/0,J365/23, but practically that not going to happen, so I guess it's fine.

Ok so I've found a way to make it run on Mac by replacing "date -d" with "date -r" (just looked at man date)... However I've noticed something strange: if the default DST offset is -3600 or std + 3600 it fails. I figured from the output that Mac date does something different it takes -STD + 3600 seconds as default instead.

Do you think that's normal or is it Apple not complying to POSIX?

MiSawa commented 2 years ago

Ok so I've found a way to make it run on Mac by replacing "date -d" with "date -r" (just looked at man date)... However I've noticed something strange: if the default DST offset is -3600 or std + 3600 it fails. I figured from the output that Mac date does something different it takes -STD + 3600 seconds as default instead.

Do you think that's normal or is it Apple not complying to POSIX?

Negative std doesn't make sense to me... std - 3600 does though. I mean, it should match the actual real-world daylight saving time that USA for example have. For example, if it's 9AM currently in STD and DST starts tomorrow, then 24 hour later should be 10AM, as to follow the fact that the sun raise earlier.

Yuri6037 commented 2 years ago

Ok so I've found a way to make it run on Mac by replacing "date -d" with "date -r" (just looked at man date)... However I've noticed something strange: if the default DST offset is -3600 or std + 3600 it fails. I figured from the output that Mac date does something different it takes -STD + 3600 seconds as default instead. Do you think that's normal or is it Apple not complying to POSIX?

Negative std doesn't make sense to me... std - 3600 does though. I mean, it should match the actual real-world daylight saving time that USA for example have. For example, if it's 9AM currently in STD and DST starts tomorrow, then 24 hour later should be 10AM, as to follow the fact that the sun raise earlier.

I've checked again and it really chooses UTC - STD + 3600 as default offset.

EDIT: I think it makes sense because time-tz uses UTC + offset = local whereas POSIX does at inverse so according to posix representation that would be translated to STD + 3600 where STD is defined in POSIX representation. So it does indeed match +1 in standard time. However in time-tz I use UTC so STD is not really STD but instead -STD. Is this correct or am I missing something?

MiSawa commented 2 years ago

Ahhh ooooops, I was really confused about what we refer with std, my bad. Yeah in my Linux box, it works if I changed

                     let offset = v.offset.map(|v| v.to_seconds()).unwrap_or(tmp + 3600); 

to

                     let offset = v.offset.map(|v| v.to_seconds()).unwrap_or(tmp - 3600); 

which means -std+3600 (std here is the thing in input string).

Yuri6037 commented 2 years ago

Here that should fix it.

MiSawa commented 2 years ago

Here that should fix it.

Confirmed, thank you!