Introduce --ci flag in tidy

* add --ci flag in tidy

This commit introduces --ci flag in tidy because currently bootstrap can't pass its ci env information to tidy. It also modifies how CiInfo initialize its ci_env variable. tidy codes which uses CiEnv::is_ci for checking ci are now using ci_env in CiInfo.
* address review
- Fix comment
- Use Option for ci flag in order to have true/false explicitly or unspecified (implicit false)
* integrate CiInfo into TidyCtx
* remove CiInfo
* CiEnv::current() should be called when ci flag is not added
* extract base_commit() to a separate function
* use &TidyCtx instead of clone

src/bootstrap/src/core/build_steps/test.rs

@@ -1320,6 +1320,9 @@ fn run(self, builder: &Builder<'_>) {
         if builder.config.cmd.bless() {
             cmd.arg("--bless");
         }
+        if builder.config.is_running_on_ci() {
+            cmd.arg("--ci=true");
+        }
         if let Some(s) =
             builder.config.cmd.extra_checks().or(builder.config.tidy_extra_checks.as_deref())
         {

src/tools/tidy/src/alphabetical/tests.rs

@@ -5,7 +5,7 @@
 
 #[track_caller]
 fn test(lines: &str, name: &str, expected_msg: &str, expected_bad: bool) {
-    let tidy_ctx = TidyCtx::new(Path::new("/"), false, TidyFlags::default());
+    let tidy_ctx = TidyCtx::new(Path::new("/"), false, None, TidyFlags::default());
     let mut check = tidy_ctx.start_check("alphabetical-test");
     check_lines(Path::new(name), lines, &tidy_ctx, &mut check);
 
@@ -37,7 +37,7 @@ fn bless_test(before: &str, after: &str) {
     let temp_path = tempfile::Builder::new().tempfile().unwrap().into_temp_path();
     std::fs::write(&temp_path, before).unwrap();
 
-    let tidy_ctx = TidyCtx::new(Path::new("/"), false, TidyFlags::new(true));
+    let tidy_ctx = TidyCtx::new(Path::new("/"), false, None, TidyFlags::new(true));
 
     let mut check = tidy_ctx.start_check("alphabetical-test");
     check_lines(&temp_path, before, &tidy_ctx, &mut check);

src/tools/tidy/src/arg_parser.rs

@@ -15,6 +15,7 @@ pub struct TidyArgParser {
     pub npm: PathBuf,
     pub verbose: bool,
     pub bless: bool,
+    pub ci: Option<bool>,
     pub extra_checks: Option<Vec<String>>,
     pub pos_args: Vec<String>,
 }
@@ -59,6 +60,7 @@ fn command() -> Command {
             )
             .arg(Arg::new("verbose").help("verbose").long("verbose").action(ArgAction::SetTrue))
             .arg(Arg::new("bless").help("target files are modified").long("bless").action(ArgAction::SetTrue))
+            .arg(Arg::new("ci").help("ci flag").long("ci").default_missing_value("true").num_args(0..=1).value_parser(value_parser!(bool)))
             .arg(
                 Arg::new("extra_checks")
                     .help("extra checks")
@@ -78,6 +80,7 @@ fn build(matches: ArgMatches) -> Self {
             npm: matches.get_one::<PathBuf>("npm").unwrap().clone(),
             verbose: *matches.get_one::<bool>("verbose").unwrap(),
             bless: *matches.get_one::<bool>("bless").unwrap(),
+            ci: matches.get_one::<bool>("ci").cloned(),
             extra_checks: None,
             pos_args: vec![],
         };

src/tools/tidy/src/arg_parser/tests.rs

@@ -19,6 +19,7 @@ fn test_tidy_parser_full() {
         "yarn",
         "--verbose",
         "--bless",
+        "--ci",
         "--extra-checks",
         "if-installed:auto:js,auto:if-installed:py,if-installed:auto:cpp,if-installed:auto:spellcheck",
         "--", // pos_args
@@ -38,6 +39,8 @@ fn test_tidy_parser_full() {
     assert_eq!(parsed_args.npm, PathBuf::from("yarn"));
     assert!(parsed_args.verbose);
     assert!(parsed_args.bless);
+    assert!(parsed_args.ci.is_some());
+    assert!(parsed_args.ci.unwrap());
     assert_eq!(
         parsed_args.extra_checks,
         Some(vec![
@@ -166,3 +169,46 @@ fn test_tidy_parser_missing_npm_path() {
     let cmd = TidyArgParser::command();
     assert!(cmd.try_get_matches_from(args).is_err());
 }
+
+// --ci has some variations
+#[test]
+fn test_tidy_parse_ci_flag() {
+    // They are requried
+    let base_args = vec![
+        "rust-tidy",
+        "--root-path",
+        "/home/user/rust",
+        "--cargo-path",
+        "/home/user/rust/build/x86_64-unknown-linux-gnu/stage0/bin/cargo",
+        "--output-dir",
+        "/home/user/rust/build",
+        "--concurrency",
+        "16",
+        "--npm-path",
+        "yarn",
+    ];
+
+    // No --ci
+    let parsed_args = TidyArgParser::build(TidyArgParser::command().get_matches_from(&base_args));
+    assert!(parsed_args.ci.is_none());
+
+    // --ci
+    let mut args1 = base_args.clone();
+    args1.push("--ci");
+    let parsed_args = TidyArgParser::build(TidyArgParser::command().get_matches_from(args1));
+    assert!(parsed_args.ci.is_some());
+
+    // --ci=true
+    let mut args2 = base_args.clone();
+    args2.extend_from_slice(&["--ci", "true"]);
+    let parsed_args = TidyArgParser::build(TidyArgParser::command().get_matches_from(args2));
+    assert!(parsed_args.ci.is_some());
+    assert!(parsed_args.ci.unwrap());
+
+    // --ci=false
+    let mut args2 = base_args.clone();
+    args2.extend_from_slice(&["--ci", "false"]);
+    let parsed_args = TidyArgParser::build(TidyArgParser::command().get_matches_from(args2));
+    assert!(parsed_args.ci.is_some());
+    assert!(!parsed_args.ci.unwrap());
+}

src/tools/tidy/src/deps.rs

@@ -6,7 +6,6 @@
 use std::io::Write;
 use std::path::Path;
 
-use build_helper::ci::CiEnv;
 use cargo_metadata::semver::Version;
 use cargo_metadata::{Metadata, Package, PackageId};
 
@@ -637,7 +636,7 @@ pub fn check(root: &Path, cargo: &Path, tidy_ctx: TidyCtx) {
     check_proc_macro_dep_list(root, cargo, bless, &mut check);
 
     for &WorkspaceInfo { path, exceptions, crates_and_deps, submodules } in WORKSPACES {
-        if has_missing_submodule(root, submodules) {
+        if has_missing_submodule(root, submodules, tidy_ctx.is_running_on_ci()) {
             continue;
         }
 
@@ -757,8 +756,8 @@ fn check_proc_macro_dep_list(root: &Path, cargo: &Path, bless: bool, check: &mut
 /// Used to skip a check if a submodule is not checked out, and not in a CI environment.
 ///
 /// This helps prevent enforcing developers to fetch submodules for tidy.
-pub fn has_missing_submodule(root: &Path, submodules: &[&str]) -> bool {
-    !CiEnv::is_ci()
+pub fn has_missing_submodule(root: &Path, submodules: &[&str], is_ci: bool) -> bool {
+    !is_ci
         && submodules.iter().any(|submodule| {
             let path = root.join(submodule);
             !path.exists()

src/tools/tidy/src/diagnostics.rs

@@ -4,6 +4,9 @@
 use std::path::{Path, PathBuf};
 use std::sync::{Arc, Mutex};
 
+use build_helper::ci::CiEnv;
+use build_helper::git::{GitConfig, get_closest_upstream_commit};
+use build_helper::stage0_parser::{Stage0Config, parse_stage0_file};
 use termcolor::Color;
 
 /// CLI flags used by tidy.
@@ -28,11 +31,24 @@ pub fn new(bless: bool) -> Self {
 pub struct TidyCtx {
     tidy_flags: TidyFlags,
     diag_ctx: Arc<Mutex<DiagCtxInner>>,
+    ci_env: CiEnv,
+    pub base_commit: Option<String>,
 }
 
 impl TidyCtx {
-    pub fn new(root_path: &Path, verbose: bool, tidy_flags: TidyFlags) -> Self {
-        Self {
+    pub fn new(
+        root_path: &Path,
+        verbose: bool,
+        ci_flag: Option<bool>,
+        tidy_flags: TidyFlags,
+    ) -> Self {
+        let ci_env = match ci_flag {
+            Some(true) => CiEnv::GitHubActions,
+            Some(false) => CiEnv::None,
+            None => CiEnv::current(),
+        };
+
+        let mut tidy_ctx = Self {
             diag_ctx: Arc::new(Mutex::new(DiagCtxInner {
                 running_checks: Default::default(),
                 finished_checks: Default::default(),
@@ -40,13 +56,22 @@ pub fn new(root_path: &Path, verbose: bool, tidy_flags: TidyFlags) -> Self {
                 verbose,
             })),
             tidy_flags,
-        }
+            ci_env,
+            base_commit: None,
+        };
+        tidy_ctx.base_commit = find_base_commit(&tidy_ctx);
+
+        tidy_ctx
     }
 
     pub fn is_bless_enabled(&self) -> bool {
         self.tidy_flags.bless
     }
 
+    pub fn is_running_on_ci(&self) -> bool {
+        self.ci_env.is_running_in_ci()
+    }
+
     pub fn start_check<Id: Into<CheckId>>(&self, id: Id) -> RunningCheck {
         let mut id = id.into();
 
@@ -75,6 +100,46 @@ pub fn into_failed_checks(self) -> Vec<FinishedCheck> {
     }
 }
 
+fn find_base_commit(tidy_ctx: &TidyCtx) -> Option<String> {
+    let mut check = tidy_ctx.start_check("CI history");
+
+    let stage0 = parse_stage0_file();
+    let Stage0Config { nightly_branch, git_merge_commit_email, .. } = stage0.config;
+
+    let base_commit = match get_closest_upstream_commit(
+        None,
+        &GitConfig {
+            nightly_branch: &nightly_branch,
+            git_merge_commit_email: &git_merge_commit_email,
+        },
+        tidy_ctx.ci_env,
+    ) {
+        Ok(Some(commit)) => Some(commit),
+        Ok(None) => {
+            error_if_in_ci("no base commit found", tidy_ctx.is_running_on_ci(), &mut check);
+            None
+        }
+        Err(error) => {
+            error_if_in_ci(
+                &format!("failed to retrieve base commit: {error}"),
+                tidy_ctx.is_running_on_ci(),
+                &mut check,
+            );
+            None
+        }
+    };
+
+    base_commit
+}
+
+fn error_if_in_ci(msg: &str, is_ci: bool, check: &mut RunningCheck) {
+    if is_ci {
+        check.error(msg);
+    } else {
+        check.warning(format!("{msg}. Some checks will be skipped."));
+    }
+}
+
 struct DiagCtxInner {
     running_checks: HashSet<CheckId>,
     finished_checks: HashSet<FinishedCheck>,
@@ -175,7 +240,7 @@ impl RunningCheck {
     /// Useful if you want to run some functions from tidy without configuring
     /// diagnostics.
     pub fn new_noop() -> Self {
-        let ctx = TidyCtx::new(Path::new(""), false, TidyFlags::default());
+        let ctx = TidyCtx::new(Path::new(""), false, None, TidyFlags::default());
         ctx.start_check("noop")
     }
 

src/tools/tidy/src/error_codes.rs

@@ -36,11 +36,11 @@
 const IGNORE_UI_TEST_CHECK: &[&str] =
     &["E0461", "E0465", "E0514", "E0554", "E0640", "E0717", "E0729"];
 
-pub fn check(root_path: &Path, search_paths: &[&Path], ci_info: &crate::CiInfo, tidy_ctx: TidyCtx) {
+pub fn check(root_path: &Path, search_paths: &[&Path], tidy_ctx: TidyCtx) {
     let mut check = tidy_ctx.start_check("error_codes");
 
     // Check that no error code explanation was removed.
-    check_removed_error_code_explanation(ci_info, &mut check);
+    check_removed_error_code_explanation(&tidy_ctx.base_commit, &mut check);
 
     // Stage 1: create list
     let error_codes = extract_error_codes(root_path, &mut check);
@@ -57,8 +57,8 @@ pub fn check(root_path: &Path, search_paths: &[&Path], ci_info: &crate::CiInfo,
     check_error_codes_used(search_paths, &error_codes, &mut check, &no_longer_emitted);
 }
 
-fn check_removed_error_code_explanation(ci_info: &crate::CiInfo, check: &mut RunningCheck) {
-    let Some(base_commit) = &ci_info.base_commit else {
+fn check_removed_error_code_explanation(base_commit: &Option<String>, check: &mut RunningCheck) {
+    let Some(base_commit) = base_commit else {
         check.verbose_msg("Skipping error code explanation removal check");
         return;
     };

src/tools/tidy/src/extdeps.rs

@@ -19,7 +19,7 @@ pub fn check(root: &Path, tidy_ctx: TidyCtx) {
     let mut check = tidy_ctx.start_check("extdeps");
 
     for &WorkspaceInfo { path, submodules, .. } in crate::deps::WORKSPACES {
-        if crate::deps::has_missing_submodule(root, submodules) {
+        if crate::deps::has_missing_submodule(root, submodules, tidy_ctx.is_running_on_ci()) {
             continue;
         }
 

src/tools/tidy/src/extra_checks/mod.rs

@@ -23,9 +23,6 @@
 use std::str::FromStr;
 use std::{env, fmt, fs, io};
 
-use build_helper::ci::CiEnv;
-
-use crate::CiInfo;
 use crate::diagnostics::TidyCtx;
 
 mod rustdoc_js;
@@ -53,7 +50,6 @@
 pub fn check(
     root_path: &Path,
     outdir: &Path,
-    ci_info: &CiInfo,
     librustdoc_path: &Path,
     tools_path: &Path,
     npm: &Path,
@@ -67,7 +63,6 @@ pub fn check(
     if let Err(e) = check_impl(
         root_path,
         outdir,
-        ci_info,
         librustdoc_path,
         tools_path,
         npm,
@@ -83,7 +78,6 @@ pub fn check(
 fn check_impl(
     root_path: &Path,
     outdir: &Path,
-    ci_info: &CiInfo,
     librustdoc_path: &Path,
     tools_path: &Path,
     npm: &Path,
@@ -121,9 +115,12 @@ fn check_impl(
     };
     lint_args.retain(|ck| ck.is_non_if_installed_or_matches(root_path, outdir));
     if lint_args.iter().any(|ck| ck.auto) {
-        crate::files_modified_batch_filter(ci_info, &mut lint_args, |ck, path| {
-            ck.is_non_auto_or_matches(path)
-        });
+        crate::files_modified_batch_filter(
+            &tidy_ctx.base_commit,
+            tidy_ctx.is_running_on_ci(),
+            &mut lint_args,
+            |ck, path| ck.is_non_auto_or_matches(path),
+        );
     }
 
     macro_rules! extra_check {
@@ -321,7 +318,7 @@ macro_rules! extra_check {
         } else {
             eprintln!("spellchecking files");
         }
-        let res = spellcheck_runner(root_path, &outdir, &cargo, &args);
+        let res = spellcheck_runner(root_path, &outdir, &cargo, &args, tidy_ctx.is_running_on_ci());
         if res.is_err() {
             rerun_with_bless("spellcheck", "fix typos");
         }
@@ -629,9 +626,16 @@ fn spellcheck_runner(
     outdir: &Path,
     cargo: &Path,
     args: &[&str],
+    is_ci: bool,
 ) -> Result<(), Error> {
-    let bin_path =
-        ensure_version_or_cargo_install(outdir, cargo, "typos-cli", "typos", SPELLCHECK_VER)?;
+    let bin_path = ensure_version_or_cargo_install(
+        outdir,
+        cargo,
+        "typos-cli",
+        "typos",
+        SPELLCHECK_VER,
+        is_ci,
+    )?;
     match Command::new(bin_path).current_dir(src_root).args(args).status() {
         Ok(status) => {
             if status.success() {
@@ -713,6 +717,7 @@ fn ensure_version_or_cargo_install(
     pkg_name: &str,
     bin_name: &str,
     version: &str,
+    is_ci: bool,
 ) -> Result<PathBuf, Error> {
     if let Ok(bin_path) = ensure_version(build_dir, bin_name, version) {
         return Ok(bin_path);
@@ -746,7 +751,7 @@ fn ensure_version_or_cargo_install(
     // On CI, we set opt-level flag for quicker installation.
     // Since lower opt-level decreases the tool's performance,
     // we don't set this option on local.
-    if CiEnv::is_ci() {
+    if is_ci {
         cmd.env("RUSTFLAGS", "-Copt-level=0");
     }
 

src/tools/tidy/src/lib.rs

@@ -6,12 +6,6 @@
 use std::ffi::OsStr;
 use std::process::Command;
 
-use build_helper::ci::CiEnv;
-use build_helper::git::{GitConfig, get_closest_upstream_commit};
-use build_helper::stage0_parser::{Stage0Config, parse_stage0_file};
-
-use crate::diagnostics::{RunningCheck, TidyCtx};
-
 macro_rules! static_regex {
     ($re:literal) => {{
         static RE: ::std::sync::LazyLock<::regex::Regex> =
@@ -42,60 +36,6 @@ macro_rules! t {
     };
 }
 
-pub struct CiInfo {
-    pub git_merge_commit_email: String,
-    pub nightly_branch: String,
-    pub base_commit: Option<String>,
-    pub ci_env: CiEnv,
-}
-
-impl CiInfo {
-    pub fn new(tidy_ctx: TidyCtx) -> Self {
-        let mut check = tidy_ctx.start_check("CI history");
-
-        let stage0 = parse_stage0_file();
-        let Stage0Config { nightly_branch, git_merge_commit_email, .. } = stage0.config;
-
-        let mut info = Self {
-            nightly_branch,
-            git_merge_commit_email,
-            ci_env: CiEnv::current(),
-            base_commit: None,
-        };
-        let base_commit = match get_closest_upstream_commit(None, &info.git_config(), info.ci_env) {
-            Ok(Some(commit)) => Some(commit),
-            Ok(None) => {
-                info.error_if_in_ci("no base commit found", &mut check);
-                None
-            }
-            Err(error) => {
-                info.error_if_in_ci(
-                    &format!("failed to retrieve base commit: {error}"),
-                    &mut check,
-                );
-                None
-            }
-        };
-        info.base_commit = base_commit;
-        info
-    }
-
-    pub fn git_config(&self) -> GitConfig<'_> {
-        GitConfig {
-            nightly_branch: &self.nightly_branch,
-            git_merge_commit_email: &self.git_merge_commit_email,
-        }
-    }
-
-    pub fn error_if_in_ci(&self, msg: &str, check: &mut RunningCheck) {
-        if self.ci_env.is_running_in_ci() {
-            check.error(msg);
-        } else {
-            check.warning(format!("{msg}. Some checks will be skipped."));
-        }
-    }
-}
-
 pub fn git_diff<S: AsRef<OsStr>>(base_commit: &str, extra_arg: S) -> Option<String> {
     let output = Command::new("git").arg("diff").arg(base_commit).arg(extra_arg).output().ok()?;
     Some(String::from_utf8_lossy(&output.stdout).into())
@@ -107,15 +47,16 @@ pub fn git_diff<S: AsRef<OsStr>>(base_commit: &str, extra_arg: S) -> Option<Stri
 ///
 /// if in CI, no elements will be removed.
 pub fn files_modified_batch_filter<T>(
-    ci_info: &CiInfo,
+    base_commit: &Option<String>,
+    is_ci: bool,
     items: &mut Vec<T>,
     pred: impl Fn(&T, &str) -> bool,
 ) {
-    if CiEnv::is_ci() {
+    if is_ci {
         // assume everything is modified on CI because we really don't want false positives there.
         return;
     }
-    let Some(base_commit) = &ci_info.base_commit else {
+    let Some(base_commit) = base_commit else {
         eprintln!("No base commit, assuming all files are modified");
         return;
     };
@@ -150,9 +91,13 @@ pub fn files_modified_batch_filter<T>(
 }
 
 /// Returns true if any modified file matches the predicate, if we are in CI, or if unable to list modified files.
-pub fn files_modified(ci_info: &CiInfo, pred: impl Fn(&str) -> bool) -> bool {
+pub fn files_modified(
+    base_commit: &Option<String>,
+    is_ci: bool,
+    pred: impl Fn(&str) -> bool,
+) -> bool {
     let mut v = vec![()];
-    files_modified_batch_filter(ci_info, &mut v, |_, p| pred(p));
+    files_modified_batch_filter(base_commit, is_ci, &mut v, |_, p| pred(p));
     !v.is_empty()
 }
 

src/tools/tidy/src/main.rs

@@ -40,11 +40,11 @@ fn main() {
 
     let verbose = parsed_args.verbose;
     let bless = parsed_args.bless;
+    let ci = parsed_args.ci;
     let extra_checks = parsed_args.extra_checks;
     let pos_args = parsed_args.pos_args;
 
-    let tidy_ctx = TidyCtx::new(&root_path, verbose, TidyFlags::new(bless));
-    let ci_info = CiInfo::new(tidy_ctx.clone());
+    let tidy_ctx = TidyCtx::new(&root_path, verbose, ci, TidyFlags::new(bless));
 
     let drain_handles = |handles: &mut VecDeque<ScopedJoinHandle<'_, ()>>| {
         // poll all threads for completion before awaiting the oldest one
@@ -101,12 +101,12 @@ macro_rules! check {
         check!(rustdoc_gui_tests, &tests_path);
         check!(rustdoc_css_themes, &librustdoc_path);
         check!(rustdoc_templates, &librustdoc_path);
-        check!(rustdoc_json, &src_path, &ci_info);
+        check!(rustdoc_json, &src_path);
         check!(known_bug, &crashes_path);
         check!(unknown_revision, &tests_path);
 
         // Checks that only make sense for the compiler.
-        check!(error_codes, &root_path, &[&compiler_path, &librustdoc_path], &ci_info);
+        check!(error_codes, &root_path, &[&compiler_path, &librustdoc_path]);
         check!(target_policy, &root_path);
         check!(gcc_submodule, &root_path, &compiler_path);
 
@@ -154,7 +154,6 @@ macro_rules! check {
             extra_checks,
             &root_path,
             &output_directory,
-            &ci_info,
             &librustdoc_path,
             &tools_path,
             &npm,

src/tools/tidy/src/rustdoc_json.rs

@@ -8,16 +8,18 @@
 
 const RUSTDOC_JSON_TYPES: &str = "src/rustdoc-json-types";
 
-pub fn check(src_path: &Path, ci_info: &crate::CiInfo, tidy_ctx: TidyCtx) {
+pub fn check(src_path: &Path, tidy_ctx: TidyCtx) {
     let mut check = tidy_ctx.start_check(CheckId::new("rustdoc_json").path(src_path));
 
-    let Some(base_commit) = &ci_info.base_commit else {
+    let Some(base_commit) = &tidy_ctx.base_commit else {
         check.verbose_msg("No base commit, skipping rustdoc_json check");
         return;
     };
 
     // First we check that `src/rustdoc-json-types` was modified.
-    if !crate::files_modified(ci_info, |p| p.starts_with(RUSTDOC_JSON_TYPES)) {
+    if !crate::files_modified(&tidy_ctx.base_commit, tidy_ctx.is_running_on_ci(), |p| {
+        p.starts_with(RUSTDOC_JSON_TYPES)
+    }) {
         // `rustdoc-json-types` was not modified so nothing more to check here.
         return;
     }