Skip to content

Commit 55a57fc

Browse files
authored
Rollup merge of #143615 - Kobzol:doc-std, r=jieyouxu
Fix handling of no_std targets in `doc::Std` step The previous logic was wrong for no_std targets, it just didn't do anything. The logic was added there because by default, the `Std` step would otherwise have a list of all std crates to check, but these would fail for no_std targets. What has to happen instead is to select the default set of packages to check/doc/build, which currently happens in the `std_cargo` function, but the `self.crates` list was overriding that. In general, using `crates: Vec<String>` in the `Std` steps is quite fishy, because it's difficult to distinguish between all crates (either they are all enumerated or `crates` is empty) and the default (e.g. `x <kind> [library]`) vs a subset (e.g. `x <kind> core`). I wanted to improve that using an enum that would distinguish these situations, avoid passing `-p` for all of the crates explicitly, and unify the selection of packages to compile/check/... in `std_cargo`, based on this enum. However, I found out from some other bootstrap comments that when you pass `-p` explicitly for all crates, cargo behaves differently (apparently for check it will also check targets/examples etc. with `-p`, but not without it). Furthermore, the doc step has a special case where it does not document the `sysroot` package. So as usually, unifying this logic would get into some edge cases... So instead I opted for a seemingly simpler solution, where I try to prefilter only two allowed crates (core and alloc) for no_std targets in the `std_crates_for_run_make` function. It's not perfect, but I think it's better than the status quo (words to live by when working on bootstrap...). Fixes [this Zulip topic](https://rust-lang.zulipchat.com/#narrow/channel/326414-t-infra.2Fbootstrap/topic/docs.20for.20non-host.20targets.3F). r? `@jieyouxu`
2 parents 9e6af56 + 6c38d38 commit 55a57fc

File tree

5 files changed

+174
-68
lines changed

5 files changed

+174
-68
lines changed

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

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -449,26 +449,24 @@ fn copy_self_contained_objects(
449449
target_deps
450450
}
451451

452-
/// Resolves standard library crates for `Std::run_make` for any build kind (like check, build, clippy, etc.).
452+
/// Resolves standard library crates for `Std::run_make` for any build kind (like check, doc,
453+
/// build, clippy, etc.).
453454
pub fn std_crates_for_run_make(run: &RunConfig<'_>) -> Vec<String> {
454-
// FIXME: Extend builder tests to cover the `crates` field of `Std` instances.
455-
if cfg!(test) {
456-
return vec![];
457-
}
458-
459-
let has_alias = run.paths.iter().any(|set| set.assert_single_path().path.ends_with("library"));
455+
let mut crates = run.make_run_crates(builder::Alias::Library);
456+
457+
// For no_std targets, we only want to check core and alloc
458+
// Regardless of core/alloc being selected explicitly or via the "library" default alias,
459+
// we only want to keep these two crates.
460+
// The set of no_std crates should be kept in sync with what `Builder::std_cargo` does.
461+
// Note: an alternative design would be to return an enum from this function (Default vs Subset)
462+
// of crates. However, several steps currently pass `-p <package>` even if all crates are
463+
// selected, because Cargo behaves differently in that case. To keep that behavior without
464+
// making further changes, we pre-filter the no-std crates here.
460465
let target_is_no_std = run.builder.no_std(run.target).unwrap_or(false);
461-
462-
// For no_std targets, do not add any additional crates to the compilation other than what `compile::std_cargo` already adds for no_std targets.
463466
if target_is_no_std {
464-
vec![]
465-
}
466-
// If the paths include "library", build the entire standard library.
467-
else if has_alias {
468-
run.make_run_crates(builder::Alias::Library)
469-
} else {
470-
run.cargo_crates_in_set()
467+
crates.retain(|c| c == "core" || c == "alloc");
471468
}
469+
crates
472470
}
473471

474472
/// Tries to find LLVM's `compiler-rt` source directory, for building `library/profiler_builtins`.

src/bootstrap/src/core/build_steps/doc.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -665,7 +665,11 @@ impl Step for Std {
665665
}
666666

667667
fn metadata(&self) -> Option<StepMetadata> {
668-
Some(StepMetadata::doc("std", self.target).stage(self.stage))
668+
Some(
669+
StepMetadata::doc("std", self.target)
670+
.stage(self.stage)
671+
.with_metadata(format!("crates=[{}]", self.crates.join(","))),
672+
)
669673
}
670674
}
671675

src/bootstrap/src/core/builder/mod.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,8 @@ pub struct StepMetadata {
146146
target: TargetSelection,
147147
built_by: Option<Compiler>,
148148
stage: Option<u32>,
149+
/// Additional opaque string printed in the metadata
150+
metadata: Option<String>,
149151
}
150152

151153
impl StepMetadata {
@@ -170,7 +172,7 @@ impl StepMetadata {
170172
}
171173

172174
fn new(name: &'static str, target: TargetSelection, kind: Kind) -> Self {
173-
Self { name, kind, target, built_by: None, stage: None }
175+
Self { name, kind, target, built_by: None, stage: None, metadata: None }
174176
}
175177

176178
pub fn built_by(mut self, compiler: Compiler) -> Self {
@@ -183,6 +185,11 @@ impl StepMetadata {
183185
self
184186
}
185187

188+
pub fn with_metadata(mut self, metadata: String) -> Self {
189+
self.metadata = Some(metadata);
190+
self
191+
}
192+
186193
pub fn get_stage(&self) -> Option<u32> {
187194
self.stage.or(self
188195
.built_by

0 commit comments

Comments
 (0)