-
Notifications
You must be signed in to change notification settings - Fork 888
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 x86_64-unknown-linux-musl build support #1517
Conversation
☔ The latest upstream changes (presumably 6d47fed) made this pull request unmergeable. Please resolve the merge conflicts. |
b7ff768
to
6de827e
Compare
Hey @strfry, First, thanks for the help :)
EDIT: I see the error now.
I'm not sure, I upstreamed a few commits from musl / alpine needed changes into llvm 5.0 which was 2 releases ago now in the hopes that later we could get rustup support for alpine. @alexcrichton could you weigh in with some direction/feedback? Best, |
@martell , I started out with just adding bash (and then later rustc and cargo), but the way this is supposed to work is that the rustc is passed in through a Docker volume from the Travis host system. And that doesn't work when the container is a different architecture than the host distro. The other part i ment is getting rust itself to compile x86_64-musl as a host architecture, so that it's provided for rustup to download. |
@martell I think it's probably not worth doing this until we get a musl host rustc working, otherwise this rustup won't have anything to download! When it works though it'll be easiest to compile a static rustup here, just using cross compilation from a glibc environment to |
Seeing as #1053 is blocked on upstream rust-lang/rust#57439, I think it makes sense to close this PR in the meantime. Please note that a closed PR isn't a rejected PR, the work here can be reused and resubmitted. |
Sorry for the delay, I'll close this for now until the host support has been resolved. |
IIUC this needs EDIT: https://github.com/rust-lang/rustup.rs/blob/master/rustup-init.sh would also need to detect if host is musl based. I tested it on Void Linux and Alpine Linux and checking output of
|
6f06325
to
4549859
Compare
@alexcrichton soft ping |
4bc6899
to
56b721e
Compare
Ah oops thanks for the ping. I'm not sure the container will work here though since it's primarily used with glibc toolchains, although you can edit .travis.yml to see the failure on the PR. Could the build be configured to do a cross build from a glibc toolchain to a musl toolchain? |
f8f4803
to
44ddbe9
Compare
Finally got some time this weekend to have another look. @alexcrichton I believe this should be mergeable now. Thanks again for the suggestion @rye. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not in a position to review the musl-specific bits, but one comment I do have is regarding the docker machine.
@lzutao put a lot of effort into allowing us to reuse the Rust compiler docker images in order to ensure that we were matching libcs etc wherever possible. Is there a suitable rust-compiler docker for musl to save the creation of the docker from ubuntu?
@kinnison Yes, Rustc has |
@lzutao Do you think you could look at what's been done on this PR and then see if you can come up with a proposed patch to reuse the rustc docker if appropriate? |
This all looks good to me, thanks @martell! @kinnison FWIW musl is pretty special in that it doesn't require the same docker containers as other targets. The main difference is that the musl target is by default statically linked instead of having a dependency on a dynamically linked libc. For glibc platforms you need a special toolchain for targeting a really old glibc (which is our goal in rust-lang/rust and what rustup wants to mirror), but for musl there's no need since any toolchain can produce a workable binary! In that sense this should be good to go IMO w/o docker changes. |
Awesome, I'm happy to do any docker changes as a follow up if anyone wishes. @alexcrichton I do not have permission to merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR might still fail because of missing sccache.
Note that I'm personally just commenting here, I'm leaving the final review up to @kinnison |
If there's no value in using the rustc docker for now, then I'll wait on the |
@kinnison done :) |
@martell You could use ci/docker/armv7-unknown-linux-gnueabihf/Dockerfile as an example to |
The appveyor failure is spurious (possibly due to too many parallel tests at once?) |
I pushed some changes to my branch. You could cherry-pick it to your PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Edit ci/cloudfront-invalidation.txt
and add musl targets.
It's not entirely clear if the cloudfront invalidation would be necessary, but it can't hurt to have it amended. |
Ok, I take it at #1882 |
Thanks @lzutao |
Closes #1053