Add new mode to bootstrap for building std during dist builds

[adamgemmell]

View all comments

This PR adds a new Mode, DistStd, to bootstrap, as proposed by @Kobzol a while ago, which is used when building the standard library during x dist builds. This mode:

• Uses the dist profile in the library workspace, added in Move bootstrap configuration to library workspace #149514
  • Normal builds revert back to using release/dev, which fixes building std without optimisations
• Disallows overriding any profile fields. Configuration in dist builds that do this should instead place the configuration in the library workspace so that it can also be used by build-std.

Note that RUSTFLAGS can still be set based on configuration, and this will be addressed in an upcoming patch as we still need to work out how build-std can consume target-specific configuration. Probably this will go in the cargo config.

[rustbot]

r? @clubby789

rustbot has assigned @clubby789.
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

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: bootstrap
• bootstrap expanded to 6 candidates
• Random selection from Mark-Simulacrum, clubby789, jieyouxu

[adamgemmell]

r? @Kobzol

[rustbot]

kobzol is not on the review rotation at the moment.
They may take a while to respond.

[bjorn3]

What is the motivation for this? This would make it harder for distros to change how the standard library gets built, right? For example a distro might want to enforce overflow checks for the standard library for security reasons. Or they might want to disable LTO because the codegen backend they chose doesn't support LTO. Or they might want to build the standard library with full debuginfo. I also personally have used ./x.py dist a couple of times to get a standalone toolchain for testing with the expectation that it is just a superset of what ./x.py build produces. Some config options suddenly getting ignored or suddenly getting a panic because I didn't restore some config options to the default would be annoying.

[adamgemmell]

The motivation is enforcing that any configuration for dist builds lives somewhere that build-std can see it. We had some discussion on zulip a while ago: https://rust-lang.zulipchat.com/#narrow/channel/326414-t-infra.2Fbootstrap/topic/Standard.20library.20dist.20configuration. From that the two main complications that led to this approach were:

1. Bootstrap unconditionally sets some configuration based on bootstrap.toml defaults, like rust.debuginfo-level-std (which inherits from rust.debug). We'd like to avoid duplication where possible to avoid divergence - perhaps std configuration should defer to what's in the standard library profile by default.

2. Dist jobs may set configuration via ./configure which masks what's in the library workspace. Some kind of sanity check would be nice here, which is certainly doable in this function for profile vars (though @jakub Beránek recommended against this unless necessary). Arbitrary RUSTFLAGS are harder, but I think over time we can get to a spot where codegen configuration is in the profile or even .cargo/config other than RUSTFLAGS.

The intention is that panic never triggers, it's a sanity check for bootstrap itself.

Yes, this is quite disruptive and annoying, and anyone doing dist builds would need to translate all their options to the library manifest, but distros need to do this anyway so the build-std artifacts have the same compatibility as the prebuilt std.

I'm not sure on the best way to make this less disruptive - perhaps it should only ignore config options when DEPLOY=1, and should at least print a warning.

[bjorn3]

but distros need to do this anyway so the build-std artifacts have the same compatibility as the prebuilt std.

For example for the debuginfo options, if a distro wants to enable full debuginfo for their precompiled standard library, they probably don't want to enable full debuginfo for end users that use -Zbuild-std. Several of the options the precompiled standard library is compiled with are just to because of the fact that users on stable can't use -Zbuild-std to enable them if they need them. They don't make sense to enforce for -Zbuild-std.

[adamgemmell]

The initial version of build-std we're stabilising doesn't support customising the standard library, it should match the distributed std as closely as possible. When we change Cargo to use the dist profile, we'll add an unstable option that keeps the "user profile" behaviour build-std currently has.

[bjorn3]

You are adding several of these flags even to the non-dist profiles.

[adamgemmell]

I've only moved over options that are unconditionally passed by bootstrap. This is because I'd like to explore doing something similar to RUSTFLAGS as I have for profile variables here, but I'm happy to defer that.

[bjorn3]

The -Cembed-bitcode=yes is only applicable to the precompiled standard library as it has to work with both LTO and non-LTO builds. For cargo built standard library the appropriate value already passed automatically and -Cembed-bitcode=yes for a non-LTO build would only increase the target dir size. And -Zinline-mir is also probably only necessary for ./x.py build when incr comp is enabled, though that one would just be a no-op for -Zbuild-std and is thus fine to keep.

[adamgemmell]

LTO is off for the standard library's own profile. Presumably if it were on then LTO would run twice for it if the user requested it too?

[bjorn3]

-Cembed-bitcode=yes allows a crate to be used as dependency of a crate that is LTOed. Cargo will always enable this when LTO is enabled and will always pass -Cembed-bitcode=no when LTO is disabled. The precompiled standard library needs to be compiled with -Cembed-bitcode=yes even though it isn't actually getting LTOed to ensure users can enable LTO without needing to recompile the standard library. With -Zbuild-std, -Cembed-bitcode already gets to true by cargo iff the user requested LTO due to the standard library just being another dependency. No need to force -Cembed-bitcode=yes when the user didn't request LTO.

[adamgemmell]

It won't be with build-std's currently approved RFCs, as until we introduce build-std=match-profile cargo won't look at the user's profile when configuring std.

[rust-log-analyzer]

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

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

  PR_CI_JOB: 1
  IMAGE: x86_64-gnu-next-trait-solver-polonius
##[endgroup]
    Updating crates.io index
error: failed to get `time` as a dependency of package `cookie_store v0.21.1`
    ... which satisfies dependency `cookie_store = "^0.21.1"` of package `ureq v3.0.8`
    ... which satisfies dependency `ureq = "^3"` of package `citool v0.1.0 (/home/runner/work/rust/rust/src/ci/citool)`

Caused by:
  failed to load source for dependency `time`

Caused by:

[adamgemmell]

@bors retry

[rust-bors]

❗ You can only retry pull requests that are approved and have a previously failed auto build.

[bjorn3]

I've retried the PR CI from the github interface for you. Bors is not involved with PR CI, only try runs and pre-merge CI.