Move crt-static and linking to build script on windows #1227
Conversation
|
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. |
|
Thanks for this! We'll actually need to keep the current |
build.rs
Outdated
| @@ -10,6 +10,18 @@ fn main() { | |||
| if rustc_minor_version().expect("Failed to get rustc version") >= 30 { | |||
| println!("cargo:rustc-cfg=core_cvoid"); | |||
| } | |||
|
|
|||
| if cfg!(target_env = "msvc") { | |||
There was a problem hiding this comment.
Doing this based on cfgs is actually wrong because this is for the host triple, while the decision of what to link should be based on the target triple. You'd need to analyze the TARGET environment variable.
There was a problem hiding this comment.
Thanks, I wasn't aware of that. I'll follow up with another commit soon.
What about the cfg below for crt-static, is that ok?
Nvm, just found it at the bottom of this doc.
|
Looks good to me! r=me with green CI |
|
Of course, though I'm not familiar with appveyor and would appreciate some guidance. |
|
@ArmsOfSorrow basically I think we should add a "test-crate" that is That would be just adding a |
JohnTitor
added
S-waiting-on-review
S-waiting-on-author
and removed
S-waiting-on-review
labels
|
Hey @ArmsOfSorrow, I've tested the code from https://users.rust-lang.org/t/linker-error-when-building-no-std-windows-dll-that-uses-libc/23976 without this PR but I cannot reproduce the linker error. Could you check if it's still an issue? If so, it'd be great if you could provide your environment information and test code. |
|
Oof, that's quite an old issue. I don't think I got the test code for it anymore. The goal behind this was to get UWP support IIRC, but that's already been done a few releases ago. I'm not sure if this PR is still relevant. |
|
Then I'm going to just clone this and the issue as the fix seems unnecessary now. Feel free to reach out if it becomes an issue again. |
Seems to fix #1203.
I've tested with a local
cdylibdepending on libc in bothstdandno_stdmode. Not quite sure about thecrt-staticcase, althoughlibcitself did compile when passingRUSTFLAGS='-C target-feature=+crt-static'.I'd appreciate someone else testing this to avoid "works on my machine"-issues.