Skip to content

Commit 58c2374

Browse files
authored
fix(build-std): determine root crates by target spec std:bool (#14899)
### What does this PR try to resolve? In #14183 Cargo starts bailing out if the `metadata.std` field in a target spec JSON is set to `false`. This is problematic because std for some targets are actually buildable even they've declared as std-unsupported. This patch removes the hard error, and instead determines the required root crates by the `metadata.std` field. That is to say, if a target is explicitly declared as `std: false`, `-Zbuild-std` will build `core` and `compiler-builtins` only, no `std` will be built. This patch doesn't change the behavior of `-Zbuild-std` with explicit crates set. For example `-Zbuild-std=std` will force building `std`. See Zulip discussion: https://rust-lang.zulipchat.com/#narrow/channel/246057-t-cargo/topic/help.20debugging.20a.20docs.2Ers.20issue.20with.20a.20new.20cargo.20error ### How should we test and review this PR? e18b64f and 125e873 might need a closer look. Cargo relies on how std workspace is organized with these commits. Here is an e2e test, if you are willing to test manually. * Use the current nightly toolchain `rustc 1.85.0-nightly (acabb5248 2024-12-04)` * `rustup component add rust-src --toolchain nightly` * `rustup target add aarch64-unknown-none --toolchain nightly` * Create a `#![no_std]` lib package. * Run `target/debug/cargo build -Zbuild-std --target aarch64-unknown-none` in that package. Previously it failed with a message `building std is not supported on the following targets`
2 parents bf79c8b + feb398a commit 58c2374

File tree

6 files changed

+175
-97
lines changed

6 files changed

+175
-97
lines changed

.github/workflows/main.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,8 @@ jobs:
161161
- run: rustup update --no-self-update stable
162162
- run: rustup update --no-self-update ${{ matrix.rust }} && rustup default ${{ matrix.rust }}
163163
- run: rustup target add ${{ matrix.other }}
164+
- run: rustup target add aarch64-unknown-none # need this for build-std mock tests
165+
if: startsWith(matrix.rust, 'nightly')
164166
- run: rustup component add rustc-dev llvm-tools-preview rust-docs
165167
if: startsWith(matrix.rust, 'nightly')
166168
- run: sudo apt update -y && sudo apt install lldb gcc-multilib libsecret-1-0 libsecret-1-dev -y

src/cargo/core/compiler/standard_lib.rs

Lines changed: 71 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -9,80 +9,52 @@ use crate::core::resolver::HasDevUnits;
99
use crate::core::{PackageId, PackageSet, Resolve, Workspace};
1010
use crate::ops::{self, Packages};
1111
use crate::util::errors::CargoResult;
12-
use crate::GlobalContext;
12+
1313
use std::collections::{HashMap, HashSet};
1414
use std::path::PathBuf;
1515

1616
use super::BuildConfig;
1717

18-
/// Parse the `-Zbuild-std` flag.
19-
pub fn parse_unstable_flag(value: Option<&str>) -> Vec<String> {
18+
fn std_crates<'a>(crates: &'a [String], default: &'static str, units: &[Unit]) -> HashSet<&'a str> {
19+
let mut crates = HashSet::from_iter(crates.iter().map(|s| s.as_str()));
2020
// This is a temporary hack until there is a more principled way to
2121
// declare dependencies in Cargo.toml.
22-
let value = value.unwrap_or("std");
23-
let mut crates: HashSet<&str> = value.split(',').collect();
22+
if crates.is_empty() {
23+
crates.insert(default);
24+
}
2425
if crates.contains("std") {
2526
crates.insert("core");
2627
crates.insert("alloc");
2728
crates.insert("proc_macro");
2829
crates.insert("panic_unwind");
2930
crates.insert("compiler_builtins");
30-
} else if crates.contains("core") {
31-
crates.insert("compiler_builtins");
32-
}
33-
crates.into_iter().map(|s| s.to_string()).collect()
34-
}
35-
36-
pub(crate) fn std_crates(gctx: &GlobalContext, units: Option<&[Unit]>) -> Option<Vec<String>> {
37-
let crates = gctx.cli_unstable().build_std.as_ref()?.clone();
38-
39-
// Only build libtest if it looks like it is needed.
40-
let mut crates = crates.clone();
41-
// If we know what units we're building, we can filter for libtest depending on the jobs.
42-
if let Some(units) = units {
31+
// Only build libtest if it looks like it is needed (libtest depends on libstd)
32+
// If we know what units we're building, we can filter for libtest depending on the jobs.
4333
if units
4434
.iter()
4535
.any(|unit| unit.mode.is_rustc_test() && unit.target.harness())
4636
{
47-
// Only build libtest when libstd is built (libtest depends on libstd)
48-
if crates.iter().any(|c| c == "std") && !crates.iter().any(|c| c == "test") {
49-
crates.push("test".to_string());
50-
}
51-
}
52-
} else {
53-
// We don't know what jobs are going to be run, so download libtest just in case.
54-
if !crates.iter().any(|c| c == "test") {
55-
crates.push("test".to_string())
37+
crates.insert("test");
5638
}
39+
} else if crates.contains("core") {
40+
crates.insert("compiler_builtins");
5741
}
5842

59-
Some(crates)
43+
crates
6044
}
6145

6246
/// Resolve the standard library dependencies.
6347
pub fn resolve_std<'gctx>(
6448
ws: &Workspace<'gctx>,
6549
target_data: &mut RustcTargetData<'gctx>,
6650
build_config: &BuildConfig,
67-
crates: &[String],
6851
) -> CargoResult<(PackageSet<'gctx>, Resolve, ResolvedFeatures)> {
6952
if build_config.build_plan {
7053
ws.gctx()
7154
.shell()
7255
.warn("-Zbuild-std does not currently fully support --build-plan")?;
7356
}
7457

75-
// check that targets support building std
76-
if crates.contains(&"std".to_string()) {
77-
let unsupported_targets = target_data.get_unsupported_std_targets();
78-
if !unsupported_targets.is_empty() {
79-
anyhow::bail!(
80-
"building std is not supported on the following targets: {}",
81-
unsupported_targets.join(", ")
82-
)
83-
}
84-
}
85-
8658
let src_path = detect_sysroot_src_path(target_data)?;
8759
let std_ws_manifest_path = src_path.join("Cargo.toml");
8860
let gctx = ws.gctx();
@@ -93,12 +65,10 @@ pub fn resolve_std<'gctx>(
9365
// `[dev-dependencies]`. No need for us to generate a `Resolve` which has
9466
// those included because we'll never use them anyway.
9567
std_ws.set_require_optional_deps(false);
96-
// `sysroot` is not in the default set because it is optional, but it needs
97-
// to be part of the resolve in case we do need it or `libtest`.
98-
let mut spec_pkgs = Vec::from(crates);
99-
spec_pkgs.push("sysroot".to_string());
100-
let spec = Packages::Packages(spec_pkgs);
101-
let specs = spec.to_package_id_specs(&std_ws)?;
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)?;
10272
let features = match &gctx.cli_unstable().build_std_features {
10373
Some(list) => list.clone(),
10474
None => vec![
@@ -128,11 +98,13 @@ pub fn resolve_std<'gctx>(
12898
))
12999
}
130100

131-
/// Generate a list of root `Unit`s for the standard library.
101+
/// Generates a map of root units for the standard library for each kind requested.
132102
///
133-
/// The given slice of crate names is the root set.
103+
/// * `crates` is the arg value from `-Zbuild-std`.
104+
/// * `units` is the root units of the build.
134105
pub fn generate_std_roots(
135106
crates: &[String],
107+
units: &[Unit],
136108
std_resolve: &Resolve,
137109
std_features: &ResolvedFeatures,
138110
kinds: &[CompileKind],
@@ -141,15 +113,52 @@ pub fn generate_std_roots(
141113
profiles: &Profiles,
142114
target_data: &RustcTargetData<'_>,
143115
) -> CargoResult<HashMap<CompileKind, Vec<Unit>>> {
144-
// Generate the root Units for the standard library.
145-
let std_ids = crates
116+
// Generate a map of Units for each kind requested.
117+
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)] {
122+
if kinds.is_empty() {
123+
continue;
124+
}
125+
generate_roots(
126+
&mut ret,
127+
default_crate,
128+
crates,
129+
units,
130+
std_resolve,
131+
std_features,
132+
&kinds,
133+
package_set,
134+
interner,
135+
profiles,
136+
target_data,
137+
)?;
138+
}
139+
140+
Ok(ret)
141+
}
142+
143+
fn generate_roots(
144+
ret: &mut HashMap<CompileKind, Vec<Unit>>,
145+
default: &'static str,
146+
crates: &[String],
147+
units: &[Unit],
148+
std_resolve: &Resolve,
149+
std_features: &ResolvedFeatures,
150+
kinds: &[&CompileKind],
151+
package_set: &PackageSet<'_>,
152+
interner: &UnitInterner,
153+
profiles: &Profiles,
154+
target_data: &RustcTargetData<'_>,
155+
) -> CargoResult<()> {
156+
let std_ids = std_crates(crates, default, units)
146157
.iter()
147158
.map(|crate_name| std_resolve.query(crate_name))
148159
.collect::<CargoResult<Vec<PackageId>>>()?;
149-
// Convert PackageId to Package.
150160
let std_pkgs = package_set.get_many(std_ids)?;
151-
// Generate a map of Units for each kind requested.
152-
let mut ret = HashMap::new();
161+
153162
for pkg in std_pkgs {
154163
let lib = pkg
155164
.targets()
@@ -162,33 +171,34 @@ pub fn generate_std_roots(
162171
let mode = CompileMode::Build;
163172
let features = std_features.activated_features(pkg.package_id(), FeaturesFor::NormalOrDev);
164173
for kind in kinds {
165-
let list = ret.entry(*kind).or_insert_with(Vec::new);
166-
let unit_for = UnitFor::new_normal(*kind);
174+
let kind = **kind;
175+
let list = ret.entry(kind).or_insert_with(Vec::new);
176+
let unit_for = UnitFor::new_normal(kind);
167177
let profile = profiles.get_profile(
168178
pkg.package_id(),
169179
/*is_member*/ false,
170180
/*is_local*/ false,
171181
unit_for,
172-
*kind,
182+
kind,
173183
);
174184
list.push(interner.intern(
175185
pkg,
176186
lib,
177187
profile,
178-
*kind,
188+
kind,
179189
mode,
180190
features.clone(),
181-
target_data.info(*kind).rustflags.clone(),
182-
target_data.info(*kind).rustdocflags.clone(),
183-
target_data.target_config(*kind).links_overrides.clone(),
191+
target_data.info(kind).rustflags.clone(),
192+
target_data.info(kind).rustdocflags.clone(),
193+
target_data.target_config(kind).links_overrides.clone(),
184194
/*is_std*/ true,
185195
/*dep_hash*/ 0,
186196
IsArtifact::No,
187197
None,
188198
));
189199
}
190200
}
191-
Ok(ret)
201+
Ok(())
192202
}
193203

194204
fn detect_sysroot_src_path(target_data: &RustcTargetData<'_>) -> CargoResult<PathBuf> {

src/cargo/core/features.rs

Lines changed: 6 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -759,7 +759,6 @@ unstable_cli_options!(
759759
avoid_dev_deps: bool = ("Avoid installing dev-dependencies if possible"),
760760
binary_dep_depinfo: bool = ("Track changes to dependency artifacts"),
761761
bindeps: bool = ("Allow Cargo packages to depend on bin, cdylib, and staticlib crates, and use the artifacts built by those crates"),
762-
#[serde(deserialize_with = "deserialize_build_std")]
763762
build_std: Option<Vec<String>> = ("Enable Cargo to compile the standard library itself as part of a crate graph compilation"),
764763
build_std_features: Option<Vec<String>> = ("Configure features enabled for the standard library itself when building the standard library"),
765764
cargo_lints: bool = ("Enable the `[lints.cargo]` table"),
@@ -873,19 +872,6 @@ const STABILIZED_LINTS: &str = "The `[lints]` table is now always available.";
873872
const STABILIZED_CHECK_CFG: &str =
874873
"Compile-time checking of conditional (a.k.a. `-Zcheck-cfg`) is now always enabled.";
875874

876-
fn deserialize_build_std<'de, D>(deserializer: D) -> Result<Option<Vec<String>>, D::Error>
877-
where
878-
D: serde::Deserializer<'de>,
879-
{
880-
let Some(crates) = <Option<Vec<String>>>::deserialize(deserializer)? else {
881-
return Ok(None);
882-
};
883-
let v = crates.join(",");
884-
Ok(Some(
885-
crate::core::compiler::standard_lib::parse_unstable_flag(Some(&v)),
886-
))
887-
}
888-
889875
#[derive(Debug, Copy, Clone, Default, Deserialize, Ord, PartialOrd, Eq, PartialEq)]
890876
#[serde(default)]
891877
pub struct GitFeatures {
@@ -1148,7 +1134,8 @@ impl CliUnstable {
11481134
}
11491135
}
11501136

1151-
fn parse_features(value: Option<&str>) -> Vec<String> {
1137+
/// Parse a comma-separated list
1138+
fn parse_list(value: Option<&str>) -> Vec<String> {
11521139
match value {
11531140
None => Vec::new(),
11541141
Some("") => Vec::new(),
@@ -1197,7 +1184,7 @@ impl CliUnstable {
11971184
match k {
11981185
// Permanently unstable features
11991186
// Sorted alphabetically:
1200-
"allow-features" => self.allow_features = Some(parse_features(v).into_iter().collect()),
1187+
"allow-features" => self.allow_features = Some(parse_list(v).into_iter().collect()),
12011188
"print-im-a-teapot" => self.print_im_a_teapot = parse_bool(k, v)?,
12021189

12031190
// Stabilized features
@@ -1216,7 +1203,7 @@ impl CliUnstable {
12161203
// until we feel confident to remove entirely.
12171204
//
12181205
// See rust-lang/cargo#11168
1219-
let feats = parse_features(v);
1206+
let feats = parse_list(v);
12201207
let stab_is_not_empty = feats.iter().any(|feat| {
12211208
matches!(
12221209
feat.as_str(),
@@ -1256,10 +1243,8 @@ impl CliUnstable {
12561243
"avoid-dev-deps" => self.avoid_dev_deps = parse_empty(k, v)?,
12571244
"binary-dep-depinfo" => self.binary_dep_depinfo = parse_empty(k, v)?,
12581245
"bindeps" => self.bindeps = parse_empty(k, v)?,
1259-
"build-std" => {
1260-
self.build_std = Some(crate::core::compiler::standard_lib::parse_unstable_flag(v))
1261-
}
1262-
"build-std-features" => self.build_std_features = Some(parse_features(v)),
1246+
"build-std" => self.build_std = Some(parse_list(v)),
1247+
"build-std-features" => self.build_std_features = Some(parse_list(v)),
12631248
"cargo-lints" => self.cargo_lints = parse_empty(k, v)?,
12641249
"codegen-backend" => self.codegen_backend = parse_empty(k, v)?,
12651250
"config-include" => self.config_include = parse_empty(k, v)?,

src/cargo/ops/cargo_compile/mod.rs

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

292-
let std_resolve_features = if let Some(crates) = &gctx.cli_unstable().build_std {
292+
let std_resolve_features = if gctx.cli_unstable().build_std.is_some() {
293293
let (std_package_set, std_resolve, std_features) =
294-
standard_lib::resolve_std(ws, &mut target_data, &build_config, crates)?;
294+
standard_lib::resolve_std(ws, &mut target_data, &build_config)?;
295295
pkg_set.add_set(std_package_set);
296296
Some((std_resolve, std_features))
297297
} else {
@@ -398,10 +398,11 @@ pub fn create_bcx<'a, 'gctx>(
398398
Vec::new()
399399
};
400400

401-
let std_roots = if let Some(crates) = standard_lib::std_crates(gctx, Some(&units)) {
401+
let std_roots = if let Some(crates) = gctx.cli_unstable().build_std.as_ref() {
402402
let (std_resolve, std_features) = std_resolve_features.as_ref().unwrap();
403403
standard_lib::generate_std_roots(
404404
&crates,
405+
&units,
405406
std_resolve,
406407
std_features,
407408
&explicit_host_kinds,

src/cargo/ops/cargo_fetch.rs

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

6666
// If -Zbuild-std was passed, download dependencies for the standard library.
67-
// We don't know ahead of time what jobs we'll be running, so tell `std_crates` that.
68-
if let Some(crates) = standard_lib::std_crates(gctx, None) {
69-
let (std_package_set, _, _) =
70-
standard_lib::resolve_std(ws, &mut data, &build_config, &crates)?;
67+
if gctx.cli_unstable().build_std.is_some() {
68+
let (std_package_set, _, _) = standard_lib::resolve_std(ws, &mut data, &build_config)?;
7169
packages.add_set(std_package_set);
7270
}
7371

0 commit comments

Comments
 (0)