Skip to content

Support Android for get current_zone() - #628

Merged
HowardHinnant merged 1 commit into
HowardHinnant:masterfrom
ndusart:android
May 14, 2024
Merged

HowardHinnant merged 1 commit into
HowardHinnant:masterfrom
ndusart:android

Conversation

@ndusart

@ndusart ndusart commented Dec 1, 2020

Copy link
Copy Markdown
Contributor

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
Copy Markdown

@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
Copy Markdown
Owner

Does this patch fix your problem?

@TheStormN

Copy link
Copy Markdown
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

ndusart commented Feb 22, 2024 •

Copy link
Copy Markdown
Contributor Author

@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

TheStormN commented Feb 22, 2024 •

Copy link
Copy Markdown
Contributor

@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

ndusart commented Feb 23, 2024

Copy link
Copy Markdown
Contributor Author

Thanks for the information, that's interesting.

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

@TheStormN

Copy link
Copy Markdown
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
Copy Markdown
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.

4 participants