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.

Sign up for GitHub

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

Jump to bottom

Add deployment-target --print flag for Apple targets #105354

Merged
merged 1 commit into from May 8, 2023
Merged

Add deployment-target --print flag for Apple targets #105354

merged 1 commit into from May 8, 2023

Conversation

BlackHoleFox
Copy link
Contributor

This is very useful for crates that need to know what the Apple OS deployment target is for their build scripts or inside of a build environment. Right now, the defaults just get copy/pasted around the ecosystem since they've been stable for so long. But with #104385 in progress, that won't be true anymore and everything will need to move. Ideally whenever it happens again, this could be less painful as everything can ask the compiler what its default is instead.

To show examples of the copy/paste proliferation, here's some crates and/or apps that do:

@rustbot
Copy link
Collaborator

rustbot commented Dec 6, 2022

r? @compiler-errors

(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 6, 2022
@rustbot
Copy link
Collaborator

rustbot commented Dec 6, 2022

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

@BlackHoleFox
Copy link
Contributor Author

@rustbot label O-macos O-ios

@rustbot rustbot added O-ios Operating system: iOS O-macos Operating system: macOS labels Dec 6, 2022
@BlackHoleFox
Copy link
Contributor Author

So far I'm unsure of the flag name. minimum-version might be better but deployment-version might be easier to find?

@BlackHoleFox BlackHoleFox force-pushed the apple-deployment-printer branch 2 times, most recently from 2d1db73 to 149846e Compare December 6, 2022 07:12
@compiler-errors
Copy link
Member

r? compiler

@rustbot rustbot assigned lcnr and unassigned compiler-errors Dec 7, 2022
@oli-obk
Copy link
Contributor

oli-obk commented Dec 16, 2022

r? @oli-obk

@rustbot rustbot assigned oli-obk and unassigned lcnr Dec 16, 2022
@oli-obk
Copy link
Contributor

oli-obk commented Dec 16, 2022

This adds a new insta-stable compiler flag. This seems like a good solution to the problem for me.

@rfcbot fcp merge

@rfcbot
Copy link

rfcbot commented Dec 16, 2022 •
edited by michaelwoerister

Team member @oli-obk has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Dec 16, 2022
petrochenkov
petrochenkov reviewed Dec 17, 2022
View reviewed changes
src/doc/rustc/src/command-line-arguments.md Outdated Show resolved Hide resolved
compiler/rustc_target/src/spec/apple_base.rs Outdated
"ios" => ios_deployment_target(),
"watchos" => watchos_deployment_target(),
"tvos" => tvos_deployment_target(),
_ => unreachable!("unknown Apple target OS"),
Copy link
Contributor

Choose a reason for hiding this comment

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

This arm is reachable with custom target specs.
I suggest returning an Option here, and relying on it in the driver instead of using sess.target.is_like_osx.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, thanks, and good to know. Moved up to rustc_driver. For the future, why is doing it there better? Do custom target specs go through a different codepath that wouldn't panic?

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rustbot rustbot added the A-testsuite Area: The testsuite used to check the correctness of rustc label Dec 21, 2022
@BlackHoleFox
Copy link
Contributor Author

There's now also a test for valid use of the new flag. It isn't very picky about the output beyond requiring that it only contains {Digit}.0. Imo its not worth hardcoding rustc's minimum macOS version in another place because there's already compile tests that verify it. That and it makes updating it later more tedious.

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label May 4, 2023
Manishearth added a commit to Manishearth/rust that referenced this pull request May 4, 2023
@Manishearth
Rollup merge of rust-lang#105354 - BlackHoleFox:apple-deployment-prin…
7157ef7 
…ter, r=oli-obk

Add deployment-target --print flag for Apple targets

This is very useful for crates that need to know what the Apple OS deployment target is for their build scripts or inside of a build environment. Right now, the defaults just get copy/pasted around the ecosystem since they've been stable for so long. But with rust-lang#104385 in progress, that won't be true anymore and everything will need to move. Ideally whenever it happens again, this could be less painful as everything can ask the compiler what its default is instead.

To show examples of the copy/paste proliferation, here's some crates and/or apps that do:
- [cc](https://github.com/rust-lang/cc-rs/pull/708/files), Soon
-  [mac-notification-sys](https://github.com/h4llow3En/mac-notification-sys/pull/46/files#diff-d0d98998092552a1d3259338c2c71e118a5b8343dd4703c0c7f552ada7f9cb42R10-R12)
- [PyO3](https://github.com/PyO3/maturin/blob/ccb02d1aa1cc41e82a3572a3c8b35cace15f3e78/src/target.rs#L755-L758)
- [Anki](https://github.com/ankitects/anki/blob/613b5c1034cc9943f3f68d818ae22b2e0acec877/build/runner/src/bundle/artifacts.rs#L49-L54)
- [jsc-rs](https://github.com/Brooooooklyn/jsc-rs/blob/37767267568fb2de62fc441473e7d158dd980520/xtask/src/build.rs#L402-L405)
... and probably more that a simple GitHub codesearch didn't see
@Manishearth Manishearth mentioned this pull request May 4, 2023
Closed
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request May 4, 2023
@matthiaskrgr
Rollup merge of rust-lang#105354 - BlackHoleFox:apple-deployment-prin…
e1b2d17 
…ter, r=oli-obk

Add deployment-target --print flag for Apple targets

This is very useful for crates that need to know what the Apple OS deployment target is for their build scripts or inside of a build environment. Right now, the defaults just get copy/pasted around the ecosystem since they've been stable for so long. But with rust-lang#104385 in progress, that won't be true anymore and everything will need to move. Ideally whenever it happens again, this could be less painful as everything can ask the compiler what its default is instead.

To show examples of the copy/paste proliferation, here's some crates and/or apps that do:
- [cc](https://github.com/rust-lang/cc-rs/pull/708/files), Soon
-  [mac-notification-sys](https://github.com/h4llow3En/mac-notification-sys/pull/46/files#diff-d0d98998092552a1d3259338c2c71e118a5b8343dd4703c0c7f552ada7f9cb42R10-R12)
- [PyO3](https://github.com/PyO3/maturin/blob/ccb02d1aa1cc41e82a3572a3c8b35cace15f3e78/src/target.rs#L755-L758)
- [Anki](https://github.com/ankitects/anki/blob/613b5c1034cc9943f3f68d818ae22b2e0acec877/build/runner/src/bundle/artifacts.rs#L49-L54)
- [jsc-rs](https://github.com/Brooooooklyn/jsc-rs/blob/37767267568fb2de62fc441473e7d158dd980520/xtask/src/build.rs#L402-L405)
... and probably more that a simple GitHub codesearch didn't see
@matthiaskrgr matthiaskrgr mentioned this pull request May 4, 2023
Closed
@JohnTitor
Copy link
Member

Failed in rollup: #111219 (comment)
@bors r-

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 5, 2023
@BlackHoleFox
Copy link
Contributor Author

Should be ready again, the UI test wasn't flexible enough before to handle different deployment versions. I fixed that and tried it out locally with multiple MACOSX_DEPLOYMENT_VERSIONS.

@rustbot ready

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

oli-obk commented May 8, 2023

@bors r+

@bors
Copy link
Contributor

bors commented May 8, 2023

📌 Commit a427d41 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 May 8, 2023
@JohnTitor JohnTitor mentioned this pull request May 8, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request May 8, 2023
@bors
Auto merge of rust-lang#111346 - JohnTitor:rollup-6g5cg9z, r=JohnTitor
ce04288 
Rollup of 7 pull requests

Successful merges:

 - rust-lang#105354 (Add deployment-target --print flag for Apple targets)
 - rust-lang#110377 (Update max_atomic_width of armv7r and armv7_sony_vita targets to 64.)
 - rust-lang#110638 (STD support for PSVita)
 - rust-lang#111211 (Don't compute trait super bounds unless they're positive)
 - rust-lang#111315 (Remove `identity_future` from stdlib)
 - rust-lang#111331 (Mark s390x condition code register as clobbered in inline assembly)
 - rust-lang#111332 (Improve inline asm for LoongArch)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit e3eb6a8 into rust-lang:master May 8, 2023
11 checks passed
@rustbot rustbot added this to the 1.71.0 milestone May 8, 2023
@BlackHoleFox BlackHoleFox deleted the apple-deployment-printer branch May 8, 2023 19:24
@BlackHoleFox BlackHoleFox mentioned this pull request May 8, 2023
Closed
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label May 25, 2023
@thomcc thomcc mentioned this pull request Jul 30, 2023
Open
@BlackHoleFox BlackHoleFox mentioned this pull request Aug 4, 2023
Merged
@messense messense mentioned this pull request Aug 12, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@petrochenkov petrochenkov petrochenkov left review comments

@lcnr lcnr lcnr left review comments

@oli-obk oli-obk oli-obk requested changes

Assignees

@oli-obk oli-obk

Labels
A-testsuite Area: The testsuite used to check the correctness of rustc disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. O-ios Operating system: iOS 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-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
Milestone
1.71.0
Development

Successfully merging this pull request may close these issues.

None yet