diff --git a/src/ci/docker/host-x86_64/mingw-check-1/Dockerfile b/src/ci/docker/host-x86_64/mingw-check-1/Dockerfile index a877de1f7b246..9bdcf00dccc13 100644 --- a/src/ci/docker/host-x86_64/mingw-check-1/Dockerfile +++ b/src/ci/docker/host-x86_64/mingw-check-1/Dockerfile @@ -39,7 +39,6 @@ RUN pip3 install --no-deps --no-cache-dir --require-hashes -r /tmp/reuse-require COPY host-x86_64/mingw-check-1/check-default-config-profiles.sh /scripts/ COPY host-x86_64/mingw-check-1/validate-toolstate.sh /scripts/ -COPY host-x86_64/mingw-check-1/validate-error-codes.sh /scripts/ # Check library crates on all tier 1 targets. # We disable optimized compiler built-ins because that requires a C toolchain for the target. @@ -52,7 +51,6 @@ ENV SCRIPT \ python3 ../x.py check --stage 1 --target=i686-pc-windows-gnu --host=i686-pc-windows-gnu && \ python3 ../x.py check --stage 1 --set build.optimized-compiler-builtins=false core alloc std --target=aarch64-unknown-linux-gnu,i686-pc-windows-msvc,i686-unknown-linux-gnu,x86_64-apple-darwin,x86_64-pc-windows-gnu,x86_64-pc-windows-msvc && \ /scripts/validate-toolstate.sh && \ - /scripts/validate-error-codes.sh && \ reuse --include-submodules lint && \ python3 ../x.py test collect-license-metadata && \ # Runs checks to ensure that there are no issues in our JS code. diff --git a/src/ci/docker/host-x86_64/mingw-check-1/validate-error-codes.sh b/src/ci/docker/host-x86_64/mingw-check-1/validate-error-codes.sh deleted file mode 100755 index e9aa948eb877f..0000000000000 --- a/src/ci/docker/host-x86_64/mingw-check-1/validate-error-codes.sh +++ /dev/null @@ -1,20 +0,0 @@ -#!/bin/bash -# Checks that no error code explanation is removed. - -set -eo pipefail - -if [[ -z "$BASE_COMMIT" ]]; then - echo "not checking error code explanations removal" - exit 0 -fi - -echo "Check if an error code explanation was removed..." - -if (git diff "$BASE_COMMIT" --name-status | grep '^D' \ - | grep --quiet "compiler/rustc_error_codes/src/error_codes/"); then - echo "Error code explanations should never be removed!" - echo "Take a look at E0001 to see how to handle it." - exit 1 -fi - -echo "No error code explanation was removed!" diff --git a/src/ci/docker/host-x86_64/mingw-check-tidy/Dockerfile b/src/ci/docker/host-x86_64/mingw-check-tidy/Dockerfile index 8d2c5e004e479..62cd8a3121296 100644 --- a/src/ci/docker/host-x86_64/mingw-check-tidy/Dockerfile +++ b/src/ci/docker/host-x86_64/mingw-check-tidy/Dockerfile @@ -39,7 +39,6 @@ RUN pip3 install --no-deps --no-cache-dir --require-hashes -r /tmp/reuse-require && pip3 install virtualenv COPY host-x86_64/mingw-check-1/validate-toolstate.sh /scripts/ -COPY host-x86_64/mingw-check-1/validate-error-codes.sh /scripts/ RUN bash -c 'npm install -g eslint@$(cat /tmp/eslint.version)' diff --git a/src/tools/tidy/src/error_codes.rs b/src/tools/tidy/src/error_codes.rs index e2d1b85797ffe..bb61412f67882 100644 --- a/src/tools/tidy/src/error_codes.rs +++ b/src/tools/tidy/src/error_codes.rs @@ -43,9 +43,18 @@ macro_rules! verbose_print { }; } -pub fn check(root_path: &Path, search_paths: &[&Path], verbose: bool, bad: &mut bool) { +pub fn check( + root_path: &Path, + search_paths: &[&Path], + verbose: bool, + ci_info: &crate::CiInfo, + bad: &mut bool, +) { let mut errors = Vec::new(); + // Check that no error code explanation was removed. + check_removed_error_code_explanation(ci_info, bad); + // Stage 1: create list let error_codes = extract_error_codes(root_path, &mut errors); if verbose { @@ -68,6 +77,27 @@ pub fn check(root_path: &Path, search_paths: &[&Path], verbose: bool, bad: &mut } } +fn check_removed_error_code_explanation(ci_info: &crate::CiInfo, bad: &mut bool) { + let Some(base_commit) = &ci_info.base_commit else { + eprintln!("Skipping error code explanation removal check"); + return; + }; + let Some(diff) = crate::git_diff(base_commit, "--name-status") else { + *bad = true; + eprintln!("removed error code explanation tidy check: Failed to run git diff"); + return; + }; + if diff.lines().any(|line| { + line.starts_with('D') && line.contains("compiler/rustc_error_codes/src/error_codes/") + }) { + *bad = true; + eprintln!("tidy check error: Error code explanations should never be removed!"); + eprintln!("Take a look at E0001 to see how to handle it."); + return; + } + println!("No error code explanation was removed!"); +} + /// Stage 1: Parses a list of error codes from `error_codes.rs`. fn extract_error_codes(root_path: &Path, errors: &mut Vec) -> Vec { let path = root_path.join(Path::new(ERROR_CODES_PATH)); diff --git a/src/tools/tidy/src/lib.rs b/src/tools/tidy/src/lib.rs index 28aa80225b177..237737f0f1697 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 0b66017b86522..ef6ff5c9277aa 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,12 +112,12 @@ 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); // Checks that only make sense for the compiler. - check!(error_codes, &root_path, &[&compiler_path, &librustdoc_path], verbose); + check!(error_codes, &root_path, &[&compiler_path, &librustdoc_path], verbose, &ci_info); check!(fluent_alphabetical, &compiler_path, bless); check!(fluent_period, &compiler_path); check!(target_policy, &root_path); diff --git a/src/tools/tidy/src/rustdoc_json.rs b/src/tools/tidy/src/rustdoc_json.rs index 2377356e14dca..dfbb35d69f17a 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;