From 66060e64753b0ca41bda90130054e261fa5523be Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 21 Jun 2025 15:57:09 +0200 Subject: [PATCH] Create new `CiInfo` type in tidy checks to centralize CI related checks --- src/tools/tidy/src/lib.rs | 61 ++++++++++++++++++++++++++++++ src/tools/tidy/src/main.rs | 6 ++- src/tools/tidy/src/rustdoc_json.rs | 48 +++-------------------- 3 files changed, 71 insertions(+), 44 deletions(-) diff --git a/src/tools/tidy/src/lib.rs b/src/tools/tidy/src/lib.rs index 28aa80225b17..237737f0f169 100644 --- a/src/tools/tidy/src/lib.rs +++ b/src/tools/tidy/src/lib.rs @@ -3,6 +3,12 @@ //! This library contains the tidy lints and exposes it //! to be used by tools. +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 termcolor::WriteColor; macro_rules! static_regex { @@ -63,6 +69,61 @@ fn tidy_error(args: &str) -> std::io::Result<()> { Ok(()) } +pub struct CiInfo { + pub git_merge_commit_email: String, + pub nightly_branch: String, + pub base_commit: Option, + pub ci_env: CiEnv, +} + +impl CiInfo { + pub fn new(bad: &mut bool) -> Self { + 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", bad); + None + } + Err(error) => { + info.error_if_in_ci(&format!("failed to retrieve base commit: {error}"), bad); + 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, bad: &mut bool) { + if self.ci_env.is_running_in_ci() { + *bad = true; + eprintln!("tidy check error: {msg}"); + } else { + eprintln!("tidy check warning: {msg}. Some checks will be skipped."); + } + } +} + +pub fn git_diff>(base_commit: &str, extra_arg: S) -> Option { + let output = Command::new("git").arg("diff").arg(base_commit).arg(extra_arg).output().ok()?; + Some(String::from_utf8_lossy(&output.stdout).into()) +} + pub mod alphabetical; pub mod bins; pub mod debug_artifacts; diff --git a/src/tools/tidy/src/main.rs b/src/tools/tidy/src/main.rs index 0b66017b8652..6bc557b36fd6 100644 --- a/src/tools/tidy/src/main.rs +++ b/src/tools/tidy/src/main.rs @@ -48,7 +48,9 @@ fn main() { let extra_checks = cfg_args.iter().find(|s| s.starts_with("--extra-checks=")).map(String::as_str); - let bad = std::sync::Arc::new(AtomicBool::new(false)); + let mut bad = false; + let ci_info = CiInfo::new(&mut bad); + let bad = std::sync::Arc::new(AtomicBool::new(bad)); let drain_handles = |handles: &mut VecDeque>| { // poll all threads for completion before awaiting the oldest one @@ -110,7 +112,7 @@ fn main() { check!(rustdoc_css_themes, &librustdoc_path); check!(rustdoc_templates, &librustdoc_path); check!(rustdoc_js, &librustdoc_path, &tools_path, &src_path); - check!(rustdoc_json, &src_path); + check!(rustdoc_json, &src_path, &ci_info); check!(known_bug, &crashes_path); check!(unknown_revision, &tests_path); diff --git a/src/tools/tidy/src/rustdoc_json.rs b/src/tools/tidy/src/rustdoc_json.rs index 2377356e14dc..dfbb35d69f17 100644 --- a/src/tools/tidy/src/rustdoc_json.rs +++ b/src/tools/tidy/src/rustdoc_json.rs @@ -1,56 +1,20 @@ //! Tidy check to ensure that `FORMAT_VERSION` was correctly updated if `rustdoc-json-types` was //! updated as well. -use std::ffi::OsStr; use std::path::Path; -use std::process::Command; use std::str::FromStr; -use build_helper::ci::CiEnv; -use build_helper::git::{GitConfig, get_closest_upstream_commit}; -use build_helper::stage0_parser::parse_stage0_file; - const RUSTDOC_JSON_TYPES: &str = "src/rustdoc-json-types"; -fn git_diff>(base_commit: &str, extra_arg: S) -> Option { - let output = Command::new("git").arg("diff").arg(base_commit).arg(extra_arg).output().ok()?; - Some(String::from_utf8_lossy(&output.stdout).into()) -} - -fn error_if_in_ci(ci_env: CiEnv, msg: &str, bad: &mut bool) { - if ci_env.is_running_in_ci() { - *bad = true; - eprintln!("error in `rustdoc_json` tidy check: {msg}"); - } else { - eprintln!("{msg}. Skipping `rustdoc_json` tidy check"); - } -} - -pub fn check(src_path: &Path, bad: &mut bool) { +pub fn check(src_path: &Path, ci_info: &crate::CiInfo, bad: &mut bool) { println!("Checking tidy rustdoc_json..."); - let stage0 = parse_stage0_file(); - let ci_env = CiEnv::current(); - let base_commit = match get_closest_upstream_commit( - None, - &GitConfig { - nightly_branch: &stage0.config.nightly_branch, - git_merge_commit_email: &stage0.config.git_merge_commit_email, - }, - ci_env, - ) { - Ok(Some(commit)) => commit, - Ok(None) => { - error_if_in_ci(ci_env, "no base commit found", bad); - return; - } - Err(error) => { - error_if_in_ci(ci_env, &format!("failed to retrieve base commit: {error}"), bad); - return; - } + let Some(base_commit) = &ci_info.base_commit else { + eprintln!("No base commit, skipping rustdoc_json check"); + return; }; // First we check that `src/rustdoc-json-types` was modified. - match git_diff(&base_commit, "--name-status") { + match crate::git_diff(&base_commit, "--name-status") { Some(output) => { if !output .lines() @@ -68,7 +32,7 @@ pub fn check(src_path: &Path, bad: &mut bool) { } } // Then we check that if `FORMAT_VERSION` was updated, the `Latest feature:` was also updated. - match git_diff(&base_commit, src_path.join("rustdoc-json-types")) { + match crate::git_diff(&base_commit, src_path.join("rustdoc-json-types")) { Some(output) => { let mut format_version_updated = false; let mut latest_feature_comment_updated = false;