Skip to content

Conversation

@basilgello
Copy link
Contributor

It is known from date documentation that Apple chose not to provide leap second info in compiled zoneinfo files shipped with OSX and iOS.

This PR sets MISSING_LEAP_SECONDS definition if both __APPLE__ and USE_OS_TZDB is set.

Tested on Kodi builds targeting OSX 10.14 and 10.15

Signed-off-by: Vasyl Gello <vasek.gello@gmail.com>
@basilgello
Copy link
Contributor Author

basilgello commented Oct 23, 2024

@HowardHinnant and separate from that, I have ported Microsoft's STL underlying implementation of std::chrono::time_zone and other tz.h stuff to date library here because of microsoft/STL#4997 . Not sure if you ever want to accept such a big patch but it passes all the tests except two that I can not figure out myself ATM.

@HowardHinnant
Copy link
Owner

The date documentation needs to be updated. Apple has been shipping leap second information in zoneinfo for several years now. And unfortunately I did not take note as to when they started. And I have no idea about iOS.

Apart from the date documentation, does this patch actually fix a test or use case on the platforms you tested?

@HowardHinnant
Copy link
Owner

On your port of Microsoft's STL implementation, I suspect that there are copyright issues. They are both open source, but might be incompatible. I would think that at the very least the copyright would need to be included.

@basilgello
Copy link
Contributor Author

TBH I did not test Kodi on OSX without this patch. Let me do that and confirm if Kodi works (or foes not). Android, for example, crashed if leap seconds were to be read.

@basilgello
Copy link
Contributor Author

basilgello commented Oct 23, 2024

Re STL and copyright - makes sense! I will add the copyright notice and check the compatibility of licenses.

STL is licensed under Apache 2.0 with LLVM exception. Date is under MIT. Both are permissive.

@basilgello
Copy link
Contributor Author

@HowardHinnant You are right: tests pass including those requiring leap seconds! I am retracting this PR but you should check posix.pass.cpp on Mac with -DUSE_SYSTEM_TZ_DB: OSX seems not to understand negative SAVE for Europe/Dublin (just like ICU!)

@basilgello basilgello closed this Oct 25, 2024
@HowardHinnant
Copy link
Owner

Thanks for checking this.

Yes, the negative save for Dublin was put into the IANA database relatively recently, maybe 5 years ago. And still many platforms haven't updated their software to cope with a negative save. It has been years since I checked, but last time I did Apple was using an ancient tzcode even though they stay current with tzdata.

However my Posix::time_zone gets the negative save correct, as does my text tzdb parser. So this test is picking up an error in the binary tz database that ships with these platforms.

@basilgello
Copy link
Contributor Author

Correct. We should fix it somehow!

@basilgello
Copy link
Contributor Author

For https://github.com/HowardHinnant/date/blob/master/test/date_test/durations_output.pass.cpp#L316 the reproducer is
https://godbolt.org/#z:OYLghAFBqd5QCxAUwB7IMYFcAuB7AJwBoBLECAQwOAGcAFCgO2QBsBlBPAd0ZAHIAjHyIY8AWwAOJFhRwk8jBs3ace/AAzDRk6cgIgAbhgkSA%2BgYEBOUzJzIaOU6gBsAFhHipLPQDkKY5H4tT10CAHlcDm5ePk0iNEwAQWoaIPj0DDYcABMSGOEAMwUcNgwKbxABdyLGHABVGmQ6VA1hACtKohlGUAwAUgAmACFBoa6SACNUyABKIjwJOQVUvmE8Az0CEmz7eDmCLFqSAIAVPDwWaYg5mjwsAgxAgSIHXMUmVii1QWEuAgoJPwhHMWPwAKzCGJxPD8ACiGVwhAA1C5XEixDQjEiLJYAHS2ew4JEQADCo1GRCRsNy%2BAISMGAGYBDNhDhWjM5ghkBQdvprkQANYgMHqLr8VyQ1rzNKpUVs2J8OZwWBIUQ7akkWmkcgFXSMfyBH5EGolMoVKrG4oNJotBVEDrPbq9cnDF6sTBLXgQZCMbIki5YMS8MHxX0AGTyyB8gYmehADOcRAkeBomvkjH9LEDwaTKbTCgjzGjYlj%2BgTbu8GE9WSoOEz2eFFY96ZrBBwhajMbj5YctfrQcbvbbHeLpfjzhudwehsZeQwWZ29IGAwQOBwElSAHpN/8uLjgJqEFgJlhGg9ij6cLjtJvVFRsgg8vrapvsrJkJuxBQHHpN3OFx%2Bb52K%2B764gggwDH06iJFBiR5ESX55NcsF9AA7CM0FIlhSKnnkwBIkBgQgDsOAUNIIAgMAyCOIcmo0H0DIYTBmHYSSYQ%2BGwJwkgAIgIAhIhQiICTQ9IMtxSJUTRjB0QxJIYAgVAMbCECvBRsj4GhIyodxMwMUx2FImxHFcbx/GCfgSIFCJDHiZJpi0Tg9EMnJCkEEpKk5BRBTIGIGnoWhOl6bBBlGZxPF8QJQkbqJtnUfZ0mObJ8mKQyymqSAUiiJpAW6YxwWsexYWmZFFmMNZYkSXFDlOS5KVpZ5IDPng2Xabl%2BkFcZ4VmUJp4xZVUkyc5yVualHnZBRYgkBgBDNf5rVBSxWGhSZEXmXgOE0FwfV2dVslcMNpg4O56WTdNs1aYFeWLYZhUrd1FkYttVUJTVw3HQ1k0sCwJAtZd7VLbdXUletGDlbFA2JUNrnveNICPEcv1tflAOdcVa0EWD/XxYNtUjfVsM7BgP1zX9yM3ajq1CZjO0vUl0Ojel/xLLJAhKYjC2JCFgNo0Jb7U89ONvQzDWExQ7NXZzHVFZTFkIPzEOvfT%2BMUVyVbnTlHNcxT93rQK8vY5DuMwxRArSOr80S1r0s60iACy%2Bu7VDdVjRNyDAGLJNI9dy1A%2BjADiDu007eMu5RJDu%2BL/3k9bwNIicgeC0rod2P8kdkz7PMWXQCeG0LysZdRHsXV7ksozH6OwjnivO%2BlaBFxrlvYRABh4NsMzfpSzet9kMxWZ3Ldtxu/fdzMZXD23p5cOPPenkQZNd23GLTzMoPL9kNDLxvWELz3fPL3Lc/XTvMx68v9vLwHy/x8v2eUsSA895XHPp9zMvrZNjBPQrdM1w1TPyLJZw6g2ae01lLO6scEBfwNtXEOjNZAAOcgmdQwDUpp29q/G22RoGOyNsLWG/9mrOQABxuBQSA4uYDt4PxmB/fea9n4YIpmidG8E3Z6BwUHPB%2BdCEswGAyChDco4ZyqLHQhnDE6/wIQgohJIBD8MpAItBoDG7UJHmwqixB74j14RLMmBBqL3E/qgpiAVFRij4BCIgMQhDSj4GSYYowkS3HuI8JcTJWTsjmEKEUFiJREDEJUFB1ipQwlWC8EAcovEWIGJKO0YTPEKg5EQDYBBUwKBAK4IAA%3D%3D

Please note that removing CONSTCD14 from both lines makes stuff compiles without any /std:c++ qualifier. And /std:c++11, /std:c++14 and /std:c++17 produce one error pattern while /std:c++20 results in another one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants