From 21805038412de0224f7cefe3b4c98cc7fda17c39 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 18 Jun 2025 17:27:01 +0200 Subject: [PATCH 1/4] Add CI check to ensure that rustdoc JSON `FORMAT_VERSION` is correctly updated --- src/tools/tidy/src/lib.rs | 1 + src/tools/tidy/src/main.rs | 1 + src/tools/tidy/src/rustdoc_json.rs | 66 ++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+) create mode 100644 src/tools/tidy/src/rustdoc_json.rs diff --git a/src/tools/tidy/src/lib.rs b/src/tools/tidy/src/lib.rs index e8a12d563358d..28aa80225b177 100644 --- a/src/tools/tidy/src/lib.rs +++ b/src/tools/tidy/src/lib.rs @@ -83,6 +83,7 @@ pub mod pal; pub mod rustdoc_css_themes; pub mod rustdoc_gui_tests; pub mod rustdoc_js; +pub mod rustdoc_json; pub mod rustdoc_templates; pub mod style; pub mod target_policy; diff --git a/src/tools/tidy/src/main.rs b/src/tools/tidy/src/main.rs index 776f1bde2eb71..420260a97a04b 100644 --- a/src/tools/tidy/src/main.rs +++ b/src/tools/tidy/src/main.rs @@ -110,6 +110,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); 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 new file mode 100644 index 0000000000000..a98c4f4cc3e6e --- /dev/null +++ b/src/tools/tidy/src/rustdoc_json.rs @@ -0,0 +1,66 @@ +//! Tidy check to ensure that `FORMAT_VERSION` was correctly updated if `rustdoc-json-types` was +//! updated as well. + +use std::process::Command; + +fn git_diff(base_commit: &str, extra_arg: &str) -> 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 fn check(bad: &mut bool) { + let Ok(base_commit) = std::env::var("BASE_COMMIT") else { + // Not in CI so nothing we can check here. + println!("not checking rustdoc JSON `FORMAT_VERSION` update"); + return; + }; + + // First we check that `src/rustdoc-json-types` was modified. + match git_diff(&base_commit, "--name-status") { + Some(output) => { + if !output + .lines() + .any(|line| line.starts_with("M") && line.contains("src/rustdoc-json-types")) + { + // `rustdoc-json-types` was not modified so nothing more to check here. + return; + } + } + None => { + *bad = true; + eprintln!("Failed to run `git diff`"); + return; + } + } + // Then we check that if `FORMAT_VERSION` was updated, the `Latest feature:` was also updated. + match git_diff(&base_commit, "src/rustdoc-json-types") { + Some(output) => { + let mut format_version_updated = false; + let mut latest_feature_comment_updated = false; + for line in output.lines() { + if line.starts_with("+pub const FORMAT_VERSION: u32 =") { + format_version_updated = true; + } else if line.starts_with("+// Latest feature:") { + latest_feature_comment_updated = true; + } + } + if format_version_updated != latest_feature_comment_updated { + *bad = true; + if latest_feature_comment_updated { + eprintln!( + "`Latest feature` comment was updated whereas `FORMAT_VERSION` wasn't" + ); + } else { + eprintln!( + "`Latest feature` comment was not updated whereas `FORMAT_VERSION` was" + ); + } + } + } + None => { + *bad = true; + eprintln!("Failed to run `git diff`"); + return; + } + } +} From bbe8a2ad19c0486986a535c103e57fabfbb81441 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Thu, 19 Jun 2025 16:50:01 +0200 Subject: [PATCH 2/4] Generate base commit in rustdoc_json tidy checks --- src/build_helper/src/git.rs | 2 +- src/tools/tidy/src/rustdoc_json.rs | 31 ++++++++++++++++++++++++++---- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/build_helper/src/git.rs b/src/build_helper/src/git.rs index 438cd14389c1c..9d1195aadf848 100644 --- a/src/build_helper/src/git.rs +++ b/src/build_helper/src/git.rs @@ -198,7 +198,7 @@ fn get_latest_upstream_commit_that_modified_files( /// author. /// /// If we are in CI, we simply return our first parent. -fn get_closest_upstream_commit( +pub fn get_closest_upstream_commit( git_dir: Option<&Path>, config: &GitConfig<'_>, env: CiEnv, diff --git a/src/tools/tidy/src/rustdoc_json.rs b/src/tools/tidy/src/rustdoc_json.rs index a98c4f4cc3e6e..3f78d565deb26 100644 --- a/src/tools/tidy/src/rustdoc_json.rs +++ b/src/tools/tidy/src/rustdoc_json.rs @@ -3,16 +3,39 @@ use std::process::Command; +use build_helper::ci::CiEnv; +use build_helper::git::{GitConfig, get_closest_upstream_commit}; +use build_helper::stage0_parser::parse_stage0_file; + fn git_diff(base_commit: &str, extra_arg: &str) -> 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 fn check(bad: &mut bool) { - let Ok(base_commit) = std::env::var("BASE_COMMIT") else { - // Not in CI so nothing we can check here. - println!("not checking rustdoc JSON `FORMAT_VERSION` update"); - return; + println!("Checking tidy rustdoc_json..."); + let stage0 = parse_stage0_file(); + 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, + }, + CiEnv::current(), + ) { + Ok(Some(commit)) => commit, + Ok(None) => { + *bad = true; + eprintln!("No base commit found, skipping rustdoc_json check"); + return; + } + Err(error) => { + *bad = true; + eprintln!( + "Failed to retrieve base commit for rustdoc_json check because of `{error}`, skipping it" + ); + return; + } }; // First we check that `src/rustdoc-json-types` was modified. From 636769490445a477746a1242e40fa11b633388a5 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 20 Jun 2025 15:02:15 +0200 Subject: [PATCH 3/4] Pass `src_path` to rustdoc_json tidy check --- src/tools/tidy/src/main.rs | 2 +- src/tools/tidy/src/rustdoc_json.rs | 22 ++++++++++++---------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/tools/tidy/src/main.rs b/src/tools/tidy/src/main.rs index 420260a97a04b..0b66017b86522 100644 --- a/src/tools/tidy/src/main.rs +++ b/src/tools/tidy/src/main.rs @@ -110,7 +110,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); + check!(rustdoc_json, &src_path); 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 3f78d565deb26..808341fbd0613 100644 --- a/src/tools/tidy/src/rustdoc_json.rs +++ b/src/tools/tidy/src/rustdoc_json.rs @@ -1,18 +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 build_helper::ci::CiEnv; use build_helper::git::{GitConfig, get_closest_upstream_commit}; use build_helper::stage0_parser::parse_stage0_file; -fn git_diff(base_commit: &str, extra_arg: &str) -> Option { +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 fn check(bad: &mut bool) { +pub fn check(src_path: &Path, bad: &mut bool) { println!("Checking tidy rustdoc_json..."); let stage0 = parse_stage0_file(); let base_commit = match get_closest_upstream_commit( @@ -26,13 +28,13 @@ pub fn check(bad: &mut bool) { Ok(Some(commit)) => commit, Ok(None) => { *bad = true; - eprintln!("No base commit found, skipping rustdoc_json check"); + eprintln!("error: no base commit found for rustdoc_json check"); return; } Err(error) => { *bad = true; eprintln!( - "Failed to retrieve base commit for rustdoc_json check because of `{error}`, skipping it" + "error: failed to retrieve base commit for rustdoc_json check because of `{error}`" ); return; } @@ -46,17 +48,18 @@ pub fn check(bad: &mut bool) { .any(|line| line.starts_with("M") && line.contains("src/rustdoc-json-types")) { // `rustdoc-json-types` was not modified so nothing more to check here. + println!("`rustdoc-json-types` was not modified."); return; } } None => { *bad = true; - eprintln!("Failed to run `git diff`"); + eprintln!("error: failed to run `git diff` in rustdoc_json check"); return; } } // Then we check that if `FORMAT_VERSION` was updated, the `Latest feature:` was also updated. - match git_diff(&base_commit, "src/rustdoc-json-types") { + match 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; @@ -71,19 +74,18 @@ pub fn check(bad: &mut bool) { *bad = true; if latest_feature_comment_updated { eprintln!( - "`Latest feature` comment was updated whereas `FORMAT_VERSION` wasn't" + "error: `Latest feature` comment was updated whereas `FORMAT_VERSION` wasn't" ); } else { eprintln!( - "`Latest feature` comment was not updated whereas `FORMAT_VERSION` was" + "error: `Latest feature` comment was not updated whereas `FORMAT_VERSION` was" ); } } } None => { *bad = true; - eprintln!("Failed to run `git diff`"); - return; + eprintln!("error: failed to run `git diff` in rustdoc_json check"); } } } From 0fc950735ac35784b2ba49679f164f3ae2dfc8f7 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 20 Jun 2025 16:12:41 +0200 Subject: [PATCH 4/4] Improve error message for rustdoc_json_types tidy check Only emit git errors if we are in CI environment --- src/build_helper/src/ci.rs | 6 +++++- src/tools/tidy/src/rustdoc_json.rs | 30 ++++++++++++++++++++---------- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/src/build_helper/src/ci.rs b/src/build_helper/src/ci.rs index 60f319129a0bd..9d114c70a671b 100644 --- a/src/build_helper/src/ci.rs +++ b/src/build_helper/src/ci.rs @@ -17,7 +17,11 @@ impl CiEnv { } pub fn is_ci() -> bool { - Self::current() != CiEnv::None + Self::current().is_running_in_ci() + } + + pub fn is_running_in_ci(self) -> bool { + self != CiEnv::None } /// Checks if running in rust-lang/rust managed CI job. diff --git a/src/tools/tidy/src/rustdoc_json.rs b/src/tools/tidy/src/rustdoc_json.rs index 808341fbd0613..f179acf4a5106 100644 --- a/src/tools/tidy/src/rustdoc_json.rs +++ b/src/tools/tidy/src/rustdoc_json.rs @@ -9,33 +9,41 @@ 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) { 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, }, - CiEnv::current(), + ci_env, ) { Ok(Some(commit)) => commit, Ok(None) => { - *bad = true; - eprintln!("error: no base commit found for rustdoc_json check"); + error_if_in_ci(ci_env, "no base commit found", bad); return; } Err(error) => { - *bad = true; - eprintln!( - "error: failed to retrieve base commit for rustdoc_json check because of `{error}`" - ); + error_if_in_ci(ci_env, &format!("failed to retrieve base commit: {error}"), bad); return; } }; @@ -45,7 +53,7 @@ pub fn check(src_path: &Path, bad: &mut bool) { Some(output) => { if !output .lines() - .any(|line| line.starts_with("M") && line.contains("src/rustdoc-json-types")) + .any(|line| line.starts_with("M") && line.contains(RUSTDOC_JSON_TYPES)) { // `rustdoc-json-types` was not modified so nothing more to check here. println!("`rustdoc-json-types` was not modified."); @@ -74,11 +82,13 @@ pub fn check(src_path: &Path, bad: &mut bool) { *bad = true; if latest_feature_comment_updated { eprintln!( - "error: `Latest feature` comment was updated whereas `FORMAT_VERSION` wasn't" + "error in `rustdoc_json` tidy check: `Latest feature` comment was updated \ + whereas `FORMAT_VERSION` wasn't in `{RUSTDOC_JSON_TYPES}/lib.rs`" ); } else { eprintln!( - "error: `Latest feature` comment was not updated whereas `FORMAT_VERSION` was" + "error in `rustdoc_json` tidy check: `Latest feature` comment was not \ + updated whereas `FORMAT_VERSION` was in `{RUSTDOC_JSON_TYPES}/lib.rs`" ); } }