Skip to content

Force posix-style quoting on lld, independent of host platform - #77543

Merged
bors merged 1 commit into
rust-lang:masterfrom
Mark-Simulacrum:rsp-quoting
Oct 5, 2020
Merged

Force posix-style quoting on lld, independent of host platform#77543
bors merged 1 commit into
rust-lang:masterfrom
Mark-Simulacrum:rsp-quoting

Conversation

@Mark-Simulacrum

@Mark-SimulacrumMark-Simulacrum commented Oct 4, 2020

Copy link
Copy Markdown
Member

This just blindly applies the logic from @eddyb's comment here: #76466 (comment)

Hopefully, this fixed#76466 -- I cannot test this though.

@rust-highfive

rust-highfive commented Oct 4, 2020

Copy link
Copy Markdown
Contributor

r? @eddyb

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

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 4, 2020
@Mark-Simulacrum
Mark-Simulacrumforce-pushed the rsp-quoting branch 3 times, most recently from 9b62c0b to ec7a77aCompareOctober 4, 2020 20:20
@Mark-SimulacrumMark-Simulacrum mentioned this pull request Oct 4, 2020
Closed
@Mark-Simulacrum

Mark-Simulacrum commented Oct 4, 2020

Copy link
Copy Markdown
MemberAuthor

@bors try

@bors

bors commented Oct 4, 2020

Copy link
Copy Markdown
Collaborator

⌛ Trying commit ec7a77a4631bb005d78b755a9584cbdbf2c756d6 with merge c4ca2fa230dcaef515acf148ec70c1850bbe5dc1...

@bors

bors commented Oct 4, 2020

Copy link
Copy Markdown
Collaborator

☀️ Try build successful - checks-actions, checks-azure
Build commit: c4ca2fa230dcaef515acf148ec70c1850bbe5dc1 (c4ca2fa230dcaef515acf148ec70c1850bbe5dc1)

@Mark-Simulacrum

Mark-Simulacrum commented Oct 5, 2020

Copy link
Copy Markdown
MemberAuthor

It looks like we have a confirmation that this is a fix for the LLD bug, so I am unilaterally beta accepting - this fixes a critical regression, and seems like a straightforward and reasonable patch.

@Mark-SimulacrumMark-Simulacrum added beta-accepted Accepted for backporting to the compiler in the beta channel. beta-nominated Nominated for backporting to the compiler in the beta channel. labels Oct 5, 2020
@Mark-Simulacrum
Instruct lld that our @ files are posix-style, not Windows
e8325b0
An upstream LLVM change changed behavior here to respect the host system quoting
rules; previously the posix-style format was always used for @files.
@eddyb

eddyb commented Oct 5, 2020

Copy link
Copy Markdown
Contributor

@bors r+

@bors

bors commented Oct 5, 2020

Copy link
Copy Markdown
Collaborator

📌 Commit e8325b0 has been approved by eddyb

@borsbors 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 Oct 5, 2020
@Mark-Simulacrum

Mark-Simulacrum commented Oct 5, 2020

Copy link
Copy Markdown
MemberAuthor

@bors p=1

@bors

bors commented Oct 5, 2020

Copy link
Copy Markdown
Collaborator

⌛ Testing commit e8325b0 with merge f317a93...

@bors

bors commented Oct 5, 2020

Copy link
Copy Markdown
Collaborator

☀️ Test successful - checks-actions, checks-azure
Approved by: eddyb
Pushing f317a93 to master...

@borsbors added the merged-by-bors This PR was explicitly merged by bors. label Oct 5, 2020
@bors
bors merged commit f317a93 into rust-lang:masterOct 5, 2020
@rustbotrustbot added this to the 1.49.0 milestone Oct 5, 2020
@Mark-SimulacrumMark-Simulacrum mentioned this pull request Oct 5, 2020
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 5, 2020
@bors
Auto merge ofrust-lang#77577- Mark-Simulacrum:stable-next, r=pietro…
90ed0a8
…albini
[stable] 1.47 release
This PR includes backports of:
* Fix miscompile in SimplifyBranchSame rust-lang#77549
* Force posix-style quoting on lld, independent of host platform rust-lang#77543
Note that both are still beta-nominated/beta-accepted, as they need to be backported to 1.48 as well (future beta branch).
@Mark-SimulacrumMark-Simulacrum removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Oct 8, 2020
@Mark-SimulacrumMark-Simulacrum modified the milestones: 1.49.0, 1.48.0Oct 8, 2020
@Mark-SimulacrumMark-Simulacrum mentioned this pull request Oct 8, 2020
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 8, 2020
@bors
Auto merge ofrust-lang#77705- Mark-Simulacrum:beta-next, r=Mark-Sim…
212e76c
…ulacrum
[beta] backports
This backports the following:
* Improve build-manifest to work with the improved promote-release rust-lang#77407
* Force posix-style quoting on lld, independent of host platform rust-lang#77543
* Fix miscompile in SimplifyBranchSame rust-lang#77549
* Update RLS and Rustfmt rust-lang#77590
* Move `EarlyOtherwiseBranch` to mir-opt-level 2 rust-lang#77582
@cuviper

cuviper commented Oct 23, 2020

Copy link
Copy Markdown
Member

@Mark-Simulacrum is there a reason this was only for LldFlavor::Wasm? There's now evidence of a problem elsewhere:
https://users.rust-lang.org/t/linking-with-lld-fails-but-only-in-32-bit-due-to-space-in-path/50521/

@Mark-Simulacrum
Mark-Simulacrum deleted the rsp-quoting branch October 23, 2020 01:35
@Mark-Simulacrum

Mark-Simulacrum commented Oct 23, 2020

Copy link
Copy Markdown
MemberAuthor

@eddyb indicated it would break things if it wasn't (https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/last.20minute.20beta.20backport/near/212289115); I did not follow up beyond that. Given the insta-stable backport I was hesitant to be too general, especially as I personally cannot test on windows at all.

@mati865

mati865 commented Oct 23, 2020

Copy link
Copy Markdown
Member

Mentioned llvm/llvm-project@928e9e1 did change only ELF backed. So there must have been another change for COFF that was missed.

@Boscop

Boscop commented Oct 30, 2020

Copy link
Copy Markdown

I'd really appreciate if this can be fixed :)

(Currently it's preventing me from building my VST plugin for 32-bit with lld.)

@Mark-Simulacrum

Mark-Simulacrum commented Oct 30, 2020

Copy link
Copy Markdown
MemberAuthor

I highly recommend filing an issue if this is still a problem. I suspect digging through lld commit history to figure out if perhaps the quoting default changed on other platforms as well would be worthwhile to try and track down the problem.

@Boscop

Boscop commented Oct 31, 2020

Copy link
Copy Markdown

@Mark-Simulacrum Do you mean filing an issue on this repo? Or where?

@Mark-Simulacrum

Mark-Simulacrum commented Oct 31, 2020

Copy link
Copy Markdown
MemberAuthor

Yes, on this repository, ideally with steps for reproduction.

@BoscopBoscop mentioned this pull request Nov 2, 2020
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@eddybeddybeddyb approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@eddybeddyb

Labels

beta-acceptedAccepted for backporting to the compiler in the beta channel.merged-by-borsThis PR was explicitly merged by bors.S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Milestone

1.48.0

Development

Successfully merging this pull request may close these issues.

Linker error with wasm target with spaces in install path

8 participants

@Mark-Simulacrum@rust-highfive@bors@eddyb@cuviper@mati865@Boscop@rustbot