Rollup merge of #152734 - Shunpoco:cleanup-bootstrap-ci, r=Kobzol

Respect the `--ci` flag in more places in bootstrap

### Motivation
Currently, the way of checking CI environment in bootstrap is not unified. Sometimes `--ci` flag is respected, but in other cases only GITHUB_ACTIONS env var is checked.

### Change
This PR modifies the way of treating the flag in bootstrap. If `--ci` (or `--ci=true`) is added, bootstrap's config set ci_env as `CiEnv::GithubActions`. This PR also modifies some lines in bootstrap to respect the config's CiEnv instance.

### Note
I haven't touched anything in tidy, because I want to raise another PR for introducing --ci flag in tidy. In the PR, I will need to change how tidy treats CI information. So currently I leave it as it is.

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

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,

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<Option<Vec<String>>, 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();

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.

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;
@@ -1330,7 +1328,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

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<ChangeId>,
     pub bypass_bootstrap_lock: bool,
@@ -318,7 +318,7 @@ pub struct Config {
 
     /// Default value for `--extra-checks`
     pub tidy_extra_checks: Option<String>,
-    pub is_running_on_ci: bool,
+    pub ci_env: CiEnv,
 
     /// Cache for determining path modifications
     pub path_modification_cache: Arc<Mutex<HashMap<Vec<&'static str>, 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."

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]

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<PathBuf>,
-    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<DownloadContext<'a>> 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,

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");

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()));

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);