Skip to content

Do not assume dynamic linking in musl/mips targets #908

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

Merged
merged 1 commit into from
Jan 24, 2018

Conversation

malbarbo
Copy link
Contributor

No description provided.

@malbarbo
Copy link
Contributor Author

Required for rust-lang/rust#47663

@alexcrichton
Copy link
Member

Due to the usage of target_feature I think this won't break the current target when landing, right?

@malbarbo
Copy link
Contributor Author

I think it will break... The target definition is initialized using ..super::linux_base::opts(), which returns crt_static_respected = false...

@malbarbo
Copy link
Contributor Author

Considering that we are not changing the default linking mode in rust-lang/rust#47663 anymore, can this be merged?

@alexcrichton
Copy link
Member

@malbarbo hm I'm not sure I could say w/o testing, mind testing before and after this change to see if the targets build correctly?

@malbarbo
Copy link
Contributor Author

I had tested it with both static and dynamic linking. I tested it again after changing crt_static_respected = false. Both works as expected.

I had forgotten to commit the -fPIC changes, but I pushed it now.

@alexcrichton
Copy link
Member

Ok!

@bors: r+

@bors
Copy link
Contributor

bors commented Jan 24, 2018

📌 Commit 042b707 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Jan 24, 2018

⌛ Testing commit 042b707 with merge 56444a4...

bors added a commit that referenced this pull request Jan 24, 2018
Do not assume dynamic linking in musl/mips targets
@bors
Copy link
Contributor

bors commented Jan 24, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 56444a4 to master...

@bors bors merged commit 042b707 into rust-lang:master Jan 24, 2018
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.

3 participants