Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Failing to compile on ios and android after update to 0.2 #9

Closed
dignifiedquire opened this issue Dec 9, 2019 · 12 comments
Closed

Failing to compile on ios and android after update to 0.2 #9

dignifiedquire opened this issue Dec 9, 2019 · 12 comments

Comments

@dignifiedquire
Copy link

dignifiedquire commented Dec 9, 2019

Details:

worked fine with 0.1. 5

@r10s
Copy link

r10s commented Dec 9, 2019

maybe i did sth. wrong ianar (i am not a rust-expert :) if some additional information are needed, please just ping me :)

@svartalf
Copy link
Owner

Thanks for a bug report! It was not really clarified what targets are supported for 0.1 version, apparently, this information was lost during the ownership transfer, sorry about that.

arm-linux-androideabi and aarch64-linux-android Android targets are already tested in CI and I dropped few iOS targets too (armv7-apple-ios and x86_64-apple-ios), so far everything compiles correctly, so it would be really helpful to provide a bit more information about your environments.

From what I understand, Android build targets Android 4.3 Jelly Bean (API 18) on a armv7-linux-androideabi platform?
It is also a bit unclear how exactly it compiles for iOS target, could you maybe provide executed command or smth, so I'll try to reproduce the issues.

@r10s
Copy link

r10s commented Dec 10, 2019

hi @svartalf thanks for your help!

on android we use armv7-linux-androideabi, aarch64-linux-android, i686-linux-android, x86_64-linux-android using the most recent ndk. __ANDROID_API__ is set to 18 for 32bit and to 21 for 64bit. rust compiling succeeds, however, at the end linking fails, ndk-make complains about that sethostname is not there, see https://gist.github.com/r10s/3c620bddc01d8640e62287b966b10f93 (the first warning is not releated), with hostname 0.1.5, there is no problem.

on ios, rust compiling fails with the error shown above, the used targets are ´aarch64-apple-ios´ and ´x86_64-apple-ios´ - at least this is what xcode claims.

@svartalf
Copy link
Owner

Yes, hostname::set, which is utilizes sethostname for *nix systems was introduced in 0.2.0, that's why linking fails in your case.
It is still unclear to me, why CI builds are green, but at least we have a starting point for investigation now.

@svartalf
Copy link
Owner

Alright, here goes the follow-up:

For iOS we could extend the following check to include the ios targets also:

hostname/src/nix.rs

Lines 42 to 46 in b7b4fae

#[cfg(not(any(target_os = "macos")))]
let size = hostname.len() as libc::size_t;
#[cfg(any(target_os = "macos"))]
let size = hostname.len() as libc::c_int;
,
same as nix crate does, that should handle the issue.

As for Android, I see that sethostnamefunction was introduced in Android API 23 (see NDK sources), and it is probably the reason why linking fails.
Not sure how to handle that exactly, though: dynamic function loading seems rather complicated in that case, I would probably go with some sort of set feature, which will enable the hostname::set function.

@r10s, any thoughts about it?

@svartalf
Copy link
Owner

Okay, ce41122 should handle the iOS issue and 72fc33f hides hostname::set function under a #[cfg(feature = "set")] feature (disabled by default), so it should handle any linking problems.

@dignifiedquire @r10s can you test it? Following patch section in the Cargo manifest should properly override the dependency:

[patch.crates-io]
hostname = { git = "https://github.com/svartalf/hostname.git", branch = "ios" }

@r10s
Copy link

r10s commented Dec 10, 2019

@svartalf thanks, that looks good! and incredibly quick, thanks a lot. unfortunately, my rust-cargo-skills are not sufficient to try this out easily, the hostname dependency is not used directly by deltachat.

i tried putting the patch to all Cargo.tomls i could find, but for some reasons xcode did not picked it up. maybe we do a pr on deltachat that reverts the change to 0.1.5 we've just done for now to get things out.

@dignifiedquire
Copy link
Author

Compiled it successfully for ios (I don't have android setup)

@r10s
Copy link

r10s commented Dec 11, 2019

@dignifiedquire thanks a lot for testing!

@svartalf
Copy link
Owner

Quick follow-up: how does the testing going on? Had you any chance to test it both for iOS and Android?

@svartalf
Copy link
Owner

So, I hide the hostname::set function with a feature gate; as it was a bit of a rush to publish it as a default part of v0.2.0 and I tried to test the Android/iOS compatibility locally as much as I can, it should be okay now.

@hpk42, @dignifiedquire, can we test your case once again with the hostname = "0.3" version?

@r10s
Copy link

r10s commented Dec 19, 2019

sorry, i did not came back to this sooner. not sure if we're back on the hostname-origin-crate, however. but if so, we will see if things work :) thanks a lot for helping :)

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

No branches or pull requests

3 participants