Skip to content

Pass deployment target to cc linker with -m*-version-min= #136333

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

madsmtm
Copy link
Contributor

@madsmtm madsmtm commented Jan 31, 2025

Clang supports many ways of passing the deployment target, and prefers that you use the -target flag for doing so. This, however, works poorly work with Clang configuration files (at least before llvm/llvm-project#111387) and Zig CC.

To support GCC, we already passed the deployment target using the older, more widely supported -arch + -mmacosx-version-min= flag combination when compiling for macOS; let's do a similar thing for the other Apple targets too using -miphoneos-version-min=, -mtargetos= etc.

Note that when passing the target triple to the LLVM backend, we still have to merge the deployment target into that.

See also the similar fix in cc-rs where the problem is more prominent, exactly because we did not already do it for macOS.

@rustbot label O-apple
r? compiler
CC @thomcc @BlackHoleFox

Clang supports many ways of passing the deployment target, and prefers
that you use the `-target` flag for doing so.

This works poorly work with configuration files and Zig CC though, so
we use the older `-mmacosx-version-min=`, `-miphoneos-version-min=`,
`-mtargetos=` etc. flags to pass the deployment target instead.
@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. O-apple Operating system: Apple (macOS, iOS, tvOS, visionOS, watchOS) labels Jan 31, 2025
@lcnr
Copy link
Contributor

lcnr commented Jan 31, 2025

r? compiler

@rustbot rustbot assigned estebank and unassigned lcnr Jan 31, 2025
@briansmith
Copy link
Contributor

See rust-lang/cc-rs#1339 (comment); the cc-rs change may need to be improved.

@madsmtm
Copy link
Contributor Author

madsmtm commented Feb 3, 2025

See rust-lang/cc-rs#1339 (comment); the cc-rs change may need to be improved.

Yeah, the cc-rs change was a bit wrong (have filed rust-lang/cc-rs#1384 to fix it), but that shouldn't affect rustc, since this PR also makes sure to not pass --target in the first place.

@madsmtm
Copy link
Contributor Author

madsmtm commented Feb 5, 2025

The related cc-rs change has seen a bit of breakage, so I'll wait let's say 3 weeks with this one until we're sure that part is sorted out.

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 5, 2025
@LFS6502
Copy link
Contributor

LFS6502 commented Mar 1, 2025

@madsmtm Any update on this PR? Thanks for your contribution.

@bors
Copy link
Collaborator

bors commented Mar 28, 2025

☔ The latest upstream changes (presumably #139037) made this pull request unmergeable. Please resolve the merge conflicts.

@madsmtm
Copy link
Contributor Author

madsmtm commented Mar 28, 2025

No update, I'll try to get back to it, but I need to think a bit to ensure that this is the right approach.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
O-apple Operating system: Apple (macOS, iOS, tvOS, visionOS, watchOS) S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. 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.

7 participants