Review remarks

Author: Kobzol

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

@@ -1541,6 +1541,8 @@ impl Step for RustcLink {
     }
 }
 
+pub const RUSTC_CODEGEN_GCC: &str = "rustc_codegen_gcc";
+
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct GccCodegenBackend {
     compilers: RustcPrivateCompilers,
@@ -1551,7 +1553,7 @@ impl Step for GccCodegenBackend {
     const ONLY_HOSTS: bool = true;
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
-        run.alias("rustc_codegen_gcc").alias("cg_gcc")
+        run.alias(RUSTC_CODEGEN_GCC).alias("cg_gcc")
     }
 
     fn make_run(run: RunConfig<'_>) {
@@ -1610,7 +1612,7 @@ impl Step for GccCodegenBackend {
         let _guard = builder.msg_rustc_tool(
             Kind::Build,
             build_compiler.stage,
-            format_args!("codegen backend gcc"),
+            "codegen backend gcc",
             build_compiler.host,
             target,
         );
@@ -1620,12 +1622,14 @@ impl Step for GccCodegenBackend {
 
     fn metadata(&self) -> Option<StepMetadata> {
         Some(
-            StepMetadata::build("rustc_codegen_gcc", self.compilers.target())
+            StepMetadata::build(RUSTC_CODEGEN_GCC, self.compilers.target())
                 .built_by(self.compilers.build_compiler()),
         )
     }
 }
 
+pub const RUSTC_CODEGEN_CRANELIFT: &str = "rustc_codegen_cranelift";
+
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct CraneliftCodegenBackend {
     pub compilers: RustcPrivateCompilers,
@@ -1636,7 +1640,7 @@ impl Step for CraneliftCodegenBackend {
     const ONLY_HOSTS: bool = true;
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
-        run.alias("rustc_codegen_cranelift").alias("cg_clif")
+        run.alias(RUSTC_CODEGEN_CRANELIFT).alias("cg_clif")
     }
 
     fn make_run(run: RunConfig<'_>) {
@@ -1688,13 +1692,13 @@ impl Step for CraneliftCodegenBackend {
         );
         cargo
             .arg("--manifest-path")
-            .arg(builder.src.join("compiler/rustc_codegen_cranelift/Cargo.toml"));
+            .arg(builder.src.join(format!("compiler/{RUSTC_CODEGEN_CRANELIFT}/Cargo.toml")));
         rustc_cargo_env(builder, &mut cargo, target);
 
         let _guard = builder.msg_rustc_tool(
             Kind::Build,
             build_compiler.stage,
-            format_args!("codegen backend cranelift"),
+            "codegen backend cranelift",
             build_compiler.host,
             target,
         );
@@ -1704,7 +1708,7 @@ impl Step for CraneliftCodegenBackend {
 
     fn metadata(&self) -> Option<StepMetadata> {
         Some(
-            StepMetadata::build("rustc_codegen_cranelift", self.compilers.target())
+            StepMetadata::build(RUSTC_CODEGEN_CRANELIFT, self.compilers.target())
                 .built_by(self.compilers.build_compiler()),
         )
     }
@@ -1716,32 +1720,28 @@ fn write_codegen_backend_stamp(
     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_")
-        });
-        let codegen_backend = match files.next() {
-            Some(f) => f,
-            None => panic!("no dylibs built for codegen backend?"),
-        };
-        if let Some(f) = files.next() {
-            panic!(
-                "codegen backend built two dylibs:\n{}\n{}",
-                codegen_backend.display(),
-                f.display()
-            );
-        }
+    if dry_run {
+        return stamp;
+    }
 
-        let codegen_backend = codegen_backend.to_str().unwrap();
-        stamp = stamp.add_stamp(codegen_backend);
-        t!(stamp.write());
+    let mut files = files.into_iter().filter(|f| {
+        let filename = f.file_name().unwrap().to_str().unwrap();
+        is_dylib(f) && filename.contains("rustc_codegen_")
+    });
+    let codegen_backend = match files.next() {
+        Some(f) => f,
+        None => panic!("no dylibs built for codegen backend?"),
+    };
+    if let Some(f) = files.next() {
+        panic!("codegen backend built two dylibs:\n{}\n{}", codegen_backend.display(), f.display());
     }
+
+    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.
 ///

src/bootstrap/src/core/build_steps/dist.rs

@@ -19,6 +19,7 @@ use object::read::archive::ArchiveFile;
 #[cfg(feature = "tracing")]
 use tracing::instrument;
 
+use crate::core::build_steps::compile::RUSTC_CODEGEN_CRANELIFT;
 use crate::core::build_steps::doc::DocumentationFormat;
 use crate::core::build_steps::tool::{self, RustcPrivateCompilers, Tool};
 use crate::core::build_steps::vendor::{VENDOR_DIR, Vendor};
@@ -1379,14 +1380,14 @@ impl Step for CraneliftCodegenBackend {
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
         // We only want to build the cranelift backend in `x dist` if the backend was enabled
         // in rust.codegen-backends.
-        // Sadly, we don't have access to the actual for which we're disting clif here..
+        // Sadly, we don't have access to the actual target for which we're disting clif here..
         // So we just use the host target.
         let clif_enabled_by_default = run
             .builder
             .config
             .enabled_codegen_backends(run.builder.host_target)
             .contains(&CodegenBackendKind::Cranelift);
-        run.alias("rustc_codegen_cranelift").default_condition(clif_enabled_by_default)
+        run.alias(RUSTC_CODEGEN_CRANELIFT).default_condition(clif_enabled_by_default)
     }
 
     fn make_run(run: RunConfig<'_>) {
@@ -1439,7 +1440,7 @@ impl Step for CraneliftCodegenBackend {
         let mut found_backend = false;
         for backend in fs::read_dir(&backends_src).unwrap() {
             let file_name = backend.unwrap().file_name();
-            if file_name.to_str().unwrap().contains("rustc_codegen_cranelift") {
+            if file_name.to_str().unwrap().contains(RUSTC_CODEGEN_CRANELIFT) {
                 tarball.add_file(
                     backends_src.join(file_name),
                     &backends_dst,
@@ -1455,7 +1456,7 @@ impl Step for CraneliftCodegenBackend {
 
     fn metadata(&self) -> Option<StepMetadata> {
         Some(
-            StepMetadata::dist("rustc_codegen_cranelift", self.build_compiler.host)
+            StepMetadata::dist(RUSTC_CODEGEN_CRANELIFT, self.build_compiler.host)
                 .built_by(self.build_compiler),
         )
     }

src/bootstrap/src/core/config/toml/rust.rs

@@ -5,7 +5,6 @@ use std::str::FromStr;
 
 use serde::{Deserialize, Deserializer};
 
-use crate::core::build_steps::compile::CODEGEN_BACKEND_PREFIX;
 use crate::core::config::toml::TomlConfig;
 use crate::core::config::{
     DebuginfoLevel, Merge, ReplaceOpt, RustcLto, StringOrBool, set, threads_from_config,
@@ -397,6 +396,8 @@ pub(crate) fn parse_codegen_backends(
     backends: Vec<String>,
     section: &str,
 ) -> Vec<CodegenBackendKind> {
+    const CODEGEN_BACKEND_PREFIX: &str = "rustc_codegen_";
+
     let mut found_backends = vec![];
     for backend in &backends {
         if let Some(stripped) = backend.strip_prefix(CODEGEN_BACKEND_PREFIX) {