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

Fix passing MACOSX_DEPLOYMENT_TARGET to the linker #105123

Merged
merged 1 commit into from
Dec 4, 2022

Conversation

BlackHoleFox
Copy link
Contributor

I messed up in #103929 when merging the two base files together and as a result, started ignoring MACOSX_DEPLOYMENT_TARGET at the linker level. This ended up being the cause of nighty builds not running on older macOS versions.

My original hope with the previous PR was that CI would have caught something like that but there were only tests checking the compiler target definitions in codegen tests. Because of how badly this sucks to break, I put together a new test via run-make that actually confirms the deployment target set makes it to the linker instead of just LLVM.

Closes #104570 (for real this time)

@rustbot
Copy link
Collaborator

rustbot commented Dec 1, 2022

r? @oli-obk

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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 1, 2022
@rustbot
Copy link
Collaborator

rustbot commented Dec 1, 2022

These commits modify compiler targets.
(See the Target Tier Policy.)

@rust-log-analyzer

This comment has been minimized.

@BlackHoleFox
Copy link
Contributor Author

@rustbot label O-macos

@rustbot rustbot added the O-macos Operating system: macOS label Dec 1, 2022
@oli-obk
Copy link
Contributor

oli-obk commented Dec 1, 2022

@thomcc @petrochenkov do you want to have a look at this considering you reviewed the original PR?

Copy link
Member

@thomcc thomcc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple concerns with the regression test but otherwise looks good. Sorry that I missed this issue initially.

src/test/run-make/macos-deployment-target/Makefile Outdated Show resolved Hide resolved
src/test/run-make/macos-deployment-target/Makefile Outdated Show resolved Hide resolved
@oli-obk
Copy link
Contributor

oli-obk commented Dec 3, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Dec 3, 2022

📌 Commit 56592d3 has been approved by oli-obk

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 Dec 3, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 4, 2022
…iaskrgr

Rollup of 4 pull requests

Successful merges:

 - rust-lang#104856 (Don't suggest associated function call for associated const.)
 - rust-lang#105123 (Fix passing MACOSX_DEPLOYMENT_TARGET to the linker)
 - rust-lang#105142 (Make inline const block `ExprWithBlock`)
 - rust-lang#105237 (Add regression test for rust-lang#79450)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 7fe9597 into rust-lang:master Dec 4, 2022
@rustbot rustbot added this to the 1.67.0 milestone Dec 4, 2022
@BlackHoleFox BlackHoleFox deleted the fixing-the-macos-deployment branch December 4, 2022 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-macos Operating system: macOS S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

rustup-installed nightly compiler no longer works on MacOS Mojave
7 participants