From 69df709f4833796e688abe5818e3eb6b617a1264 Mon Sep 17 00:00:00 2001 From: Shunpoco Date: Sat, 13 Dec 2025 19:41:14 +0000 Subject: [PATCH 1/7] modify setup to choose pre-push hook with spellcheck It changes setup for pre-push hook to use 3 options (tidy, with spellcheck, and skip). Since spellceck binary is installed everytime under ./build if the user clean up the build folder, it should be optional. --- src/bootstrap/src/core/build_steps/setup.rs | 94 +++++++++++++++++---- src/etc/pre-push-spellcheck.sh | 36 ++++++++ 2 files changed, 113 insertions(+), 17 deletions(-) create mode 100755 src/etc/pre-push-spellcheck.sh diff --git a/src/bootstrap/src/core/build_steps/setup.rs b/src/bootstrap/src/core/build_steps/setup.rs index 7dfd566a58ccb..c93459bb74ec2 100644 --- a/src/bootstrap/src/core/build_steps/setup.rs +++ b/src/bootstrap/src/core/build_steps/setup.rs @@ -484,6 +484,52 @@ impl Step for Hook { } } +/// It handles Git hook setup +#[derive(Clone, Debug, Eq, PartialEq)] +enum GitPrePushHookKind { + Tidy, + TidyWithSpellcheck, +} + +impl GitPrePushHookKind { + fn prompt_user() -> io::Result> { + let prompt_str = "Available options: +1. Set tidy as pre-push hook +2. Set tidy with spellcheck as pre-push hook + +Please select [default: None]:"; + + let mut input = String::new(); + loop { + print!("{prompt_str}"); + io::stdout().flush()?; + io::stdin().read_line(&mut input)?; + + let mut modified_input = input.to_lowercase(); + modified_input.retain(|ch| !ch.is_whitespace()); + + match modified_input.as_str() { + "1" => return Ok(Some(GitPrePushHookKind::Tidy)), + "2" => return Ok(Some(GitPrePushHookKind::TidyWithSpellcheck)), + "" | "none" => return Ok(None), + _ => { + eprintln!("ERROR: unrecognized option '{}'", input.trim()); + eprintln!("NOTE: press Ctrl+C to exit"); + } + } + + input.clear(); + } + } + + fn settings_path(&self) -> PathBuf { + PathBuf::new().join("src").join("etc").join(match self { + GitPrePushHookKind::Tidy => "pre-push.sh", + GitPrePushHookKind::TidyWithSpellcheck => "pre-push-spellcheck.sh", + }) + } +} + // install a git hook to automatically run tidy, if they want fn install_git_hook_maybe(builder: &Builder<'_>, config: &Config) -> io::Result<()> { let git = helpers::git(Some(&config.src)) @@ -493,37 +539,51 @@ fn install_git_hook_maybe(builder: &Builder<'_>, config: &Config) -> io::Result< let git = PathBuf::from(git.trim()); let hooks_dir = git.join("hooks"); let dst = hooks_dir.join("pre-push"); - if dst.exists() { - // The git hook has already been set up, or the user already has a custom hook. - return Ok(()); - } println!( "\nRust's CI will automatically fail if it doesn't pass `tidy`, the internal tool for ensuring code quality. If you'd like, x.py can install a git hook for you that will automatically run `test tidy` before pushing your code to ensure your code is up to par. If you decide later that this behavior is -undesirable, simply delete the `pre-push` file from .git/hooks." +undesirable, simply delete the `pre-push` file from .git/hooks. +You have two choices of hooks, the first just runs `test tidy`, the second runs the tidy command with spellcheck. +Since the spellcheck will be installed if the binary doesn't exist under `build/`, we'll recommend you to choose the first one if you frequently clean up the build directory. +It overrides the existing pre-push hook if you already have." ); - if prompt_user("Would you like to install the git hook?: [y/N]")? != Some(PromptResult::Yes) { - println!("Ok, skipping installation!"); - return Ok(()); - } + let src = match GitPrePushHookKind::prompt_user() { + Ok(git_hook_kind) => { + if let Some(git_hook_kind) = git_hook_kind { + git_hook_kind.settings_path() + } else { + println!("Skip setting pre-push hook"); + return Ok(()); + } + } + Err(e) => { + eprintln!("ERROR: could not determine pre push hook: {e}"); + return Err(e); + } + }; + if !hooks_dir.exists() { // We need to (try to) create the hooks directory first. let _ = fs::create_dir(hooks_dir); } - let src = config.src.join("src").join("etc").join("pre-push.sh"); - match fs::hard_link(src, &dst) { + + if let Ok(true) = fs::exists(&dst) { + // Remove the existing pre-push file. + if let Err(e) = fs::remove_file(&dst) { + eprintln!("ERROR: could not remove the existing hook\n{}", e); + return Err(e); + } + } + + match fs::hard_link(config.src.join(&src), &dst) { Err(e) => { - eprintln!( - "ERROR: could not create hook {}: do you already have the git hook installed?\n{}", - dst.display(), - e - ); + eprintln!("ERROR: could not create hook {}:\n{}", dst.display(), e); return Err(e); } - Ok(_) => println!("Linked `src/etc/pre-push.sh` to `.git/hooks/pre-push`"), + Ok(_) => println!("Linked `{}` to `{}`", src.display(), dst.display()), }; Ok(()) } diff --git a/src/etc/pre-push-spellcheck.sh b/src/etc/pre-push-spellcheck.sh new file mode 100755 index 0000000000000..b7bda5d309730 --- /dev/null +++ b/src/etc/pre-push-spellcheck.sh @@ -0,0 +1,36 @@ +#!/usr/bin/env bash +# +# Call `tidy` before git push +# Copy this script to .git/hooks to activate, +# and remove it from .git/hooks to deactivate. +# + +set -Euo pipefail + +# Check if the push is doing anything other than deleting remote branches +SKIP=true +while read LOCAL_REF LOCAL_SHA REMOTE_REF REMOTE_SHA; do + if [[ "$LOCAL_REF" != "(delete)" || \ + "$LOCAL_SHA" != "0000000000000000000000000000000000000000" ]]; then + SKIP=false + fi +done + +if $SKIP; then + echo "Skipping tidy check for branch deletion" + exit 0 +fi + +ROOT_DIR="$(git rev-parse --show-toplevel)" + +echo "Running pre-push script $ROOT_DIR/x test tidy" + +cd "$ROOT_DIR" +# The env var is necessary for printing diffs in py (fmt/lint) and cpp. +TIDY_PRINT_DIFF=1 ./x test tidy \ + --set build.locked-deps=true \ + --extra-checks auto:py,auto:cpp,auto:js,spellcheck +if [ $? -ne 0 ]; then + echo "You may use \`git push --no-verify\` to skip this check." + exit 1 +fi From 0e4ff305cc146b884df1f850a13ccba80c5deeab Mon Sep 17 00:00:00 2001 From: Shunpoco Date: Tue, 16 Dec 2025 09:35:39 +0000 Subject: [PATCH 2/7] wip --- src/tools/tidy/src/extra_checks/mod.rs | 64 +++++++++++++++----------- src/tools/tidy/src/lib.rs | 18 ++++++++ src/tools/tidy/src/main.rs | 2 + src/tools/tidy/src/spellcheck.rs | 30 ++++++++++++ 4 files changed, 88 insertions(+), 26 deletions(-) create mode 100644 src/tools/tidy/src/spellcheck.rs diff --git a/src/tools/tidy/src/extra_checks/mod.rs b/src/tools/tidy/src/extra_checks/mod.rs index a45af7fcf1580..f2b7d0e3e0686 100644 --- a/src/tools/tidy/src/extra_checks/mod.rs +++ b/src/tools/tidy/src/extra_checks/mod.rs @@ -42,7 +42,7 @@ const RUFF_CONFIG_PATH: &[&str] = &["src", "tools", "tidy", "config", "ruff.toml const RUFF_CACHE_PATH: &[&str] = &["cache", "ruff_cache"]; const PIP_REQ_PATH: &[&str] = &["src", "tools", "tidy", "config", "requirements.txt"]; -const SPELLCHECK_DIRS: &[&str] = &["compiler", "library", "src/bootstrap", "src/librustdoc"]; +pub const SPELLCHECK_DIRS: &[&str] = &["compiler", "library", "src/bootstrap", "src/librustdoc"]; pub fn check( root_path: &Path, @@ -74,24 +74,12 @@ pub fn check( } } -fn check_impl( - root_path: &Path, - outdir: &Path, - ci_info: &CiInfo, - librustdoc_path: &Path, - tools_path: &Path, - npm: &Path, - cargo: &Path, - extra_checks: Option<&str>, - pos_args: &[String], - tidy_ctx: &TidyCtx, -) -> Result<(), Error> { - let show_diff = - std::env::var("TIDY_PRINT_DIFF").is_ok_and(|v| v.eq_ignore_ascii_case("true") || v == "1"); - let bless = tidy_ctx.is_bless_enabled(); +pub fn has_spellcheck(lint_args: &Vec) -> bool { + lint_args.iter().any(|arg| arg.matches(ExtraCheckLang::Spellcheck, ExtraCheckKind::None)) +} - // Split comma-separated args up - let mut lint_args = match extra_checks { +pub fn parse_extra_checks(extra_checks: Option<&str>) -> Vec { + match extra_checks { Some(s) => s .strip_prefix("--extra-checks=") .unwrap() @@ -114,7 +102,27 @@ fn check_impl( }) .collect(), None => vec![], - }; + } +} + +fn check_impl( + root_path: &Path, + outdir: &Path, + ci_info: &CiInfo, + librustdoc_path: &Path, + tools_path: &Path, + npm: &Path, + cargo: &Path, + extra_checks: Option<&str>, + pos_args: &[String], + tidy_ctx: &TidyCtx, +) -> Result<(), Error> { + let show_diff = + std::env::var("TIDY_PRINT_DIFF").is_ok_and(|v| v.eq_ignore_ascii_case("true") || v == "1"); + let bless = tidy_ctx.is_bless_enabled(); + + // Split comma-separated args up + let mut lint_args = parse_extra_checks(extra_checks); 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) @@ -316,7 +324,7 @@ fn check_impl( } else { eprintln!("spellchecking files"); } - let res = spellcheck_runner(root_path, &outdir, &cargo, &args); + let res = spellcheck_runner(root_path, &outdir, &cargo, &args, true); if res.is_err() { rerun_with_bless("spellcheck", "fix typos"); } @@ -614,14 +622,18 @@ fn shellcheck_runner(args: &[&OsStr]) -> Result<(), Error> { } /// Ensure that spellchecker is installed then run it at the given path -fn spellcheck_runner( +pub fn spellcheck_runner( src_root: &Path, outdir: &Path, cargo: &Path, args: &[&str], + install: bool, ) -> Result<(), Error> { - let bin_path = - crate::ensure_version_or_cargo_install(outdir, cargo, "typos-cli", "typos", "1.38.1")?; + let bin_path = if install { + crate::ensure_version_or_cargo_install(outdir, cargo, "typos-cli", "typos", "1.38.1")? + } else { + crate::ensure_version(outdir, "typos", "1.38.1")? + }; match Command::new(bin_path).current_dir(src_root).args(args).status() { Ok(status) => { if status.success() { @@ -676,7 +688,7 @@ fn find_with_extension( } #[derive(Debug)] -enum Error { +pub enum Error { Io(io::Error), /// a is required to run b. c is extra info MissingReq(&'static str, &'static str, Option), @@ -724,7 +736,7 @@ impl From for Error { } #[derive(Debug)] -enum ExtraCheckParseError { +pub enum ExtraCheckParseError { #[allow(dead_code, reason = "shown through Debug")] UnknownKind(String), #[allow(dead_code)] @@ -738,7 +750,7 @@ enum ExtraCheckParseError { AutoRequiresLang, } -struct ExtraCheckArg { +pub struct ExtraCheckArg { auto: bool, lang: ExtraCheckLang, /// None = run all extra checks for the given lang diff --git a/src/tools/tidy/src/lib.rs b/src/tools/tidy/src/lib.rs index 756f9790e04ad..0b24e9b6ff288 100644 --- a/src/tools/tidy/src/lib.rs +++ b/src/tools/tidy/src/lib.rs @@ -158,6 +158,23 @@ pub fn files_modified(ci_info: &CiInfo, pred: impl Fn(&str) -> bool) -> bool { !v.is_empty() } +/// Check if the given executable is installed with the given version. +pub fn ensure_version(build_dir: &Path, bin_name: &str, version: &str) -> io::Result { + let tool_root_dir = build_dir.join("misc-tools"); + let tool_bin_dir = tool_root_dir.join("bin"); + let bin_path = tool_bin_dir.join(bin_name); + + if let Ok(output) = Command::new(&bin_path).arg("--version").output() + && let Ok(s) = str::from_utf8(&output.stdout) + && let Some(v) = s.trim().split_whitespace().last() + && v == version + { + return Ok(bin_path); + } + + Err(io::Error::other("no tool")) +} + /// If the given executable is installed with the given version, use that, /// otherwise install via cargo. pub fn ensure_version_or_cargo_install( @@ -253,6 +270,7 @@ pub mod rustdoc_css_themes; pub mod rustdoc_gui_tests; pub mod rustdoc_json; pub mod rustdoc_templates; +pub mod spellcheck; pub mod style; pub mod target_policy; pub mod target_specific_tests; diff --git a/src/tools/tidy/src/main.rs b/src/tools/tidy/src/main.rs index 94c24f11ed12f..f8870c2b11051 100644 --- a/src/tools/tidy/src/main.rs +++ b/src/tools/tidy/src/main.rs @@ -133,6 +133,8 @@ fn main() { check!(bins, &root_path); } + check!(spellcheck, &root_path, &output_directory, &cargo, extra_checks); + check!(style, &src_path); check!(style, &tests_path); check!(style, &compiler_path); diff --git a/src/tools/tidy/src/spellcheck.rs b/src/tools/tidy/src/spellcheck.rs new file mode 100644 index 0000000000000..b22a3ff473afa --- /dev/null +++ b/src/tools/tidy/src/spellcheck.rs @@ -0,0 +1,30 @@ +use std::path::Path; + +use crate::extra_checks::{parse_extra_checks, has_spellcheck, spellcheck_runner, SPELLCHECK_DIRS}; +use crate::diagnostics::TidyCtx; + +/// Check checks spells in modified files if the tool is installed +/// and its version is expected. +/// If the command has --extra-checks=spellcheck flag, +/// this function reinstalles and runs the tool. +/// I should remove cargo +pub fn check(root_path: &Path, outdir: &Path, cargo: &Path, extra_checks: Option<&str>, tidy_ctx: TidyCtx) { + let mut check = tidy_ctx.start_check("spellcheck"); + + let lint_args = parse_extra_checks(extra_checks); + let should_install = has_spellcheck(&lint_args); + + if should_install { + // extra-checks will do spellcheck + eprintln!("extra check will reinstall spellcheck then run"); + return; + } + + eprintln!("Doing spellcheck!!!!"); + let config_path = root_path.join("typos.toml"); + let mut args = vec!["-c", config_path.as_os_str().to_str().unwrap()]; + args.extend_from_slice(SPELLCHECK_DIRS); + if let Err(e) = spellcheck_runner(root_path, &outdir, &cargo, &args, false) { + check.error(e); + } +} From 264657bdaae9a2af5dbdff32c86131d745fc8b95 Mon Sep 17 00:00:00 2001 From: Shunpoco Date: Sat, 20 Dec 2025 12:02:47 +0000 Subject: [PATCH 3/7] temp --- src/tools/tidy/src/extra_checks/mod.rs | 3 ++- src/tools/tidy/src/spellcheck.rs | 11 +++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/tools/tidy/src/extra_checks/mod.rs b/src/tools/tidy/src/extra_checks/mod.rs index f2b7d0e3e0686..e1b2412ee2ee4 100644 --- a/src/tools/tidy/src/extra_checks/mod.rs +++ b/src/tools/tidy/src/extra_checks/mod.rs @@ -74,7 +74,8 @@ pub fn check( } } -pub fn has_spellcheck(lint_args: &Vec) -> bool { +pub fn has_spellcheck(extra_checks: Option<&str>) -> bool { + let lint_args = parse_extra_checks(extra_checks); lint_args.iter().any(|arg| arg.matches(ExtraCheckLang::Spellcheck, ExtraCheckKind::None)) } diff --git a/src/tools/tidy/src/spellcheck.rs b/src/tools/tidy/src/spellcheck.rs index b22a3ff473afa..77cc31f3577ad 100644 --- a/src/tools/tidy/src/spellcheck.rs +++ b/src/tools/tidy/src/spellcheck.rs @@ -1,6 +1,5 @@ use std::path::Path; - -use crate::extra_checks::{parse_extra_checks, has_spellcheck, spellcheck_runner, SPELLCHECK_DIRS}; +use crate::extra_checks::{has_spellcheck, spellcheck_runner, SPELLCHECK_DIRS}; use crate::diagnostics::TidyCtx; /// Check checks spells in modified files if the tool is installed @@ -11,8 +10,7 @@ use crate::diagnostics::TidyCtx; pub fn check(root_path: &Path, outdir: &Path, cargo: &Path, extra_checks: Option<&str>, tidy_ctx: TidyCtx) { let mut check = tidy_ctx.start_check("spellcheck"); - let lint_args = parse_extra_checks(extra_checks); - let should_install = has_spellcheck(&lint_args); + let should_install = has_spellcheck(extra_checks); if should_install { // extra-checks will do spellcheck @@ -24,7 +22,12 @@ pub fn check(root_path: &Path, outdir: &Path, cargo: &Path, extra_checks: Option let config_path = root_path.join("typos.toml"); let mut args = vec!["-c", config_path.as_os_str().to_str().unwrap()]; args.extend_from_slice(SPELLCHECK_DIRS); + if let Err(e) = spellcheck_runner(root_path, &outdir, &cargo, &args, false) { check.error(e); } } + +// fn runner(src_root: &Path, outdir: &Path, args: &[&str]) -> Result<(), Error> { + +// } \ No newline at end of file From 992faa0361556bd7f5e43860f5edafd8b6e2c9d8 Mon Sep 17 00:00:00 2001 From: Shunpoco Date: Sat, 20 Dec 2025 13:27:40 +0000 Subject: [PATCH 4/7] wip --- src/tools/tidy/src/extra_checks/mod.rs | 19 +++++++-------- src/tools/tidy/src/main.rs | 5 +++- src/tools/tidy/src/spellcheck.rs | 33 +++++++++++++------------- 3 files changed, 29 insertions(+), 28 deletions(-) diff --git a/src/tools/tidy/src/extra_checks/mod.rs b/src/tools/tidy/src/extra_checks/mod.rs index e1b2412ee2ee4..17ce3f524abe6 100644 --- a/src/tools/tidy/src/extra_checks/mod.rs +++ b/src/tools/tidy/src/extra_checks/mod.rs @@ -42,6 +42,10 @@ const RUFF_CONFIG_PATH: &[&str] = &["src", "tools", "tidy", "config", "ruff.toml const RUFF_CACHE_PATH: &[&str] = &["cache", "ruff_cache"]; const PIP_REQ_PATH: &[&str] = &["src", "tools", "tidy", "config", "requirements.txt"]; +pub const SPELLCHECK_BIN_NAME: &str = "typos"; +pub const SPELLCHECK_PACKAGE_NAME: &str = "typos-cli"; +pub const SPELLCHECK_VERSION: &str = "1.38.1"; +pub const SPELLCHECK_CONFIG: &str = "typos.toml"; pub const SPELLCHECK_DIRS: &[&str] = &["compiler", "library", "src/bootstrap", "src/librustdoc"]; pub fn check( @@ -314,7 +318,7 @@ fn check_impl( } if spellcheck { - let config_path = root_path.join("typos.toml"); + let config_path = root_path.join(SPELLCHECK_CONFIG); let mut args = vec!["-c", config_path.as_os_str().to_str().unwrap()]; args.extend_from_slice(SPELLCHECK_DIRS); @@ -325,7 +329,9 @@ fn check_impl( } else { eprintln!("spellchecking files"); } - let res = spellcheck_runner(root_path, &outdir, &cargo, &args, true); + + let bin_path = crate::ensure_version_or_cargo_install(outdir, cargo, SPELLCHECK_PACKAGE_NAME, SPELLCHECK_BIN_NAME, SPELLCHECK_VERSION)?; + let res = spellcheck_runner(root_path, &bin_path, &args); if res.is_err() { rerun_with_bless("spellcheck", "fix typos"); } @@ -625,16 +631,9 @@ fn shellcheck_runner(args: &[&OsStr]) -> Result<(), Error> { /// Ensure that spellchecker is installed then run it at the given path pub fn spellcheck_runner( src_root: &Path, - outdir: &Path, - cargo: &Path, + bin_path: &PathBuf, args: &[&str], - install: bool, ) -> Result<(), Error> { - let bin_path = if install { - crate::ensure_version_or_cargo_install(outdir, cargo, "typos-cli", "typos", "1.38.1")? - } else { - crate::ensure_version(outdir, "typos", "1.38.1")? - }; match Command::new(bin_path).current_dir(src_root).args(args).status() { Ok(status) => { if status.success() { diff --git a/src/tools/tidy/src/main.rs b/src/tools/tidy/src/main.rs index f8870c2b11051..12526b9539228 100644 --- a/src/tools/tidy/src/main.rs +++ b/src/tools/tidy/src/main.rs @@ -12,6 +12,7 @@ use std::thread::{self, ScopedJoinHandle, scope}; use std::{env, process}; use tidy::diagnostics::{COLOR_ERROR, COLOR_SUCCESS, TidyCtx, TidyFlags, output_message}; +use tidy::extra_checks::has_spellcheck; use tidy::*; fn main() { @@ -133,7 +134,9 @@ fn main() { check!(bins, &root_path); } - check!(spellcheck, &root_path, &output_directory, &cargo, extra_checks); + if !has_spellcheck(extra_checks) { + check!(spellcheck, &root_path, &output_directory); + } check!(style, &src_path); check!(style, &tests_path); diff --git a/src/tools/tidy/src/spellcheck.rs b/src/tools/tidy/src/spellcheck.rs index 77cc31f3577ad..8b9310556f422 100644 --- a/src/tools/tidy/src/spellcheck.rs +++ b/src/tools/tidy/src/spellcheck.rs @@ -1,33 +1,32 @@ use std::path::Path; -use crate::extra_checks::{has_spellcheck, spellcheck_runner, SPELLCHECK_DIRS}; +use crate::extra_checks::spellcheck_runner; use crate::diagnostics::TidyCtx; +pub const SPELLCHECK_BIN_NAME: &str = "typos"; +pub const SPELLCHECK_PACKAGE_NAME: &str = "typos-cli"; +pub const SPELLCHECK_VERSION: &str = "1.38.1"; +pub const SPELLCHECK_CONFIG: &str = "typos.toml"; +pub const SPELLCHECK_DIRS: &[&str] = &["compiler", "library", "src/bootstrap", "src/librustdoc"]; + /// Check checks spells in modified files if the tool is installed /// and its version is expected. /// If the command has --extra-checks=spellcheck flag, /// this function reinstalles and runs the tool. /// I should remove cargo -pub fn check(root_path: &Path, outdir: &Path, cargo: &Path, extra_checks: Option<&str>, tidy_ctx: TidyCtx) { +pub fn check(root_path: &Path, outdir: &Path, tidy_ctx: TidyCtx) { let mut check = tidy_ctx.start_check("spellcheck"); - let should_install = has_spellcheck(extra_checks); - - if should_install { - // extra-checks will do spellcheck - eprintln!("extra check will reinstall spellcheck then run"); - return; - } - eprintln!("Doing spellcheck!!!!"); - let config_path = root_path.join("typos.toml"); + let config_path = root_path.join(SPELLCHECK_CONFIG); let mut args = vec!["-c", config_path.as_os_str().to_str().unwrap()]; args.extend_from_slice(SPELLCHECK_DIRS); - if let Err(e) = spellcheck_runner(root_path, &outdir, &cargo, &args, false) { - check.error(e); + match crate::ensure_version(outdir, SPELLCHECK_BIN_NAME, SPELLCHECK_VERSION) { + Ok(bin_path) => { + if let Err(e) = spellcheck_runner(root_path, &bin_path, &args) { + check.error(e); + } + }, + Err(e) => check.error(e), } } - -// fn runner(src_root: &Path, outdir: &Path, args: &[&str]) -> Result<(), Error> { - -// } \ No newline at end of file From 1d3d1c1c717d444319593ec9d37078fc92f5a62f Mon Sep 17 00:00:00 2001 From: Shunpoco Date: Sat, 20 Dec 2025 14:59:10 +0000 Subject: [PATCH 5/7] fix --- src/tools/tidy/src/extra_checks/mod.rs | 38 ++++++++++++++++++-------- src/tools/tidy/src/main.rs | 1 + src/tools/tidy/src/spellcheck.rs | 37 ++++++++++++------------- 3 files changed, 44 insertions(+), 32 deletions(-) diff --git a/src/tools/tidy/src/extra_checks/mod.rs b/src/tools/tidy/src/extra_checks/mod.rs index 17ce3f524abe6..80205b6906049 100644 --- a/src/tools/tidy/src/extra_checks/mod.rs +++ b/src/tools/tidy/src/extra_checks/mod.rs @@ -43,10 +43,10 @@ const RUFF_CACHE_PATH: &[&str] = &["cache", "ruff_cache"]; const PIP_REQ_PATH: &[&str] = &["src", "tools", "tidy", "config", "requirements.txt"]; pub const SPELLCHECK_BIN_NAME: &str = "typos"; -pub const SPELLCHECK_PACKAGE_NAME: &str = "typos-cli"; pub const SPELLCHECK_VERSION: &str = "1.38.1"; -pub const SPELLCHECK_CONFIG: &str = "typos.toml"; -pub const SPELLCHECK_DIRS: &[&str] = &["compiler", "library", "src/bootstrap", "src/librustdoc"]; +const SPELLCHECK_PACKAGE_NAME: &str = "typos-cli"; +const SPELLCHECK_CONFIG: &str = "typos.toml"; +const SPELLCHECK_DIRS: &[&str] = &["compiler", "library", "src/bootstrap", "src/librustdoc"]; pub fn check( root_path: &Path, @@ -318,20 +318,21 @@ fn check_impl( } if spellcheck { - let config_path = root_path.join(SPELLCHECK_CONFIG); - let mut args = vec!["-c", config_path.as_os_str().to_str().unwrap()]; - - args.extend_from_slice(SPELLCHECK_DIRS); - + let args = build_spellcheck_args(root_path, bless); if bless { eprintln!("spellchecking files and fixing typos"); - args.push("--write-changes"); } else { eprintln!("spellchecking files"); } - let bin_path = crate::ensure_version_or_cargo_install(outdir, cargo, SPELLCHECK_PACKAGE_NAME, SPELLCHECK_BIN_NAME, SPELLCHECK_VERSION)?; - let res = spellcheck_runner(root_path, &bin_path, &args); + let bin_path = crate::ensure_version_or_cargo_install( + outdir, + cargo, + SPELLCHECK_PACKAGE_NAME, + SPELLCHECK_BIN_NAME, + SPELLCHECK_VERSION, + )?; + let res = spellcheck_runner(root_path, &bin_path, args); if res.is_err() { rerun_with_bless("spellcheck", "fix typos"); } @@ -628,11 +629,24 @@ fn shellcheck_runner(args: &[&OsStr]) -> Result<(), Error> { if status.success() { Ok(()) } else { Err(Error::FailedCheck("shellcheck")) } } +pub fn build_spellcheck_args(root_path: &Path, bless: bool) -> Vec { + let config_path = root_path.join(SPELLCHECK_CONFIG).as_os_str().to_str().unwrap().to_string(); + + let mut args = vec!["-c".to_string(), config_path]; + args.extend(SPELLCHECK_DIRS.iter().map(|s| s.to_string())); + + if bless { + args.push("--write-changes".to_string()); + } + + args +} + /// Ensure that spellchecker is installed then run it at the given path pub fn spellcheck_runner( src_root: &Path, bin_path: &PathBuf, - args: &[&str], + args: Vec, ) -> Result<(), Error> { match Command::new(bin_path).current_dir(src_root).args(args).status() { Ok(status) => { diff --git a/src/tools/tidy/src/main.rs b/src/tools/tidy/src/main.rs index 12526b9539228..5b53e20fe1a8f 100644 --- a/src/tools/tidy/src/main.rs +++ b/src/tools/tidy/src/main.rs @@ -134,6 +134,7 @@ fn main() { check!(bins, &root_path); } + // If extra-checks has spellcheck, do nothing because this check is redundant. if !has_spellcheck(extra_checks) { check!(spellcheck, &root_path, &output_directory); } diff --git a/src/tools/tidy/src/spellcheck.rs b/src/tools/tidy/src/spellcheck.rs index 8b9310556f422..0742f7280b945 100644 --- a/src/tools/tidy/src/spellcheck.rs +++ b/src/tools/tidy/src/spellcheck.rs @@ -1,12 +1,9 @@ use std::path::Path; -use crate::extra_checks::spellcheck_runner; -use crate::diagnostics::TidyCtx; -pub const SPELLCHECK_BIN_NAME: &str = "typos"; -pub const SPELLCHECK_PACKAGE_NAME: &str = "typos-cli"; -pub const SPELLCHECK_VERSION: &str = "1.38.1"; -pub const SPELLCHECK_CONFIG: &str = "typos.toml"; -pub const SPELLCHECK_DIRS: &[&str] = &["compiler", "library", "src/bootstrap", "src/librustdoc"]; +use crate::diagnostics::TidyCtx; +use crate::extra_checks::{ + SPELLCHECK_BIN_NAME, SPELLCHECK_VERSION, build_spellcheck_args, spellcheck_runner, +}; /// Check checks spells in modified files if the tool is installed /// and its version is expected. @@ -14,19 +11,19 @@ pub const SPELLCHECK_DIRS: &[&str] = &["compiler", "library", "src/bootstrap", " /// this function reinstalles and runs the tool. /// I should remove cargo pub fn check(root_path: &Path, outdir: &Path, tidy_ctx: TidyCtx) { - let mut check = tidy_ctx.start_check("spellcheck"); + let mut check = tidy_ctx.start_check("spellcheck"); - eprintln!("Doing spellcheck!!!!"); - let config_path = root_path.join(SPELLCHECK_CONFIG); - let mut args = vec!["-c", config_path.as_os_str().to_str().unwrap()]; - args.extend_from_slice(SPELLCHECK_DIRS); + let args = build_spellcheck_args(root_path, false); - match crate::ensure_version(outdir, SPELLCHECK_BIN_NAME, SPELLCHECK_VERSION) { - Ok(bin_path) => { - if let Err(e) = spellcheck_runner(root_path, &bin_path, &args) { - check.error(e); - } - }, - Err(e) => check.error(e), - } + match crate::ensure_version(outdir, SPELLCHECK_BIN_NAME, SPELLCHECK_VERSION) { + Ok(bin_path) => { + eprintln!("Detect spellchecker, run it..."); + if let Err(e) = spellcheck_runner(root_path, &bin_path, args) { + check.error(e); + } + } + Err(_) => { + // If ensure_version returns an error, do nothing. + } + } } From fa9531fb895f977e05b75a9f9cdd306ce7da322a Mon Sep 17 00:00:00 2001 From: Shunpoco Date: Sat, 20 Dec 2025 16:05:03 +0000 Subject: [PATCH 6/7] Revert "modify setup to choose pre-push hook with spellcheck" This reverts commit 69df709f4833796e688abe5818e3eb6b617a1264. --- src/bootstrap/src/core/build_steps/setup.rs | 94 ++++----------------- src/etc/pre-push-spellcheck.sh | 36 -------- 2 files changed, 17 insertions(+), 113 deletions(-) delete mode 100755 src/etc/pre-push-spellcheck.sh diff --git a/src/bootstrap/src/core/build_steps/setup.rs b/src/bootstrap/src/core/build_steps/setup.rs index c93459bb74ec2..7dfd566a58ccb 100644 --- a/src/bootstrap/src/core/build_steps/setup.rs +++ b/src/bootstrap/src/core/build_steps/setup.rs @@ -484,52 +484,6 @@ impl Step for Hook { } } -/// It handles Git hook setup -#[derive(Clone, Debug, Eq, PartialEq)] -enum GitPrePushHookKind { - Tidy, - TidyWithSpellcheck, -} - -impl GitPrePushHookKind { - fn prompt_user() -> io::Result> { - let prompt_str = "Available options: -1. Set tidy as pre-push hook -2. Set tidy with spellcheck as pre-push hook - -Please select [default: None]:"; - - let mut input = String::new(); - loop { - print!("{prompt_str}"); - io::stdout().flush()?; - io::stdin().read_line(&mut input)?; - - let mut modified_input = input.to_lowercase(); - modified_input.retain(|ch| !ch.is_whitespace()); - - match modified_input.as_str() { - "1" => return Ok(Some(GitPrePushHookKind::Tidy)), - "2" => return Ok(Some(GitPrePushHookKind::TidyWithSpellcheck)), - "" | "none" => return Ok(None), - _ => { - eprintln!("ERROR: unrecognized option '{}'", input.trim()); - eprintln!("NOTE: press Ctrl+C to exit"); - } - } - - input.clear(); - } - } - - fn settings_path(&self) -> PathBuf { - PathBuf::new().join("src").join("etc").join(match self { - GitPrePushHookKind::Tidy => "pre-push.sh", - GitPrePushHookKind::TidyWithSpellcheck => "pre-push-spellcheck.sh", - }) - } -} - // install a git hook to automatically run tidy, if they want fn install_git_hook_maybe(builder: &Builder<'_>, config: &Config) -> io::Result<()> { let git = helpers::git(Some(&config.src)) @@ -539,51 +493,37 @@ fn install_git_hook_maybe(builder: &Builder<'_>, config: &Config) -> io::Result< let git = PathBuf::from(git.trim()); let hooks_dir = git.join("hooks"); let dst = hooks_dir.join("pre-push"); + if dst.exists() { + // The git hook has already been set up, or the user already has a custom hook. + return Ok(()); + } println!( "\nRust's CI will automatically fail if it doesn't pass `tidy`, the internal tool for ensuring code quality. If you'd like, x.py can install a git hook for you that will automatically run `test tidy` before pushing your code to ensure your code is up to par. If you decide later that this behavior is -undesirable, simply delete the `pre-push` file from .git/hooks. -You have two choices of hooks, the first just runs `test tidy`, the second runs the tidy command with spellcheck. -Since the spellcheck will be installed if the binary doesn't exist under `build/`, we'll recommend you to choose the first one if you frequently clean up the build directory. -It overrides the existing pre-push hook if you already have." +undesirable, simply delete the `pre-push` file from .git/hooks." ); - let src = match GitPrePushHookKind::prompt_user() { - Ok(git_hook_kind) => { - if let Some(git_hook_kind) = git_hook_kind { - git_hook_kind.settings_path() - } else { - println!("Skip setting pre-push hook"); - return Ok(()); - } - } - Err(e) => { - eprintln!("ERROR: could not determine pre push hook: {e}"); - return Err(e); - } - }; - + if prompt_user("Would you like to install the git hook?: [y/N]")? != Some(PromptResult::Yes) { + println!("Ok, skipping installation!"); + return Ok(()); + } if !hooks_dir.exists() { // We need to (try to) create the hooks directory first. let _ = fs::create_dir(hooks_dir); } - - if let Ok(true) = fs::exists(&dst) { - // Remove the existing pre-push file. - if let Err(e) = fs::remove_file(&dst) { - eprintln!("ERROR: could not remove the existing hook\n{}", e); - return Err(e); - } - } - - match fs::hard_link(config.src.join(&src), &dst) { + let src = config.src.join("src").join("etc").join("pre-push.sh"); + match fs::hard_link(src, &dst) { Err(e) => { - eprintln!("ERROR: could not create hook {}:\n{}", dst.display(), e); + eprintln!( + "ERROR: could not create hook {}: do you already have the git hook installed?\n{}", + dst.display(), + e + ); return Err(e); } - Ok(_) => println!("Linked `{}` to `{}`", src.display(), dst.display()), + Ok(_) => println!("Linked `src/etc/pre-push.sh` to `.git/hooks/pre-push`"), }; Ok(()) } diff --git a/src/etc/pre-push-spellcheck.sh b/src/etc/pre-push-spellcheck.sh deleted file mode 100755 index b7bda5d309730..0000000000000 --- a/src/etc/pre-push-spellcheck.sh +++ /dev/null @@ -1,36 +0,0 @@ -#!/usr/bin/env bash -# -# Call `tidy` before git push -# Copy this script to .git/hooks to activate, -# and remove it from .git/hooks to deactivate. -# - -set -Euo pipefail - -# Check if the push is doing anything other than deleting remote branches -SKIP=true -while read LOCAL_REF LOCAL_SHA REMOTE_REF REMOTE_SHA; do - if [[ "$LOCAL_REF" != "(delete)" || \ - "$LOCAL_SHA" != "0000000000000000000000000000000000000000" ]]; then - SKIP=false - fi -done - -if $SKIP; then - echo "Skipping tidy check for branch deletion" - exit 0 -fi - -ROOT_DIR="$(git rev-parse --show-toplevel)" - -echo "Running pre-push script $ROOT_DIR/x test tidy" - -cd "$ROOT_DIR" -# The env var is necessary for printing diffs in py (fmt/lint) and cpp. -TIDY_PRINT_DIFF=1 ./x test tidy \ - --set build.locked-deps=true \ - --extra-checks auto:py,auto:cpp,auto:js,spellcheck -if [ $? -ne 0 ]; then - echo "You may use \`git push --no-verify\` to skip this check." - exit 1 -fi From 58e43474c6fb136d30a987a0527b084331b6ed8e Mon Sep 17 00:00:00 2001 From: Shunpoco Date: Sat, 20 Dec 2025 16:10:11 +0000 Subject: [PATCH 7/7] fix --- src/tools/tidy/src/extra_checks/mod.rs | 68 +++++++++++++------------- src/tools/tidy/src/lib.rs | 27 ++++++---- src/tools/tidy/src/spellcheck.rs | 12 +++-- 3 files changed, 61 insertions(+), 46 deletions(-) diff --git a/src/tools/tidy/src/extra_checks/mod.rs b/src/tools/tidy/src/extra_checks/mod.rs index 80205b6906049..34a81e5156832 100644 --- a/src/tools/tidy/src/extra_checks/mod.rs +++ b/src/tools/tidy/src/extra_checks/mod.rs @@ -78,38 +78,6 @@ pub fn check( } } -pub fn has_spellcheck(extra_checks: Option<&str>) -> bool { - let lint_args = parse_extra_checks(extra_checks); - lint_args.iter().any(|arg| arg.matches(ExtraCheckLang::Spellcheck, ExtraCheckKind::None)) -} - -pub fn parse_extra_checks(extra_checks: Option<&str>) -> Vec { - match extra_checks { - Some(s) => s - .strip_prefix("--extra-checks=") - .unwrap() - .split(',') - .map(|s| { - if s == "spellcheck:fix" { - eprintln!("warning: `spellcheck:fix` is no longer valid, use `--extra-checks=spellcheck --bless`"); - } - (ExtraCheckArg::from_str(s), s) - }) - .filter_map(|(res, src)| match res { - Ok(arg) => { - Some(arg) - } - Err(err) => { - // only warn because before bad extra checks would be silently ignored. - eprintln!("warning: bad extra check argument {src:?}: {err:?}"); - None - } - }) - .collect(), - None => vec![], - } -} - fn check_impl( root_path: &Path, outdir: &Path, @@ -365,6 +333,38 @@ fn check_impl( Ok(()) } +fn parse_extra_checks(extra_checks: Option<&str>) -> Vec { + match extra_checks { + Some(s) => s + .strip_prefix("--extra-checks=") + .unwrap() + .split(',') + .map(|s| { + if s == "spellcheck:fix" { + eprintln!("warning: `spellcheck:fix` is no longer valid, use `--extra-checks=spellcheck --bless`"); + } + (ExtraCheckArg::from_str(s), s) + }) + .filter_map(|(res, src)| match res { + Ok(arg) => { + Some(arg) + } + Err(err) => { + // only warn because before bad extra checks would be silently ignored. + eprintln!("warning: bad extra check argument {src:?}: {err:?}"); + None + } + }) + .collect(), + None => vec![], + } +} + +pub fn has_spellcheck(extra_checks: Option<&str>) -> bool { + let lint_args = parse_extra_checks(extra_checks); + lint_args.iter().any(|arg| arg.matches(ExtraCheckLang::Spellcheck, ExtraCheckKind::None)) +} + fn run_ruff( root_path: &Path, outdir: &Path, @@ -750,7 +750,7 @@ impl From for Error { } #[derive(Debug)] -pub enum ExtraCheckParseError { +enum ExtraCheckParseError { #[allow(dead_code, reason = "shown through Debug")] UnknownKind(String), #[allow(dead_code)] @@ -764,7 +764,7 @@ pub enum ExtraCheckParseError { AutoRequiresLang, } -pub struct ExtraCheckArg { +struct ExtraCheckArg { auto: bool, lang: ExtraCheckLang, /// None = run all extra checks for the given lang diff --git a/src/tools/tidy/src/lib.rs b/src/tools/tidy/src/lib.rs index 0b24e9b6ff288..0eb5d33559fec 100644 --- a/src/tools/tidy/src/lib.rs +++ b/src/tools/tidy/src/lib.rs @@ -158,21 +158,30 @@ pub fn files_modified(ci_info: &CiInfo, pred: impl Fn(&str) -> bool) -> bool { !v.is_empty() } -/// Check if the given executable is installed with the given version. +/// Check if the given executable is installed, and the cersion is expected. pub fn ensure_version(build_dir: &Path, bin_name: &str, version: &str) -> io::Result { let tool_root_dir = build_dir.join("misc-tools"); let tool_bin_dir = tool_root_dir.join("bin"); let bin_path = tool_bin_dir.join(bin_name); - if let Ok(output) = Command::new(&bin_path).arg("--version").output() - && let Ok(s) = str::from_utf8(&output.stdout) - && let Some(v) = s.trim().split_whitespace().last() - && v == version - { - return Ok(bin_path); - } + match Command::new(&bin_path).arg("--version").output() { + Ok(output) => { + let Some(v) = str::from_utf8(&output.stdout).unwrap().trim().split_whitespace().last() + else { + return Err(io::Error::other("version check failed")); + }; + + if v != version { + eprintln!( + "warning: the tool {bin_name} is detected, but version {v} doesn't match with the expected version {version}" + ); - Err(io::Error::other("no tool")) + return Err(io::Error::other("version is not expected")); + } + Ok(bin_path) + } + Err(e) => Err(e), + } } /// If the given executable is installed with the given version, use that, diff --git a/src/tools/tidy/src/spellcheck.rs b/src/tools/tidy/src/spellcheck.rs index 0742f7280b945..f5bf718bf37f5 100644 --- a/src/tools/tidy/src/spellcheck.rs +++ b/src/tools/tidy/src/spellcheck.rs @@ -17,13 +17,19 @@ pub fn check(root_path: &Path, outdir: &Path, tidy_ctx: TidyCtx) { match crate::ensure_version(outdir, SPELLCHECK_BIN_NAME, SPELLCHECK_VERSION) { Ok(bin_path) => { - eprintln!("Detect spellchecker, run it..."); + eprintln!("Spellchecker is detected. Run it automatially"); if let Err(e) = spellcheck_runner(root_path, &bin_path, args) { check.error(e); } } - Err(_) => { - // If ensure_version returns an error, do nothing. + Err(e) => { + // If the tool is not found, do nothing. + if e.kind() != std::io::ErrorKind::NotFound { + eprintln!( + "error: spellchecker has a problem. --extra-check=specllcheck may solve the problem." + ); + check.error(e); + } } } }