Rollup merge of #142627 - Kobzol:bootstrap-metadata, r=jieyouxu

Add `StepMetadata` to describe steps

This is used to replace the previous downcasting of executed steps, which wasn't very scalable. In addition to tests, we could also use the metadata e.g. for tracing.

r? `@jieyouxu`

Author: Kobzol

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

@@ -24,7 +24,8 @@ use crate::core::build_steps::tool::SourceType;
 use crate::core::build_steps::{dist, llvm};
 use crate::core::builder;
 use crate::core::builder::{
-    Builder, Cargo, Kind, PathSet, RunConfig, ShouldRun, Step, TaskPath, crate_description,
+    Builder, Cargo, Kind, PathSet, RunConfig, ShouldRun, Step, StepMetadata, TaskPath,
+    crate_description,
 };
 use crate::core::config::{DebuginfoLevel, LlvmLibunwind, RustcLto, TargetSelection};
 use crate::utils::build_stamp;
@@ -305,6 +306,14 @@ impl Step for Std {
             builder.compiler(compiler.stage, builder.config.host_target),
         ));
     }
+
+    fn metadata(&self) -> Option<StepMetadata> {
+        Some(
+            StepMetadata::build("std", self.target)
+                .built_by(self.compiler)
+                .stage(self.compiler.stage),
+        )
+    }
 }
 
 fn copy_and_stamp(
@@ -1171,6 +1180,14 @@ impl Step for Rustc {
 
         build_compiler.stage
     }
+
+    fn metadata(&self) -> Option<StepMetadata> {
+        Some(
+            StepMetadata::build("rustc", self.target)
+                .built_by(self.build_compiler)
+                .stage(self.build_compiler.stage + 1),
+        )
+    }
 }
 
 pub fn rustc_cargo(

src/bootstrap/src/core/build_steps/llvm.rs

@@ -18,7 +18,7 @@ use build_helper::git::PathFreshness;
 #[cfg(feature = "tracing")]
 use tracing::instrument;
 
-use crate::core::builder::{Builder, RunConfig, ShouldRun, Step};
+use crate::core::builder::{Builder, RunConfig, ShouldRun, Step, StepMetadata};
 use crate::core::config::{Config, TargetSelection};
 use crate::utils::build_stamp::{BuildStamp, generate_smart_stamp_hash};
 use crate::utils::exec::command;
@@ -582,6 +582,10 @@ impl Step for Llvm {
 
         res
     }
+
+    fn metadata(&self) -> Option<StepMetadata> {
+        Some(StepMetadata::build("llvm", self.target))
+    }
 }
 
 pub fn get_llvm_version(builder: &Builder<'_>, llvm_config: &Path) -> String {

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

@@ -130,6 +130,38 @@ pub trait Step: 'static + Clone + Debug + PartialEq + Eq + Hash {
         // as such calling them from ./x.py isn't logical.
         unimplemented!()
     }
+
+    /// Returns metadata of the step, for tests
+    fn metadata(&self) -> Option<StepMetadata> {
+        None
+    }
+}
+
+/// Metadata that describes an executed step, mostly for testing and tracing.
+#[allow(unused)]
+#[derive(Debug)]
+pub struct StepMetadata {
+    name: &'static str,
+    kind: Kind,
+    target: TargetSelection,
+    built_by: Option<Compiler>,
+    stage: Option<u32>,
+}
+
+impl StepMetadata {
+    pub fn build(name: &'static str, target: TargetSelection) -> Self {
+        Self { name, kind: Kind::Build, target, built_by: None, stage: None }
+    }
+
+    pub fn built_by(mut self, compiler: Compiler) -> Self {
+        self.built_by = Some(compiler);
+        self
+    }
+
+    pub fn stage(mut self, stage: u32) -> Self {
+        self.stage = Some(stage);
+        self
+    }
 }
 
 pub struct RunConfig<'a> {

src/bootstrap/src/core/builder/tests.rs

@@ -8,6 +8,7 @@ use super::*;
 use crate::Flags;
 use crate::core::build_steps::doc::DocumentationFormat;
 use crate::core::config::Config;
+use crate::utils::cache::ExecutedStep;
 use crate::utils::tests::git::{GitCtx, git_test};
 
 static TEST_TRIPLE_1: &str = "i686-unknown-haiku";
@@ -1258,31 +1259,24 @@ mod staging {
 /// Renders the executed bootstrap steps for usage in snapshot tests with insta.
 /// Only renders certain important steps.
 /// Each value in `steps` should be a tuple of (Step, step output).
-fn render_steps(steps: &[(Box<dyn Any>, Box<dyn Any>)]) -> String {
+fn render_steps(steps: &[ExecutedStep]) -> String {
     steps
         .iter()
-        .filter_map(|(step, output)| {
-            // FIXME: implement an optional method on Step to produce metadata for test, instead
-            // of this downcasting
-            if let Some((rustc, output)) = downcast_step::<compile::Rustc>(step, output) {
-                Some(format!(
-                    "[build] {} -> {}",
-                    render_compiler(rustc.build_compiler),
-                    // FIXME: return the correct stage from the `Rustc` step, now it behaves weirdly
-                    render_compiler(Compiler::new(rustc.build_compiler.stage + 1, rustc.target)),
-                ))
-            } else if let Some((std, output)) = downcast_step::<compile::Std>(step, output) {
-                Some(format!(
-                    "[build] {} -> std {} <{}>",
-                    render_compiler(std.compiler),
-                    std.compiler.stage,
-                    std.target
-                ))
-            } else if let Some((llvm, output)) = downcast_step::<llvm::Llvm>(step, output) {
-                Some(format!("[build] llvm <{}>", llvm.target))
-            } else {
-                None
+        .filter_map(|step| {
+            use std::fmt::Write;
+
+            let Some(metadata) = &step.metadata else {
+                return None;
+            };
+
+            let mut record = format!("[{}] ", metadata.kind.as_str());
+            if let Some(compiler) = metadata.built_by {
+                write!(record, "{} -> ", render_compiler(compiler));
             }
+            let stage =
+                if let Some(stage) = metadata.stage { format!("{stage} ") } else { "".to_string() };
+            write!(record, "{} {stage}<{}>", metadata.name, metadata.target);
+            Some(record)
         })
         .map(|line| {
             line.replace(TEST_TRIPLE_1, "target1")
@@ -1293,19 +1287,6 @@ fn render_steps(steps: &[(Box<dyn Any>, Box<dyn Any>)]) -> String {
         .join("\n")
 }
 
-fn downcast_step<'a, S: Step>(
-    step: &'a Box<dyn Any>,
-    output: &'a Box<dyn Any>,
-) -> Option<(&'a S, &'a S::Output)> {
-    let Some(step) = step.downcast_ref::<S>() else {
-        return None;
-    };
-    let Some(output) = output.downcast_ref::<S::Output>() else {
-        return None;
-    };
-    Some((step, output))
-}
-
 fn render_compiler(compiler: Compiler) -> String {
     format!("rustc {} <{}>", compiler.stage, compiler.host)
 }

src/bootstrap/src/utils/cache.rs

@@ -218,10 +218,15 @@ pub struct Cache {
         >,
     >,
     #[cfg(test)]
-    /// Contains steps in the same order in which they were executed
-    /// Useful for tests
-    /// Tuples (step, step output)
-    executed_steps: RefCell<Vec<(Box<dyn Any>, Box<dyn Any>)>>,
+    /// Contains step metadata of executed steps (in the same order in which they were executed).
+    /// Useful for tests.
+    executed_steps: RefCell<Vec<ExecutedStep>>,
+}
+
+#[cfg(test)]
+#[derive(Debug)]
+pub struct ExecutedStep {
+    pub metadata: Option<crate::core::builder::StepMetadata>,
 }
 
 impl Cache {
@@ -243,9 +248,8 @@ impl Cache {
 
         #[cfg(test)]
         {
-            let step: Box<dyn Any> = Box::new(step.clone());
-            let output: Box<dyn Any> = Box::new(value.clone());
-            self.executed_steps.borrow_mut().push((step, output));
+            let metadata = step.metadata();
+            self.executed_steps.borrow_mut().push(ExecutedStep { metadata });
         }
 
         stepcache.insert(step, value);
@@ -283,7 +287,7 @@ impl Cache {
     }
 
     #[cfg(test)]
-    pub fn into_executed_steps(mut self) -> Vec<(Box<dyn Any>, Box<dyn Any>)> {
+    pub fn into_executed_steps(mut self) -> Vec<ExecutedStep> {
         mem::take(&mut self.executed_steps.borrow_mut())
     }
 }