-
Notifications
You must be signed in to change notification settings - Fork 13.7k
Update cc-rs to 1.2.33, and switch rustc_codegen_ssa to use find-msvc-tools #146186
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
base: master
Are you sure you want to change the base?
Conversation
Failed to set assignee to
|
These commits modify the If this was unintentional then you should revert the changes before this PR is merged. Some changes occurred in compiler/rustc_codegen_ssa |
@bors try jobs=aarch64-msvc-1,aarch64-apple,test-various,dist-apple-various |
This comment has been minimized.
This comment has been minimized.
Update cc-rs to 1.2.33 try-job: aarch64-msvc-1 try-job: aarch64-apple try-job: test-various try-job: dist-apple-various
Let me know @dpaoliello if you want to try with |
Reminder, once the PR becomes ready for a review, use |
This comment has been minimized.
This comment has been minimized.
I switched to @rustbot ready |
This comment has been minimized.
This comment has been minimized.
The list of allowed third-party dependencies may have been modified! You must ensure that any new dependencies have compatible licenses before merging. |
@bors r=davidtwco,jieyouxu |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, wrong bors |
✌️ @dpaoliello, you can now perform try builds on this pull request! You can now post |
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
@bors2 try jobs=dist-aarch64-linux |
This comment has been minimized.
This comment has been minimized.
Update cc-rs to 1.2.33, and switch rustc_codegen_ssa to use find-msvc-tools try-job: dist-aarch64-linux
This comment has been minimized.
This comment has been minimized.
💔 Test for 501f1bd failed: CI. Failed jobs:
|
There we go:
Now, what to do about it? I'm not a Linux dev, so I have no idea what's gone wrong... |
Bisected the change next doors (#146226 (comment)) to |
🤔 gcc's
clang's
|
Let me ask other people as well EDIT: asked in #t-compiler/help > compiler `cc` bump failure Unfortunately I won't have time to dig into this, not until next weekend. |
@bors2 try jobs=dist-aarch64-linux |
Update cc-rs to 1.2.33, and switch rustc_codegen_ssa to use find-msvc-tools try-job: dist-aarch64-linux
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
💔 Test for 083d034 failed: CI. Failed jobs:
|
☔ The latest upstream changes (presumably #146018) made this pull request unmergeable. Please resolve the merge conflicts. |
@dpaoliello nice cleanup. Please consider cherrypicking lambdageek@797c4b1 which should switch |
@@ -117,7 +117,6 @@ pub(crate) fn get_linker<'a>( | |||
if sess.target.is_like_msvc | |||
&& let Some(ref tool) = msvc_tool | |||
{ | |||
cmd.args(tool.args()); |
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.
Why is this ok? It looked to me like cc::windows_registry::find_tool
can have extra arguments here, while find_msvc_tools::find_tool
cannot
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 old windows_registry::find_tool
never set any args, which is why find_msvc_tools
dropped it
https://github.com/rust-lang/cc-rs/blob/3c1325b09a78827fb2beb3ea9e8f1e3f84876b64/src/windows/find_tools.rs#L359-L371
I have concerns with that change: I don't think we can rely on I will switch |
Is the concern that it might be set to something unexpected when bootstrapping the compiler? |
Decided to implement this myself: rust-lang/cc-rs#1553 |
For my purposes, contains fixes when compiling the Rust compiler for Arm64EC.
Checked the commits since 1.2.16, and I don't see anything else that may affect Rust?
find-msvc-tools
was also factored out fromcc
to allow updating the use inrustc_codegen_ssa
(finding the linker when running the Rust compiler) to be separate from the use inrustc_llvm
(building LLVM as part of the Rust compiler).