From 4d697d243302066fcb5cb6a9daacf7e6cdc891c7 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 10 Dec 2025 15:16:41 +0100 Subject: [PATCH] Remove "tidy" tool for `tests/rustdoc` testsuite --- src/tools/compiletest/src/common.rs | 9 - src/tools/compiletest/src/executor.rs | 9 - src/tools/compiletest/src/lib.rs | 28 +-- src/tools/compiletest/src/runtest.rs | 164 +----------------- .../compiletest/src/runtest/compute_diff.rs | 57 ------ src/tools/compiletest/src/runtest/rustdoc.rs | 4 +- src/tools/compiletest/src/rustdoc_gui_test.rs | 2 - 7 files changed, 8 insertions(+), 265 deletions(-) diff --git a/src/tools/compiletest/src/common.rs b/src/tools/compiletest/src/common.rs index 02b93593cbbd4..ddbe8601de204 100644 --- a/src/tools/compiletest/src/common.rs +++ b/src/tools/compiletest/src/common.rs @@ -9,7 +9,6 @@ use camino::{Utf8Path, Utf8PathBuf}; use semver::Version; use crate::edition::Edition; -use crate::executor::ColorConfig; use crate::fatal; use crate::util::{Utf8PathBufExt, add_dylib_path, string_enum}; @@ -597,11 +596,6 @@ pub struct Config { /// FIXME: this is *way* too coarse; the user can't select *which* info to verbosely dump. pub verbose: bool, - /// Whether to use colors in test output. - /// - /// Note: the exact control mechanism is delegated to [`colored`]. - pub color: ColorConfig, - /// Where to find the remote test client process, if we're using it. /// /// Note: this is *only* used for target platform executables created by `run-make` test @@ -623,9 +617,6 @@ pub struct Config { /// created in `$test_suite_build_root/rustfix_missing_coverage.txt` pub rustfix_coverage: bool, - /// Whether to run `tidy` (html-tidy) when a rustdoc test fails. - pub has_html_tidy: bool, - /// Whether to run `enzyme` autodiff tests. pub has_enzyme: bool, diff --git a/src/tools/compiletest/src/executor.rs b/src/tools/compiletest/src/executor.rs index 4dd4b1f85aaa7..c800d11d6b2fd 100644 --- a/src/tools/compiletest/src/executor.rs +++ b/src/tools/compiletest/src/executor.rs @@ -341,15 +341,6 @@ pub(crate) struct CollectedTestDesc { pub(crate) should_fail: ShouldFail, } -/// Whether console output should be colored or not. -#[derive(Copy, Clone, Default, Debug)] -pub enum ColorConfig { - #[default] - AutoColor, - AlwaysColor, - NeverColor, -} - /// Tests with `//@ should-fail` are tests of compiletest itself, and should /// be reported as successful if and only if they would have _failed_. #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] diff --git a/src/tools/compiletest/src/lib.rs b/src/tools/compiletest/src/lib.rs index ff4cd81d33ff6..7c237102ed847 100644 --- a/src/tools/compiletest/src/lib.rs +++ b/src/tools/compiletest/src/lib.rs @@ -24,7 +24,6 @@ use core::panic; use std::collections::HashSet; use std::fmt::Write; use std::io::{self, ErrorKind}; -use std::process::{Command, Stdio}; use std::sync::{Arc, OnceLock}; use std::time::SystemTime; use std::{env, fs, vec}; @@ -43,7 +42,7 @@ use crate::common::{ }; use crate::directives::{AuxProps, DirectivesCache, FileDirectives}; use crate::edition::parse_edition; -use crate::executor::{CollectedTest, ColorConfig}; +use crate::executor::CollectedTest; /// Creates the `Config` instance for this invocation of compiletest. /// @@ -136,8 +135,9 @@ fn parse_config(args: Vec) -> Config { "overwrite stderr/stdout files instead of complaining about a mismatch", ) .optflag("", "fail-fast", "stop as soon as possible after any test fails") - .optopt("", "color", "coloring: auto, always, never", "WHEN") .optopt("", "target", "the target to build for", "TARGET") + // FIXME: Should be removed once `bootstrap` will be updated to not use this option. + .optopt("", "color", "coloring: auto, always, never", "WHEN") .optopt("", "host", "the host to build for", "HOST") .optopt("", "cdb", "path to CDB to use for CDB debuginfo tests", "PATH") .optopt("", "gdb", "path to GDB to use for GDB debuginfo tests", "PATH") @@ -276,12 +276,6 @@ fn parse_config(args: Vec) -> Config { let lldb = matches.opt_str("lldb").map(Utf8PathBuf::from); let lldb_version = matches.opt_str("lldb-version").as_deref().and_then(debuggers::extract_lldb_version); - let color = match matches.opt_str("color").as_deref() { - Some("auto") | None => ColorConfig::AutoColor, - Some("always") => ColorConfig::AlwaysColor, - Some("never") => ColorConfig::NeverColor, - Some(x) => panic!("argument for --color must be auto, always, or never, but found `{}`", x), - }; // FIXME: this is very questionable, we really should be obtaining LLVM version info from // `bootstrap`, and not trying to be figuring out that in `compiletest` by running the // `FileCheck` binary. @@ -306,16 +300,6 @@ fn parse_config(args: Vec) -> Config { let with_rustc_debug_assertions = matches.opt_present("with-rustc-debug-assertions"); let with_std_debug_assertions = matches.opt_present("with-std-debug-assertions"); let mode = matches.opt_str("mode").unwrap().parse().expect("invalid mode"); - let has_html_tidy = if mode == TestMode::Rustdoc { - Command::new("tidy") - .arg("--version") - .stdout(Stdio::null()) - .status() - .map_or(false, |status| status.success()) - } else { - // Avoid spawning an external command when we know html-tidy won't be used. - false - }; let has_enzyme = matches.opt_present("has-enzyme"); let filters = if mode == TestMode::RunMake { matches @@ -457,11 +441,9 @@ fn parse_config(args: Vec) -> Config { adb_device_status, verbose: matches.opt_present("verbose"), only_modified: matches.opt_present("only-modified"), - color, remote_test_client: matches.opt_str("remote-test-client").map(Utf8PathBuf::from), compare_mode, rustfix_coverage: matches.opt_present("rustfix-coverage"), - has_html_tidy, has_enzyme, channel: matches.opt_str("channel").unwrap(), git_hash: matches.opt_present("git-hash"), @@ -1146,10 +1128,6 @@ fn check_for_overlapping_test_paths(found_path_stems: &HashSet) { } fn early_config_check(config: &Config) { - if !config.has_html_tidy && config.mode == TestMode::Rustdoc { - warning!("`tidy` (html-tidy.org) is not installed; diffs will not be generated"); - } - if !config.profiler_runtime && config.mode == TestMode::CoverageRun { let actioned = if config.bless { "blessed" } else { "checked" }; warning!("profiler runtime is not available, so `.coverage` files won't be {actioned}"); diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 54d6e0190ddca..f28f70ed453ea 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -1,12 +1,11 @@ use std::borrow::Cow; use std::collections::{HashMap, HashSet}; use std::ffi::OsString; -use std::fs::{self, File, create_dir_all}; +use std::fs::{self, create_dir_all}; use std::hash::{DefaultHasher, Hash, Hasher}; use std::io::prelude::*; -use std::io::{self, BufReader}; use std::process::{Child, Command, ExitStatus, Output, Stdio}; -use std::{env, fmt, iter, str}; +use std::{env, fmt, io, iter, str}; use build_helper::fs::remove_and_create_dir_all; use camino::{Utf8Path, Utf8PathBuf}; @@ -23,9 +22,9 @@ use crate::directives::TestProps; use crate::errors::{Error, ErrorKind, load_errors}; use crate::output_capture::ConsoleOut; use crate::read2::{Truncated, read2_abbreviated}; -use crate::runtest::compute_diff::{DiffLine, make_diff, write_diff, write_filtered_diff}; +use crate::runtest::compute_diff::{DiffLine, make_diff, write_diff}; use crate::util::{Utf8PathBufExt, add_dylib_path, static_regex}; -use crate::{ColorConfig, help, json, stamp_file_path, warning}; +use crate::{json, stamp_file_path}; // Helper modules that implement test running logic for each test suite. // tidy-alphabetical-start @@ -2162,161 +2161,6 @@ impl<'test> TestCx<'test> { if cfg!(target_os = "freebsd") { "ISO-8859-1" } else { "UTF-8" } } - fn compare_to_default_rustdoc(&self, out_dir: &Utf8Path) { - if !self.config.has_html_tidy { - return; - } - writeln!(self.stdout, "info: generating a diff against nightly rustdoc"); - - let suffix = - self.safe_revision().map_or("nightly".into(), |path| path.to_owned() + "-nightly"); - let compare_dir = output_base_dir(self.config, self.testpaths, Some(&suffix)); - remove_and_create_dir_all(&compare_dir).unwrap_or_else(|e| { - panic!("failed to remove and recreate output directory `{compare_dir}`: {e}") - }); - - // We need to create a new struct for the lifetimes on `config` to work. - let new_rustdoc = TestCx { - config: &Config { - // FIXME: use beta or a user-specified rustdoc instead of - // hardcoding the default toolchain - rustdoc_path: Some("rustdoc".into()), - // Needed for building auxiliary docs below - rustc_path: "rustc".into(), - ..self.config.clone() - }, - ..*self - }; - - let output_file = TargetLocation::ThisDirectory(new_rustdoc.aux_output_dir_name()); - let mut rustc = new_rustdoc.make_compile_args( - &new_rustdoc.testpaths.file, - output_file, - Emit::None, - AllowUnused::Yes, - LinkToAux::Yes, - Vec::new(), - ); - let aux_dir = new_rustdoc.aux_output_dir(); - new_rustdoc.build_all_auxiliary(&aux_dir, &mut rustc); - - let proc_res = new_rustdoc.document(&compare_dir, DocKind::Html); - if !proc_res.status.success() { - writeln!(self.stderr, "failed to run nightly rustdoc"); - return; - } - - #[rustfmt::skip] - let tidy_args = [ - "--new-blocklevel-tags", "rustdoc-search,rustdoc-toolbar,rustdoc-topbar", - "--indent", "yes", - "--indent-spaces", "2", - "--wrap", "0", - "--show-warnings", "no", - "--markup", "yes", - "--quiet", "yes", - "-modify", - ]; - let tidy_dir = |dir| { - for entry in walkdir::WalkDir::new(dir) { - let entry = entry.expect("failed to read file"); - if entry.file_type().is_file() - && entry.path().extension().and_then(|p| p.to_str()) == Some("html") - { - let status = - Command::new("tidy").args(&tidy_args).arg(entry.path()).status().unwrap(); - // `tidy` returns 1 if it modified the file. - assert!(status.success() || status.code() == Some(1)); - } - } - }; - tidy_dir(out_dir); - tidy_dir(&compare_dir); - - let pager = { - let output = Command::new("git").args(&["config", "--get", "core.pager"]).output().ok(); - output.and_then(|out| { - if out.status.success() { - Some(String::from_utf8(out.stdout).expect("invalid UTF8 in git pager")) - } else { - None - } - }) - }; - - let diff_filename = format!("build/tmp/rustdoc-compare-{}.diff", std::process::id()); - - if !write_filtered_diff( - self, - &diff_filename, - out_dir, - &compare_dir, - self.config.verbose, - |file_type, extension| { - file_type.is_file() && (extension == Some("html") || extension == Some("js")) - }, - ) { - return; - } - - match self.config.color { - ColorConfig::AlwaysColor => colored::control::set_override(true), - ColorConfig::NeverColor => colored::control::set_override(false), - _ => {} - } - - if let Some(pager) = pager { - let pager = pager.trim(); - if self.config.verbose { - writeln!(self.stderr, "using pager {}", pager); - } - let output = Command::new(pager) - // disable paging; we want this to be non-interactive - .env("PAGER", "") - .stdin(File::open(&diff_filename).unwrap()) - // Capture output and print it explicitly so it will in turn be - // captured by output-capture. - .output() - .unwrap(); - assert!(output.status.success()); - writeln!(self.stdout, "{}", String::from_utf8_lossy(&output.stdout)); - writeln!(self.stderr, "{}", String::from_utf8_lossy(&output.stderr)); - } else { - warning!("no pager configured, falling back to unified diff"); - help!( - "try configuring a git pager (e.g. `delta`) with \ - `git config --global core.pager delta`" - ); - let mut out = io::stdout(); - let mut diff = BufReader::new(File::open(&diff_filename).unwrap()); - let mut line = Vec::new(); - loop { - line.truncate(0); - match diff.read_until(b'\n', &mut line) { - Ok(0) => break, - Ok(_) => {} - Err(e) => writeln!(self.stderr, "ERROR: {:?}", e), - } - match String::from_utf8(line.clone()) { - Ok(line) => { - if line.starts_with('+') { - write!(&mut out, "{}", line.green()).unwrap(); - } else if line.starts_with('-') { - write!(&mut out, "{}", line.red()).unwrap(); - } else if line.starts_with('@') { - write!(&mut out, "{}", line.blue()).unwrap(); - } else { - out.write_all(line.as_bytes()).unwrap(); - } - } - Err(_) => { - write!(&mut out, "{}", String::from_utf8_lossy(&line).reversed()).unwrap(); - } - } - } - }; - } - fn get_lines(&self, path: &Utf8Path, mut other_files: Option<&mut Vec>) -> Vec { let content = fs::read_to_string(path.as_std_path()).unwrap(); let mut ignore = false; diff --git a/src/tools/compiletest/src/runtest/compute_diff.rs b/src/tools/compiletest/src/runtest/compute_diff.rs index 3363127b3ea37..8418dcbaf9639 100644 --- a/src/tools/compiletest/src/runtest/compute_diff.rs +++ b/src/tools/compiletest/src/runtest/compute_diff.rs @@ -1,9 +1,4 @@ use std::collections::VecDeque; -use std::fs::{File, FileType}; - -use camino::Utf8Path; - -use crate::runtest::TestCx; #[derive(Debug, PartialEq)] pub enum DiffLine { @@ -109,55 +104,3 @@ pub(crate) fn write_diff(expected: &str, actual: &str, context_size: usize) -> S } output } - -/// Filters based on filetype and extension whether to diff a file. -/// -/// Returns whether any data was actually written. -pub(crate) fn write_filtered_diff( - cx: &TestCx<'_>, - diff_filename: &str, - out_dir: &Utf8Path, - compare_dir: &Utf8Path, - verbose: bool, - filter: Filter, -) -> bool -where - Filter: Fn(FileType, Option<&str>) -> bool, -{ - use std::io::{Read, Write}; - let mut diff_output = File::create(diff_filename).unwrap(); - let mut wrote_data = false; - for entry in walkdir::WalkDir::new(out_dir.as_std_path()) { - let entry = entry.expect("failed to read file"); - let extension = entry.path().extension().and_then(|p| p.to_str()); - if filter(entry.file_type(), extension) { - let expected_path = compare_dir - .as_std_path() - .join(entry.path().strip_prefix(&out_dir.as_std_path()).unwrap()); - let expected = if let Ok(s) = std::fs::read(&expected_path) { s } else { continue }; - let actual_path = entry.path(); - let actual = std::fs::read(&actual_path).unwrap(); - let diff = unified_diff::diff( - &expected, - &expected_path.to_str().unwrap(), - &actual, - &actual_path.to_str().unwrap(), - 3, - ); - wrote_data |= !diff.is_empty(); - diff_output.write_all(&diff).unwrap(); - } - } - - if !wrote_data { - writeln!(cx.stdout, "note: diff is identical to nightly rustdoc"); - assert!(diff_output.metadata().unwrap().len() == 0); - return false; - } else if verbose { - writeln!(cx.stderr, "printing diff:"); - let mut buf = Vec::new(); - diff_output.read_to_end(&mut buf).unwrap(); - std::io::stderr().lock().write_all(&mut buf).unwrap(); - } - true -} diff --git a/src/tools/compiletest/src/runtest/rustdoc.rs b/src/tools/compiletest/src/runtest/rustdoc.rs index a4558de5f1c0c..3c80521e51ecd 100644 --- a/src/tools/compiletest/src/runtest/rustdoc.rs +++ b/src/tools/compiletest/src/runtest/rustdoc.rs @@ -28,9 +28,7 @@ impl TestCx<'_> { } let res = self.run_command_to_procres(&mut cmd); if !res.status.success() { - self.fatal_proc_rec_general("htmldocck failed!", None, &res, || { - self.compare_to_default_rustdoc(&out_dir); - }); + self.fatal_proc_rec("htmldocck failed!", &res); } } } diff --git a/src/tools/compiletest/src/rustdoc_gui_test.rs b/src/tools/compiletest/src/rustdoc_gui_test.rs index f6d026ab9cfd6..c30f3d1e10ea8 100644 --- a/src/tools/compiletest/src/rustdoc_gui_test.rs +++ b/src/tools/compiletest/src/rustdoc_gui_test.rs @@ -108,11 +108,9 @@ fn incomplete_config_for_rustdoc_gui_test() -> Config { adb_test_dir: Default::default(), adb_device_status: Default::default(), verbose: Default::default(), - color: Default::default(), remote_test_client: Default::default(), compare_mode: Default::default(), rustfix_coverage: Default::default(), - has_html_tidy: Default::default(), has_enzyme: Default::default(), channel: Default::default(), git_hash: Default::default(),