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

Fix rustbuild per-target musl root #36972

Merged
merged 2 commits into from
Oct 6, 2016

Conversation

nastevens
Copy link
Contributor

In #36292, support was added to target musl libc for ARM targets using rustbuild. Specifically, that change allowed the addition of per-target musl-root options in the rustbuild config.toml so that multiple targets depending on musl could be built. However, that implementation contained a couple of omissions: the musl-root option was added to the config, but was never added to the TOML parsing, and therefore was not actually being loaded from config.toml. This PR rectifies that.

Using these changes and a heavily modified version of the buildbot Docker container, I have been able to build rust targeting armv7-unknown-linux-musleabihf and have successfully run the binaries on a Raspberry Pi 3. I'm also planning to test arm-unknown-linux-musleabi and arm-unknown-linux-musleabihf systems, but have no reason to believe that this change would not simply work on those targets.

In rust-lang#36292, support was added to target musl libc for ARM targets using
rustbuild. Specifically, that change allowed the addition of per-target
"musl-root" options in the rustbuild config.toml so that multiple
targets depending on musl could be built. However, that implementation
contained a couple of omissions: the musl-root option was added to the
config, but was never added to the TOML parsing, and therefore was not
actually being loaded from config.toml. This commit rectifies that and
allows successful building of musl-based ARM targets.
The previous panic message delivered when a musl target was specified
but musl-root was not specified incorrectly instructed the user to add
the musl-root key to the "build" section of config.toml. The key
actually needs to be added to the "rust" section.
@rust-highfive
Copy link
Contributor

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@alexcrichton
Copy link
Member

@bors: r+

Nice! Do you have those docker images on hand to share? We may be trying to get these targets up and running soon :)

@bors
Copy link
Collaborator

bors commented Oct 5, 2016

📌 Commit 7937f6c has been approved by alexcrichton

@nastevens
Copy link
Contributor Author

I just put up the Dockerfile at https://github.com/nastevens/docker-rust-arm-musl - it'd be awesome if it were useful. I'm running a build right now with all three ARM targets and will push that to docker hub when it's done (i.e. tomorrow - it took forever with just one target...)

@nastevens
Copy link
Contributor Author

Docker image is pushed: https://hub.docker.com/r/nastevens/arm-musl-rust/. Be forewarned - it is huge, around 4GB. I need to go back and refactor for size.

With that image all three arm targets work.

@alexcrichton
Copy link
Member

Thanks @nastevens!

sophiajt pushed a commit to sophiajt/rust that referenced this pull request Oct 6, 2016
…musl-root, r=alexcrichton

Fix rustbuild per-target musl root

In rust-lang#36292, support was added to target musl libc for ARM targets using rustbuild. Specifically, that change allowed the addition of per-target `musl-root` options in the rustbuild `config.toml` so that multiple targets depending on musl could be built. However, that implementation contained a couple of omissions: the `musl-root` option was added to the config, but was never added to the TOML parsing, and therefore was not actually being loaded from `config.toml`. This PR rectifies that.

Using these changes and a heavily modified version of the buildbot Docker container, I have been able to build rust targeting `armv7-unknown-linux-musleabihf` and have successfully run the binaries on a Raspberry Pi 3. I'm also planning to test `arm-unknown-linux-musleabi` and `arm-unknown-linux-musleabihf` systems, but have no reason to believe that this change would not simply work on those targets.
bors added a commit that referenced this pull request Oct 6, 2016
@bors bors merged commit 7937f6c into rust-lang:master Oct 6, 2016
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

Successfully merging this pull request may close these issues.

4 participants