search

HowardHinnant / date

A date and time library based on the C++11/14/17 <chrono> header
Other
3.15k stars 682 forks source link

posix/ptz.pass.cpp fails with USE_OS_TZDB=1 #671

Open alebastr opened 3 years ago

alebastr commented 3 years ago

Posix::time_zone tests are failing with the latest release (3.0.1) when using the OS timezone database. It might be an expected issue due to a tzdb format difference or missing data. All other tests are passing (with known issue in gcc's std::locale).

OS: Fedora Rawhide gcc: 11.1.1 clang: 12.0.0 tzdata: 2021a

<mock-chroot> sh-5.1# CXX=g++ OPTIONS="-DUSE_OS_TZDB=1" TEST_PREFIX=ptz VERBOSE=1 ./testit
...
Running test:  ptz.pass.cpp
a.out: ptz.pass.cpp:57: int main(): Assertion `is_equal(tzi->get_info(tp), tzp.get_info(tp))' failed.
./testit: line 96:   757 Aborted                 (core dumped) ./$TEST_EXE
/builddir/build/BUILD/date-3.0.1/test/posix/ptz.pass.cpp failed at run time
Compile line was: g++ -std=c++14 -DUSE_OS_TZDB=1 -I/builddir/build/BUILD/date-3.0.1 -Wall /builddir/build/BUILD/date-3.0.1/src/tz.cpp -lcurl -I/builddir/build/BUILD/date-3.0.1/include -I/builddir/build/BUILD/date-3.0.1/include/date ptz.pass.cpp
failed 1 tests in /builddir/build/BUILD/date-3.0.1/test/posix

<mock-chroot> sh-5.1# CXX=clang++ OPTIONS="-DUSE_OS_TZDB=1 -stdlib=libc++" TEST_PREFIX=ptz VERBOSE=1 ./testit
...
Running test:  ptz.pass.cpp
a.out: ptz.pass.cpp:57: int main(): Assertion `is_equal(tzi->get_info(tp), tzp.get_info(tp))' failed.
./testit: line 96:   798 Aborted                 (core dumped) ./$TEST_EXE
/builddir/build/BUILD/date-3.0.1/test/posix/ptz.pass.cpp failed at run time
Compile line was: clang++ -std=c++14 -DUSE_OS_TZDB=1 -stdlib=libc++ -I/builddir/build/BUILD/date-3.0.1 -Wall /builddir/build/BUILD/date-3.0.1/src/tz.cpp -lcurl -I/builddir/build/BUILD/date-3.0.1/include -I/builddir/build/BUILD/date-3.0.1/include/date ptz.pass.cpp
failed 1 tests in /builddir/build/BUILD/date-3.0.1/test/posix
HowardHinnant commented 3 years ago

Thanks. This exposed 3 problems, two of which I've fixed. Of the two I fixed, neither warrants a new release as they are extremely minor bugs.

  1. Under -DUSE_OS_TZDB=1 time_zone::get_info(local_time) was not zero-initializing the second sys_info in the local_info when the result was local_info::unique. This is very nearly harmless because when local_info::unique, the client only inspects the first sys_info and not the second. But the spec says the second should be zero-initialized as opposed to containing garbage. Fixed.
  2. When using -DUSE_OS_TZDB=1 the sys_info.save field only contains the information of whether or not this result refers to daylight saving (true or false). It does not contain the actual number of minutes the UTC offset is different from the non-daylight-saving result. This is because the underlying zic-compiled database contains exactly this reduced information set. So I changed the sys_info equality comparison function in the test ptz.pass.cpp to only compare the boolean information (daylight saving or not). Fixed.
  3. The binary zic-compiled output on macOS, and perhaps others such as Fedora Rawhide, reverse the daylight saving flag for Ireland because its save is negative, which causes troubles for some readers of the binary database. There is a test in ptz.pass.cpp that detects this and fails when found. This test is effectively an "expected failure" on such platforms. This test should only fail with -DUSE_OS_TZDB=1 as the text form of the database, and the Posix::time_zone both correctly handle negative values for save.
alebastr commented 3 years ago

Thanks, https://github.com/HowardHinnant/date/commit/052eebaf0086e6bbc5ead01c3f1a8f02496aa701 works for me.

The binary zic-compiled output on macOS, and perhaps others such as Fedora Rawhide, reverse the daylight saving flag for Ireland because its save is negative, which causes troubles for some readers of the binary database.

Hm, dst flag is indeed reversed for Europe/Dublin in the tzdata we have in Fedora (if the command below is the right way to check), but the ptz.pass.cpp is still passing.

% zdump -v /usr/share/zoneinfo/Europe/Dublin |grep 2021
/usr/share/zoneinfo/Europe/Dublin  Sun Mar 28 00:59:59 2021 UT = Sun Mar 28 00:59:59 2021 GMT isdst=1 gmtoff=0
/usr/share/zoneinfo/Europe/Dublin  Sun Mar 28 01:00:00 2021 UT = Sun Mar 28 02:00:00 2021 IST isdst=0 gmtoff=3600
/usr/share/zoneinfo/Europe/Dublin  Sun Oct 31 00:59:59 2021 UT = Sun Oct 31 01:59:59 2021 IST isdst=0 gmtoff=3600
/usr/share/zoneinfo/Europe/Dublin  Sun Oct 31 01:00:00 2021 UT = Sun Oct 31 01:00:00 2021 GMT isdst=1 gmtoff=0
HowardHinnant commented 3 years ago

Ah! Your binary database is better than mine on macOS (10.14.6). :-)

$ zdump -v /usr/share/zoneinfo/Europe/Dublin |grep 2021
/usr/share/zoneinfo/Europe/Dublin  Sun Mar 28 00:59:59 2021 UTC = Sun Mar 28 00:59:59 2021 GMT isdst=0
/usr/share/zoneinfo/Europe/Dublin  Sun Mar 28 01:00:00 2021 UTC = Sun Mar 28 02:00:00 2021 IST isdst=1
/usr/share/zoneinfo/Europe/Dublin  Sun Oct 31 00:59:59 2021 UTC = Sun Oct 31 01:59:59 2021 IST isdst=1
/usr/share/zoneinfo/Europe/Dublin  Sun Oct 31 01:00:00 2021 UTC = Sun Oct 31 01:00:00 2021 GMT isdst=0

Yours is right (isdst) and mine is wrong. Thus mine fails the test while yours doesn't.

HowardHinnant commented 3 years ago

Just to clarify, the only thing that is wrong on macOS is the isdst flag. Everything else is correct: the UTC offset, the start dates, the abbreviation.