From d5574c578aaa956086f52557bf67521bb598a2f1 Mon Sep 17 00:00:00 2001 From: Shunpoco Date: Tue, 17 Feb 2026 18:28:53 +0000 Subject: [PATCH] modify around --ci flag in bootstrap --ci flag in bootstrap is not respected in some cases. This commit modifies such inconsistencies in the tool. --- src/bootstrap/src/bin/main.rs | 2 +- src/bootstrap/src/core/build_steps/dist.rs | 2 +- src/bootstrap/src/core/build_steps/format.rs | 4 +-- src/bootstrap/src/core/build_steps/test.rs | 4 +-- src/bootstrap/src/core/builder/cargo.rs | 4 +-- src/bootstrap/src/core/config/config.rs | 31 ++++++++++++-------- src/bootstrap/src/core/config/tests.rs | 9 +++--- src/bootstrap/src/core/download.rs | 13 ++++++-- src/bootstrap/src/utils/cc_detect.rs | 2 +- src/bootstrap/src/utils/metrics.rs | 2 +- src/bootstrap/src/utils/render_tests.rs | 2 +- 11 files changed, 44 insertions(+), 31 deletions(-) diff --git a/src/bootstrap/src/bin/main.rs b/src/bootstrap/src/bin/main.rs index 93c7faf4f015..44eab9b87783 100644 --- a/src/bootstrap/src/bin/main.rs +++ b/src/bootstrap/src/bin/main.rs @@ -71,7 +71,7 @@ fn main() { // check_version warnings are not printed during setup, or during CI let changelog_suggestion = if matches!(config.cmd, Subcommand::Setup { .. }) - || config.is_running_on_ci + || config.is_running_on_ci() || config.dry_run() { None diff --git a/src/bootstrap/src/core/build_steps/dist.rs b/src/bootstrap/src/core/build_steps/dist.rs index eee960027a9f..ed1d96bb821a 100644 --- a/src/bootstrap/src/core/build_steps/dist.rs +++ b/src/bootstrap/src/core/build_steps/dist.rs @@ -3084,7 +3084,7 @@ impl Step for Gcc { return None; } - if builder.config.is_running_on_ci { + if builder.config.is_running_on_ci() { assert_eq!( builder.config.gcc_ci_mode, GccCiMode::BuildLocally, diff --git a/src/bootstrap/src/core/build_steps/format.rs b/src/bootstrap/src/core/build_steps/format.rs index d487995e98a0..53cb03a41fc4 100644 --- a/src/bootstrap/src/core/build_steps/format.rs +++ b/src/bootstrap/src/core/build_steps/format.rs @@ -92,7 +92,7 @@ fn update_rustfmt_version(build: &Builder<'_>) { fn get_modified_rs_files(build: &Builder<'_>) -> Result>, String> { // In CI `get_git_modified_files` returns something different to normal environment. // This shouldn't be called in CI anyway. - assert!(!build.config.is_running_on_ci); + assert!(!build.config.is_running_on_ci()); if !verify_rustfmt_version(build) { return Ok(None); @@ -142,7 +142,7 @@ pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) { // `--all` is specified or we are in CI. We check all files in CI to avoid bugs in // `get_modified_rs_files` letting regressions slip through; we also care about CI time less // since this is still very fast compared to building the compiler. - let all = all || build.config.is_running_on_ci; + let all = all || build.config.is_running_on_ci(); let mut builder = ignore::types::TypesBuilder::new(); builder.add_defaults(); diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index fda9f3bbba3a..f0fe1c03e7e1 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -3500,7 +3500,7 @@ impl Step for BootstrapPy { // Bootstrap tests might not be perfectly self-contained and can depend // on the environment, so only run them by default in CI, not locally. // See `test::Bootstrap::should_run`. - builder.config.is_running_on_ci + builder.config.is_running_on_ci() } fn make_run(run: RunConfig<'_>) { @@ -3539,7 +3539,7 @@ impl Step for Bootstrap { // Bootstrap tests might not be perfectly self-contained and can depend on the external // environment, submodules that are checked out, etc. // Therefore we only run them by default on CI. - builder.config.is_running_on_ci + builder.config.is_running_on_ci() } /// Tests the build system itself. diff --git a/src/bootstrap/src/core/builder/cargo.rs b/src/bootstrap/src/core/builder/cargo.rs index 7f4e23881e45..2e3ddd22db60 100644 --- a/src/bootstrap/src/core/builder/cargo.rs +++ b/src/bootstrap/src/core/builder/cargo.rs @@ -2,8 +2,6 @@ use std::env; use std::ffi::{OsStr, OsString}; use std::path::{Path, PathBuf}; -use build_helper::ci::CiEnv; - use super::{Builder, Kind}; use crate::core::build_steps::test; use crate::core::build_steps::tool::SourceType; @@ -1326,7 +1324,7 @@ impl Builder<'_> { // Try to use a sysroot-relative bindir, in case it was configured absolutely. cargo.env("RUSTC_INSTALL_BINDIR", self.config.bindir_relative()); - if CiEnv::is_ci() { + if self.config.is_running_on_ci() { // Tell cargo to use colored output for nicer logs in CI, even // though CI isn't printing to a terminal. // Also set an explicit `TERM=xterm` so that cargo doesn't warn diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 61eef3c01592..bc68bfe39642 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -88,7 +88,7 @@ pub const RUSTC_IF_UNCHANGED_ALLOWED_PATHS: &[&str] = &[ /// filled out from the decoded forms of the structs below. For documentation /// on each field, see the corresponding fields in /// `bootstrap.example.toml`. -#[derive(Default, Clone)] +#[derive(Clone)] pub struct Config { pub change_id: Option, pub bypass_bootstrap_lock: bool, @@ -318,7 +318,7 @@ pub struct Config { /// Default value for `--extra-checks` pub tidy_extra_checks: Option, - pub is_running_on_ci: bool, + pub ci_env: CiEnv, /// Cache for determining path modifications pub path_modification_cache: Arc, PathFreshness>>>, @@ -728,7 +728,11 @@ impl Config { ); } - let is_running_on_ci = flags_ci.unwrap_or(CiEnv::is_ci()); + let ci_env = match flags_ci { + Some(true) => CiEnv::GitHubActions, + Some(false) => CiEnv::None, + None => CiEnv::current(), + }; let dwn_ctx = DownloadContext { path_modification_cache: path_modification_cache.clone(), src: &src, @@ -739,7 +743,7 @@ impl Config { stage0_metadata: &stage0_metadata, llvm_assertions, bootstrap_cache_path: &build_bootstrap_cache_path, - is_running_on_ci, + ci_env, }; let initial_rustc = build_rustc.unwrap_or_else(|| { @@ -1168,7 +1172,7 @@ impl Config { // CI should always run stage 2 builds, unless it specifically states otherwise #[cfg(not(test))] - if flags_stage.is_none() && is_running_on_ci { + if flags_stage.is_none() && ci_env.is_running_in_ci() { match flags_cmd { Subcommand::Test { .. } | Subcommand::Miri { .. } @@ -1295,6 +1299,7 @@ impl Config { ccache, change_id: toml.change_id.inner, channel, + ci_env, clippy_info, cmd: flags_cmd, codegen_tests: rust_codegen_tests.unwrap_or(true), @@ -1345,7 +1350,6 @@ impl Config { initial_rustc, initial_rustfmt, initial_sysroot, - is_running_on_ci, jemalloc: rust_jemalloc.unwrap_or(false), jobs: Some(threads_from_config(flags_jobs.or(build_jobs).unwrap_or(0))), json_output: flags_json_output, @@ -1500,6 +1504,10 @@ impl Config { self.exec_ctx.dry_run() } + pub fn is_running_on_ci(&self) -> bool { + self.ci_env.is_running_in_ci() + } + pub fn is_explicit_stage(&self) -> bool { self.explicit_stage_from_cli || self.explicit_stage_from_config } @@ -1666,7 +1674,7 @@ impl Config { if !self.llvm_from_ci { // This happens when LLVM submodule is updated in CI, we should disable ci-rustc without an error // to not break CI. For non-CI environments, we should return an error. - if self.is_running_on_ci { + if self.is_running_on_ci() { println!("WARNING: LLVM submodule has changes, `download-rustc` will be disabled."); return None; } else { @@ -1788,8 +1796,7 @@ impl Config { .unwrap() .entry(paths.to_vec()) .or_insert_with(|| { - check_path_modifications(&self.src, &self.git_config(), paths, CiEnv::current()) - .unwrap() + check_path_modifications(&self.src, &self.git_config(), paths, self.ci_env).unwrap() }) .clone() } @@ -2223,7 +2230,7 @@ pub fn download_ci_rustc_commit<'a>( return None; } - if dwn_ctx.is_running_on_ci { + if dwn_ctx.is_running_on_ci() { eprintln!("CI rustc commit matches with HEAD and we are in CI."); eprintln!( "`rustc.download-ci` functionality will be skipped as artifacts are not available." @@ -2267,7 +2274,7 @@ pub fn check_path_modifications_<'a>( dwn_ctx.src, &git_config(dwn_ctx.stage0_metadata), paths, - CiEnv::current(), + dwn_ctx.ci_env, ) .unwrap() }) @@ -2322,7 +2329,7 @@ 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 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/config/tests.rs b/src/bootstrap/src/core/config/tests.rs index e19604d4ab12..277ede8d7745 100644 --- a/src/bootstrap/src/core/config/tests.rs +++ b/src/bootstrap/src/core/config/tests.rs @@ -42,7 +42,7 @@ fn download_ci_llvm() { .config("check") .with_default_toml_config("llvm.download-ci-llvm = \"if-unchanged\"") .create_config(); - if if_unchanged_config.llvm_from_ci && if_unchanged_config.is_running_on_ci { + if if_unchanged_config.llvm_from_ci && if_unchanged_config.is_running_on_ci() { let has_changes = if_unchanged_config.has_changes_from_upstream(LLVM_INVALIDATION_PATHS); assert!( @@ -491,13 +491,14 @@ fn test_exclude() { #[test] fn test_ci_flag() { let config = TestCtx::new().config("check").arg("--ci").arg("false").create_config(); - assert!(!config.is_running_on_ci); + assert!(!config.is_running_on_ci()); let config = TestCtx::new().config("check").arg("--ci").arg("true").create_config(); - assert!(config.is_running_on_ci); + assert!(config.is_running_on_ci()); + // If --ci flag is not added, is_running_on_ci() relies on if it is run on actual CI or not. let config = TestCtx::new().config("check").create_config(); - assert_eq!(config.is_running_on_ci, CiEnv::is_ci()); + assert_eq!(config.is_running_on_ci(), CiEnv::is_ci()); } #[test] diff --git a/src/bootstrap/src/core/download.rs b/src/bootstrap/src/core/download.rs index bf8d0cf4d534..6177233bc04e 100644 --- a/src/bootstrap/src/core/download.rs +++ b/src/bootstrap/src/core/download.rs @@ -6,6 +6,7 @@ use std::io::{BufRead, BufReader, BufWriter, ErrorKind, Write}; use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex, OnceLock}; +use build_helper::ci::CiEnv; use build_helper::git::PathFreshness; use xz2::bufread::XzDecoder; @@ -411,7 +412,13 @@ pub(crate) struct DownloadContext<'a> { pub stage0_metadata: &'a build_helper::stage0_parser::Stage0, pub llvm_assertions: bool, pub bootstrap_cache_path: &'a Option, - pub is_running_on_ci: bool, + pub ci_env: CiEnv, +} + +impl<'a> DownloadContext<'a> { + pub fn is_running_on_ci(&self) -> bool { + self.ci_env.is_running_in_ci() + } } impl<'a> AsRef> for DownloadContext<'a> { @@ -432,7 +439,7 @@ impl<'a> From<&'a Config> for DownloadContext<'a> { stage0_metadata: &value.stage0_metadata, llvm_assertions: value.llvm_assertions, bootstrap_cache_path: &value.bootstrap_cache_path, - is_running_on_ci: value.is_running_on_ci, + ci_env: value.ci_env, } } } @@ -981,7 +988,7 @@ fn download_file<'a>( match url.split_once("://").map(|(proto, _)| proto) { Some("http") | Some("https") => download_http_with_retries( dwn_ctx.host_target, - dwn_ctx.is_running_on_ci, + dwn_ctx.is_running_on_ci(), dwn_ctx.exec_ctx, &tempfile, url, diff --git a/src/bootstrap/src/utils/cc_detect.rs b/src/bootstrap/src/utils/cc_detect.rs index 0662ae304ac0..d010226f0dfd 100644 --- a/src/bootstrap/src/utils/cc_detect.rs +++ b/src/bootstrap/src/utils/cc_detect.rs @@ -223,7 +223,7 @@ fn default_compiler( let root = if let Some(path) = build.wasi_sdk_path.as_ref() { path } else { - if build.config.is_running_on_ci { + if build.config.is_running_on_ci() { panic!("ERROR: WASI_SDK_PATH must be configured for a -wasi target on CI"); } println!("WARNING: WASI_SDK_PATH not set, using default cc/cxx compiler"); diff --git a/src/bootstrap/src/utils/metrics.rs b/src/bootstrap/src/utils/metrics.rs index 9b1ccc32cb61..e685c64733c6 100644 --- a/src/bootstrap/src/utils/metrics.rs +++ b/src/bootstrap/src/utils/metrics.rs @@ -222,7 +222,7 @@ impl BuildMetrics { format_version: CURRENT_FORMAT_VERSION, system_stats, invocations, - ci_metadata: get_ci_metadata(CiEnv::current()), + ci_metadata: get_ci_metadata(build.config.ci_env), }; t!(std::fs::create_dir_all(dest.parent().unwrap())); diff --git a/src/bootstrap/src/utils/render_tests.rs b/src/bootstrap/src/utils/render_tests.rs index 55eba6c696c5..1d133a9c9e2f 100644 --- a/src/bootstrap/src/utils/render_tests.rs +++ b/src/bootstrap/src/utils/render_tests.rs @@ -179,7 +179,7 @@ impl<'a> Renderer<'a> { if self.builder.config.verbose_tests { self.render_test_outcome_verbose(outcome, test); - } else if self.builder.config.is_running_on_ci { + } else if self.builder.config.is_running_on_ci() { self.render_test_outcome_ci(outcome, test); } else { self.render_test_outcome_terse(outcome, test);