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

Add host build for aarch64-linux-android #41149

Closed
wants to merge 3 commits into from

Conversation

malbarbo
Copy link
Contributor

@malbarbo malbarbo commented Apr 7, 2017

Some care was taken writing the Dockerfiles so the images can share layers.

@rust-highfive
Copy link
Collaborator

r? @brson

(rust_highfive has picked a reviewer for you, use r? to override)

@malbarbo
Copy link
Contributor Author

malbarbo commented Apr 7, 2017

r? @alexcrichton

@rust-highfive rust-highfive assigned alexcrichton and unassigned brson Apr 7, 2017
@malbarbo
Copy link
Contributor Author

malbarbo commented Apr 7, 2017

@alexcrichton There are some points that need to be discussed.

The ndk update was necessary to fix a crash in openssl build.

The minimum version of the ndk api level to build rustc llvm is 21. You said in another thread that the firefox team need to support some old api level. According to wikipedia this is now Android 4.0.3 (api level 14-15). I don't know how to solve this. Is there a way to build the stdlib with a toolchain and the rustc and cargo with another one? Note that this is not a problem for aarch64, because the ndk is only available for api level greater than 21.

Considering the recent split of some builders, I've create one builder for each arch. Maybe we are reaching the limit?

I used --enable-extended to build cargo. I do not saw any builder using it. Is it ok?

I left out host build for arm. I think that most hardware out there supports armv7, but maybe there is interest in running rustc on it.

Some manifestation about android rustc support: termux/termux-packages#261, https://www.reddit.com/r/rust/comments/631bl2/exciting_rust_coming_to_android_and_chromebooks/

@malbarbo malbarbo changed the title Add android host builds Add android host build support Apr 7, 2017
@japaric
Copy link
Member

japaric commented Apr 8, 2017

Thanks for working on this, @malbarbo. This is exciting!

I'm not Alex (:smile:) but let me share my thoughts:

Disclaimer: I'm not familiar with Android's NDKs and API levels.

Some manifestation about android rustc support

Just to confirm, these rustc / Cargo binaries would be usable not only on Android phones but also on Chromebooks (Chrome OS)? That's what I understood from the linked reddit thread.

I don't know how to solve this. Is there a way to build the stdlib with an toolchain and the rustc and cargo with another one?

I think building is not the problem here. We could have two docker images using different NDKs with different API levels and build only std on one and the whole toolchain+Cargo on the other. The problem, I think, is uploading the binaries because both builders would produce a rust-std component so during the upload one would overwrite the other (or raise an error? I'm not sure what's the failure mode in this case).

Perhaps we could, somehow, let the rust-std created with the lowest API level always override the other one. AFAIK, rustc and Cago don't depend on the rust-std component at runtime so that might work plus it may make programs natively compiled on an Android device as portable as possible as you'll be compiling against the API-9 std.

The ndk update was necessary to fix a crash in openssl build.

Q: Is this NDK update different than raising the API level?

The minimum version of the ndk api level to build rustc is 21.

Could you say more about why older API level didn't work. I thought that API levels were more related to functionality like accessing sensors so I'm curious about why this didn't work. Perhaps Android's API levels are a bit like glibc's symbol versioning?

@alexcrichton
Copy link
Member

Thanks for the PR @malbarbo! The changes here look great to me.

Unfortunately though, yeah, we're running at the limit of our capacity and I think we'll need to allocate more before landing this. This is a pretty significant investment, three new compiler builds, and it won't be cheap to support. @rust-lang/tools, any particular thoughts here on that aspect?

As to API versioning and whatnot, I think our story here is sort of up in the air. We have historically attempted to just be as compatible as possible, resorting to runtime checks for incompatibilities across NDK revisions. In that sense Gecko requested version 9 compatibility, but we maybe able to increase that (we'd just want to check with them). I think we'll still want the standard library to be "as compatible as possible" but I don't mind making the compiler require a newer version. The OSX tests today, for example, compile libstd for 10.7 and the compiler for 10.8.

We unfortunately don't have any method of indicating the version in the target right now, though, so we'll just have to pick one and stick to it. I don't think we're ready to add multiple targets for multiple revisions just yet.

@alexcrichton
Copy link
Member

@malbarbo also if you'd like I'd be fine landing the rustbuild changes ahead of the docker config changes, that way we don't have to block that commit.

@alexcrichton
Copy link
Member

Ok so we're currently using 41/45 of our capacity on Travis on each PR. I think we need to leave some breathing room for projects like Cargo/libc and PRs to have capacity as well, so until we get more capacity or shuffle around what we have already I don't think we'll be able to expand our test coverage unfortunately. Unfortunately compiling entire compilers is pretty serious and requires whole separate builders, so I don't know of an easy way to slot this in with the existing builds.

cc @aturon

@bors
Copy link
Contributor

bors commented Apr 13, 2017

☔ The latest upstream changes (presumably #41267) made this pull request unmergeable. Please resolve the merge conflicts.

@malbarbo
Copy link
Contributor Author

@japaric

I'm not Alex (:smile:) but let me share my thoughts:

Oh, I was just pinging Alex, every one is welcome!

Just to confirm, these rustc / Cargo binaries would be usable not only on Android phones but also on Chromebooks (Chrome OS)? That's what I understood from the linked reddit thread.

Yes. According to this android apps can be installed in many Chromebooks. Installing a terminal emulator like termux would allow running rustup (which supports android!) and install rustc and cargo. The only missing part is publish rustc (and cargo) for android.

AFAIK, rustc and cargo don't depend on the rust-std component at runtime

Running ldd on rustc we can see that rustc is dynamic linked with std. Cargo is static linked (maybe this will change because cargo will be build using --enable-extend in the rust builder). Anyway, I think this is not a problem. libstd can be build with older api level. The problem is that llvm need api level 21 (I fixed the first post).

Is this NDK update different than raising the API level?

Yes. A ndk update means updating the tool chains (gcc, etc). A new ndk generally support all previous api levels, so it can be used to target (almost) any android version.

Could you say more about why older API level didn't work. I thought that API levels were more related to functionality like accessing sensors so I'm curious about why this didn't work. Perhaps Android's API levels are a bit like glibc's symbol versioning?

Android uses bionic libc, each api level correspond to a new version of bionic and each new version of bonic implements more libc (posix) functions. So using old api level means that some libc functions may be missing. Also some functions was declared static when it should not be, see this. Trying to build llvm with an api level older than 21 produces error related with stat/fstat functions and structures.

Instead of using two different builders, I'm trying to build rustc using to different api levels. Let's see.

@malbarbo
Copy link
Contributor Author

@alexcrichton Thanks for the feedback. I understand the limited capacity issue.

Considering that there are interest in running rustc in android and that it will allow programming in rust using chomebooks, I think we should try to add at least one builder.

We can add the builder for aarch64 (the PR needs to be updated). I'm trying to build armv7 using two different api level, if I make it we will have two options.

Considering that our capacity is limited I think we need a policy to decide where the capacity will be used. It seems that for now the policy is FCFS. It would be great to have download statistics so we could decide which new target add to the builders and which ones to retire.

@aturon
Copy link
Member

aturon commented Apr 13, 2017

To echo what @alexcrichton is saying: while we can potentially grow our capacity further, we need to figure out a sustainable model going forward as we add an increasing number of platforms; the costs add up quickly. I'll be working with the core team and Mozilla to develop a strategy here, but it will take some time. If we can find other avenues in the meantime, that'd be great.

@alexcrichton
Copy link
Member

@malbarbo ok yeah starting out conservatively seems fine with just an aarch64 builder. And yes we definitely need a policy in this regard!

@shepmaster shepmaster added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Apr 14, 2017
@malbarbo
Copy link
Contributor Author

@alexcrichton I noticed that there is a difference from rust-std packages for targets that have host support and targets that do not have. Can you tell me if this in intentional?

Here is the list of files in a rust-std tarball for target only build. And here is a list of additional files for a host build. It seems odd to have rustc artifacts when installing a target using rustup. This happens for example, when install aarch64-unknown-linux-gnu (with rustup target add aarch64-unknown-linux-gnu) in a x86_64 system. The rust-std tarball for aarch64-unknown-linux-gnu has 50MB while the rust-std tarball for aarch64-linux-android has only 16MB.

@alexcrichton
Copy link
Member

Yeah while unfortunate it's intentional right now. We never got around to having separate packages for rustc and libstd crates, so the "rust-std" package is actually "all compiled libraries for this target" which may or may not include the rustc libs depending on what we're distributing.

Also update sscache to 2017-04-04 and reorganize Dockerfile so we can
share some image layers
@malbarbo malbarbo changed the title Add android host build support Add host build for aarch64-linux-android Apr 19, 2017
@malbarbo
Copy link
Contributor Author

@alexcrichton Changed the PR to only add aarch64 builder.

@alexcrichton alexcrichton added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). T-core Relevant to the core team, which will review and decide on the PR/issue. labels Apr 20, 2017
@alexcrichton alexcrichton removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Apr 20, 2017
@alexcrichton
Copy link
Member

Ok thanks @malbarbo, in the meantime I've tagged this as waiting on the core team while we figure out a way to get more capacity.

@bors
Copy link
Contributor

bors commented Apr 21, 2017

☔ The latest upstream changes (presumably #41445) made this pull request unmergeable. Please resolve the merge conflicts.

@carols10cents
Copy link
Member

Merge conflicts! Moving this back to you @malbarbo :)

@carols10cents carols10cents added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels May 22, 2017
@malbarbo
Copy link
Contributor Author

@carols10cents after #41864 to add an aarch64 android builder is just move a dir out of disabled and add a entry to .travis.yml. I was waiting on the team decision. Should I close this PR and open an issue to track the team decision?

@carols10cents
Copy link
Member

@malbarbo Ah, I see. I'll poke people!

@shepmaster
Copy link
Member

@malbarbo thanks for your contribution! As you suggested, it's probably better to track this in some other way, especially as we are still figuring out the best way to support adding more and more build types to the Rust build system.

I'm going to close this PR for now. 😿

@oilel
Copy link

oilel commented Jan 16, 2023

I solved it according to termux/termux-packages#8214 . I just installed binutils.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-core Relevant to the core team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants