Runtime support for setting the binary tzdb directory path

[DavisVaughan]

Pre-requisite for #564

Hi Howard,

I am very motivated to add support for using a binary tzdb on Windows. I hope that that is something that you would also be interested in, and would be willing to add support for, even though you are on a feature freeze.

As a first step, I've added a PR to my own R package (that uses <date>) with changes to the tz code to allow for setting the binary tzdb path at runtime. It seems to work pretty well for me with binary files I pulled from CCTZ's github repo. It currently is only allowed for non-Windows platforms, because I have a feeling the binary parser might have to be tweaked for Windows. Nevertheless, this is a required step for Windows support, since I'd have to provide a binary tzdb in a custom location for that platform. It will also be useful for me on non-Windows platforms, as I can standardize on a specific tzdb version across all platforms.

I've documented the changes in the PR, and would love the opportunity to include them as a PR into date if you think they are appropriate. I've tried to be as backwards compatible as possible. The first 3 commits are relevant to you. Ignore the large amount of changed files, those are just the binary zoneinfo files.
r-lib/clock#6

Let me know what you think!

[jkhoogland]

Hi Vaughan,
there was already a separate PR 611 submitted some time back for this.
Regards,
Jiri

[HowardHinnant]

And I'm behind on reviewing PR's. I should have more time available soon.

[DavisVaughan]

Jiri,

Thanks for bringing that up!

I read that PR pretty closely before opening this. While I'd probably use pieces of that to fixup the support for Windows in the binary tzdb parser, I'll admit that I wasn't the biggest fan of relying on environment variables for the paths. It is otherwise a nice step towards what I eventually want to accomplish!

We've had issues in the past where we relied on CCTZ, which uses the TZDIR env var, and situations out of our control changed the TZDIR environment variable out from under us, causing issues like this one tidyverse/lubridate#928 (this affected a lot of R users on Mac, sadly). It would be much nicer to be able to point date to a specific path at runtime that is completely separate from any env var. That would have prevented this issue. This is also an old outstanding CCTZ issue requesting support for something like this (also by a lubridate R package co-author) google/cctz#6. With this approach, clients could probably even point set_tz_dir() to TZDIR if they wanted to.

I don't think I quite understood what the TZDATA_DIR env var in that PR was for. I've already managed to use a custom path for the text version of the tzdb at runtime by first setting INSTALL to a dummy value at compile time, and then using set_install() at runtime. It is entirely possible that I just missed the point, though (it may be CMAKE related, which I do not use or understand).

I also liked that my approach separated out the idea of USE_OS_TZDB from USE_BINARY_TZDB. When you use a custom path to a binary tzdb, that is, by definition, not the one that the OS supplied. There is some code in current_zone() that looks to only work with a binary tzdb that is truly OS supplied. I'm fairly certain that using a custom location for the binary tzdb would not work here, since it relies on some simlinks. https://github.com/DavisVaughan/civil/blob/424c9954661e8784ae2a1028af7495c63d0bfb0b/src/tz.cpp#L3875-L3904

Overall I think my suggestion is a just bit more focused, and builds off some of my own pain points with those env vars.

[wabscale]

Hi Davis I have submitted another PR with just the changes to allow users to use a set_tz_dir function as you have described. The set_install function is used for the downloaded database, so I believe it should be kept separate.

#627

[DavisVaughan]

Hmm, I’m not certain that would feed well into a future PR where support for the binary parser on Windows is added.

If binary support is added on Windows, we will need to find the custom supplied tzdb, but I don’t think the way to do that on Windows would be by calling discover_tz_dir(). In fact, that function is currently not part of the Windows build (see the ifndef right above it), so it would have to be enabled if we did go this route.

It also still feels odd to me to mix the idea of USE_OS_TZDB with a user defined location, which kind of implies that it probably wasn’t the OS one.