From 9b443be80c711ba6f69a16bf6fad3a3eab08da30 Mon Sep 17 00:00:00 2001 From: jyn Date: Mon, 29 Jun 2026 09:43:54 +0200 Subject: [PATCH 1/3] tidy: make it more clear that it has to be run under bootstrap, not as a standalone command - error if RUSTC isn't set, which means it'll fall back to the user-installed default (wrong) - be more specific in `--bless` suggestions that they should use `x test tidy`, not tidy directly. --- src/tools/tidy/src/deps.rs | 3 +++ src/tools/tidy/src/extra_checks/mod.rs | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/tools/tidy/src/deps.rs b/src/tools/tidy/src/deps.rs index 2b12e653f3748..6e34357794acc 100644 --- a/src/tools/tidy/src/deps.rs +++ b/src/tools/tidy/src/deps.rs @@ -683,6 +683,9 @@ pub fn check(root: &Path, cargo: &Path, tidy_ctx: TidyCtx) { /// Ensure the list of proc-macro crate transitive dependencies is up to date fn check_proc_macro_dep_list(root: &Path, cargo: &Path, bless: bool, check: &mut RunningCheck) { + if std::env::var("RUSTC").is_err() { + panic!("tidy must be run under bootstrap (./x test tidy), not as a standalone command"); + } let mut cmd = cargo_metadata::MetadataCommand::new(); cmd.cargo_path(cargo) .manifest_path(root.join("Cargo.toml")) diff --git a/src/tools/tidy/src/extra_checks/mod.rs b/src/tools/tidy/src/extra_checks/mod.rs index 008cc0322534a..0ee5b3534dd91 100644 --- a/src/tools/tidy/src/extra_checks/mod.rs +++ b/src/tools/tidy/src/extra_checks/mod.rs @@ -200,7 +200,9 @@ fn js_prepare(root_path: &Path, outdir: &Path, npm: &Path, tidy_ctx: &TidyCtx) - fn show_bless_help(mode: &str, action: &str, bless: bool) { if !bless { - eprintln!("rerun tidy with `--extra-checks={mode} --bless` to {action}"); + eprintln!( + "rerun with `--bless` to {action}: `./x.py test tidy --extra-checks={mode} --bless`" + ); } } From 264bf6d3048831540e314f5f4228501e34aace37 Mon Sep 17 00:00:00 2001 From: jyn Date: Mon, 29 Jun 2026 09:43:54 +0200 Subject: [PATCH 2/3] enable `TIDY_PRINT_DIFF` by default for non-interactive cases, such as CI, this makes it easier to tell what went wrong without having to interactively debug the problem. --- src/tools/tidy/src/extra_checks/mod.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/tools/tidy/src/extra_checks/mod.rs b/src/tools/tidy/src/extra_checks/mod.rs index 0ee5b3534dd91..75b02520a3d46 100644 --- a/src/tools/tidy/src/extra_checks/mod.rs +++ b/src/tools/tidy/src/extra_checks/mod.rs @@ -207,7 +207,9 @@ fn show_bless_help(mode: &str, action: &str, bless: bool) { } fn show_diff() -> bool { - std::env::var("TIDY_PRINT_DIFF").is_ok_and(|v| v.eq_ignore_ascii_case("true") || v == "1") + let disable_diff = + std::env::var("TIDY_PRINT_DIFF").is_ok_and(|v| v.eq_ignore_ascii_case("false") || v == "0"); + !disable_diff } fn check_spellcheck(root_path: &Path, outdir: &Path, cargo: &Path, tidy_ctx: &TidyCtx) { From c286fbb18d8a1407a221bae6e69f92d94db977a7 Mon Sep 17 00:00:00 2001 From: jyn Date: Fri, 3 Jul 2026 14:30:37 +0200 Subject: [PATCH 3/3] rip out TIDY_PRINT_DIFF and just always show diffs by default. not sure why you'd want to hide them. --- src/ci/docker/host-x86_64/tidy/Dockerfile | 3 +- src/etc/pre-push.sh | 3 +- src/tools/tidy/src/extra_checks/mod.rs | 37 +++++++++-------------- 3 files changed, 16 insertions(+), 27 deletions(-) diff --git a/src/ci/docker/host-x86_64/tidy/Dockerfile b/src/ci/docker/host-x86_64/tidy/Dockerfile index e84d61e0b99cc..22c947a3ca7ef 100644 --- a/src/ci/docker/host-x86_64/tidy/Dockerfile +++ b/src/ci/docker/host-x86_64/tidy/Dockerfile @@ -39,5 +39,4 @@ COPY host-x86_64/pr-check-1/validate-toolstate.sh /scripts/ # NOTE: intentionally uses python2 for x.py so we can test it still works. # validate-toolstate only runs in our CI, so it's ok for it to only support python3. -ENV SCRIPT="TIDY_PRINT_DIFF=1 python2.7 ../x.py test \ - src/tools/tidy tidyselftest --extra-checks=py,cpp,js,spellcheck" +ENV SCRIPT="python2.7 ../x.py test src/tools/tidy tidyselftest --extra-checks=py,cpp,js,spellcheck" diff --git a/src/etc/pre-push.sh b/src/etc/pre-push.sh index 210edbf0cda66..fb2007db9f12c 100755 --- a/src/etc/pre-push.sh +++ b/src/etc/pre-push.sh @@ -33,8 +33,7 @@ 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 \ +./x test tidy \ --set build.locked-deps=true \ --extra-checks auto:py,auto:cpp,auto:js if [ $? -ne 0 ]; then diff --git a/src/tools/tidy/src/extra_checks/mod.rs b/src/tools/tidy/src/extra_checks/mod.rs index 75b02520a3d46..d358858b24598 100644 --- a/src/tools/tidy/src/extra_checks/mod.rs +++ b/src/tools/tidy/src/extra_checks/mod.rs @@ -10,12 +10,11 @@ //! configuration after a double dash (`--extra-checks=py -- foo.py`) //! 2. Build configuration based on args/environment: //! - Formatters by default are in check only mode -//! - If in CI (TIDY_PRINT_DIFF=1 is set), check and print the diff //! - If `--bless` is provided, formatters may run //! - Pass any additional config after the `--`. If no files are specified, //! use a default. -//! 3. Print the output of the given command. If it fails and `TIDY_PRINT_DIFF` -//! is set, rerun the tool to print a suggestion diff (for e.g. CI) +//! 3. Print the output of the given command. If it fails, rerun the tool to print a suggestion +//! diff. use std::ffi::{OsStr, OsString}; use std::path::{Path, PathBuf}; @@ -206,12 +205,6 @@ fn show_bless_help(mode: &str, action: &str, bless: bool) { } } -fn show_diff() -> bool { - let disable_diff = - std::env::var("TIDY_PRINT_DIFF").is_ok_and(|v| v.eq_ignore_ascii_case("false") || v == "0"); - !disable_diff -} - fn check_spellcheck(root_path: &Path, outdir: &Path, cargo: &Path, tidy_ctx: &TidyCtx) { let mut check = tidy_ctx.start_check("extra_checks:spellcheck"); @@ -318,7 +311,7 @@ fn check_python_lint( let res = run_ruff(root_path, outdir, py_path, &cfg_args, &file_args, args); - if res.is_err() && !bless && show_diff() { + if res.is_err() && !bless { eprintln!("\npython linting failed! Printing diff suggestions:"); let diff_res = run_ruff( @@ -362,18 +355,16 @@ fn check_python_fmt( let res = run_ruff(root_path, outdir, py_path, &cfg_args, &file_args, &args); if res.is_err() && !bless { - if show_diff() { - eprintln!("\npython formatting does not match! Printing diff:"); - - let _ = run_ruff( - root_path, - outdir, - py_path, - &cfg_args, - &file_args, - &["format".as_ref(), "--diff".as_ref()], - ); - } + eprintln!("\npython formatting does not match! Printing diff:"); + + let _ = run_ruff( + root_path, + outdir, + py_path, + &cfg_args, + &file_args, + &["format".as_ref(), "--diff".as_ref()], + ); show_bless_help("py:fmt", "reformat Python code", bless); } @@ -425,7 +416,7 @@ fn check_cpp_fmt( let args = merge_args(&cfg_args_clang_format, &file_args_clang_format); let res = py_runner(py_path, false, None, "clang-format", &args); - if res.is_err() && !bless && show_diff() { + if res.is_err() && !bless { eprintln!("\nclang-format linting failed! Printing diff suggestions:"); let mut cfg_args_diff = cfg_args.clone();