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

Change implementation of -Z gcc-ld and lld-wrapper again #100200

Merged
merged 1 commit into from
Sep 2, 2022

Conversation

petrochenkov
Copy link
Contributor

@petrochenkov petrochenkov commented Aug 6, 2022

This PR partially reverts #97375 and uses the strategy described in #97402 (comment) instead, thus fixes #97755.

@rustbot rustbot added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 6, 2022
@rust-highfive
Copy link
Collaborator

r? @compiler-errors

(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 Aug 6, 2022
@compiler-errors
Copy link
Member

r? compiler

@michaelwoerister
Copy link
Member

Sorry for not following up sooner.

@petrochenkov, do you have a reviewer in mind for this? The strategy outlined #97402 (comment) sounds reasonable to me, but I'm lacking the knowledge to determine if there could be problems with it.

cc @Mark-Simulacrum, since this is touching dist.

@Mark-Simulacrum
Copy link
Member

bootstrap changes here look fine to me.

@petrochenkov
Copy link
Contributor Author

r? @lqd maybe

@lqd
Copy link
Member

lqd commented Aug 24, 2022

Sorry for the wait, I'm now back from vacation and will take look at this tomorrow.

@lqd
Copy link
Member

lqd commented Aug 29, 2022

A test to ensure that -Zgcc-ld=lld works on osx now and in the future would have been good to have (especially with an eye towards the rust-lld MCP) so I've been trying to reproduce the initial issue, without success unfortunately.

I'm not sure which version of clang (or macos) was used and when/why it starts switching to response files instead of CLI args. Even with many long link args (the CLI was 23K chars), it still seems to work on an M1 on Monterey (maybe this only happens on x64 darwin but it seems doubtful).

This PR looks good to me as well, but I'd like to make sure: @tschuett, does the thumbs up you left on the OP mean that you were able to test this PR on the project triggering #97755 successfully ? Would you have a repository where we could reproduce the issue by any chance ?

I'd like to have an answer and hopefully I can reduce a test out of that, before merging this PR.

But if it's not possible, and @tschuett has indeed validated the fix, r=me (+mark).

@tschuett
Copy link

I have 195 crates to build for my application.

> clang --version
Apple clang version 13.1.6 (clang-1316.0.21.2.5)
Target: x86_64-apple-darwin21.6.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin
> rustc +nightly --verbose --version
rustc 1.65.0-nightly (ce36e8825 2022-08-28)
binary: rustc
commit-hash: ce36e88256f09078519f8bc6b21e4dc88f88f523
commit-date: 2022-08-28
host: x86_64-apple-darwin
release: 1.65.0-nightly
LLVM version: 15.0.0

and it failed again. I cannot test this PR. sorry.

@lqd
Copy link
Member

lqd commented Aug 29, 2022

Thank you; I have the same clang version.

We could temporarily switch the try builder to osx if we wanted to make a single build for you to try out, but things aren't going to get much worse than "-Zgcc-ld=lld is not working sometimes", and I've stress tested this branch on linux as well without issues.

So I'll test the nightly where this PR lands for the successful osx cases I saw, and hopefully you can keep us updated on how it behaves for you -- we'll keep an eye on possible new issues as well.

@bors r=lqd,Mark-Simulacrum

Thanks @petrochenkov.

@bors
Copy link
Contributor

bors commented Aug 29, 2022

📌 Commit 6b68921 has been approved by lqd,Mark-Simulacrum

It is now in the queue for this repository.

@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 Aug 29, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 1, 2022
…imulacrum

Change implementation of `-Z gcc-ld` and `lld-wrapper` again

This PR partially reverts rust-lang#97375 and uses the strategy described in rust-lang#97402 (comment) instead, thus fixes rust-lang#97755.
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Sep 1, 2022
…imulacrum

Change implementation of `-Z gcc-ld` and `lld-wrapper` again

This PR partially reverts rust-lang#97375 and uses the strategy described in rust-lang#97402 (comment) instead, thus fixes rust-lang#97755.
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Sep 1, 2022
…imulacrum

Change implementation of `-Z gcc-ld` and `lld-wrapper` again

This PR partially reverts rust-lang#97375 and uses the strategy described in rust-lang#97402 (comment) instead, thus fixes rust-lang#97755.
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Sep 1, 2022
…imulacrum

Change implementation of `-Z gcc-ld` and `lld-wrapper` again

This PR partially reverts rust-lang#97375 and uses the strategy described in rust-lang#97402 (comment) instead, thus fixes rust-lang#97755.
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Sep 2, 2022
…imulacrum

Change implementation of `-Z gcc-ld` and `lld-wrapper` again

This PR partially reverts rust-lang#97375 and uses the strategy described in rust-lang#97402 (comment) instead, thus fixes rust-lang#97755.
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Sep 2, 2022
…imulacrum

Change implementation of `-Z gcc-ld` and `lld-wrapper` again

This PR partially reverts rust-lang#97375 and uses the strategy described in rust-lang#97402 (comment) instead, thus fixes rust-lang#97755.
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Sep 2, 2022
…imulacrum

Change implementation of `-Z gcc-ld` and `lld-wrapper` again

This PR partially reverts rust-lang#97375 and uses the strategy described in rust-lang#97402 (comment) instead, thus fixes rust-lang#97755.
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Sep 2, 2022
…imulacrum

Change implementation of `-Z gcc-ld` and `lld-wrapper` again

This PR partially reverts rust-lang#97375 and uses the strategy described in rust-lang#97402 (comment) instead, thus fixes rust-lang#97755.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 2, 2022
…imulacrum

Change implementation of `-Z gcc-ld` and `lld-wrapper` again

This PR partially reverts rust-lang#97375 and uses the strategy described in rust-lang#97402 (comment) instead, thus fixes rust-lang#97755.
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Sep 2, 2022
…imulacrum

Change implementation of `-Z gcc-ld` and `lld-wrapper` again

This PR partially reverts rust-lang#97375 and uses the strategy described in rust-lang#97402 (comment) instead, thus fixes rust-lang#97755.
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Sep 2, 2022
…imulacrum

Change implementation of `-Z gcc-ld` and `lld-wrapper` again

This PR partially reverts rust-lang#97375 and uses the strategy described in rust-lang#97402 (comment) instead, thus fixes rust-lang#97755.
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 2, 2022
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#100121 (Try normalizing types without RevealAll in ParamEnv in MIR validation)
 - rust-lang#100200 (Change implementation of `-Z gcc-ld` and `lld-wrapper` again)
 - rust-lang#100814 ( Porting 'compiler/rustc_trait_selection' to translatable diagnostics - Part 1)
 - rust-lang#101215 (Also replace the version placeholder in rustc_attr)
 - rust-lang#101260 (Use `FILE_ATTRIBUTE_TAG_INFO` to get reparse tag)
 - rust-lang#101323 (Remove unused .toggle-label CSS rule)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit ed37111 into rust-lang:master Sep 2, 2022
@rustbot rustbot added this to the 1.65.0 milestone Sep 2, 2022
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. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

lld-wrapper failed on Darwin
9 participants