implemented custom differ

Author: Orion Gonzalez

config.example.toml

@@ -419,6 +419,9 @@
 # passed to cargo invocations.
 #jobs = 0
 
+# What custom diff tool to use for displaying compiletest tests.
+#display-diff-tool = "difft --color=always --background=light --display=side-by-side"
+
 # =============================================================================
 # General install configuration options
 # =============================================================================

src/bootstrap/src/core/build_steps/test.rs

@@ -1834,6 +1834,9 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the
         if builder.config.cmd.only_modified() {
             cmd.arg("--only-modified");
         }
+        if let Some(display_diff_tool) = &builder.config.display_diff_tool {
+            cmd.arg("--display-diff-tool").arg(display_diff_tool);
+        }
 
         let mut flags = if is_rustdoc { Vec::new() } else { vec!["-Crpath".to_string()] };
         flags.push(format!("-Cdebuginfo={}", builder.config.rust_debuginfo_level_tests));

src/bootstrap/src/core/config/config.rs

@@ -368,6 +368,9 @@ pub struct Config {
     /// The paths to work with. For example: with `./x check foo bar` we get
     /// `paths=["foo", "bar"]`.
     pub paths: Vec<PathBuf>,
+
+    /// What custom diff tool to use for displaying compiletest tests.
+    pub display_diff_tool: Option<String>,
 }
 
 #[derive(Clone, Debug, Default)]
@@ -892,6 +895,7 @@ define_config! {
         android_ndk: Option<PathBuf> = "android-ndk",
         optimized_compiler_builtins: Option<bool> = "optimized-compiler-builtins",
         jobs: Option<u32> = "jobs",
+        display_diff_tool: Option<String> = "display-diff-tool",
     }
 }
 
@@ -1512,6 +1516,7 @@ impl Config {
             android_ndk,
             optimized_compiler_builtins,
             jobs,
+            display_diff_tool,
         } = toml.build.unwrap_or_default();
 
         config.jobs = Some(threads_from_config(flags.jobs.unwrap_or(jobs.unwrap_or(0))));
@@ -2158,6 +2163,7 @@ impl Config {
         config.rust_debuginfo_level_tests = debuginfo_level_tests.unwrap_or(DebuginfoLevel::None);
         config.optimized_compiler_builtins =
             optimized_compiler_builtins.unwrap_or(config.channel != "dev");
+        config.display_diff_tool = display_diff_tool;
 
         let download_rustc = config.download_rustc_commit.is_some();
         // See https://github.com/rust-lang/compiler-team/issues/326

src/tools/compiletest/src/lib.rs

@@ -175,7 +175,9 @@ pub fn parse_config(args: Vec<String>) -> Config {
             "git-merge-commit-email",
             "email address used for finding merge commits",
             "EMAIL",
-        );
+        )
+        .optopt("", "display-diff-tool", "What custom diff tool to use for displaying compiletest tests.", "COMMAND")
+    ;
 
     let (argv0, args_) = args.split_first().unwrap();
     if args.len() == 1 || args[1] == "-h" || args[1] == "--help" {
@@ -364,7 +366,7 @@ pub fn parse_config(args: Vec<String>) -> Config {
         git_merge_commit_email: matches.opt_str("git-merge-commit-email").unwrap(),
 
         profiler_runtime: matches.opt_present("profiler-runtime"),
-        diff_command: env::var("COMPILETEST_DIFF_TOOL").ok(),
+        diff_command: matches.opt_str("display-diff-tool"),
     }
 }
 

src/tools/compiletest/src/runtest.rs

@@ -2459,7 +2459,7 @@ impl<'test> TestCx<'test> {
         }
     }
 
-    fn compare_output(&self, kind: &str, actual: &str, expected: &str) -> usize {
+    fn compare_output(&self, stream: &str, actual: &str, expected: &str) -> usize {
         let are_different = match (self.force_color_svg(), expected.find('\n'), actual.find('\n')) {
             // FIXME: We ignore the first line of SVG files
             // because the width parameter is non-deterministic.
@@ -2499,56 +2499,66 @@ impl<'test> TestCx<'test> {
             (expected, actual)
         };
 
+        // Write the actual output to a file in build/
+        let test_name = self.config.compare_mode.as_ref().map_or("", |m| m.to_str());
+        let actual_path = self
+            .output_base_name()
+            .with_extra_extension(self.revision.unwrap_or(""))
+            .with_extra_extension(test_name)
+            .with_extra_extension(stream);
+
+        if let Err(err) = fs::write(&actual_path, &actual) {
+            self.fatal(&format!("failed to write {stream} to `{actual_path:?}`: {err}",));
+        }
+        println!("Saved the actual {stream} to {actual_path:?}");
+
+        let expected_path =
+            expected_output_path(self.testpaths, self.revision, &self.config.compare_mode, stream);
+
         if !self.config.bless {
             if expected.is_empty() {
-                println!("normalized {}:\n{}\n", kind, actual);
+                println!("normalized {}:\n{}\n", stream, actual);
             } else {
-                println!("diff of {}:\n", kind);
-                print!("{}", write_diff(expected, actual, 3));
+                println!("diff of {stream}:\n");
+                if let Some(diff_command) = self.config.diff_command.as_deref() {
+                    let mut args = diff_command.split_whitespace();
+                    let name = args.next().unwrap();
+                    match Command::new(name)
+                        .args(args)
+                        .args([&expected_path, &actual_path])
+                        .output()
+                    {
+                        Err(err) => {
+                            self.fatal(&format!(
+                                "failed to call custom diff command `{diff_command}`: {err}"
+                            ));
+                        }
+                        Ok(output) => {
+                            let output = String::from_utf8_lossy_owned(output.stdout).unwrap();
+                            print!("{output}");
+                        }
+                    }
+                } else {
+                    print!("{}", write_diff(expected, actual, 3));
+                }
             }
-        }
-
-        let mode = self.config.compare_mode.as_ref().map_or("", |m| m.to_str());
-        let output_file = self
-            .output_base_name()
-            .with_extra_extension(self.revision.unwrap_or(""))
-            .with_extra_extension(mode)
-            .with_extra_extension(kind);
-
-        let mut files = vec![output_file];
-        if self.config.bless {
+        } else {
             // Delete non-revision .stderr/.stdout file if revisions are used.
             // Without this, we'd just generate the new files and leave the old files around.
             if self.revision.is_some() {
                 let old =
-                    expected_output_path(self.testpaths, None, &self.config.compare_mode, kind);
+                    expected_output_path(self.testpaths, None, &self.config.compare_mode, stream);
                 self.delete_file(&old);
             }
-            files.push(expected_output_path(
-                self.testpaths,
-                self.revision,
-                &self.config.compare_mode,
-                kind,
-            ));
-        }
 
-        for output_file in &files {
-            if actual.is_empty() {
-                self.delete_file(output_file);
-            } else if let Err(err) = fs::write(&output_file, &actual) {
-                self.fatal(&format!(
-                    "failed to write {} to `{}`: {}",
-                    kind,
-                    output_file.display(),
-                    err,
-                ));
+            if let Err(err) = fs::write(&expected_path, &actual) {
+                self.fatal(&format!("failed to write {stream} to `{expected_path:?}`: {err}"));
             }
+            println!("Blessing the {stream} of {test_name} in {expected_path:?}");
         }
 
-        println!("\nThe actual {0} differed from the expected {0}.", kind);
-        for output_file in files {
-            println!("Actual {} saved to {}", kind, output_file.display());
-        }
+        println!("\nThe actual {0} differed from the expected {0}.", stream);
+
         if self.config.bless { 0 } else { 1 }
     }