From 3559c7ddaff9bde0a5111c619288f1af823e6529 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 22 Jul 2026 12:42:29 +1000 Subject: [PATCH 1/2] Prefer `cfg!(not(test))` when skipping code paths during unit tests --- src/bootstrap/src/core/build_steps/doc.rs | 4 ++-- src/bootstrap/src/core/build_steps/tool.rs | 1 - src/bootstrap/src/core/config/config.rs | 28 ++++++++-------------- src/bootstrap/src/core/sanity.rs | 13 +++++----- src/bootstrap/src/utils/helpers.rs | 1 - 5 files changed, 18 insertions(+), 29 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/doc.rs b/src/bootstrap/src/core/build_steps/doc.rs index 9299a47c70c57..df3fb001e5bc5 100644 --- a/src/bootstrap/src/core/build_steps/doc.rs +++ b/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; } diff --git a/src/bootstrap/src/core/build_steps/tool.rs b/src/bootstrap/src/core/build_steps/tool.rs index 3318575e1f0e5..a6ecdea06623f 100644 --- a/src/bootstrap/src/core/build_steps/tool.rs +++ b/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, } diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index fd4a3af74a2be..55f287fa1f8e2 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/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." diff --git a/src/bootstrap/src/core/sanity.rs b/src/bootstrap/src/core/sanity.rs index 330be66ddebd6..92b1f35b64f2f 100644 --- a/src/bootstrap/src/core/sanity.rs +++ b/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 }); diff --git a/src/bootstrap/src/utils/helpers.rs b/src/bootstrap/src/utils/helpers.rs index c36d43f0cb5a4..394e14c80a1ef 100644 --- a/src/bootstrap/src/utils/helpers.rs +++ b/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)); From 480c37dbd387701724b999506f7dca7152b0caea Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 22 Jul 2026 12:39:30 +1000 Subject: [PATCH 2/2] Prefer `cfg!(not(test))` when skipping downloads Assertions have been added to some code paths that were previously not built during `cfg(test)`, to make sure they aren't accidentally executed. --- src/bootstrap/src/core/build_steps/gcc.rs | 17 +++----- src/bootstrap/src/core/build_steps/llvm.rs | 3 +- src/bootstrap/src/core/config/toml/llvm.rs | 2 - src/bootstrap/src/core/download.rs | 50 +++++++++++----------- src/bootstrap/src/lib.rs | 1 + 5 files changed, 34 insertions(+), 39 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/gcc.rs b/src/bootstrap/src/core/build_steps/gcc.rs index 34da87a8d1cef..992279fe1e133 100644 --- a/src/bootstrap/src/core/build_steps/gcc.rs +++ b/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 { - 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 { - 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"]) diff --git a/src/bootstrap/src/core/build_steps/llvm.rs b/src/bootstrap/src/core/build_steps/llvm.rs index 572df47d3a62c..0a13bf5d487b3 100644 --- a/src/bootstrap/src/core/build_steps/llvm.rs +++ b/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) { diff --git a/src/bootstrap/src/core/config/toml/llvm.rs b/src/bootstrap/src/core/config/toml/llvm.rs index 8bc6b4ef2491c..2538c591df79e 100644 --- a/src/bootstrap/src/core/config/toml/llvm.rs +++ b/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, diff --git a/src/bootstrap/src/core/download.rs b/src/bootstrap/src/core/download.rs index 76fd2cb0bc14b..d19c928e7c553 100644 --- a/src/bootstrap/src/core/download.rs +++ b/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 = 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>, - _out: &Path, -) -> Option { - 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>, out: &Path, ) -> Option { - 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>, _out: &Path) {} - -#[cfg(not(test))] pub(crate) fn download_beta_toolchain<'a>(dwn_ctx: impl AsRef>, 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( dwn_ctx: impl AsRef>, 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 diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index 5b2da5e4788c4..0035d6f3af668 100644 --- a/src/bootstrap/src/lib.rs +++ b/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};