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

Build macOS distribution artifacts with XCode 13 #104650

Merged
merged 2 commits into from
Nov 25, 2022

Conversation

BlackHoleFox
Copy link
Contributor

@BlackHoleFox BlackHoleFox commented Nov 20, 2022

After all of the rust-lang/rust Apple runners started using macOS 12, the builds created by CI began to use XCode 14.0.1. Due to this (as far as we can tell), XCode's build tools started to ignore the MACOSX_DEPLOYMENT_TARGET being defined by us for the distributed builds that let both rustc and libstd work on older versions. The current idea is that since XCode 14's macOS SDK doesn't support deployment targets before 10.13, it uses some default of its own. You can see the difference between stable's and the most recent nighty's supported versions here.

I wasn't able to confirm my SDK versioning hypothesis locally since I think there's something jammed with my XCode installation, but hopefully this should still fix it for releases.

Closes #104570

r? @Mark-Simulacrum

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Nov 20, 2022
Do this because XCode 14 no longer supports a macOS deployment target
of anything before 10.13. We need 10.7+(-ish, really 10.9+) for now.
@thomcc thomcc added the O-macos Operating system: macOS label Nov 20, 2022
@Mark-Simulacrum
Copy link
Member

Hm, if so, I guess this will increase motivation for #104385 (comment) (as the last comment there notes) in terms of how straightforward targeting older versions is for dist builds...

Overall this seems like an okay stopgap though, let's see if it helps. @bors r+ rollup=never

@bors
Copy link
Contributor

bors commented Nov 20, 2022

📌 Commit cda219e has been approved by 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 Nov 20, 2022
@BlackHoleFox
Copy link
Contributor Author

fwiw I don't think this will (or intend it to) help with #104385 directly because 10.12 is still below what Apple is supporting in new versions of the macOS SDK. It would make it easier though, for example, to bump to 10.13 in 2-3 years assuming Apple hasn't dropped that too.

@Mark-Simulacrum
Copy link
Member

Hm, maybe my comment wasn't clear, but my point is rather that if XCode is dropping support for older macos, it becomes impractical for us to target those older versions for long (GitHub actions will stop providing older XCode versions etc).

@BlackHoleFox
Copy link
Contributor Author

You were clear enough I think :) We might have just talked past each other. I'm with you that the maintenance cost goes up for supporting old macOS (like here) but I don't think that "how easy is this" should be the thing determining when we move the support level forward. I especially don't think GitHub Actions, an entirely unrelated third party, should passively constrict Rust. Even in the future where XCode 13 goes poof, we should still consider 10.12 support beyond "what's directly in the CI images?" because there's other low-burden ways to handle it. I think too many people use old Macs to follow that methodology.

With the above said I don't think the current situation (maybe being forced to go from 10.12 to 10.13) is a good example because anything that runs 10.12 can run 10.13 but hopefully the idea manifests better whenever 10.13 is dropped by Apple's SDKs.

@thomcc
Copy link
Member

thomcc commented Nov 22, 2022

other low-burden ways to handle it

Can you elaborate on what these are? My impression is that it's pretty annoying...

@ehuss ehuss added beta-nominated Nominated for backporting to the compiler in the beta channel. stable-nominated Nominated for backporting to the compiler in the stable channel. labels Nov 22, 2022
@ehuss
Copy link
Contributor

ehuss commented Nov 22, 2022

Nominating for beta since this will be required to avoid regressing there.

Also nominating for stable, since this should be included iff there is a stable release for other reasons to similarly avoid a regression. (Although since we're already halfway through the cycle, I suspect this is very unlikely.)

@BlackHoleFox
Copy link
Contributor Author

Can you elaborate on what these are? My impression is that it's pretty annoying...

As I'm pretty sure its permitted by Apple's ToU, a trusted infra member could vendor XCode 13's macOS SDK (or the most minimal XCode 13 install) into an S3 bucket and fetch that for doing release builds. It might be a bit slower but it wouldn't be on every PR / bors run.

Nominating for beta since this will be required to avoid regressing there.
...
Also nominating for stable

Should it be confirmed this works on nightly first before the backport effort?

@ehuss
Copy link
Contributor

ehuss commented Nov 22, 2022

Should it be confirmed this works on nightly first before the backport effort?

Backports usually aren't processed until after this lands, and often with some longish delay, and it needs to be approved by the team. Those delays usually allow for dealing with fixes that don't work as expected. Also, from my own understanding of the changes here, the risk looks low to me.

As for the concerns about supporting this long-term, I personally wouldn't be too worried about it. GitHub so far has been pretty good about keep very old versions of XCode in their macOS images, and keeping old macOS images around for years. For example, the macOS 10.15 image has XCode 10.3 (which was released about 3.5 years ago), which looks to support a deploy target of macOS 10.6 (which was released 13 years ago). Of course these things age out faster than some would like, but I don't think it is too problematic.

@bors
Copy link
Contributor

bors commented Nov 24, 2022

⌛ Testing commit cda219e with merge fbe416da901d83cdb67299d17714cc232c057fb7...

@bors
Copy link
Contributor

bors commented Nov 24, 2022

💥 Test timed out

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 24, 2022
@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@thomcc
Copy link
Member

thomcc commented Nov 24, 2022

I don't see a reason to think this is likely to be non-spurious.

@bors retry

@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 Nov 24, 2022
@bors
Copy link
Contributor

bors commented Nov 25, 2022

⌛ Testing commit cda219e with merge 8a75c5a...

@bors
Copy link
Contributor

bors commented Nov 25, 2022

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing 8a75c5a to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 25, 2022
@bors bors merged commit 8a75c5a into rust-lang:master Nov 25, 2022
@rustbot rustbot added this to the 1.67.0 milestone Nov 25, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (8a75c5a): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.7% [-3.5%, -2.4%] 3
All ❌✅ (primary) - - 0

Cycles

This benchmark run did not return any relevant results for this metric.

@BlackHoleFox BlackHoleFox deleted the stuck-with-xcode-13 branch November 27, 2022 18:28
@pietroalbini pietroalbini added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Dec 12, 2022
@pietroalbini
Copy link
Member

Accepting for "beta" (by now stable) backport.

@pietroalbini pietroalbini removed stable-nominated Nominated for backporting to the compiler in the stable channel. beta-nominated Nominated for backporting to the compiler in the beta channel. labels Dec 12, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 12, 2022
…troalbini

[stable] Prepare 1.66.0 release

This PR prepares the artifacts for the 1.66.0 release. The following PRs have been backported:

* rust-lang#104782
* rust-lang#105023
* rust-lang#104558
* rust-lang#104610
* rust-lang#103989
* rust-lang#104650
* rust-lang#105539
* rust-lang#105477

r? `@ghost`
Aaron1011 pushed a commit to Aaron1011/rust that referenced this pull request Jan 6, 2023
…Mark-Simulacrum

Build macOS distribution artifacts with XCode 13

After all of the `rust-lang/rust` Apple runners started using macOS 12, the builds created by CI began to use XCode 14.0.1. Due to this (as far as we can tell), XCode's build tools started to ignore the `MACOSX_DEPLOYMENT_TARGET` being defined by us for the distributed builds that let both `rustc` and `libstd` work on older versions. The current idea is that since XCode 14's macOS SDK doesn't support deployment targets before 10.13, it uses some default of its own. You can see the difference between stable's and the most recent nighty's supported versions [here](rust-lang#104570 (comment)).

I wasn't able to confirm my SDK versioning hypothesis locally since I think there's something jammed with my XCode installation, but hopefully this should still fix it for releases.

Closes rust-lang#104570

r? `@Mark-Simulacrum`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc beta-accepted Accepted for backporting to the compiler in the beta channel. merged-by-bors This PR was explicitly merged by bors. 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-infra Relevant to the infrastructure 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
9 participants