alexcrichton / filetime

Accessing file timestamps in a platform-agnostic fashion in Rust
Apache License 2.0
122 stars 56 forks source link

Do not include utimes module on android #61

Closed link2xt closed 4 years ago

link2xt commented 4 years ago

Android variant is a copy of utimensat with a modification to remove futimens call, so it should follow the pattern used for utimensat module used for "solaris, illumos, ..." case.

Follow-up for #59

Fixes #62

link2xt commented 4 years ago

@alexcrichton Sorry for the noise, I have not tested that #59 builds.

This one I just tested, it builds on all platforms delta chat uses. Good news is that once it is set up, android platform will be tested nightly.

alexcrichton commented 4 years ago

Could this add CI that android compiles?

link2xt commented 4 years ago

It's not that easy, even if android compiles, it is impossible to test without NDK. It would be nice, but we don't do this on our repo either, instead we have this setup in android repo: https://github.com/deltachat/deltachat-android/blob/master/ndk-make.sh

link2xt commented 4 years ago

I added a separate action to build for android without testing. It fails without this fix indeed, see #63

Dushistov commented 4 years ago

May be libc crate should be fixed instead? Android based on Linux kernel, and there is suitable syscalls for futimes, just Android's libc do not provide function for this, but it is possible to use syscall directly. And modern enough NDK should have it https://android-review.googlesource.com/c/platform/bionic/+/63321 , though this would require too high level of API, so it is possible for me to use.

link2xt commented 4 years ago

The bug is already fixed here, the only problem is that I accidentally copied the mod utimes line which is completely irrelevant here, without testing.

libc crate is supposed to be an FFI binding for system libc, so it is arguably correct the way it is.

alexcrichton commented 4 years ago

Thanks!