bootstrap: Prefer cfg!(not(test)) when skipping code paths during unit tests

src/bootstrap/src/core/build_steps/doc.rs

@@ -1399,8 +1399,8 @@ impl Step for RustcBook {
     /// "rustbook" is used to convert it to HTML.
     fn run(self, builder: &Builder<'_>) {
         // FIXME: Temporary workaround for https://github.com/rust-lang/rust/issues/158378
-        #[cfg(not(test))] // So this check doesn't affect the bootstrap tests
-        if self.target == "i686-pc-windows-msvc" {
+        // Make sure this workaround doesn't break unit tests on the affected host.
+        if cfg!(not(test)) && self.target == "i686-pc-windows-msvc" {
             eprintln!("WARNING: Skipping rustc book build to work around #158378");
             return;
         }

src/bootstrap/src/core/build_steps/gcc.rs

@@ -13,6 +13,8 @@ use std::fs;
 use std::path::{Path, PathBuf};
 use std::sync::OnceLock;
 
+use build_helper::git::PathFreshness;
+
 use crate::core::builder::{Builder, Cargo, Kind, RunConfig, ShouldRun, Step};
 use crate::core::config::TargetSelection;
 use crate::utils::build_stamp::{BuildStamp, generate_smart_stamp_hash};
@@ -135,9 +137,11 @@ pub enum GccBuildStatus {
 /// Tries to download GCC from CI if it is enabled and GCC artifacts
 /// are available for the given target.
 /// Returns a path to the libgccjit.so file.
-#[cfg(not(test))]
 fn try_download_gcc(builder: &Builder<'_>, target_pair: GccTargetPair) -> Option<PathBuf> {
-    use build_helper::git::PathFreshness;
+    // Don't actually download GCC during unit tests.
+    if cfg!(test) {
+        return None;
+    }
 
     // Try to download GCC from CI if configured and available
     if !matches!(builder.config.gcc_ci_mode, crate::core::config::GccCiMode::DownloadFromCi) {
@@ -194,11 +198,6 @@ fn try_download_gcc(builder: &Builder<'_>, target_pair: GccTargetPair) -> Option
     }
 }
 
-#[cfg(test)]
-fn try_download_gcc(_builder: &Builder<'_>, _target_pair: GccTargetPair) -> Option<PathBuf> {
-    None
-}
-
 /// This returns information about whether GCC should be built or if it's already built.
 /// It transparently handles downloading GCC from CI if needed.
 ///
@@ -367,15 +366,13 @@ pub fn add_cg_gcc_cargo_flags(cargo: &mut Cargo, gcc: &GccOutput) {
 }
 
 /// The absolute path to the downloaded GCC artifacts.
-#[cfg(not(test))]
 fn ci_gcc_root(config: &crate::Config, target: TargetSelection) -> PathBuf {
     config.out.join(target).join("ci-gcc")
 }
 
 /// Detect whether GCC sources have been modified locally or not.
-#[cfg(not(test))]
 fn detect_gcc_freshness(config: &crate::Config, is_git: bool) -> build_helper::git::PathFreshness {
-    use build_helper::git::PathFreshness;
+    assert!(cfg!(not(test)), "unit tests shouldn't care about GCC freshness");
 
     if is_git {
         config.check_path_modifications(&["src/gcc", "src/bootstrap/download-ci-gcc-stamp"])

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

@@ -200,8 +200,9 @@ pub const LLVM_INVALIDATION_PATHS: &[&str] = &[
 ];
 
 /// Detect whether LLVM sources have been modified locally or not.
-#[cfg_attr(test, expect(dead_code))]
 pub(crate) fn detect_llvm_freshness(config: &Config, is_git: bool) -> PathFreshness {
+    assert!(cfg!(not(test)), "unit tests shouldn't care about LLVM freshness");
+
     if is_git {
         config.check_path_modifications(LLVM_INVALIDATION_PATHS)
     } else if let Some(info) = crate::utils::channel::read_commit_info_file(&config.src) {

src/bootstrap/src/core/build_steps/tool.rs

@@ -1235,7 +1235,6 @@ impl Step for LlvmBitcodeLinker {
 }
 
 #[derive(Debug, Clone, Hash, PartialEq, Eq)]
-#[cfg_attr(test, expect(dead_code))]
 pub struct LibcxxVersionTool {
     pub target: TargetSelection,
 }

src/bootstrap/src/core/config/config.rs

@@ -57,7 +57,6 @@ use crate::core::download::{
 };
 use crate::utils::channel;
 use crate::utils::exec::{ExecutionContext, command};
-#[cfg_attr(test, expect(unused_imports))]
 use crate::utils::helpers::{exe, fail, get_host_target};
 use crate::{
     CodegenBackendKind, GitInfo, OnceLock, TargetSelection, check_ci_llvm, exit, helpers, t,
@@ -1274,8 +1273,7 @@ impl Config {
         }
 
         // CI should always run stage 2 builds, unless it specifically states otherwise
-        #[cfg(not(test))]
-        if flags_stage.is_none() && ci_env.is_running_in_ci() {
+        if cfg!(not(test)) && flags_stage.is_none() && ci_env.is_running_in_ci() {
             match flags_cmd {
                 Subcommand::Test { .. }
                 | Subcommand::Miri { .. }
@@ -2305,24 +2303,14 @@ fn postprocess_toml(
     toml.merge(None, &mut Default::default(), override_toml, ReplaceOpt::Override);
 }
 
-#[cfg(test)]
-pub fn check_stage0_version(
-    _program_path: &Path,
-    _component_name: &'static str,
-    _src_dir: &Path,
-    _exec_ctx: &ExecutionContext,
-) {
-}
-
 /// check rustc/cargo version is same or lower with 1 apart from the building one
-#[cfg(not(test))]
 pub fn check_stage0_version(
     program_path: &Path,
     component_name: &'static str,
     src_dir: &Path,
     exec_ctx: &ExecutionContext,
 ) {
-    if exec_ctx.dry_run() {
+    if cfg!(test) || exec_ctx.dry_run() {
         return;
     }
 
@@ -2515,8 +2503,9 @@ pub fn parse_download_ci_llvm<'a>(
         }
 
         // Fetching the LLVM submodule is unnecessary for self-tests.
-        #[cfg(not(test))]
-        update_submodule(dwn_ctx, rust_info, "src/llvm-project");
+        if cfg!(not(test)) {
+            update_submodule(dwn_ctx, rust_info, "src/llvm-project");
+        }
 
         // Check for untracked changes in `src/llvm-project` and other important places.
         let has_changes = has_changes_from_upstream(dwn_ctx, LLVM_INVALIDATION_PATHS);
@@ -2537,8 +2526,11 @@ pub fn parse_download_ci_llvm<'a>(
                 );
             }
 
-            #[cfg(not(test))]
-            if b && dwn_ctx.is_running_on_ci() && CiEnv::is_rust_lang_managed_ci_job() {
+            if cfg!(not(test))
+                && b
+                && dwn_ctx.is_running_on_ci()
+                && CiEnv::is_rust_lang_managed_ci_job()
+            {
                 // On rust-lang CI, we must always rebuild LLVM if there were any modifications to it
                 panic!(
                     "`llvm.download-ci-llvm` cannot be set to `true` on CI. Use `if-unchanged` instead."

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

@@ -4,7 +4,6 @@
 use serde::{Deserialize, Deserializer};
 
 use crate::core::config::StringOrBool;
-#[cfg_attr(test, expect(unused_imports))]
 use crate::core::config::toml::{Merge, ReplaceOpt, TomlConfig};
 use crate::{HashMap, HashSet, PathBuf, define_config, exit};
 
@@ -46,7 +45,6 @@ define_config! {
 
 /// Compares the current `Llvm` options against those in the CI LLVM builder and detects any incompatible options.
 /// It does this by destructuring the `Llvm` instance to make sure every `Llvm` field is covered and not missing.
-#[cfg(not(test))]
 pub fn check_incompatible_options_for_ci_llvm(
     current_config_toml: TomlConfig,
     ci_config_toml: TomlConfig,

src/bootstrap/src/core/download.rs

@@ -8,13 +8,16 @@ use std::sync::{Arc, Mutex, OnceLock};
 
 use build_helper::ci::CiEnv;
 use build_helper::git::PathFreshness;
+use build_helper::stage0_parser::VersionMetadata;
 use xz2::bufread::XzDecoder;
 
+use crate::core::build_steps::llvm::detect_llvm_freshness;
+use crate::core::config::toml::llvm::check_incompatible_options_for_ci_llvm;
 use crate::core::config::{BUILDER_CONFIG_FILENAME, TargetSelection};
 use crate::utils::build_stamp::BuildStamp;
 use crate::utils::exec::{ExecutionContext, command};
 use crate::utils::helpers::{exe, hex_encode, move_file};
-use crate::{Config, t};
+use crate::{Config, exit, t};
 
 static SHOULD_FIX_BINS_AND_DYLIBS: OnceLock<bool> = OnceLock::new();
 
@@ -243,16 +246,11 @@ impl Config {
         download_component(dwn_ctx, &self.out, mode, filename, prefix, key, destination);
     }
 
-    #[cfg(test)]
-    pub(crate) fn maybe_download_ci_llvm(&self) {}
-
-    #[cfg(not(test))]
     pub(crate) fn maybe_download_ci_llvm(&self) {
-        use build_helper::git::PathFreshness;
-
-        use crate::core::build_steps::llvm::detect_llvm_freshness;
-        use crate::core::config::toml::llvm::check_incompatible_options_for_ci_llvm;
-        use crate::exit;
+        // Never try to download CI LLVM during unit tests.
+        if cfg!(test) {
+            return;
+        }
 
         if !self.llvm_from_ci {
             return;
@@ -334,8 +332,10 @@ impl Config {
         };
     }
 
-    #[cfg(not(test))]
     fn download_ci_llvm(&self, llvm_sha: &str) {
+        // For unit tests, downloading should have been blocked by `maybe_download_ci_llvm`.
+        assert!(cfg!(not(test)), "unit tests shouldn't be downloading CI LLVM");
+
         let llvm_assertions = self.llvm_assertions;
 
         let cache_prefix = format!("llvm-{llvm_sha}-{llvm_assertions}");
@@ -503,22 +503,16 @@ pub(crate) fn is_download_ci_available(target_triple: &str, llvm_assertions: boo
     }
 }
 
-#[cfg(test)]
-pub(crate) fn maybe_download_rustfmt<'a>(
-    _dwn_ctx: impl AsRef<DownloadContext<'a>>,
-    _out: &Path,
-) -> Option<PathBuf> {
-    Some(PathBuf::new())
-}
-
 /// NOTE: rustfmt is a completely different toolchain than the bootstrap compiler, so it can't
 /// reuse target directories or artifacts
-#[cfg(not(test))]
 pub(crate) fn maybe_download_rustfmt<'a>(
     dwn_ctx: impl AsRef<DownloadContext<'a>>,
     out: &Path,
 ) -> Option<PathBuf> {
-    use build_helper::stage0_parser::VersionMetadata;
+    // Don't actually download rustfmt during unit tests.
+    if cfg!(test) {
+        return Some(PathBuf::new());
+    }
 
     let dwn_ctx = dwn_ctx.as_ref();
 
@@ -573,11 +567,12 @@ pub(crate) fn maybe_download_rustfmt<'a>(
     Some(rustfmt_path)
 }
 
-#[cfg(test)]
-pub(crate) fn download_beta_toolchain<'a>(_dwn_ctx: impl AsRef<DownloadContext<'a>>, _out: &Path) {}
-
-#[cfg(not(test))]
 pub(crate) fn download_beta_toolchain<'a>(dwn_ctx: impl AsRef<DownloadContext<'a>>, out: &Path) {
+    // Don't actually download a beta toolchain during unit tests.
+    if cfg!(test) {
+        return;
+    }
+
     let dwn_ctx = dwn_ctx.as_ref();
     dwn_ctx.exec_ctx.do_if_verbose(|| {
         println!("downloading stage0 beta artifacts");
@@ -600,7 +595,6 @@ pub(crate) fn download_beta_toolchain<'a>(dwn_ctx: impl AsRef<DownloadContext<'a
 }
 
 #[allow(clippy::too_many_arguments)]
-#[cfg_attr(test, expect(dead_code))]
 fn download_toolchain<'a>(
     dwn_ctx: impl AsRef<DownloadContext<'a>>,
     out: &Path,
@@ -611,6 +605,8 @@ fn download_toolchain<'a>(
     destination: &str,
     mode: DownloadSource,
 ) {
+    assert!(cfg!(not(test)), "unit tests shouldn't be downloading a toolchain");
+
     let dwn_ctx = dwn_ctx.as_ref();
     let host = dwn_ctx.host_target.triple;
     let bin_root = out.join(host).join(sysroot);
@@ -1035,6 +1031,8 @@ fn download_http_with_retries(
     help_on_error: &str,
 ) {
     println!("downloading {url}");
+    assert!(cfg!(not(test)), "unit tests shouldn't be downloading things: {url:?}");
+
     // Try curl. If that fails and we are on windows, fallback to PowerShell.
     // options should be kept in sync with
     // src/bootstrap/src/core/download.rs

src/bootstrap/src/core/sanity.rs

@@ -14,10 +14,7 @@ use std::ffi::{OsStr, OsString};
 use std::path::PathBuf;
 use std::{env, fs};
 
-#[cfg(not(test))]
-use crate::builder::Builder;
-use crate::builder::Kind;
-#[cfg(not(test))]
+use crate::builder::{Builder, Kind};
 use crate::core::build_steps::tool;
 use crate::core::config::{CompilerBuiltins, Target};
 use crate::utils::exec::command;
@@ -41,7 +38,6 @@ const STAGE0_MISSING_TARGETS: &[&str] = &[
 
 /// Minimum version threshold for libstdc++ required when using prebuilt LLVM
 /// from CI (with`llvm.download-ci-llvm` option).
-#[cfg(not(test))]
 const LIBSTDCXX_MIN_VERSION_THRESHOLD: usize = 8;
 
 impl Finder {
@@ -109,8 +105,11 @@ pub fn check(build: &mut Build) {
     }
 
     // Ensure that a compatible version of libstdc++ is available on the system when using `llvm.download-ci-llvm`.
-    #[cfg(not(test))]
-    if !build.config.dry_run() && !build.host_target.is_msvc() && build.config.llvm_from_ci {
+    if cfg!(not(test))
+        && !build.config.dry_run()
+        && !build.host_target.is_msvc()
+        && build.config.llvm_from_ci
+    {
         let builder = Builder::new(build);
         let libcxx_version = builder.ensure(tool::LibcxxVersionTool { target: build.host_target });
 

src/bootstrap/src/lib.rs

@@ -15,6 +15,7 @@
 //!
 //! More documentation can be found in each respective module below, and you can
 //! also check out the `src/bootstrap/README.md` file for more information.
+#![allow(clippy::assertions_on_constants, reason = "false positive for `assert!(cfg!(..))`")]
 
 use std::cell::Cell;
 use std::collections::{BTreeSet, HashMap, HashSet};

src/bootstrap/src/utils/helpers.rs

@@ -595,7 +595,6 @@ pub fn detail_exit(code: i32, is_test: bool) -> ! {
     }
 }
 
-#[cfg_attr(test, expect(dead_code))]
 pub fn fail(s: &str) -> ! {
     eprintln!("\n\n{s}\n\n");
     detail_exit(1, cfg!(test));