Remove "tidy" tool for tests/rustdoc testsuite

Author: GuillaumeGomez

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,
 

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)]

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<String>) -> 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<String>) -> 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<String>) -> 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<String>) -> 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<Utf8PathBuf>) {
 }
 
 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}");

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<String>>) -> Vec<usize> {
         let content = fs::read_to_string(path.as_std_path()).unwrap();
         let mut ignore = false;

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<Filter>(
-    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
-}

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);
             }
         }
     }