Auto merge of #145874 - Kobzol:fix-dist-builds, r=jieyouxu

Remove unnecessary stage2 host builds from cross-compiled dist builders

This is a repeated regression (https://github.com/rust-lang/rust/issues/138004, https://github.com/rust-lang/rust/issues/138123) that was reintroduced in https://github.com/rust-lang/rust/pull/145472. I thought that we have a test for it, but alas, the "correct" test required `--disable-docs`. I added the test in this PR, and re-added the `dist::Std` build optimization that solves this.

r? `@jieyouxu`

3 files changed, 112 insertions, 31 deletions

diff --git a/src/bootstrap/src/core/build_steps/dist.rs b/src/bootstrap/src/core/build_steps/dist.rs
index 3c322256cf2..b01832708df 100644
--- a/src/bootstrap/src/core/build_steps/dist.rs
+++ b/src/bootstrap/src/core/build_steps/dist.rs
@@ -764,10 +764,22 @@ pub struct Std {
 
 impl Std {
     pub fn new(builder: &Builder<'_>, target: TargetSelection) -> Self {
-        Std {
-            build_compiler: builder.compiler(builder.top_stage, builder.config.host_target),
-            target,
-        }
+        // This is an important optimization mainly for CI.
+        // Normally, to build stage N libstd, we need stage N rustc.
+        // However, if we know that we will uplift libstd from stage 1 anyway, building the stage N
+        // rustc can be wasteful.
+        // In particular, if we do a cross-compiling dist stage 2 build from T1 to T2, we need:
+        // - stage 2 libstd for T2 (uplifted from stage 1, where it was built by T1 rustc)
+        // - stage 2 rustc for T2
+        // However, without this optimization, we would also build stage 2 rustc for **T1**, which
+        // is completely wasteful.
+        let build_compiler =
+            if compile::Std::should_be_uplifted_from_stage_1(builder, builder.top_stage, target) {
+                builder.compiler(1, builder.host_target)
+            } else {
+                builder.compiler(builder.top_stage, builder.host_target)
+            };
+        Std { build_compiler, target }
     }
 }
 
diff --git a/src/bootstrap/src/core/builder/mod.rs b/src/bootstrap/src/core/builder/mod.rs
index 4f1bed13c13..76cade25a03 100644
--- a/src/bootstrap/src/core/builder/mod.rs
+++ b/src/bootstrap/src/core/builder/mod.rs
@@ -145,8 +145,7 @@ pub trait Step: 'static + Clone + Debug + PartialEq + Eq + Hash {
 }
 
 /// Metadata that describes an executed step, mostly for testing and tracing.
-#[allow(unused)]
-#[derive(Debug, PartialEq, Eq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct StepMetadata {
     name: String,
     kind: Kind,
diff --git a/src/bootstrap/src/core/builder/tests.rs b/src/bootstrap/src/core/builder/tests.rs
index 5260b43f706..4d47562c7ce 100644
--- a/src/bootstrap/src/core/builder/tests.rs
+++ b/src/bootstrap/src/core/builder/tests.rs
@@ -1133,8 +1133,7 @@ mod snapshot {
         [dist] mingw <host>
         [build] rustc 0 <host> -> GenerateCopyright 1 <host>
         [dist] rustc <host>
-        [build] rustc 2 <host> -> std 2 <host>
-        [dist] rustc 2 <host> -> std 2 <host>
+        [dist] rustc 1 <host> -> std 1 <host>
         [dist] rustc 1 <host> -> rustc-dev 2 <host>
         [dist] src <>
         [dist] reproducible-artifacts <host>
@@ -1198,8 +1197,7 @@ mod snapshot {
         [build] rustc 1 <host> -> rust-analyzer-proc-macro-srv 2 <host>
         [build] rustc 0 <host> -> GenerateCopyright 1 <host>
         [dist] rustc <host>
-        [build] rustc 2 <host> -> std 2 <host>
-        [dist] rustc 2 <host> -> std 2 <host>
+        [dist] rustc 1 <host> -> std 1 <host>
         [dist] rustc 1 <host> -> rustc-dev 2 <host>
         [dist] rustc 1 <host> -> analysis 2 <host>
         [dist] src <>
@@ -1216,7 +1214,6 @@ mod snapshot {
         [build] rustc 1 <host> -> miri 2 <host>
         [build] rustc 1 <host> -> cargo-miri 2 <host>
         [dist] rustc 1 <host> -> miri 2 <host>
-        [dist] rustc 1 <host> -> std 1 <host>
         [dist] rustc 1 <host> -> extended 2 <host>
         [dist] reproducible-artifacts <host>
         ");
@@ -1287,8 +1284,7 @@ mod snapshot {
         [dist] mingw <target1>
         [build] rustc 0 <host> -> GenerateCopyright 1 <host>
         [dist] rustc <host>
-        [build] rustc 2 <host> -> std 2 <host>
-        [dist] rustc 2 <host> -> std 2 <host>
+        [dist] rustc 1 <host> -> std 1 <host>
         [build] rustc 2 <host> -> std 2 <target1>
         [dist] rustc 2 <host> -> std 2 <target1>
         [dist] rustc 1 <host> -> rustc-dev 2 <host>
@@ -1350,8 +1346,7 @@ mod snapshot {
         [dist] rustc <host>
         [build] rustdoc 2 <target1>
         [dist] rustc <target1>
-        [build] rustc 2 <host> -> std 2 <host>
-        [dist] rustc 2 <host> -> std 2 <host>
+        [dist] rustc 1 <host> -> std 1 <host>
         [dist] rustc 1 <host> -> rustc-dev 2 <host>
         [dist] rustc 1 <host> -> rustc-dev 2 <target1>
         [dist] src <>
@@ -1433,10 +1428,8 @@ mod snapshot {
         [dist] rustc <host>
         [build] rustdoc 2 <target1>
         [dist] rustc <target1>
-        [build] rustc 2 <host> -> std 2 <host>
-        [dist] rustc 2 <host> -> std 2 <host>
-        [build] rustc 2 <host> -> std 2 <target1>
-        [dist] rustc 2 <host> -> std 2 <target1>
+        [dist] rustc 1 <host> -> std 1 <host>
+        [dist] rustc 1 <host> -> std 1 <target1>
         [dist] rustc 1 <host> -> rustc-dev 2 <host>
         [dist] rustc 1 <host> -> rustc-dev 2 <target1>
         [dist] src <>
@@ -1490,10 +1483,8 @@ mod snapshot {
         ");
     }
 
-    /// This also serves as an important regression test for <https://github.com/rust-lang/rust/issues/138123>
-    /// and <https://github.com/rust-lang/rust/issues/138004>.
     #[test]
-    fn dist_all_cross() {
+    fn dist_all_cross_extended() {
         let ctx = TestCtx::new();
         insta::assert_snapshot!(
             ctx
@@ -1545,8 +1536,7 @@ mod snapshot {
         [build] rustc 1 <host> -> rust-analyzer-proc-macro-srv 2 <target1>
         [build] rustc 0 <host> -> GenerateCopyright 1 <host>
         [dist] rustc <target1>
-        [build] rustc 2 <host> -> std 2 <target1>
-        [dist] rustc 2 <host> -> std 2 <target1>
+        [dist] rustc 1 <host> -> std 1 <target1>
         [dist] rustc 1 <host> -> rustc-dev 2 <target1>
         [dist] rustc 1 <host> -> analysis 2 <target1>
         [dist] src <>
@@ -1564,15 +1554,82 @@ mod snapshot {
         [build] rustc 1 <host> -> cargo-miri 2 <target1>
         [dist] rustc 1 <host> -> miri 2 <target1>
         [build] rustc 1 <host> -> LlvmBitcodeLinker 2 <target1>
-        [dist] rustc 1 <host> -> std 1 <target1>
         [doc] rustc 2 <target1> -> std 2 <target1> crates=[]
         [dist] rustc 1 <host> -> extended 2 <target1>
         [dist] reproducible-artifacts <target1>
         ");
     }
 
-    // Enable dist cranelift tarball by default with `x dist` if cranelift is enabled in
-    // `rust.codegen-backends`.
+    /// Simulates e.g. the powerpc64 builder, which is fully cross-compiled from x64, but it does
+    /// not build docs. Crutically, it shouldn't build host stage 2 rustc.
+    ///
+    /// This is a regression test for <https://github.com/rust-lang/rust/issues/138123>
+    /// and <https://github.com/rust-lang/rust/issues/138004>.
+    #[test]
+    fn dist_all_cross_extended_no_docs() {
+        let ctx = TestCtx::new();
+        let steps = ctx
+            .config("dist")
+            .hosts(&[TEST_TRIPLE_1])
+            .targets(&[TEST_TRIPLE_1])
+            .args(&[
+                "--set",
+                "rust.channel=nightly",
+                "--set",
+                "build.extended=true",
+                "--set",
+                "build.docs=false",
+            ])
+            .get_steps();
+
+        // Make sure that we don't build stage2 host rustc
+        steps.assert_no_match(|m| {
+            m.name == "rustc"
+                && m.built_by.map(|b| b.stage) == Some(1)
+                && *m.target.triple == host_target()
+        });
+
+        insta::assert_snapshot!(
+                steps.render(), @r"
+        [dist] mingw <target1>
+        [build] llvm <host>
+        [build] llvm <target1>
+        [build] rustc 0 <host> -> rustc 1 <host>
+        [build] rustc 0 <host> -> WasmComponentLd 1 <host>
+        [build] rustc 1 <host> -> std 1 <target1>
+        [build] rustc 1 <host> -> std 1 <host>
+        [build] rustc 1 <host> -> rustc 2 <target1>
+        [build] rustc 1 <host> -> WasmComponentLd 2 <target1>
+        [build] rustdoc 2 <target1>
+        [build] rustc 1 <host> -> rust-analyzer-proc-macro-srv 2 <target1>
+        [build] rustc 0 <host> -> GenerateCopyright 1 <host>
+        [build] rustc 0 <host> -> RustInstaller 1 <host>
+        [dist] rustc <target1>
+        [dist] rustc 1 <host> -> std 1 <target1>
+        [dist] rustc 1 <host> -> rustc-dev 2 <target1>
+        [dist] rustc 1 <host> -> analysis 2 <target1>
+        [dist] src <>
+        [build] rustc 1 <host> -> cargo 2 <target1>
+        [dist] rustc 1 <host> -> cargo 2 <target1>
+        [build] rustc 1 <host> -> rust-analyzer 2 <target1>
+        [dist] rustc 1 <host> -> rust-analyzer 2 <target1>
+        [build] rustc 1 <host> -> rustfmt 2 <target1>
+        [build] rustc 1 <host> -> cargo-fmt 2 <target1>
+        [dist] rustc 1 <host> -> rustfmt 2 <target1>
+        [build] rustc 1 <host> -> clippy-driver 2 <target1>
+        [build] rustc 1 <host> -> cargo-clippy 2 <target1>
+        [dist] rustc 1 <host> -> clippy 2 <target1>
+        [build] rustc 1 <host> -> miri 2 <target1>
+        [build] rustc 1 <host> -> cargo-miri 2 <target1>
+        [dist] rustc 1 <host> -> miri 2 <target1>
+        [build] rustc 1 <host> -> LlvmBitcodeLinker 2 <target1>
+        [dist] rustc 1 <host> -> extended 2 <target1>
+        [dist] reproducible-artifacts <target1>
+        ");
+    }
+
+    /// Enable dist cranelift tarball by default with `x dist` if cranelift is enabled in
+    /// `rust.codegen-backends`.
     #[test]
     fn dist_cranelift_by_default() {
         let ctx = TestCtx::new();
@@ -1619,8 +1676,7 @@ mod snapshot {
         [build] rustc 0 <host> -> GenerateCopyright 1 <host>
         [dist] rustc <host>
         [dist] rustc 1 <host> -> rustc_codegen_cranelift 2 <host>
-        [build] rustc 2 <host> -> std 2 <host>
-        [dist] rustc 2 <host> -> std 2 <host>
+        [dist] rustc 1 <host> -> std 1 <host>
         [dist] rustc 1 <host> -> rustc-dev 2 <host>
         [dist] src <>
         [dist] reproducible-artifacts <host>
@@ -2351,8 +2407,7 @@ mod snapshot {
         [doc] rustc 1 <host> -> releases 2 <host>
         [build] rustc 0 <host> -> RustInstaller 1 <host>
         [dist] docs <host>
-        [build] rustc 2 <host> -> std 2 <host>
-        [dist] rustc 2 <host> -> std 2 <host>
+        [dist] rustc 1 <host> -> std 1 <host>
         [build] rustc 1 <host> -> rust-analyzer-proc-macro-srv 2 <host>
         [build] rustc 0 <host> -> GenerateCopyright 1 <host>
         [dist] rustc <host>
@@ -2426,6 +2481,21 @@ impl ExecutedSteps {
         }
     }
 
+    /// Make sure that no metadata matches the given `func`.
+    #[track_caller]
+    fn assert_no_match<F>(&self, func: F)
+    where
+        F: Fn(StepMetadata) -> bool,
+    {
+        for metadata in self.steps.iter().filter_map(|s| s.metadata.clone()) {
+            if func(metadata.clone()) {
+                panic!(
+                    "Metadata {metadata:?} was found, even though it should have not been present"
+                );
+            }
+        }
+    }
+
     fn contains(&self, metadata: &StepMetadata) -> bool {
         self.steps
             .iter()