Always set the deployment target when building std #133092

madsmtm · 2024-11-16T04:30:04Z

cc has a bug/feature (I guess depending on how you look at it) where the default deployment target is taken from the SDK instead of from rustc. This causes compiler-builtins to build compiler-rt with the wrong deployment target on iOS.

I've been meaning to change how cc works in this regard, but that's a lengthy process, so let's fix it in bootstrap for now.

The behaviour can be seen locally with ./x build library --set build.optimized-compiler-builtins=true for various target triples, and then inspecting with otool -l build/host/stage1/lib/rustlib/*/lib/libcompiler_builtins-*.rlib | rg 'minos|version'. I have added a rmake test that ensures that we now have the same version everywhere.

Fixes #128419
Fixes rust-lang/compiler-builtins#650
See also rust-lang/cargo#13115, rust-lang/cc-rs#1171
See #133092 (comment) for a description of how the change works.

@rustbot label O-apple

rustbot · 2024-11-16T04:30:12Z

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

madsmtm · 2024-11-16T05:35:11Z

Okay... So this didn't actually work initially, since bootstrap is for some ~~godforsaken~~ reason calling cc to get default flags, and then later passing passing them to Cargo, which invokes the build script, which calls cc again to get default flags... So all the build flags end up getting duplicated (!), and the values in CFLAGS that were looked up previously end up being prioritized.

I've rebased and fixed that in the first and second commits, but it may break more than I think, so this should probably get a try build before merging.

rustbot · 2024-11-16T07:20:00Z

This PR changes how LLVM is built. Consider updating src/bootstrap/download-ci-llvm-stamp.

This PR modifies tests/run-make/. If this PR is trying to port a Makefile
run-make test to use rmake.rs, please update the
run-make port tracking issue
so we can track our progress. You can either modify the tracking issue
directly, or you can comment on the tracking issue and link this PR.

cc @jieyouxu

madsmtm · 2024-11-16T07:22:31Z

This PR changes how LLVM is built. Consider updating src/bootstrap/download-ci-llvm-stamp.

Not sure if I should? It changes in theory (we pass different flags), but in practice it shouldn't (because those flags were already set by cmake-rs, so they were just duplicated)?

src/bootstrap/src/core/build_steps/compile.rs

Mark-Simulacrum · 2024-11-24T18:50:02Z

r=me modulo comment

Mark-Simulacrum · 2024-11-28T22:09:53Z

@bors r+

bors · 2024-11-28T22:09:55Z

📌 Commit ff3dab4 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

…t, r=Mark-Simulacrum Always set the deployment target when building std `cc` has [a bug/feature](rust-lang/cc-rs#1171) (I guess depending on how you look at it) where the default deployment target is taken from the SDK instead of from `rustc`. This causes `compiler-builtins` to build `compiler-rt` with the wrong deployment target on iOS. I've been meaning to change how `cc` works in this regard, but that's a lengthy process, so let's fix it in bootstrap for now. The behaviour be seen locally with `./x build library --set build.optimized-compiler-builtins=true` for various target triples, and then inspecting with `otool -l build/host/stage1/lib/rustlib/*/lib/libcompiler_builtins-*.rlib | rg 'minos|version'`. I have added a rmake test that ensures that we now have the same version everywhere. Fixes rust-lang#128419 Fixes rust-lang/compiler-builtins#650 See also rust-lang/cargo#13115 `@rustbot` label O-apple

Rollup of 7 pull requests Successful merges: - rust-lang#131323 (Support `clobber_abi` in AVR inline assembly) - rust-lang#133092 (Always set the deployment target when building std) - rust-lang#133134 (Don't use a SyntheticProvider for literally every type) - rust-lang#133538 (Better diagnostic for fn items in variadic functions) - rust-lang#133590 (Rename `-Zparse-only`) - rust-lang#133592 (Misc: better instructions for envrc, ignore `/build` instead of `build/`) - rust-lang#133608 (Revert rust-lang#133418 (Store coverage source regions as `Span`) due to regression rust-lang#133606) r? `@ghost` `@rustbot` modify labels: rollup

Zalathar · 2024-11-29T06:19:47Z

Could this have caused the failure at #133609 (comment)?

jieyouxu · 2024-11-29T06:26:18Z

This looks most likely I agree.
@bors r-

bors · 2024-12-04T10:46:24Z

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

bors · 2024-12-10T06:40:32Z

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

alex-semenyuk · 2025-01-20T09:33:14Z

@madsmtm
Thanks for your contribution
From wg-triage. Could you please take a look comments above

madsmtm · 2025-01-20T10:19:27Z

Yeah, I'm aware that it failed, just haven't gotten around to looking into it yet

madsmtm · 2025-01-27T01:33:23Z

Okay, so I believe I've fixed that error, the issue was that bootstrap was setting CMAKE_C_FLAGS, which prevented cmake-rs from setting the define itself based on cc-rs' defaults. While doing this I also found rust-lang/cmake-rs#232 - though I don't believe we're affected directly by that in CI.
@rustbot ready

Since it is unlikely that I caught everything, could someone do @bors rollup=iffy or similar?

Commands that end up invoking cc-rs, i.e. Cargo (through build scripts) and cmake-rs don't need the CFLAGS from cc-rs itself, as they will just end up as duplicates. We do still choose to pass them in certain places, but now it's at least clear which flags are default, and which flags are extra flags added on.

madsmtm · 2025-01-31T03:10:14Z

Actually, I decided to downscope this, as I realized there were more implications that I originally thought.

Functionally, I've now split Build::cflags into Build::default_cflags and Build::extra_cflags. Most places call both, with the exception of Cargo, which only calls extra_cflags (to allow the default ones to be selected by cc-rs). In the future, I'd like to do the same for our use of cmake-rs, and probably other places too, but that requires more work.

With that change done, passing the *_DEPLOYMENT_TARGET variables in bootstrap when building with Cargo allows compiler-builtins' build script to pick a higher deployment target.

Finally, I've slightly tweaked jobs.yml to reflect the changes here.

Note that specifying MACOSX_STD_DEPLOYMENT_TARGET is still completely unnecessary here, but it's nice to have for future changes where we might want to version `std` and `rustc`'s deployment target separately.

rustbot assigned Mark-Simulacrum Nov 16, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) O-apple Operating system: Apple (macOS, iOS, tvOS, visionOS, watchOS) labels Nov 16, 2024

This was referenced Nov 16, 2024

Print name of env var in --print=deployment-target #133041

Merged

"was built for iOS 16.4" warning after updating from 1.79 to 1.80 #128419

Open

madsmtm force-pushed the bootstrap-deployment-target branch from fd382a0 to dd8ca01 Compare November 16, 2024 05:34

madsmtm force-pushed the bootstrap-deployment-target branch 3 times, most recently from 50812ff to 2294b05 Compare November 16, 2024 06:55

rustbot added the A-run-make Area: port run-make Makefiles to rmake.rs label Nov 16, 2024

madsmtm force-pushed the bootstrap-deployment-target branch from 2294b05 to f816d33 Compare November 16, 2024 07:05

madsmtm marked this pull request as ready for review November 16, 2024 07:19

Mark-Simulacrum reviewed Nov 24, 2024

View reviewed changes

src/bootstrap/src/core/build_steps/compile.rs Outdated Show resolved Hide resolved

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 28, 2024

Zalathar mentioned this pull request Nov 29, 2024

Rollup of 7 pull requests #133609

Closed

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 Nov 29, 2024

madsmtm force-pushed the bootstrap-deployment-target branch from ff3dab4 to 4e1dcf8 Compare January 27, 2025 00:09

rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Jan 27, 2025

This comment has been minimized.

Sign in to view

madsmtm force-pushed the bootstrap-deployment-target branch 2 times, most recently from 73af7e4 to b91a267 Compare January 27, 2025 00:32

This comment has been minimized.

Sign in to view

madsmtm force-pushed the bootstrap-deployment-target branch from d053536 to 55096ad Compare January 27, 2025 01:32

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 Jan 27, 2025

madsmtm force-pushed the bootstrap-deployment-target branch 2 times, most recently from 84f7291 to bc33388 Compare January 31, 2025 02:59

This comment has been minimized.

Sign in to view

madsmtm force-pushed the bootstrap-deployment-target branch from bc33388 to a0331a4 Compare January 31, 2025 03:06

madsmtm added 3 commits January 31, 2025 04:07

Always set the deployment target when building std

9318fbb

Explain why MACOSX_STD_DEPLOYMENT_TARGET exist

0a3fd1f

madsmtm force-pushed the bootstrap-deployment-target branch from a0331a4 to 0a3fd1f Compare January 31, 2025 03:07

Update deployment target setup in jobs.yml

9fa819e

Note that specifying MACOSX_STD_DEPLOYMENT_TARGET is still completely unnecessary here, but it's nice to have for future changes where we might want to version `std` and `rustc`'s deployment target separately.

jieyouxu assigned jieyouxu and unassigned jieyouxu Jan 31, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Always set the deployment target when building std #133092

Always set the deployment target when building std #133092

madsmtm commented Nov 16, 2024 •

edited

Loading

rustbot commented Nov 16, 2024

madsmtm commented Nov 16, 2024 •

edited

Loading

rustbot commented Nov 16, 2024

madsmtm commented Nov 16, 2024

Mark-Simulacrum commented Nov 24, 2024

Mark-Simulacrum commented Nov 28, 2024

bors commented Nov 28, 2024

Zalathar commented Nov 29, 2024

jieyouxu commented Nov 29, 2024

bors commented Dec 4, 2024

bors commented Dec 10, 2024

alex-semenyuk commented Jan 20, 2025

madsmtm commented Jan 20, 2025

This comment has been minimized.

This comment has been minimized.

madsmtm commented Jan 27, 2025 •

edited

Loading

This comment has been minimized.

madsmtm commented Jan 31, 2025 •

edited

Loading

Always set the deployment target when building std #133092

Are you sure you want to change the base?

Always set the deployment target when building std #133092

Conversation

madsmtm commented Nov 16, 2024 • edited Loading

rustbot commented Nov 16, 2024

madsmtm commented Nov 16, 2024 • edited Loading

rustbot commented Nov 16, 2024

madsmtm commented Nov 16, 2024

Mark-Simulacrum commented Nov 24, 2024

Mark-Simulacrum commented Nov 28, 2024

bors commented Nov 28, 2024

Zalathar commented Nov 29, 2024

jieyouxu commented Nov 29, 2024

bors commented Dec 4, 2024

bors commented Dec 10, 2024

alex-semenyuk commented Jan 20, 2025

madsmtm commented Jan 20, 2025

This comment has been minimized.

This comment has been minimized.

madsmtm commented Jan 27, 2025 • edited Loading

This comment has been minimized.

madsmtm commented Jan 31, 2025 • edited Loading

madsmtm commented Nov 16, 2024 •

edited

Loading

madsmtm commented Nov 16, 2024 •

edited

Loading

madsmtm commented Jan 27, 2025 •

edited

Loading

madsmtm commented Jan 31, 2025 •

edited

Loading