Repository navigation
Implement USE_OS_TZDB for Android - #842
Conversation
|
@HowardHinnant I fixed private problem :) |
| enum class choose {earliest, latest}; | ||
|
|
||
| struct tzdb; | ||
| static std::unique_ptr<tzdb> init_tzdb(); |
There was a problem hiding this comment.
On macOS with -Wall:
../date/include/date/tz.h:143:30: warning: unused function 'init_tzdb' [-Wunused-function]
143 | static std::unique_ptr<tzdb> init_tzdb();
| ^~~~~~~~~
1 warning generated.
There was a problem hiding this comment.
Maybe it is worth guarding with Android ifdefs here and leave the original declaration guarded by non-Android ifdefs. Otherwise on Android, the first declaration of function is non-static but then a static friend function is expected, throwing a hard error
There was a problem hiding this comment.
The whole idea was to allow init_tzdb accessing time_zone::parse_from_android_tzdata but keep that one private for all other possible users.
There was a problem hiding this comment.
I'd be happy to test a fix to this PR.
There was a problem hiding this comment.
Cool! Pushed the fix. Now it builds against Clang 18.1.0 in Android NDK r27b.
-Wall on Android:
[ 0%] Building CXX object CMakeFiles/date-tz.dir/src/tz.cpp.o
/tmp/android-ndk-r27b/toolchains/llvm/prebuilt/linux-x86_64/bin/clang++ --target=armv7-none-linux-androideabi21 --sysroot=/tmp/android-ndk-r27b/toolchains/llvm/prebuilt/linux-x86_64/sysroot -DAUTO_DOWNLOAD=0 -DHAS_REMOTE_API=0 -DHAS_STRING_VIEW=1 -DINSTALL=. -DONLY_C_LOCALE=0 -DUSE_OS_TZDB=1 -I/tmp/date/include -g -DANDROID -fdata-sections -ffunction-sections -funwind-tables -fstack-protector-strong -no-canonical-prefixes -D_FORTIFY_SOURCE=2 -march=armv7-a -mthumb -Wformat -Werror=format-security -Wall -std=gnu++17 -fPIC -pthread -MD -MT CMakeFiles/date-tz.dir/src/tz.cpp.o -MF CMakeFiles/date-tz.dir/src/tz.cpp.o.d -o CMakeFiles/date-tz.dir/src/tz.cpp.o -c /tmp/date/src/tz.cpp
/tmp/date/src/tz.cpp:195:23: warning: unused variable 'folder_delimiter' [-Wunused-const-variable]
195 | static CONSTDATA char folder_delimiter = '/';
| ^~~~~~~~~~~~~~~~
/tmp/date/src/tz.cpp:655:1: warning: unused function 'parse_month' [-Wunused-function]
655 | parse_month(std::istream& in)
| ^~~~~~~~~~~
/tmp/date/src/tz.cpp:2896:1: warning: unused function 'get_version' [-Wunused-function]
2896 | get_version()
| ^~~~~~~~~~~
3 warnings generated.
Should I guard these unused-s too?
There was a problem hiding this comment.
Sure, but could you put that in a separate PR since I've already merged this one? Thanks!
Signed-off-by: Vasyl Gello <vasek.gello@gmail.com>
2a3b5a5 to
151f4a2
Compare
|
I tried to use latest to get this but it wont build on android. I get multiple errors while trying to build with
Am i doing something wrong? EDIT: btw should it work on all android devices (reading from |
|
It is supposed to work on any device, not just rooted. And I have checked (and using it myself) against recent NDKs (25, 26, 27). Which NDK are you trying to build it with? |
|
@basilgello i am building with NDK 25 right now: And i checked i have the latest code. From my gradle file i call cmake with EDIT: also tried with ndk 27.0.11902837 and got the same error |
|
Yes you need |
|
@basilgello thanks i actually in my case i dont use data CMakeLists.txt so i should not need |
|
It is a function |
|
You can check if CMake build passes different set of flags to clang++ than you specified. Just |
|
@basilgello ok i see something must be wrong with my setup. Will check thanks |
This patch implements native tzdata parser for Android. This allows developers to re-use built-in Android tzdata without the need to ship and update IANA TZ data.