Skip to content

Commit 99dff6d

Browse files
authored
fix(build-std): make Resolve align to what to build (#14938)
### What does this PR try to resolve? Blocked on <#14943> (or can just merge this one). Fixes #14935 #14935 failed because since 125e873 [`std_resolve`][1] only includes `sysroot` as primary package. When any custom Cargo feature is provided via `-Zbuild-std-feature`, the default feature set `panic-unwind` would be gone, so no `panic_unwind` crate presents in `std_resolve`. When then calling [`std_resolve.query`][2] with the default set of crates from [`std_crates`][3], which automatically includes `panic_unwind` when `std` presents, it'll result in spec not found because `panic_unwind` was not in `std_resolve` anyway. [1]: https://github.com/rust-lang/cargo/blob/addcc8ca715bc7fe20df66afd6efbf3c77ef43f8/src/cargo/core/compiler/standard_lib.rs#L96 [2]: https://github.com/rust-lang/cargo/blob/addcc8ca715bc7fe20df66afd6efbf3c77ef43f8/src/cargo/core/compiler/standard_lib.rs#L158 [3]: https://github.com/rust-lang/cargo/blob/addcc8ca715bc7fe20df66afd6efbf3c77ef43f8/src/cargo/core/compiler/standard_lib.rs#L156 ### How should we test and review this PR? This patch is kinda a revert of 125e873 in terms of the behavior. With this, now `std_resolve` is always resolved to the same set of packages that Cargo will use to generate the unit graph, (technically the same set of crates + `sysroot`), by sharing the same set of primary packages via `std_crates` functions. Note that when multiple `--target`s provided, if std is specified or there is one might support std, Cargo will always resolve std dep graph. To test it manually, run ``` RUSTFLAGS="-C panic=abort" cargo +nightly-2024-12-15 b -Zbuild-std=std,panic_abort -Zbuild-std-features=panic_immediate_abort ``` change to this PR's cargo with the same nightly rustc, it would succeed. I am a bit reluctant to add an new end-2end build-std test, but I still did it. A bit scared when mock-std gets out-of-sync of features in std in rust-lang/rust.
2 parents 8682bb4 + 0149bca commit 99dff6d

File tree

5 files changed

+71
-13
lines changed

5 files changed

+71
-13
lines changed

src/cargo/core/compiler/build_context/target_info.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -619,6 +619,16 @@ impl TargetInfo {
619619
.iter()
620620
.any(|sup| sup.as_str() == split.as_str())
621621
}
622+
623+
/// Checks if a target maybe support std.
624+
///
625+
/// If no explictly stated in target spec json, we treat it as "maybe support".
626+
///
627+
/// This is only useful for `-Zbuild-std` to determine the default set of
628+
/// crates it is going to build.
629+
pub fn maybe_support_std(&self) -> bool {
630+
matches!(self.supports_std, Some(true) | None)
631+
}
622632
}
623633

624634
/// Takes rustc output (using specialized command line args), and calculates the file prefix and

src/cargo/core/compiler/standard_lib.rs

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,14 @@ fn std_crates<'a>(crates: &'a [String], default: &'static str, units: &[Unit]) -
4444
}
4545

4646
/// Resolve the standard library dependencies.
47+
///
48+
/// * `crates` is the arg value from `-Zbuild-std`.
4749
pub fn resolve_std<'gctx>(
4850
ws: &Workspace<'gctx>,
4951
target_data: &mut RustcTargetData<'gctx>,
5052
build_config: &BuildConfig,
53+
crates: &[String],
54+
kinds: &[CompileKind],
5155
) -> CargoResult<(PackageSet<'gctx>, Resolve, ResolvedFeatures)> {
5256
if build_config.build_plan {
5357
ws.gctx()
@@ -65,10 +69,21 @@ pub fn resolve_std<'gctx>(
6569
// `[dev-dependencies]`. No need for us to generate a `Resolve` which has
6670
// those included because we'll never use them anyway.
6771
std_ws.set_require_optional_deps(false);
68-
// `sysroot` + the default feature set below should give us a good default
69-
// Resolve, which includes `libtest` as well.
70-
let specs = Packages::Packages(vec!["sysroot".into()]);
71-
let specs = specs.to_package_id_specs(&std_ws)?;
72+
let specs = {
73+
// If there is anything looks like needing std, resolve with it.
74+
// If not, we assume only `core` maye be needed, as `core the most fundamental crate.
75+
//
76+
// This may need a UI overhaul if `build-std` wants to fully support multi-targets.
77+
let maybe_std = kinds
78+
.iter()
79+
.any(|kind| target_data.info(*kind).maybe_support_std());
80+
let mut crates = std_crates(crates, if maybe_std { "std" } else { "core" }, &[]);
81+
// `sysroot` is not in the default set because it is optional, but it needs
82+
// to be part of the resolve in case we do need it or `libtest`.
83+
crates.insert("sysroot");
84+
let specs = Packages::Packages(crates.into_iter().map(Into::into).collect());
85+
specs.to_package_id_specs(&std_ws)?
86+
};
7287
let features = match &gctx.cli_unstable().build_std_features {
7388
Some(list) => list.clone(),
7489
None => vec![
@@ -115,10 +130,10 @@ pub fn generate_std_roots(
115130
) -> CargoResult<HashMap<CompileKind, Vec<Unit>>> {
116131
// Generate a map of Units for each kind requested.
117132
let mut ret = HashMap::new();
118-
let (core_only, maybe_std): (Vec<&CompileKind>, Vec<_>) = kinds.iter().partition(|kind|
119-
// Only include targets that explicitly don't support std
120-
target_data.info(**kind).supports_std == Some(false));
121-
for (default_crate, kinds) in [("core", core_only), ("std", maybe_std)] {
133+
let (maybe_std, maybe_core): (Vec<&CompileKind>, Vec<_>) = kinds
134+
.iter()
135+
.partition(|kind| target_data.info(**kind).maybe_support_std());
136+
for (default_crate, kinds) in [("core", maybe_core), ("std", maybe_std)] {
122137
if kinds.is_empty() {
123138
continue;
124139
}

src/cargo/ops/cargo_compile/mod.rs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -289,9 +289,14 @@ pub fn create_bcx<'a, 'gctx>(
289289
resolved_features,
290290
} = resolve;
291291

292-
let std_resolve_features = if gctx.cli_unstable().build_std.is_some() {
293-
let (std_package_set, std_resolve, std_features) =
294-
standard_lib::resolve_std(ws, &mut target_data, &build_config)?;
292+
let std_resolve_features = if let Some(crates) = &gctx.cli_unstable().build_std {
293+
let (std_package_set, std_resolve, std_features) = standard_lib::resolve_std(
294+
ws,
295+
&mut target_data,
296+
&build_config,
297+
crates,
298+
&build_config.requested_kinds,
299+
)?;
295300
pkg_set.add_set(std_package_set);
296301
Some((std_resolve, std_features))
297302
} else {

src/cargo/ops/cargo_fetch.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,14 @@ pub fn fetch<'a>(
6464
}
6565

6666
// If -Zbuild-std was passed, download dependencies for the standard library.
67-
if gctx.cli_unstable().build_std.is_some() {
68-
let (std_package_set, _, _) = standard_lib::resolve_std(ws, &mut data, &build_config)?;
67+
if let Some(crates) = &gctx.cli_unstable().build_std {
68+
let (std_package_set, _, _) = standard_lib::resolve_std(
69+
ws,
70+
&mut data,
71+
&build_config,
72+
crates,
73+
&build_config.requested_kinds,
74+
)?;
6975
packages.add_set(std_package_set);
7076
}
7177

tests/build-std/main.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -418,3 +418,25 @@ fn test_proc_macro() {
418418
"#]])
419419
.run();
420420
}
421+
422+
#[cargo_test(build_std_real)]
423+
fn test_panic_abort() {
424+
// See rust-lang/cargo#14935
425+
let p = project()
426+
.file(
427+
"Cargo.toml",
428+
r#"
429+
[package]
430+
name = "foo"
431+
edition = "2021"
432+
"#,
433+
)
434+
.file("src/lib.rs", "#![no_std]")
435+
.build();
436+
437+
p.cargo("check")
438+
.build_std_arg("std,panic_abort")
439+
.env("RUSTFLAGS", "-C panic=abort")
440+
.arg("-Zbuild-std-features=panic_immediate_abort")
441+
.run();
442+
}

0 commit comments

Comments
 (0)