Split CodegenBackend step into two and handle stamp sysroot copy explicitly

2 files changed, 181 insertions, 148 deletions

diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs
index e1ee0773107..dc350c5c34a 100644
--- a/src/bootstrap/src/core/build_steps/compile.rs
+++ b/src/bootstrap/src/core/build_steps/compile.rs
@@ -23,8 +23,7 @@ use crate::core::build_steps::tool::{RustcPrivateCompilers, SourceType, copy_lld
 use crate::core::build_steps::{dist, llvm};
 use crate::core::builder;
 use crate::core::builder::{
-    Builder, Cargo, Kind, PathSet, RunConfig, ShouldRun, Step, StepMetadata, TaskPath,
-    crate_description,
+    Builder, Cargo, Kind, RunConfig, ShouldRun, Step, StepMetadata, crate_description,
 };
 use crate::core::config::{DebuginfoLevel, LlvmLibunwind, RustcLto, TargetSelection};
 use crate::utils::build_stamp;
@@ -1539,99 +1538,131 @@ impl Step for RustcLink {
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
-pub struct CodegenBackend {
+pub struct GccCodegenBackend {
     compilers: RustcPrivateCompilers,
-    backend: CodegenBackendKind,
 }
 
-fn needs_codegen_config(run: &RunConfig<'_>) -> bool {
-    let mut needs_codegen_cfg = false;
-    for path_set in &run.paths {
-        needs_codegen_cfg = match path_set {
-            PathSet::Set(set) => set.iter().any(|p| is_codegen_cfg_needed(p, run)),
-            PathSet::Suite(suite) => is_codegen_cfg_needed(suite, run),
-        }
+impl Step for GccCodegenBackend {
+    type Output = BuildStamp;
+    const ONLY_HOSTS: bool = true;
+
+    fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
+        run.alias("rustc_codegen_gcc").alias("cg_gcc")
     }
-    needs_codegen_cfg
-}
 
-pub(crate) const CODEGEN_BACKEND_PREFIX: &str = "rustc_codegen_";
+    fn make_run(run: RunConfig<'_>) {
+        run.builder.ensure(GccCodegenBackend {
+            compilers: RustcPrivateCompilers::new(run.builder, run.builder.top_stage, run.target),
+        });
+    }
 
-fn is_codegen_cfg_needed(path: &TaskPath, run: &RunConfig<'_>) -> bool {
-    let path = path.path.to_str().unwrap();
+    #[cfg_attr(
+        feature = "tracing",
+        instrument(
+            level = "debug",
+            name = "GccCodegenBackend::run",
+            skip_all,
+            fields(
+                compilers = ?self.compilers,
+            ),
+        ),
+    )]
+    fn run(self, builder: &Builder<'_>) -> Self::Output {
+        let target = self.compilers.target();
+        let build_compiler = self.compilers.build_compiler();
 
-    let is_explicitly_called = |p| -> bool { run.builder.paths.contains(p) };
-    let should_enforce = run.builder.kind == Kind::Dist || run.builder.kind == Kind::Install;
+        let stamp = build_stamp::codegen_backend_stamp(
+            builder,
+            build_compiler,
+            target,
+            &CodegenBackendKind::Gcc,
+        );
 
-    if path.contains(CODEGEN_BACKEND_PREFIX) {
-        let mut needs_codegen_backend_config = true;
-        for backend in run.builder.config.codegen_backends(run.target) {
-            if path.ends_with(&(CODEGEN_BACKEND_PREFIX.to_owned() + backend.name())) {
-                needs_codegen_backend_config = false;
-            }
-        }
-        if (is_explicitly_called(&PathBuf::from(path)) || should_enforce)
-            && needs_codegen_backend_config
-        {
-            run.builder.info(
-                "WARNING: no codegen-backends config matched the requested path to build a codegen backend. \
-                HELP: add backend to codegen-backends in bootstrap.toml.",
+        if builder.config.keep_stage.contains(&build_compiler.stage) {
+            trace!("`keep-stage` requested");
+            builder.info(
+                "WARNING: Using a potentially old codegen backend. \
+                This may not behave well.",
             );
-            return true;
+            // Codegen backends are linked separately from this step today, so we don't do
+            // anything here.
+            return stamp;
         }
+
+        let mut cargo = builder::Cargo::new(
+            builder,
+            build_compiler,
+            Mode::Codegen,
+            SourceType::InTree,
+            target,
+            Kind::Build,
+        );
+        cargo.arg("--manifest-path").arg(builder.src.join("compiler/rustc_codegen_gcc/Cargo.toml"));
+        rustc_cargo_env(builder, &mut cargo, target);
+
+        let gcc = builder.ensure(Gcc { target });
+        add_cg_gcc_cargo_flags(&mut cargo, &gcc);
+
+        let _guard = builder.msg_rustc_tool(
+            Kind::Build,
+            build_compiler.stage,
+            format_args!("codegen backend gcc"),
+            build_compiler.host,
+            target,
+        );
+        let files = run_cargo(builder, cargo, vec![], &stamp, vec![], false, false);
+        write_codegen_backend_stamp(stamp, files, builder.config.dry_run())
     }
 
-    false
+    fn metadata(&self) -> Option<StepMetadata> {
+        Some(
+            StepMetadata::build("rustc_codegen_gcc", self.compilers.target())
+                .built_by(self.compilers.build_compiler()),
+        )
+    }
 }
 
-impl Step for CodegenBackend {
-    type Output = ();
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
+pub struct CraneliftCodegenBackend {
+    compilers: RustcPrivateCompilers,
+}
+
+impl Step for CraneliftCodegenBackend {
+    type Output = BuildStamp;
     const ONLY_HOSTS: bool = true;
-    /// Only the backends specified in the `codegen-backends` entry of `bootstrap.toml` are built.
-    const DEFAULT: bool = true;
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
-        run.paths(&["compiler/rustc_codegen_cranelift", "compiler/rustc_codegen_gcc"])
+        run.alias("rustc_codegen_cranelift").alias("cg_clif")
     }
 
     fn make_run(run: RunConfig<'_>) {
-        if needs_codegen_config(&run) {
-            return;
-        }
-
-        for backend in run.builder.config.codegen_backends(run.target) {
-            if backend.is_llvm() {
-                continue; // Already built as part of rustc
-            }
-
-            run.builder.ensure(CodegenBackend {
-                compilers: RustcPrivateCompilers::new(
-                    run.builder,
-                    run.builder.top_stage,
-                    run.target,
-                ),
-                backend: backend.clone(),
-            });
-        }
+        run.builder.ensure(CraneliftCodegenBackend {
+            compilers: RustcPrivateCompilers::new(run.builder, run.builder.top_stage, run.target),
+        });
     }
 
     #[cfg_attr(
         feature = "tracing",
         instrument(
             level = "debug",
-            name = "CodegenBackend::run",
+            name = "CraneliftCodegenBackend::run",
             skip_all,
             fields(
                 compilers = ?self.compilers,
-                backend = ?self.backend,
             ),
         ),
     )]
-    fn run(self, builder: &Builder<'_>) {
-        let backend = self.backend;
+    fn run(self, builder: &Builder<'_>) -> Self::Output {
         let target = self.compilers.target();
         let build_compiler = self.compilers.build_compiler();
 
+        let stamp = build_stamp::codegen_backend_stamp(
+            builder,
+            build_compiler,
+            target,
+            &CodegenBackendKind::Cranelift,
+        );
+
         if builder.config.keep_stage.contains(&build_compiler.stage) {
             trace!("`keep-stage` requested");
             builder.info(
@@ -1640,11 +1671,9 @@ impl Step for CodegenBackend {
             );
             // Codegen backends are linked separately from this step today, so we don't do
             // anything here.
-            return;
+            return stamp;
         }
 
-        let out_dir = builder.cargo_out(build_compiler, Mode::Codegen, target);
-
         let mut cargo = builder::Cargo::new(
             builder,
             build_compiler,
@@ -1655,30 +1684,35 @@ impl Step for CodegenBackend {
         );
         cargo
             .arg("--manifest-path")
-            .arg(builder.src.join(format!("compiler/{}/Cargo.toml", backend.crate_name())));
+            .arg(builder.src.join("compiler/rustc_codegen_cranelift/Cargo.toml"));
         rustc_cargo_env(builder, &mut cargo, target);
 
-        // Ideally, we'd have a separate step for the individual codegen backends,
-        // like we have in tests (test::CodegenGCC) but that would require a lot of restructuring.
-        // If the logic gets more complicated, it should probably be done.
-        if backend.is_gcc() {
-            let gcc = builder.ensure(Gcc { target });
-            add_cg_gcc_cargo_flags(&mut cargo, &gcc);
-        }
-
-        let tmp_stamp = BuildStamp::new(&out_dir).with_prefix("tmp");
-
         let _guard = builder.msg_rustc_tool(
             Kind::Build,
             build_compiler.stage,
-            format_args!("codegen backend {}", backend.name()),
+            format_args!("codegen backend cranelift"),
             build_compiler.host,
             target,
         );
-        let files = run_cargo(builder, cargo, vec![], &tmp_stamp, vec![], false, false);
-        if builder.config.dry_run() {
-            return;
-        }
+        let files = run_cargo(builder, cargo, vec![], &stamp, vec![], false, false);
+        write_codegen_backend_stamp(stamp, files, builder.config.dry_run())
+    }
+
+    fn metadata(&self) -> Option<StepMetadata> {
+        Some(
+            StepMetadata::build("rustc_codegen_cranelift", self.compilers.target())
+                .built_by(self.compilers.build_compiler()),
+        )
+    }
+}
+
+/// Write filtered `files` into the passed build stamp and returns it.
+fn write_codegen_backend_stamp(
+    mut stamp: BuildStamp,
+    files: Vec<PathBuf>,
+    dry_run: bool,
+) -> BuildStamp {
+    if !dry_run {
         let mut files = files.into_iter().filter(|f| {
             let filename = f.file_name().unwrap().to_str().unwrap();
             is_dylib(f) && filename.contains("rustc_codegen_")
@@ -1694,32 +1728,27 @@ impl Step for CodegenBackend {
                 f.display()
             );
         }
-        let stamp = build_stamp::codegen_backend_stamp(builder, build_compiler, target, &backend);
-        let codegen_backend = codegen_backend.to_str().unwrap();
-        t!(stamp.add_stamp(codegen_backend).write());
-    }
 
-    fn metadata(&self) -> Option<StepMetadata> {
-        Some(
-            StepMetadata::build(&self.backend.crate_name(), self.compilers.target())
-                .built_by(self.compilers.build_compiler()),
-        )
+        let codegen_backend = codegen_backend.to_str().unwrap();
+        stamp = stamp.add_stamp(codegen_backend);
+        t!(stamp.write());
     }
+    stamp
 }
 
+pub(crate) const CODEGEN_BACKEND_PREFIX: &str = "rustc_codegen_";
+
 /// Creates the `codegen-backends` folder for a compiler that's about to be
 /// assembled as a complete compiler.
 ///
-/// This will take the codegen artifacts produced by `compiler` and link them
+/// This will take the codegen artifacts recorded in the given `stamp` and link them
 /// into an appropriate location for `target_compiler` to be a functional
 /// compiler.
 fn copy_codegen_backends_to_sysroot(
     builder: &Builder<'_>,
-    compiler: Compiler,
+    stamp: BuildStamp,
     target_compiler: Compiler,
 ) {
-    let target = target_compiler.host;
-
     // Note that this step is different than all the other `*Link` steps in
     // that it's not assembling a bunch of libraries but rather is primarily
     // moving the codegen backend into place. The codegen backend of rustc is
@@ -1735,25 +1764,18 @@ fn copy_codegen_backends_to_sysroot(
         return;
     }
 
-    for backend in builder.config.codegen_backends(target) {
-        if backend.is_llvm() {
-            continue; // Already built as part of rustc
-        }
-
-        let stamp = build_stamp::codegen_backend_stamp(builder, compiler, target, backend);
-        if stamp.path().exists() {
-            let dylib = t!(fs::read_to_string(stamp.path()));
-            let file = Path::new(&dylib);
-            let filename = file.file_name().unwrap().to_str().unwrap();
-            // change `librustc_codegen_cranelift-xxxxxx.so` to
-            // `librustc_codegen_cranelift-release.so`
-            let target_filename = {
-                let dash = filename.find('-').unwrap();
-                let dot = filename.find('.').unwrap();
-                format!("{}-{}{}", &filename[..dash], builder.rust_release(), &filename[dot..])
-            };
-            builder.copy_link(file, &dst.join(target_filename), FileType::NativeLibrary);
-        }
+    if stamp.path().exists() {
+        let dylib = t!(fs::read_to_string(stamp.path()));
+        let file = Path::new(&dylib);
+        let filename = file.file_name().unwrap().to_str().unwrap();
+        // change `librustc_codegen_cranelift-xxxxxx.so` to
+        // `librustc_codegen_cranelift-release.so`
+        let target_filename = {
+            let dash = filename.find('-').unwrap();
+            let dot = filename.find('.').unwrap();
+            format!("{}-{}{}", &filename[..dash], builder.rust_release(), &filename[dot..])
+        };
+        builder.copy_link(file, &dst.join(target_filename), FileType::NativeLibrary);
     }
 }
 
@@ -2162,44 +2184,52 @@ impl Step for Assemble {
         );
         build_compiler.stage = actual_stage;
 
-        #[cfg(feature = "tracing")]
-        let _codegen_backend_span =
-            span!(tracing::Level::DEBUG, "building requested codegen backends").entered();
-        for backend in builder.config.codegen_backends(target_compiler.host) {
-            if backend.is_llvm() {
-                debug!("llvm codegen backend is already built as part of rustc");
-                continue; // Already built as part of rustc
-            }
+        let mut codegen_backend_stamps = vec![];
+        {
+            #[cfg(feature = "tracing")]
+            let _codegen_backend_span =
+                span!(tracing::Level::DEBUG, "building requested codegen backends").entered();
+
+            for backend in builder.config.codegen_backends(target_compiler.host) {
+                // FIXME: this is a horrible hack used to make `x check` work when other codegen
+                // backends are enabled.
+                // `x check` will check stage 1 rustc, which copies its rmetas to the stage0 sysroot.
+                // Then it checks codegen backends, which correctly use these rmetas.
+                // Then it needs to check std, but for that it needs to build stage 1 rustc.
+                // This copies the build rmetas into the stage0 sysroot, effectively poisoning it,
+                // because we then have both check and build rmetas in the same sysroot.
+                // That would be fine on its own. However, when another codegen backend is enabled,
+                // then building stage 1 rustc implies also building stage 1 codegen backend (even if
+                // it isn't used for anything). And since that tries to use the poisoned
+                // rmetas, it fails to build.
+                // We don't actually need to build rustc-private codegen backends for checking std,
+                // so instead we skip that.
+                // Note: this would be also an issue for other rustc-private tools, but that is "solved"
+                // by check::Std being last in the list of checked things (see
+                // `Builder::get_step_descriptions`).
+                if builder.kind == Kind::Check && builder.top_stage == 1 {
+                    continue;
+                }
 
-            // FIXME: this is a horrible hack used to make `x check` work when other codegen
-            // backends are enabled.
-            // `x check` will check stage 1 rustc, which copies its rmetas to the stage0 sysroot.
-            // Then it checks codegen backends, which correctly use these rmetas.
-            // Then it needs to check std, but for that it needs to build stage 1 rustc.
-            // This copies the build rmetas into the stage0 sysroot, effectively poisoning it,
-            // because we then have both check and build rmetas in the same sysroot.
-            // That would be fine on its own. However, when another codegen backend is enabled,
-            // then building stage 1 rustc implies also building stage 1 codegen backend (even if
-            // it isn't used for anything). And since that tries to use the poisoned
-            // rmetas, it fails to build.
-            // We don't actually need to build rustc-private codegen backends for checking std,
-            // so instead we skip that.
-            // Note: this would be also an issue for other rustc-private tools, but that is "solved"
-            // by check::Std being last in the list of checked things (see
-            // `Builder::get_step_descriptions`).
-            if builder.kind == Kind::Check && builder.top_stage == 1 {
-                continue;
+                let prepare_compilers = || {
+                    RustcPrivateCompilers::from_build_and_target_compiler(
+                        build_compiler,
+                        target_compiler,
+                    )
+                };
+
+                let stamp = match backend {
+                    CodegenBackendKind::Cranelift => {
+                        builder.ensure(CraneliftCodegenBackend { compilers: prepare_compilers() })
+                    }
+                    CodegenBackendKind::Gcc => {
+                        builder.ensure(GccCodegenBackend { compilers: prepare_compilers() })
+                    }
+                    CodegenBackendKind::Llvm | CodegenBackendKind::Custom(_) => continue,
+                };
+                codegen_backend_stamps.push(stamp);
             }
-            builder.ensure(CodegenBackend {
-                compilers: RustcPrivateCompilers::from_build_and_target_compiler(
-                    build_compiler,
-                    target_compiler,
-                ),
-                backend: backend.clone(),
-            });
         }
-        #[cfg(feature = "tracing")]
-        drop(_codegen_backend_span);
 
         let stage = target_compiler.stage;
         let host = target_compiler.host;
@@ -2260,7 +2290,9 @@ impl Step for Assemble {
         }
 
         debug!("copying codegen backends to sysroot");
-        copy_codegen_backends_to_sysroot(builder, build_compiler, target_compiler);
+        for stamp in codegen_backend_stamps {
+            copy_codegen_backends_to_sysroot(builder, stamp, target_compiler);
+        }
 
         if builder.config.lld_enabled {
             let lld_wrapper =
diff --git a/src/bootstrap/src/core/builder/mod.rs b/src/bootstrap/src/core/builder/mod.rs
index e15941938f1..d16ffbe34ce 100644
--- a/src/bootstrap/src/core/builder/mod.rs
+++ b/src/bootstrap/src/core/builder/mod.rs
@@ -959,7 +959,8 @@ impl<'a> Builder<'a> {
                 compile::Std,
                 compile::Rustc,
                 compile::Assemble,
-                compile::CodegenBackend,
+                compile::CraneliftCodegenBackend,
+                compile::GccCodegenBackend,
                 compile::StartupObjects,
                 tool::BuildManifest,
                 tool::Rustbook,