Conversation
Outdated..To run the tests on the Android Emulator run ``` ./gradlew androidNativeX64TestBinaries && adb push okio/build/bin/androidNativeX64/debugTest/test.kexe /data/local/tmp && adb shell /data/local/tmp/test.kexe ```For me all the tests passed except those: Will investigate further and update you. |
Also, OutdatedUPDATE: I got most tests running. Those are the only failing tests. ``` [ FAILED ] 3 tests, listed below: [ FAILED ] okio.NativeSystemFileSystemTest.canonicalizeDotReturnsCurrentWorkingDirectory [ FAILED ] okio.NativeSystemFileSystemTest.listOnRelativePathWhichIsNotDotReturnsRelativePaths [ FAILED ] okio.NativeSystemFileSystemTest.listOrNullOnRelativePathWhichIsNotDotReturnsRelativePaths ```To run tests now, do the following on the terminal. |
|
UPDATE again. Now the only failling tests are Best way to run the tests as of now. |
|
After reading the Knowing that, I can say that Okio does work as intended on Android native, which is useful for a bunch of Android tech apps such as Termux or Android IDEs. |
|
@JakeWharton should i edit |
|
@JakeWharton are you free to look at this? I think there are two ways to make opendir/closedir and this stuff on Android native
And also it would be nice if we could create a new nativeNonAndroid sourceset. Although if we made it then we can't use the option 1, as the platform.posix.DIR is a typealias on Linux and we can't create a typealias to a typealias. All the tests succeeded expect listOnRelativePathWhichIsNotDotReturnsRelativePaths and listOrNullOnRelativePathWhichIsNotDotReturnsRelativePaths I can probably fix this tests and edit the github workflow to run them all for androidNative. What do you think? Info: |
|
@JakeWharton any update on this one? |
|
Nope. I'm not working right now, so probably won't be looking at this for a month or two. |
swankjesse
left a comment
There was a problem hiding this comment.
Looks very promising. Thanks for driving this.
I wanna look into making androidNativeMain a child of linuxMain
okio/src/androidNativeMain/kotlin/okio/AndroidNativePosixVariant.kt
Outdated
Show resolved
Hide resolved
okio/src/androidNativeMain/kotlin/okio/AndroidNativePosixVariant.kt
Outdated
Show resolved
Hide resolved
|
@swankjesse I tried another approach on the latest commit. Can you review it and tell me what do you think? |
a043378 to
93f8268
Compare
|
Oops looks like the workaround didn't work on CI.. |
c80ee4a to
fa1210c
Compare
| internal expect class PosixDirectory(path: Path): Closeable { | ||
| val isInvalid: Boolean | ||
| fun nextEntry(): CPointer<dirent>? | ||
| } |
There was a problem hiding this comment.
This is the cleanest working approach so far.
I probably should’ve marked this PR as a draft from the beginning…
Man, everything feels way more complicated than it needs to be.
Why doesn’t platform.posix just expose DIR on Android?!
There was a problem hiding this comment.
Seems like these should all be value classes to avoid the memory allocation. Does anything prevent that?
There was a problem hiding this comment.
@JakeWharton
The problem is that the actual implementation needs to store
private val dir = opendir(path.toString())And if you meant that the value should be DIR. Then this is kinda complicated.
as we cannot use expect/actual for DIR in only two source sets. (androidNative, nativeNonAndroid)
that's because this code will not compile under nativeNonAndroid
actual typealias DIR = platform.posix.DIRIt will fail with an error cannot create a typealias to a typealias.
As platform.posix.DIR is a typealias on Linux.
we could create a new nativeNonLinux source set that will exclude androidNative and linux.
What do you think?
EDIT: I just realized that we could do this
internal expect value class PosixDirectory(private val dir: COpaquePointer) : Closeable {
fun nextEntry(): CPointer<dirent>?
override fun close()
}
internal expect fun openPosixDirectory(path: Path): PosixDirectory?and
internal actual value class PosixDirectory(private val dir: COpaquePointer) : Closeable {
actual fun nextEntry() = readdir(dir.reinterpret())
actual override fun close() {
closedir(dir.reinterpret()) // Ignore errno from closedir.
}
}
internal actual fun openPosixDirectory(path: Path): PosixDirectory? {
return opendir(path.toString())?.let(::PosixDirectory)
}Does this seem good?
And also we can make openPosixDirectory inline.
Do you think it's worth it?
And btw, I really hate the Kotlin compiler advising me to not use inline on functions..
c1d512a to
4fb4578
Compare
and workaround `platform.posix.DIR` not being available there.
…ons. Unfortunately we cannot typealias `DIR` in a shared source set. Because it is already a typealias on Linux.
4fb4578 to
8aabd6d
Compare
This adds support for androidNative targets.
This is the second attempt at trying to do this. Now I can confirm that it does compile and work.
The problem with the first attempt (#1726) was that it didn't account for the fact that platform.posix.DIR doesn't exist on androidNative. Therefore, opendir, readdir, and closedir can't be used from common code. This fixes this by using expect/actual for all this stuff.
To run the tests. Run the following commands in order.
(You need to have an Android Emulator running first.)
Closes: #1692, Closes #1242