Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Android for get current_zone() #628

Merged
merged 1 commit into from May 14, 2024

Conversation

ndusart
Copy link
Contributor

@ndusart ndusart commented Dec 1, 2020

Hello.

In Android, the timezone is not stored in /etc/localtime or any other file. We have to look for the value of the system property persist.sys.timezone instead.

This PR add this check for Android in current_zone() function.

@wanfranck
Copy link

@ndusart @HowardHinnant is there any plan towards merging it to master branch?

We encountered a runtime_error related to date::current_zone() on Android device and now thinking around possible workarounds with that.

@HowardHinnant
Copy link
Owner

Does this patch fix your problem?

@TheStormN
Copy link
Contributor

@HowardHinnant this patch really fixes the issue for me. Of course it should be noted that the library only works when an external TZ database is provided as it is unable to parse the one provided by Android.

However the PR as it is now have some issues.

  1. The std::string result is not used, so it can be removed.
  2. The ifdef guards should be changed to #if defined(ANDROID) || defined(__ANDROID__)

@ndusart
Copy link
Contributor Author

ndusart commented Feb 22, 2024

@TheStormN you are totally right, a TZ database needs to be included for this to work

But I wonder, since this library has been ported in the standard library, isn't this transparently supported by the std::chrono in latest NDK ?

@TheStormN
Copy link
Contributor

TheStormN commented Feb 22, 2024

@ndusart The only compilers which have support for time zones are MSVC 2022 and GCC 14. Clang libc++(used in NDK) is still very far from ready, even in their master branch. They have some code for Linux only. The TZ database which comes integrated with Android however is in binary format(single file) which of course can be parsed, there is C code available for that, but someone will have to port that parser to libc++.

P.S. If you are interested in that I can give you some hints:

@ndusart
Copy link
Contributor Author

ndusart commented Feb 23, 2024

Thanks for the information, that's interesting.

I rebased this PR to integrate the suggestions by @TheStormN.

@TheStormN
Copy link
Contributor

Thanks a lot @ndusart ! Hopefully @HowardHinnant will merge it.

P.S. the check >= 1 is a bit strange. It could be just > 0. Zero seems more natural number than one which looks like a magic number, but that's really a minor thing. :)

@TheStormN
Copy link
Contributor

@HowardHinnant Can we please get this merged, so I can remove at least part of the local patches?

@HowardHinnant HowardHinnant merged commit f986299 into HowardHinnant:master May 14, 2024
@ndusart ndusart deleted the android branch May 14, 2024 08:33
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.

None yet

4 participants