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

musl: build toolchain libs with -fPIC #59468

Merged
merged 1 commit into from
Mar 29, 2019
Merged

Conversation

mati865
Copy link
Contributor

@mati865 mati865 commented Mar 27, 2019

Fixes #59411

@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 27, 2019
@Centril
Copy link
Contributor

Centril commented Mar 27, 2019

r? @nagisa

@rust-highfive rust-highfive assigned nagisa and unassigned nikomatsakis Mar 27, 2019
@pnkfelix
Copy link
Member

Hmm. The only reason I could imagine to try to avoid doing this would be if we didn't want to apply position-independent code everywhere for MUSL, and the only reason I could imagine doing that would be if we thought there would be lost optimization opportunities.

But, given that we are seeing an important customer (ripgrep) fail to build, and this change fixes it, I'm going to go ahead and take the plunge here, approving the change without first trying to evaluation the performance impact (which I honestly suspect will be insignificant in most and maybe all cases).

@pnkfelix
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Mar 28, 2019

📌 Commit c764890 has been approved by pnkfelix

@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 Mar 28, 2019
@Centril
Copy link
Contributor

Centril commented Mar 28, 2019

r? @pnkfelix

@mati865
Copy link
Contributor Author

mati865 commented Mar 28, 2019

In fact this PR sort of brings PIC back. Before #58575 x86_64-unknown-linux-musl was build from this script: https://github.com/rust-lang/rust/blob/237bf3244fffef501cf37d4bda00e1fce3fcfb46/src/ci/docker/scripts/musl.sh.

The day will come when all binutils < 2.26 distros are EOL (for Ubuntu 14.04 it's end of the next month and for CentOS 6 it's end of 2020). That will be beautiful day because PIC and another workaround can be dropped without causing breakage to supported distros.

@pnkfelix
Copy link
Member

thanks @mati865 for the clarification!

Centril added a commit to Centril/rust that referenced this pull request Mar 29, 2019
bors added a commit that referenced this pull request Mar 29, 2019
Rollup of 11 pull requests

Successful merges:

 - #58019 (Combine all builtin late lints and make lint checking parallel)
 - #59358 (Use `track_errors` instead of hand rolling)
 - #59394 (warn -> deny duplicate match bindings)
 - #59401 (bootstrap: build crates under libtest with -Z emit-stack-sizes)
 - #59423 (Visit path in `walk_mac`)
 - #59468 (musl: build toolchain libs with -fPIC)
 - #59476 (Use `SmallVec` in `TokenStreamBuilder`.)
 - #59496 (Remove unnecessary with_globals calls)
 - #59498 (Use 'write_all' instead of 'write' in example code)
 - #59503 (Stablize {f32,f64}::copysign().)
 - #59511 (Fix missed fn rename in #59284)

Failed merges:

r? @ghost
bors added a commit that referenced this pull request Mar 29, 2019
Rollup of 11 pull requests

Successful merges:

 - #58019 (Combine all builtin late lints and make lint checking parallel)
 - #59358 (Use `track_errors` instead of hand rolling)
 - #59394 (warn -> deny duplicate match bindings)
 - #59401 (bootstrap: build crates under libtest with -Z emit-stack-sizes)
 - #59423 (Visit path in `walk_mac`)
 - #59468 (musl: build toolchain libs with -fPIC)
 - #59476 (Use `SmallVec` in `TokenStreamBuilder`.)
 - #59496 (Remove unnecessary with_globals calls)
 - #59498 (Use 'write_all' instead of 'write' in example code)
 - #59503 (Stablize {f32,f64}::copysign().)
 - #59511 (Fix missed fn rename in #59284)

Failed merges:

r? @ghost
@bors bors merged commit c764890 into rust-lang:master Mar 29, 2019
@mati865 mati865 deleted the musl_toolchain branch March 29, 2019 11:28
BurntSushi added a commit to BurntSushi/ripgrep that referenced this pull request Apr 3, 2019
Rust is having problems with trusty, in particular, see this bug I
filed: rust-lang/rust#59411

This was purpotedly fixed in
rust-lang/rust#59468,
but it appears the issue is still occurring.

This commit tries to update to Ubuntu 16.04 in the hope that it will fix
this problem.
BurntSushi added a commit to BurntSushi/ripgrep that referenced this pull request Apr 3, 2019
Rust is having problems with trusty, in particular, see this bug I
filed: rust-lang/rust#59411

This was purpotedly fixed in
rust-lang/rust#59468,
but it appears the issue is still occurring.

This commit tries to update to Ubuntu 16.04 in the hope that it will fix
this problem.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants