diff --git a/src/bootstrap/src/core/build_steps/check.rs b/src/bootstrap/src/core/build_steps/check.rs index 002b46cb7fbec..4c3e49c885d8b 100644 --- a/src/bootstrap/src/core/build_steps/check.rs +++ b/src/bootstrap/src/core/build_steps/check.rs @@ -13,7 +13,8 @@ use crate::core::build_steps::tool::{ prepare_tool_cargo, }; use crate::core::builder::{ - self, Alias, Builder, Cargo, Kind, RunConfig, ShouldRun, Step, StepMetadata, crate_description, + self, Alias, Builder, Cargo, Kind, RunConfig, ShouldRun, Step, StepMetadata, StepTask, + crate_description, }; use crate::core::config::TargetSelection; use crate::utils::build_stamp::{self, BuildStamp}; @@ -219,13 +220,9 @@ impl PrepareRustcRmetaSysroot { } } -impl Step for PrepareRustcRmetaSysroot { +impl StepTask for PrepareRustcRmetaSysroot { type Output = RmetaSysroot; - fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { - run.never() - } - fn run(self, builder: &Builder<'_>) -> Self::Output { // Check rustc let stamp = builder.ensure(Rustc::from_build_compiler( @@ -263,13 +260,9 @@ impl PrepareStdRmetaSysroot { } } -impl Step for PrepareStdRmetaSysroot { +impl StepTask for PrepareStdRmetaSysroot { type Output = RmetaSysroot; - fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { - run.never() - } - fn run(self, builder: &Builder<'_>) -> Self::Output { // Check std let stamp = builder.ensure(Std { diff --git a/src/bootstrap/src/core/build_steps/clean.rs b/src/bootstrap/src/core/build_steps/clean.rs index fec4a79de3c75..36894b2f92c58 100644 --- a/src/bootstrap/src/core/build_steps/clean.rs +++ b/src/bootstrap/src/core/build_steps/clean.rs @@ -21,8 +21,9 @@ impl Step for CleanAll { type Output = (); fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { - // Only runs as the default `./x clean` step; cannot be selected explicitly. - run.never() + // Normally this step is invoked implicitly via `./x clean`, but all + // steps are required to register at least one explicit path/alias. + run.alias("default") } fn is_default_step(_builder: &Builder<'_>) -> bool { diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs index 2335efc870a9c..74951737c326e 100644 --- a/src/bootstrap/src/core/build_steps/compile.rs +++ b/src/bootstrap/src/core/build_steps/compile.rs @@ -22,9 +22,9 @@ use tracing::span; use crate::core::build_steps::gcc::{Gcc, GccOutput, GccTargetPair}; use crate::core::build_steps::tool::{RustcPrivateCompilers, SourceType, copy_lld_artifacts}; use crate::core::build_steps::{dist, llvm}; -use crate::core::builder; use crate::core::builder::{ - Builder, Cargo, Kind, RunConfig, ShouldRun, Step, StepMetadata, apply_pgo, crate_description, + self, Builder, Cargo, Kind, RunConfig, ShouldRun, Step, StepMetadata, StepTask, apply_pgo, + crate_description, }; use crate::core::config::toml::target::DefaultLinuxLinkerOverride; use crate::core::config::{ @@ -740,13 +740,9 @@ impl StdLink { } } -impl Step for StdLink { +impl StepTask for StdLink { type Output = (); - fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { - run.never() - } - /// Link all libstd rlibs/dylibs into the sysroot location. /// /// Links those artifacts generated by `compiler` to the `stage` compiler's @@ -1530,13 +1526,9 @@ impl RustcLink { } } -impl Step for RustcLink { +impl StepTask for RustcLink { type Output = (); - fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { - run.never() - } - /// Same as `StdLink`, only for librustc fn run(self, builder: &Builder<'_>) { let build_compiler = self.build_compiler; @@ -1904,13 +1896,9 @@ impl Sysroot { } } -impl Step for Sysroot { +impl StepTask for Sysroot { type Output = PathBuf; - fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { - run.never() - } - /// Returns the sysroot that `compiler` is supposed to use. /// For the stage0 compiler, this is stage0-sysroot (because of the initial std build). /// For all other stages, it's the same stage directory that the compiler lives in. diff --git a/src/bootstrap/src/core/build_steps/dist.rs b/src/bootstrap/src/core/build_steps/dist.rs index b74dd7dc3b987..a71a33cd12d1d 100644 --- a/src/bootstrap/src/core/build_steps/dist.rs +++ b/src/bootstrap/src/core/build_steps/dist.rs @@ -29,7 +29,7 @@ use crate::core::build_steps::tool::{ }; use crate::core::build_steps::vendor::Vendor; use crate::core::build_steps::{compile, llvm}; -use crate::core::builder::{Builder, Kind, RunConfig, ShouldRun, Step, StepMetadata}; +use crate::core::builder::{Builder, Kind, RunConfig, ShouldRun, Step, StepMetadata, StepTask}; use crate::core::config::{GccCiMode, TargetSelection}; use crate::utils::build_stamp::{self, BuildStamp}; use crate::utils::channel::{self, Info}; @@ -704,13 +704,9 @@ pub struct DebuggerScripts { pub target: TargetSelection, } -impl Step for DebuggerScripts { +impl StepTask for DebuggerScripts { type Output = (); - fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { - run.never() - } - fn run(self, builder: &Builder<'_>) { let target = self.target; let sysroot = self.sysroot; diff --git a/src/bootstrap/src/core/build_steps/doc.rs b/src/bootstrap/src/core/build_steps/doc.rs index 4787578e19db0..02441fef6e6c0 100644 --- a/src/bootstrap/src/core/build_steps/doc.rs +++ b/src/bootstrap/src/core/build_steps/doc.rs @@ -16,7 +16,8 @@ use crate::core::build_steps::tool::{ self, RustcPrivateCompilers, SourceType, Tool, prepare_tool_cargo, }; use crate::core::builder::{ - self, Builder, Compiler, Kind, RunConfig, ShouldRun, Step, StepMetadata, crate_description, + self, Builder, Compiler, Kind, RunConfig, ShouldRun, Step, StepMetadata, StepTask, + crate_description, }; use crate::core::config::{Config, TargetSelection}; use crate::helpers::{submodule_path_of, symlink_dir, t, up_to_date}; @@ -138,13 +139,9 @@ struct RustbookSrc { build_compiler: Option, } -impl Step for RustbookSrc

{ +impl StepTask for RustbookSrc

{ type Output = (); - fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { - run.never() - } - /// Invoke `rustbook` for `target` for the doc book `name` from the `src` path. /// /// This will not actually generate any documentation if the documentation has @@ -574,18 +571,9 @@ pub struct SharedAssets { target: TargetSelection, } -impl Step for SharedAssets { +impl StepTask for SharedAssets { type Output = SharedAssetsPaths; - fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { - // Other tasks depend on this, no need to execute it on its own - run.never() - } - - fn is_default_step(_builder: &Builder<'_>) -> bool { - false - } - /// Generate shared resources used by other pieces of documentation. fn run(self, builder: &Builder<'_>) -> Self::Output { let out = builder.doc_out(self.target); diff --git a/src/bootstrap/src/core/build_steps/synthetic_targets.rs b/src/bootstrap/src/core/build_steps/synthetic_targets.rs index 964ccf1c31088..4d793fe0cf2ea 100644 --- a/src/bootstrap/src/core/build_steps/synthetic_targets.rs +++ b/src/bootstrap/src/core/build_steps/synthetic_targets.rs @@ -8,7 +8,7 @@ //! that calls create_synthetic_target. use crate::Compiler; -use crate::core::builder::{Builder, ShouldRun, Step}; +use crate::core::builder::{Builder, StepTask}; use crate::core::config::TargetSelection; #[derive(Debug, Clone, PartialEq, Eq, Hash)] @@ -17,17 +17,9 @@ pub(crate) struct MirOptPanicAbortSyntheticTarget { pub(crate) base: TargetSelection, } -impl Step for MirOptPanicAbortSyntheticTarget { +impl StepTask for MirOptPanicAbortSyntheticTarget { type Output = TargetSelection; - fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { - run.never() - } - - fn is_default_step(_builder: &Builder<'_>) -> bool { - true - } - fn run(self, builder: &Builder<'_>) -> Self::Output { create_synthetic_target(builder, self.compiler, "miropt-abort", self.base, |spec| { spec.insert("panic-strategy".into(), "abort".into()); diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 9ebe3191db40f..2bb70835bc77d 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -28,7 +28,7 @@ use crate::core::build_steps::tool::{ use crate::core::build_steps::toolstate::ToolState; use crate::core::build_steps::{compile, dist, llvm}; use crate::core::builder::{ - self, Alias, Builder, Compiler, Kind, RunConfig, ShouldRun, Step, StepMetadata, + self, Alias, Builder, Compiler, Kind, RunConfig, ShouldRun, Step, StepMetadata, StepTask, crate_description, }; use crate::core::config::TargetSelection; @@ -2129,13 +2129,9 @@ struct Compiletest { compare_mode: Option<&'static str>, } -impl Step for Compiletest { +impl StepTask for Compiletest { type Output = (); - fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { - run.never() - } - fn run(self, builder: &Builder<'_>) { if builder.test_target == TestTarget::DocOnly { return; @@ -2854,13 +2850,8 @@ struct BookTest { dependencies: Vec<&'static str>, } -impl Step for BookTest { +impl StepTask for BookTest { type Output = (); - const IS_HOST: bool = true; - - fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { - run.never() - } fn run(self, builder: &Builder<'_>) { // External docs are different from local because: @@ -3663,13 +3654,9 @@ pub struct RemoteCopyLibs { target: TargetSelection, } -impl Step for RemoteCopyLibs { +impl StepTask for RemoteCopyLibs { type Output = (); - fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { - run.never() - } - fn run(self, builder: &Builder<'_>) { let build_compiler = self.build_compiler; let target = self.target; diff --git a/src/bootstrap/src/core/build_steps/test/failed_tests.rs b/src/bootstrap/src/core/build_steps/test/failed_tests.rs index 8be6e50fd3db1..56c9eec6890d3 100644 --- a/src/bootstrap/src/core/build_steps/test/failed_tests.rs +++ b/src/bootstrap/src/core/build_steps/test/failed_tests.rs @@ -3,7 +3,7 @@ use std::fs::{self, File}; use std::io::{BufRead, BufReader, ErrorKind}; use std::path::{Path, PathBuf}; -use crate::core::builder::{Builder, ShouldRun, Step}; +use crate::core::builder::{Builder, StepTask}; use crate::t; #[derive(Clone)] @@ -29,13 +29,9 @@ impl RecordFailedTests { /// and the `RecordFailedTest` type doesn't (need to) signify anything. #[derive(Clone, Copy, Eq, PartialEq, Hash, Debug)] pub struct SetupFailedTestsFile; -impl Step for SetupFailedTestsFile { +impl StepTask for SetupFailedTestsFile { type Output = RecordFailedTests; - fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { - run.never() - } - fn run(self, builder: &Builder<'_>) -> Self::Output { if !builder.config.cmd.record() || builder.config.dry_run() { return RecordFailedTests { failed_tests_path: None }; diff --git a/src/bootstrap/src/core/build_steps/tool.rs b/src/bootstrap/src/core/build_steps/tool.rs index e5d24280092d9..42a0ccaf564ac 100644 --- a/src/bootstrap/src/core/build_steps/tool.rs +++ b/src/bootstrap/src/core/build_steps/tool.rs @@ -16,10 +16,9 @@ use std::{env, fs}; use crate::core::build_steps::compile::is_lto_stage; use crate::core::build_steps::toolstate::ToolState; use crate::core::build_steps::{compile, llvm}; -use crate::core::builder; use crate::core::builder::{ - Builder, Cargo as CargoCommand, RunConfig, ShouldRun, Step, StepMetadata, apply_pgo, - cargo_profile_var, + self, Builder, Cargo as CargoCommand, RunConfig, ShouldRun, Step, StepMetadata, StepTask, + apply_pgo, cargo_profile_var, }; use crate::core::config::{DebuginfoLevel, RustcLto, TargetSelection}; use crate::utils::exec::{BootstrapCommand, command}; @@ -66,13 +65,9 @@ pub struct ToolBuildResult { pub build_compiler: Compiler, } -impl Step for ToolBuild { +impl StepTask for ToolBuild { type Output = ToolBuildResult; - fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { - run.never() - } - /// Builds a tool in `src/tools` /// /// This will build the specified tool with the specified `host` compiler in @@ -1239,17 +1234,8 @@ pub enum LibcxxVersion { Llvm(usize), } -impl Step for LibcxxVersionTool { +impl StepTask for LibcxxVersionTool { type Output = LibcxxVersion; - const IS_HOST: bool = true; - - fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { - run.never() - } - - fn is_default_step(_builder: &Builder<'_>) -> bool { - false - } fn run(self, builder: &Builder<'_>) -> LibcxxVersion { let out_dir = builder.out.join(self.target.to_string()).join("libcxx-version"); diff --git a/src/bootstrap/src/core/builder/cli_paths/snapshots/x_clean.snap b/src/bootstrap/src/core/builder/cli_paths/snapshots/x_clean.snap index 71f12ecd501a7..30eb798f39938 100644 --- a/src/bootstrap/src/core/builder/cli_paths/snapshots/x_clean.snap +++ b/src/bootstrap/src/core/builder/cli_paths/snapshots/x_clean.snap @@ -4,4 +4,4 @@ expression: clean --- [Clean] clean::CleanAll targets: [aarch64-unknown-linux-gnu] - - Set({}) + - Set({clean::default}) diff --git a/src/bootstrap/src/core/builder/mod.rs b/src/bootstrap/src/core/builder/mod.rs index 9b14250f74e6d..2a2c9e639bb34 100644 --- a/src/bootstrap/src/core/builder/mod.rs +++ b/src/bootstrap/src/core/builder/mod.rs @@ -52,15 +52,15 @@ pub struct Builder<'a> { /// What to build or what action to perform. pub kind: Kind, - /// A cache of outputs of [`Step`]s so we can avoid running steps we already + /// A cache of outputs of [`StepTask`]s so we can avoid running steps we already /// ran. cache: Cache, - /// A stack of [`Step`]s to run before we can run this builder. The output + /// A stack of [`StepTask`]s to run before we can run this builder. The output /// of steps is cached in [`Self::cache`]. stack: RefCell>>, - /// The total amount of time we spent running [`Step`]s in [`Self::stack`]. + /// The total amount of time we spent running [`StepTask`]s in [`Self::stack`]. time_spent_on_dependencies: Cell, /// The paths passed on the command line. Used by steps to figure out what @@ -101,6 +101,36 @@ impl dyn AnyDebug { // Feel free to add other `dyn Any` methods as necessary. } +/// A cacheable unit of work that can be performed via [`Builder::ensure`]. +/// +/// This trait was split off from [`Step`], so that helper tasks don't have +/// to implement all of the step APIs related to command-line processing. +pub trait StepTask: 'static + Clone + Debug + PartialEq + Eq + Hash { + /// Result type of [`StepTask::run`]. + type Output: Clone; + + /// Runs this task. See [`Step::run`]. + fn run(self, builder: &Builder<'_>) -> Self::Output; + + /// Returns metadata of the step, for tests. + fn metadata(&self) -> Option { + None + } +} + +/// Every [`Step`] is also a [`StepTask`]. +impl StepTask for S { + type Output = ::Output; + + fn run(self, builder: &Builder<'_>) -> Self::Output { + ::run(self, builder) + } + + fn metadata(&self) -> Option { + ::metadata(self) + } +} + pub trait Step: 'static + Clone + Debug + PartialEq + Eq + Hash { /// Result type of `Step::run`. type Output: Clone; @@ -152,13 +182,7 @@ pub trait Step: 'static + Clone + Debug + PartialEq + Eq + Hash { /// Called directly by the bootstrap `Step` handler when not triggered indirectly by other `Step`s using [`Builder::ensure`]. /// For example, `./x.py test bootstrap` runs this for `test::Bootstrap`. Similarly, `./x.py test` runs it for every step /// that is listed by the `describe` macro in [`Builder::get_step_descriptions`]. - fn make_run(_run: RunConfig<'_>) { - // It is reasonable to not have an implementation of make_run for rules - // who do not want to get called from the root context. This means that - // they are likely dependencies (e.g., sysroot creation) or similar, and - // as such calling them from ./x.py isn't logical. - unimplemented!() - } + fn make_run(_run: RunConfig<'_>); /// Returns metadata of the step, for tests fn metadata(&self) -> Option { @@ -617,12 +641,6 @@ impl<'a> ShouldRun<'a> { self } - // allows being more explicit about why should_run in Step returns the value passed to it - pub fn never(mut self) -> ShouldRun<'a> { - self.paths.insert(PathSet::empty()); - self - } - /// Given a set of requested paths, return the subset which match the Step for this `ShouldRun`, /// removing the matches from `paths`. /// @@ -744,13 +762,9 @@ struct Libdir { target: TargetSelection, } -impl Step for Libdir { +impl StepTask for Libdir { type Output = PathBuf; - fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { - run.never() - } - fn run(self, builder: &Builder<'_>) -> PathBuf { let relative_sysroot_libdir = builder.sysroot_libdir_relative(self.compiler); let sysroot = builder.sysroot(self.compiler).join(relative_sysroot_libdir).join("rustlib"); @@ -1577,8 +1591,11 @@ Alternatively, you can set `build.local-rebuild=true` and use a stage0 compiler /// Ensure that a given step is built, returning its output. This will /// cache the step, so it is safe (and good!) to call this as often as /// needed to ensure that all dependencies are built. + /// + /// This actually takes [`StepTask`] instead of [`Step`], to allow for + /// helper tasks that don't need to implement the full step interface. #[track_caller] - pub fn ensure(&'a self, step: S) -> S::Output { + pub fn ensure(&'a self, step: S) -> S::Output { { let mut stack = self.stack.borrow_mut(); for stack_step in stack.iter() { @@ -1747,14 +1764,14 @@ Alternatively, you can set `build.local-rebuild=true` and use a stage0 compiler } /// Return qualified step name, e.g. `compile::Rustc`. -pub fn pretty_step_name() -> String { +pub fn pretty_step_name() -> String { // Normalize step type path to only keep the module and the type name let path = type_name::().rsplit("::").take(2).collect::>(); path.into_iter().rev().collect::>().join("::") } /// Renders `step` using its `Debug` implementation and extract the field arguments out of it. -fn step_debug_args(step: &S) -> String { +fn step_debug_args(step: &S) -> String { let step_dbg_repr = format!("{step:?}"); // Some steps do not have any arguments, so they do not have the braces @@ -1766,7 +1783,7 @@ fn step_debug_args(step: &S) -> String { } } -fn pretty_print_step(step: &S) -> String { +fn pretty_print_step(step: &S) -> String { format!("{} {{ {} }}", pretty_step_name::(), step_debug_args(step)) } diff --git a/src/bootstrap/src/utils/cache.rs b/src/bootstrap/src/utils/cache.rs index 8c5b3529979d7..40d73cac5dd74 100644 --- a/src/bootstrap/src/utils/cache.rs +++ b/src/bootstrap/src/utils/cache.rs @@ -24,7 +24,7 @@ use std::ops::Deref; use std::sync::{LazyLock, Mutex}; use std::{fmt, mem}; -use crate::core::builder::Step; +use crate::core::builder::StepTask; /// Represents an interned value of type `T`, allowing for efficient comparisons and retrieval. /// @@ -236,7 +236,7 @@ impl Cache { } /// Stores the result of a computation step in the cache. - pub fn put(&self, step: S, value: S::Output) { + pub fn put(&self, step: S, value: S::Output) { let mut cache = self.cache.borrow_mut(); let type_id = TypeId::of::(); let stepcache = cache @@ -256,7 +256,7 @@ impl Cache { } /// Retrieves a cached result for the given step, if available. - pub fn get(&self, step: &S) -> Option { + pub fn get(&self, step: &S) -> Option { let mut cache = self.cache.borrow_mut(); let type_id = TypeId::of::(); let stepcache = cache @@ -270,7 +270,7 @@ impl Cache { #[cfg(test)] impl Cache { - pub(crate) fn inspect_all_steps_of_type( + pub(crate) fn inspect_all_steps_of_type( &self, map_fn: impl Fn(&S, &S::Output) -> T, ) -> Vec { @@ -284,7 +284,7 @@ impl Cache { values } - pub fn contains(&self) -> bool { + pub fn contains(&self) -> bool { self.cache.borrow().contains_key(&TypeId::of::()) } diff --git a/src/bootstrap/src/utils/metrics.rs b/src/bootstrap/src/utils/metrics.rs index e685c64733c66..e2c6124f07ab5 100644 --- a/src/bootstrap/src/utils/metrics.rs +++ b/src/bootstrap/src/utils/metrics.rs @@ -17,7 +17,7 @@ use build_helper::metrics::{ use sysinfo::{CpuRefreshKind, RefreshKind, System}; use crate::Build; -use crate::core::builder::{Builder, Step}; +use crate::core::builder::{Builder, StepTask}; use crate::utils::helpers::t; // Update this number whenever a breaking change is made to the build metrics. @@ -67,7 +67,7 @@ impl BuildMetrics { BuildMetrics { state } } - pub(crate) fn enter_step(&self, step: &S, builder: &Builder<'_>) { + pub(crate) fn enter_step(&self, step: &S, builder: &Builder<'_>) { // Do not record dry runs, as they'd be duplicates of the actual steps. if builder.config.dry_run() { return; diff --git a/src/bootstrap/src/utils/step_graph.rs b/src/bootstrap/src/utils/step_graph.rs index d6a2d89482ee4..8dc07f6b39a5c 100644 --- a/src/bootstrap/src/utils/step_graph.rs +++ b/src/bootstrap/src/utils/step_graph.rs @@ -4,7 +4,7 @@ use std::io::BufWriter; use std::panic::Location; use std::path::Path; -use crate::core::builder::{AnyDebug, Step, pretty_step_name}; +use crate::core::builder::{AnyDebug, StepTask, pretty_step_name}; use crate::t; use crate::utils::tracing::format_location; @@ -23,7 +23,7 @@ pub struct StepGraph { impl StepGraph { #[track_caller] - pub fn register_step_execution( + pub fn register_step_execution( &mut self, step: &S, parent: Option<&Box>, @@ -61,7 +61,7 @@ impl StepGraph { } #[track_caller] - pub fn register_cached_step( + pub fn register_cached_step( &mut self, step: &S, parent: &Box,