Feature - Setting the binary tzdb directory path at runtime#639
Feature - Setting the binary tzdb directory path at runtime#639DavisVaughan wants to merge 3 commits intoHowardHinnant:masterfrom
Conversation
This re-includes USE_OS_TZDB as a subset of USE_BINARY_TZDB. If USE_BINARY_TZDB is set, but USE_OS_TZDB is not, then the client must call set_tz_dir() before initializing the tzdb to set the path to the binary tzdb.
|
@DavisVaughan Can you explain what you mean with "the binary tzdb parser does not yet work on Windows"? |
|
On Windows only the text version of the tzdb is allowed. On non-Windows you can choose to either use the text tzdb or the binary tzdb, and there are parsers for both, with the binary one typically being much faster. But the binary parser hasn't been extended to support Windows yet, i.e.:
|
|
I see, so I am right in interpreting that on Windows, one can use
Does it mean that it is known not to work, or just it hasn't been made available? |
|
Hasn't been made available. Also in the mix: The latest VS tools implement this library as part of C++20: https://gcc.godbolt.org/z/da3YKeKa8 This includes the IANA database. |
Right, and that is what I do here, which ends up pointing at a text version of the tzdb that I ship with this R package With these compilation flags set: |
Closes #626
A step towards #564
This PR allows for setting the time zone database directory path at runtime. It does so through a new helper,
set_tz_dir(), which must be called before the tzdb is initialized if the user has declared that they are going to manually set the path at runtime.Because the binary tzdb parser does not yet work on Windows, this PR does not enable the runtime setting of the path on Windows. That said, the eventual goal to resolve #564 is to fix the binary tzdb parser on Windows, and then to enable runtime setting of the directory path there. That should bring really nice performance improvements to Windows.
There is a new option,
USE_BINARY_TZDB.USE_OS_TZDBcan be thought of as a subset of this. IfUSE_BINARY_TZDBis turned on, butUSE_OS_TZDBis not, then the client is required to callset_tz_dir()before initializing the tzdb, otherwise a runtime error is thrown. IfUSE_OS_TZDBis on, then the automatic detection of the OS tzdb is done instead, andset_tz_dir()is not exposed.If
USE_BINARY_TZDBis not defined, then it is set toUSE_OS_TZDBto be compatible with all previous versions of date.There are two additional changes that I felt had to be made to merge this PR.
Previously,
discover_tz_dir()andget_tz_dir()were always defined if on a non-Windows platform, even ifUSE_OS_TZDBwas not defined. I considered this a bug.discover_tz_dir()is now only defined whenUSE_OS_TZDBis on (otherwise you don't need to discover it), andget_tz_dir()is defined if eitherUSE_OS_TZDBorUSE_BINARY_TZDBis defined. It is required forget_tz_dir()to be implemented this way to eventually add support forUSE_BINARY_TZDBon Windows (i.e. we can't just turn off support for it when on Windows, as is currently done).I have changed the non-Windows implementation of
current_zone()to only callget_tz_dir()whenUSE_OS_TZDBis on. This is a direct result of the previously mentioned change. It seems likecurrent_zone()only callsget_tz_dir()in embedded systems and is related to a symlink to an OS tzdb path, so it really feels likeUSE_OS_TZDBshould have to be active for this to work properly anyways. It would not be sufficient forUSE_BINARY_TZDBto be on, butUSE_OS_TZDBto be off, because this could be a custom location that the symlink probably won't point to.To make sure this is working, I turned it on over in the project I am working on. It seems to work nicely for me on my Mac r-lib/clock#49
I'm not very familiar with C++ testing, so feel free to make changes against this PR as you see fit if you'd like to accept it.