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.

Sign up for GitHub

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

Jump to bottom

Enable rust-lld on dist-x86_64-musl #70619

Merged
merged 1 commit into from
Apr 10, 2020
Merged

Enable rust-lld on dist-x86_64-musl #70619

merged 1 commit into from
Apr 10, 2020

Conversation

stream-punk
Copy link
Contributor

@stream-punk stream-punk commented Mar 31, 2020 •
edited
Loading

Add rust-lld to rustup llvm-tools-preview on nightly for musl

I am using a musl distro on my workstation, with RUSTFLAGS="-C target-feature=-crt-static" this works fine. I know that x86_64-unknown-linux-musl was originally only meant as a target and not as a host. But most problems have been fixed, and I have fewer problems with unknown (rustup) than when I am using x86_64-alpine-linux-musl (rust installed by the distro). The only thing I am missing is rust-lld in llvm-tools-preview on nightly.

I needed rust-lld for a wasm tutorial. I built rust-lld and tested it with that tutorial, and it worked well. I asked here where to request to enable lld and ended up doing this PR.

I compared llvm-tools-preview nightly-x86_64-unknown-linux-musl and nightly-x86_64-unknown-linux-gnu: only rust-lld is missing in musl.

I tested the change using:

./src/ci/docker/run.sh dist-x86_64-musl

And I checked that the resulting rust-lld binary runs.

Enable rust-lld on dist-x86_64-musl
a61a7c5 
Add rust-lld to llvm-tools-preview on nightly for musl
@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Mark-Simulacrum (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.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 31, 2020
@stream-punk
Copy link
Contributor Author

@mati865 I see you did many changes to this file. My change is very simple. Could you review it or let me know who I should ask?

@mati865
Copy link
Contributor

mati865 commented Apr 7, 2020

No objections, I had even thought LLD is available in musl toochain already.
Though I'm not a reviewer and cannot approve it.

This was asked on Zulip last week but nobody answered.

@stream-punk
Copy link
Contributor Author

No objections, I had even thought LLD is available in musl toochain already.
Though I'm not a reviewer and cannot approve it.

Thanks for the feedback

@stream-punk
Copy link
Contributor Author

stream-punk commented Apr 7, 2020 •
edited
Loading

Hi @pietroalbini, sorry to bother you, I tried to find a overlap between people who might be reviewer and have worked the docker-ci and you turned up.

Could review it? Or who should I ask?

@pietroalbini
Copy link
Member

Thanks for this PR! @Mark-Simulacrum is the best person to review the PR, let's wait for their insights.

@Mark-Simulacrum
Copy link
Member

I may be the best person but I also don't know what we have as a policy here. Having thought a bit more about this over the past week or so though I'm inclined to just approve (since it's a -preview component and one we're unlikely to stabilize as is). In the worst case we can just revert, it's a one line patch after all :)

@bors r+

@bors
Copy link
Contributor

bors commented Apr 7, 2020

📌 Commit a61a7c5 has been approved by Mark-Simulacrum

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 7, 2020
@bors
Copy link
Contributor

bors commented Apr 7, 2020

⌛ Testing commit a61a7c5 with merge 09dcdc5ff7286270196d6cb0977490390571bde2...

@bors
Copy link
Contributor

bors commented Apr 7, 2020

💔 Test failed - checks-azure

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 7, 2020
@pietroalbini
Copy link
Member

Seems spurious.

@bors retry

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 7, 2020
@stream-punk
Copy link
Contributor Author

stream-punk commented Apr 8, 2020 •
edited
Loading

@pietroalbini it seems some kind of timeout. I guess the build takes now slightly longer. Sorry no, I misread that. No idea what happens.

@bors
Copy link
Contributor

bors commented Apr 10, 2020

⌛ Testing commit a61a7c5 with merge 167510f...

@bors
Copy link
Contributor

bors commented Apr 10, 2020

☀️ Test successful - checks-azure
Approved by: Mark-Simulacrum
Pushing 167510f to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 10, 2020
@bors bors merged commit 167510f into rust-lang:master Apr 10, 2020
@stream-punk
Copy link
Contributor Author

stream-punk commented Apr 16, 2020 •
edited
Loading

I updated nightly and tested the wasm workflow on musl-distro: rust-lld works 👍 Currently the rustup-update has to be forced, because not all components are built.

@stream-punk stream-punk mentioned this pull request Jul 28, 2020
Merged
Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 30, 2020
@Manishearth
Rollup merge of rust-lang#74871 - etherealist:musl_doc, r=Mark-Simula…
8892785 
…crum

Enable docs on dist-x86_64-musl

Add the `rust-docs` component to toolchain `x86_64-unknown-linux-musl`, which allows people using rustup on their musl-based linux distribution to download the rust-docs.

`--disable-docs` is based on the assumption that `x86_64-unknown-linux-musl` is only a cross-compile target.

I have tested that the docs are built. I assume the build-system will automatically detect the docs and create a `rust-docs` component. I will [monitor](https://rust-lang.github.io/rustup-components-history/x86_64-unknown-linux-musl.html) the components and create a follow-up PR, if the docs aren't published.

See also rust-lang#70619, where we enabled `rust-lld` to enable the wasm-workflow on musl-based linux distributions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Mark-Simulacrum Mark-Simulacrum

Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@stream-punk @rust-highfive @mati865 @pietroalbini @Mark-Simulacrum @bors