Skip to content
New issue

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

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

Already on GitHub? # to your account

Implement kotlinx-datetime for the Android native targets #344

Merged
merged 3 commits into from
Mar 1, 2024

Conversation

dkhalanskyjb
Copy link
Collaborator

To test, run an Android emulator, and then, in the command line, ./gradlew androidNativeArm64TestBinaries && adb push core/build/bin/androidNativeArm64/debugTest/test.kexe /data/local/tmp/ && adb shell /data/local/tmp/test.kexe

Change Arm64 to another platfom (X86, X64, or Arm32) as needed.

Fixes #311

core/build.gradle.kts Outdated Show resolved Hide resolved
Base automatically changed from pureKotlinDarwin to master February 26, 2024 11:26
Copy link
Member

@ilya-g ilya-g left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder would it be possible to restructure native sources later, so that the source tree resembled source set tree more?
E.g.
native
-- tzfile
---- tzdbOnFilesystem
------ darwin
------ linux
---- androidNative
-- windows

@dkhalanskyjb
Copy link
Collaborator Author

So, native/ would be

src/
test/
tzfile/
windows/

? Looks a bit strange to me. But I agree that the directory structure is too cumbersome now.

To test, run an Android emulator, and then, in the command line,
./gradlew androidNativeArm64TestBinaries && adb push core/build/bin/androidNativeArm64/debugTest/test.kexe /data/local/tmp/ && adb shell /data/local/tmp/test.kexe

Change `Arm64` to another platfom (`X86`, `X64`, or `Arm32`)
as needed.
For some reason, even though the code itself compiles and works,
metadata couldn't compile. This is fixed by more carefully
separating which code is compiled for each target, so unneeded
code doesn't get included in the Native Android implementation.
@dkhalanskyjb dkhalanskyjb merged commit 290a666 into master Mar 1, 2024
1 check passed
@dkhalanskyjb dkhalanskyjb deleted the nativeAndroid branch March 1, 2024 14:13
Comment on lines +25 to +28
for (path in listOf(
Path.fromString("/system/usr/share/zoneinfo/tzdata"), // immutable fallback tzdb
Path.fromString("/apex/com.android.tzdata/etc/tz/tzdata"), // an up-to-date tzdb, may not exist
)) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the immutable fallback be last in list? @dkhalanskyjb

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@natario1, why? On line 42, the last put to execute will determine what gets used in the end. So, for Europe/London, for example, first, the map will receive the value from the fallback, but then, the value will be overwritten by the up-to-date timezone database. No?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, sorry for bothering then! It just felt wrong, I'm used to see 'fallbacks' at the end of lists. Thanks for working on this 🙏

@hfhbd
Copy link
Contributor

hfhbd commented Mar 24, 2024

Unrelated to the actual changes but there is also a new API to run android emulator tests by using Gradle: https://developer.android.com/studio/test/gradle-managed-devices
See https://github.com/hfhbd/kotlinx-uuid/blob/main/kotlinx-uuid-core/build.gradle.kts#L63 for a sample. Do you want to migrate to this API? If so, I can provide a PR.

@dkhalanskyjb
Copy link
Collaborator Author

@hfhbd, it would be very nice to have tests for the Android native targets, but aren't these Android tests for the JVM version?

@hfhbd
Copy link
Contributor

hfhbd commented Mar 25, 2024

@dkhalanskyjb Honestly, I don't know! They do work for the JVM version (of course) and you can bundle your c/native code in a .aar file to call it from Java too.
But the docs (and many information on the internet too) about android native are very thin compared to the Java ones, so asking someone from Google/create an issue would be helpful.

@dkhalanskyjb
Copy link
Collaborator Author

@hfhbd, if you want to dive deep into this and discover a way to test native Android code on the emulator, we'll gladly accept your PR. I'd advise not spending time on this, though. If my intuitive understanding is correct, this would be a lot of work. The way Kotlin/Native builds tests is by compiling them into a separate executable, and you'd need to write some JVM wrapper to call that executable and check the test results.

@natario1
Copy link

In case it helps, I built https://github.com/deepmedia/multiplatform-testing a few years ago, when trying to push for androidNative support in kotlinx repos (lack of testing was said to be one of the blockers).

It can download Android tools, create and spin up matching emulators, and run Android Native tests as a linux executable. At the time, I found a couple of bugs in atomicfu and k/n runtime thanks to it.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support Native Tier 3 targets
4 participants