Allow nix to compile on android

[roblabla]

Fixes #313

[Susurrus]

These need doc comments. Also, if you search the libc crate you'll see these are available on Linux and all BSD platforms. We shouldn't artificially restrict them to just android here.

[roblabla]

They are still available on other platforms, just defined as libc::c_int above ? CF line 201

Maybe I should use cfg_if! to make it clearer.

[roblabla]

It's worth noting that this change is necessary because of rust-lang/libc#511

[Susurrus]

They're defined as c_int for all platforms.

Since you mentioned the upstream issue, please also add a FIXME that links to the upstream issue and states what can be done when it's resolved. We need to track these tasks in the code, not just here in the PRs.

[roblabla]

So there's another symbol that's apparently missing from android : __errno_location. Instead, it stores it in tread local storage, and has a function to retrieve a pointer to it called __errno : https://github.com/LineageOS/android_bionic/blob/cm-14.1/libc/bionic/__errno.cpp

errno is simply defined as dereferencing the symbol there : https://github.com/LineageOS/android_bionic/blob/cm-14.1/libc/include/errno.h#L44

When we try to compile something for android, we get a linker error saying that __errno_location is undefined.

Now, because the bionic __errno is defined as volatile in bionic, I don't know if it's a good idea to simply make __errno_location return it ? I'm pretty illiterate when it comes to what volatile does.

[roblabla]

More problems, this time for the target aarch64-linux-android. For some reason, libc::SO_PASSCRED, libc::SO_PEERCRED, libc::SO_SNDBUFFORCE are not defined on android in libc (in addition to being undefined on linux arm). I believe this is a bug, so I'm pushing it upstream. In the meantime, I'll also #cfg them out.

https://github.com/nix-rust/nix/blob/master/src/sys/socket/consts.rs#L43 is marked as not(target_arch="arm").

[roblabla]

It should now be possible to compile binaries using nix for both arm-linux-androideabi and aarch64-linux-android.

Should I document every #[cfg(not(target_os = "android"))] ? I'm not too sure what I'm supposed to put in there :/.

[Susurrus]

@roblabla There are basically two categories of things here you're asking about, one is things that currently fail when testing on Android that shouldn't and things that shouldn't be tested on Android. I'd prefer to wait until upstream is fixed so that we only have cases of the latter. Those don't need to be documented. If we have things that can't currently be tested because they're blocking on upstream, those need to be documented. But like I said, I'd rather wait until upstream is fixed to merge this PR so that we only have "good" fixes here.

[roblabla]

The only change that'll ever be fixed by libc is the use of 'as' in the bitflags though, along with missing symbols. I'll try digging what symbols are missing in libc compared to the most recent version of android, so we have a clearer picture of the situation.

…

On June 29, 2017 6:29:12 AM GMT+02:00, Susurrus ***@***.***> wrote: @roblabla There are basically two categories of things here you're asking about, one is things that currently fail when testing on Android that shouldn't and things that shouldn't be tested on Android. I'd prefer to wait until upstream is fixed so that we only have cases of the latter. Those don't need to be documented. If we have things that can't currently be tested because they're blocking on upstream, those need to be documented. But like I said, I'd rather wait until upstream is fixed to merge this PR so that we only have "good" fixes here. -- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: #631 (comment)

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

[Susurrus]

Yes, well libc isn't going to add symbols that don't exist on Android. For things that don't exist on Android, cfg-ing them out of tests and builds is correct and doesn't need explicit documentation.

[roblabla]

Linking rust-lang/libc#632 since it seems to contain some items of interest.

[Susurrus]

@roblabla We now have all android targets being built in CI, so if you rebase this you can see how well things will compile on those targets. Probably not worth doing so until rust-lang/libc#632 settles down, however.

[ndusart]

@roblabla rust-lang/libc#632 is fixed, we could remove some of the #[cfg()] in the first commit now :)

[roblabla]

With rust-lang/libc#634 now merged, most of the constants are now defined. The only ones left are :

• SO_PASSCRED
• SO_PEERCRED
• SO_SNDBUFFORCE

They should be defined though, as a quick grep -R reveals they are in the unified headers, under asm-generic. Time for a second libc PR I guess

[Susurrus]

@roblabla I added those constants in rust-lang/libc#639 already as I saw they were needed for Android and I was already adding socket constants as part of #636.

[Susurrus]

So give this thing a rebase and remove some of those cfgs and we'll see what happens!

[roblabla]

rust-lang/libc#642 is necessary now

[Susurrus]

I'd also like to move Android builds to Tier 2 as part of this. Could you modify the README and also .travis.yml to do that as well?

[roblabla]

Should be fine now

[Susurrus]

Could you better document your changes to the libc_bitflags macro? munch an ident isn't very helpful to someone who doesn't use the macro system in Rust. It'd be better if you described exactly what it allowed, especially if you added an example use-case. The module comment at the top could use some additions for the two use cases you added as well.

I'd also prefer your changes to macros.rs be removed into a separate commit before the others if you're comfortable with git rebase, otherwise it's not a big deal.

[roblabla]

Is this better ? I basically only added one use-case, just split accross 2 arms to support the trailing comma on the last ident.

[roblabla]

Right, so this change caused the build to fail on arm(v7)-unknown-linux-gnueabi because libc doesn't define those flags on arm. rust-lang/libc#644 will fix this failure.

For the i686 and x86_64 android, there are a bunch of type failures. Which is surprising because there isn't much in the arch-specific files of libc (https://github.com/rust-lang/libc/blob/master/src/unix/notbsd/android/b32/arm.rs). I'll investigate.

[ndusart]

The failures on Android with Intel-cpu are due because the test suite is using rustc 1.13 which has no support at all for x86_64-linux-android and for which std was broken for i686-linux-android (see fix), this fix was introduced in version 1.18 of rust.

So the explanation was that std::os::raw::c_char returned from CStr::as_ptr is different than libc::c_char for older stdlib.

I think x86_64-linux-android is not considered stable yet, libc still use the beta channel for testing it.
We should consider using at least rustc 1.18 for testing i686-linux-android, and at least stable for x86_64-linux-android.

I could also advise you to test (at least the build phase) locally on your machine with the latest rustc for the different targets. You won't have to commit just to test and you'd have seen that difference.

[roblabla]

Ah I see. So I bumped the minimum version to 1.18 for i686/x86_64. I also updated the libc PR to also define MAP_32BIT on x86/x86_64 android targets.

I think this should be everything necessary.

[Susurrus]

@roblabla Love the separate macro commit, thanks for that. That let me cherry-pick it into #527, as I needed that functionality as well.

This is pretty close to being mergeable:

• The requirement for Rust 1.18 for the x86 Android targets needs to be documented. I suggest adding it to the README like (requires Rust >= 1.18) to the end of the lines stating they have Tier 2 support.
• We need some CHANGELOG entries. Under Changed add that Android is now a Tier 2 target and under Fixed bind and errno_location now work correctly on Android.

[Susurrus]

Please add those CHANGELOG entries in with the corresponding commits.

If you're feeling particularly diligent, you could also reorder the 2 last commits. I like to have commits ordered such that building things will always succeed. This means fixing the errors and then requiring builds to pass for the fixed target.

[roblabla]

@Susurrus actually, the commits are in the right order. Github interface is just confused, probably because of all the rebase and push --force ? https://github.com/roblabla/nix/commits/feature-androidWorks

[roblabla]

Done.

[Susurrus]

LGTM, thanks for all your hard work!

bors r+

[bors]

Build succeeded

• buildbot/nix-rust/nix fbsd11
• continuous-integration/travis-ci/push