Skip to content

Commit

Permalink
fix(build-std): determine root crates by target spec std:bool (#14899)
Browse files Browse the repository at this point in the history
### 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`
  • Loading branch information
epage authored Dec 9, 2024
2 parents bf79c8b + feb398a commit 58c2374
Show file tree
Hide file tree
Showing 6 changed files with 175 additions and 97 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,8 @@ jobs:
- run: rustup update --no-self-update stable
- run: rustup update --no-self-update ${{ matrix.rust }} && rustup default ${{ matrix.rust }}
- run: rustup target add ${{ matrix.other }}
- run: rustup target add aarch64-unknown-none # need this for build-std mock tests
if: startsWith(matrix.rust, 'nightly')
- run: rustup component add rustc-dev llvm-tools-preview rust-docs
if: startsWith(matrix.rust, 'nightly')
- run: sudo apt update -y && sudo apt install lldb gcc-multilib libsecret-1-0 libsecret-1-dev -y
Expand Down
132 changes: 71 additions & 61 deletions src/cargo/core/compiler/standard_lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,80 +9,52 @@ use crate::core::resolver::HasDevUnits;
use crate::core::{PackageId, PackageSet, Resolve, Workspace};
use crate::ops::{self, Packages};
use crate::util::errors::CargoResult;
use crate::GlobalContext;

use std::collections::{HashMap, HashSet};
use std::path::PathBuf;

use super::BuildConfig;

/// Parse the `-Zbuild-std` flag.
pub fn parse_unstable_flag(value: Option<&str>) -> Vec<String> {
fn std_crates<'a>(crates: &'a [String], default: &'static str, units: &[Unit]) -> HashSet<&'a str> {
let mut crates = HashSet::from_iter(crates.iter().map(|s| s.as_str()));
// This is a temporary hack until there is a more principled way to
// declare dependencies in Cargo.toml.
let value = value.unwrap_or("std");
let mut crates: HashSet<&str> = value.split(',').collect();
if crates.is_empty() {
crates.insert(default);
}
if crates.contains("std") {
crates.insert("core");
crates.insert("alloc");
crates.insert("proc_macro");
crates.insert("panic_unwind");
crates.insert("compiler_builtins");
} else if crates.contains("core") {
crates.insert("compiler_builtins");
}
crates.into_iter().map(|s| s.to_string()).collect()
}

pub(crate) fn std_crates(gctx: &GlobalContext, units: Option<&[Unit]>) -> Option<Vec<String>> {
let crates = gctx.cli_unstable().build_std.as_ref()?.clone();

// Only build libtest if it looks like it is needed.
let mut crates = crates.clone();
// If we know what units we're building, we can filter for libtest depending on the jobs.
if let Some(units) = units {
// Only build libtest if it looks like it is needed (libtest depends on libstd)
// If we know what units we're building, we can filter for libtest depending on the jobs.
if units
.iter()
.any(|unit| unit.mode.is_rustc_test() && unit.target.harness())
{
// Only build libtest when libstd is built (libtest depends on libstd)
if crates.iter().any(|c| c == "std") && !crates.iter().any(|c| c == "test") {
crates.push("test".to_string());
}
}
} else {
// We don't know what jobs are going to be run, so download libtest just in case.
if !crates.iter().any(|c| c == "test") {
crates.push("test".to_string())
crates.insert("test");
}
} else if crates.contains("core") {
crates.insert("compiler_builtins");
}

Some(crates)
crates
}

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

// check that targets support building std
if crates.contains(&"std".to_string()) {
let unsupported_targets = target_data.get_unsupported_std_targets();
if !unsupported_targets.is_empty() {
anyhow::bail!(
"building std is not supported on the following targets: {}",
unsupported_targets.join(", ")
)
}
}

let src_path = detect_sysroot_src_path(target_data)?;
let std_ws_manifest_path = src_path.join("Cargo.toml");
let gctx = ws.gctx();
Expand All @@ -93,12 +65,10 @@ pub fn resolve_std<'gctx>(
// `[dev-dependencies]`. No need for us to generate a `Resolve` which has
// those included because we'll never use them anyway.
std_ws.set_require_optional_deps(false);
// `sysroot` is not in the default set because it is optional, but it needs
// to be part of the resolve in case we do need it or `libtest`.
let mut spec_pkgs = Vec::from(crates);
spec_pkgs.push("sysroot".to_string());
let spec = Packages::Packages(spec_pkgs);
let specs = spec.to_package_id_specs(&std_ws)?;
// `sysroot` + the default feature set below should give us a good default
// Resolve, which includes `libtest` as well.
let specs = Packages::Packages(vec!["sysroot".into()]);
let specs = specs.to_package_id_specs(&std_ws)?;
let features = match &gctx.cli_unstable().build_std_features {
Some(list) => list.clone(),
None => vec![
Expand Down Expand Up @@ -128,11 +98,13 @@ pub fn resolve_std<'gctx>(
))
}

/// Generate a list of root `Unit`s for the standard library.
/// Generates a map of root units for the standard library for each kind requested.
///
/// The given slice of crate names is the root set.
/// * `crates` is the arg value from `-Zbuild-std`.
/// * `units` is the root units of the build.
pub fn generate_std_roots(
crates: &[String],
units: &[Unit],
std_resolve: &Resolve,
std_features: &ResolvedFeatures,
kinds: &[CompileKind],
Expand All @@ -141,15 +113,52 @@ pub fn generate_std_roots(
profiles: &Profiles,
target_data: &RustcTargetData<'_>,
) -> CargoResult<HashMap<CompileKind, Vec<Unit>>> {
// Generate the root Units for the standard library.
let std_ids = crates
// Generate a map of Units for each kind requested.
let mut ret = HashMap::new();
let (core_only, maybe_std): (Vec<&CompileKind>, Vec<_>) = kinds.iter().partition(|kind|
// Only include targets that explicitly don't support std
target_data.info(**kind).supports_std == Some(false));
for (default_crate, kinds) in [("core", core_only), ("std", maybe_std)] {
if kinds.is_empty() {
continue;
}
generate_roots(
&mut ret,
default_crate,
crates,
units,
std_resolve,
std_features,
&kinds,
package_set,
interner,
profiles,
target_data,
)?;
}

Ok(ret)
}

fn generate_roots(
ret: &mut HashMap<CompileKind, Vec<Unit>>,
default: &'static str,
crates: &[String],
units: &[Unit],
std_resolve: &Resolve,
std_features: &ResolvedFeatures,
kinds: &[&CompileKind],
package_set: &PackageSet<'_>,
interner: &UnitInterner,
profiles: &Profiles,
target_data: &RustcTargetData<'_>,
) -> CargoResult<()> {
let std_ids = std_crates(crates, default, units)
.iter()
.map(|crate_name| std_resolve.query(crate_name))
.collect::<CargoResult<Vec<PackageId>>>()?;
// Convert PackageId to Package.
let std_pkgs = package_set.get_many(std_ids)?;
// Generate a map of Units for each kind requested.
let mut ret = HashMap::new();

for pkg in std_pkgs {
let lib = pkg
.targets()
Expand All @@ -162,33 +171,34 @@ pub fn generate_std_roots(
let mode = CompileMode::Build;
let features = std_features.activated_features(pkg.package_id(), FeaturesFor::NormalOrDev);
for kind in kinds {
let list = ret.entry(*kind).or_insert_with(Vec::new);
let unit_for = UnitFor::new_normal(*kind);
let kind = **kind;
let list = ret.entry(kind).or_insert_with(Vec::new);
let unit_for = UnitFor::new_normal(kind);
let profile = profiles.get_profile(
pkg.package_id(),
/*is_member*/ false,
/*is_local*/ false,
unit_for,
*kind,
kind,
);
list.push(interner.intern(
pkg,
lib,
profile,
*kind,
kind,
mode,
features.clone(),
target_data.info(*kind).rustflags.clone(),
target_data.info(*kind).rustdocflags.clone(),
target_data.target_config(*kind).links_overrides.clone(),
target_data.info(kind).rustflags.clone(),
target_data.info(kind).rustdocflags.clone(),
target_data.target_config(kind).links_overrides.clone(),
/*is_std*/ true,
/*dep_hash*/ 0,
IsArtifact::No,
None,
));
}
}
Ok(ret)
Ok(())
}

fn detect_sysroot_src_path(target_data: &RustcTargetData<'_>) -> CargoResult<PathBuf> {
Expand Down
27 changes: 6 additions & 21 deletions src/cargo/core/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -759,7 +759,6 @@ unstable_cli_options!(
avoid_dev_deps: bool = ("Avoid installing dev-dependencies if possible"),
binary_dep_depinfo: bool = ("Track changes to dependency artifacts"),
bindeps: bool = ("Allow Cargo packages to depend on bin, cdylib, and staticlib crates, and use the artifacts built by those crates"),
#[serde(deserialize_with = "deserialize_build_std")]
build_std: Option<Vec<String>> = ("Enable Cargo to compile the standard library itself as part of a crate graph compilation"),
build_std_features: Option<Vec<String>> = ("Configure features enabled for the standard library itself when building the standard library"),
cargo_lints: bool = ("Enable the `[lints.cargo]` table"),
Expand Down Expand Up @@ -873,19 +872,6 @@ const STABILIZED_LINTS: &str = "The `[lints]` table is now always available.";
const STABILIZED_CHECK_CFG: &str =
"Compile-time checking of conditional (a.k.a. `-Zcheck-cfg`) is now always enabled.";

fn deserialize_build_std<'de, D>(deserializer: D) -> Result<Option<Vec<String>>, D::Error>
where
D: serde::Deserializer<'de>,
{
let Some(crates) = <Option<Vec<String>>>::deserialize(deserializer)? else {
return Ok(None);
};
let v = crates.join(",");
Ok(Some(
crate::core::compiler::standard_lib::parse_unstable_flag(Some(&v)),
))
}

#[derive(Debug, Copy, Clone, Default, Deserialize, Ord, PartialOrd, Eq, PartialEq)]
#[serde(default)]
pub struct GitFeatures {
Expand Down Expand Up @@ -1148,7 +1134,8 @@ impl CliUnstable {
}
}

fn parse_features(value: Option<&str>) -> Vec<String> {
/// Parse a comma-separated list
fn parse_list(value: Option<&str>) -> Vec<String> {
match value {
None => Vec::new(),
Some("") => Vec::new(),
Expand Down Expand Up @@ -1197,7 +1184,7 @@ impl CliUnstable {
match k {
// Permanently unstable features
// Sorted alphabetically:
"allow-features" => self.allow_features = Some(parse_features(v).into_iter().collect()),
"allow-features" => self.allow_features = Some(parse_list(v).into_iter().collect()),
"print-im-a-teapot" => self.print_im_a_teapot = parse_bool(k, v)?,

// Stabilized features
Expand All @@ -1216,7 +1203,7 @@ impl CliUnstable {
// until we feel confident to remove entirely.
//
// See rust-lang/cargo#11168
let feats = parse_features(v);
let feats = parse_list(v);
let stab_is_not_empty = feats.iter().any(|feat| {
matches!(
feat.as_str(),
Expand Down Expand Up @@ -1256,10 +1243,8 @@ impl CliUnstable {
"avoid-dev-deps" => self.avoid_dev_deps = parse_empty(k, v)?,
"binary-dep-depinfo" => self.binary_dep_depinfo = parse_empty(k, v)?,
"bindeps" => self.bindeps = parse_empty(k, v)?,
"build-std" => {
self.build_std = Some(crate::core::compiler::standard_lib::parse_unstable_flag(v))
}
"build-std-features" => self.build_std_features = Some(parse_features(v)),
"build-std" => self.build_std = Some(parse_list(v)),
"build-std-features" => self.build_std_features = Some(parse_list(v)),
"cargo-lints" => self.cargo_lints = parse_empty(k, v)?,
"codegen-backend" => self.codegen_backend = parse_empty(k, v)?,
"config-include" => self.config_include = parse_empty(k, v)?,
Expand Down
7 changes: 4 additions & 3 deletions src/cargo/ops/cargo_compile/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,9 +289,9 @@ pub fn create_bcx<'a, 'gctx>(
resolved_features,
} = resolve;

let std_resolve_features = if let Some(crates) = &gctx.cli_unstable().build_std {
let std_resolve_features = if gctx.cli_unstable().build_std.is_some() {
let (std_package_set, std_resolve, std_features) =
standard_lib::resolve_std(ws, &mut target_data, &build_config, crates)?;
standard_lib::resolve_std(ws, &mut target_data, &build_config)?;
pkg_set.add_set(std_package_set);
Some((std_resolve, std_features))
} else {
Expand Down Expand Up @@ -398,10 +398,11 @@ pub fn create_bcx<'a, 'gctx>(
Vec::new()
};

let std_roots = if let Some(crates) = standard_lib::std_crates(gctx, Some(&units)) {
let std_roots = if let Some(crates) = gctx.cli_unstable().build_std.as_ref() {
let (std_resolve, std_features) = std_resolve_features.as_ref().unwrap();
standard_lib::generate_std_roots(
&crates,
&units,
std_resolve,
std_features,
&explicit_host_kinds,
Expand Down
6 changes: 2 additions & 4 deletions src/cargo/ops/cargo_fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,8 @@ pub fn fetch<'a>(
}

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

Expand Down
Loading

0 comments on commit 58c2374

Please sign in to comment.