Refactor change detection for rustdoc and download-rustc

Author: guliwa

src/bootstrap/src/core/build_steps/tool.rs

@@ -10,7 +10,7 @@ use crate::core::builder::{Builder, Cargo as CargoCommand, RunConfig, ShouldRun,
 use crate::core::config::TargetSelection;
 use crate::utils::channel::GitInfo;
 use crate::utils::exec::{BootstrapCommand, command};
-use crate::utils::helpers::{add_dylib_path, exe, git, t};
+use crate::utils::helpers::{add_dylib_path, exe, t};
 use crate::{Compiler, Kind, Mode, gha};
 
 #[derive(Debug, Clone, Hash, PartialEq, Eq)]
@@ -585,21 +585,12 @@ impl Step for Rustdoc {
             )
             .unwrap();
 
-            let librustdoc_src = builder.config.src.join("src/librustdoc");
-            let rustdoc_src = builder.config.src.join("src/tools/rustdoc");
-
-            // FIXME: The change detection logic here is quite similar to `Config::download_ci_rustc_commit`.
-            // It would be better to unify them.
-            let has_changes = !git(Some(&builder.config.src))
-                .allow_failure()
-                .run_always()
-                .args(["diff-index", "--quiet", &commit])
-                .arg("--")
-                .arg(librustdoc_src)
-                .arg(rustdoc_src)
-                .run(builder);
-
-            if !has_changes {
+            let dirs = vec![
+                PathBuf::from("src/librustdoc"),
+                PathBuf::from("src/tools/rustdoc"),
+            ];
+
+            if let Some(false) = builder.config.check_for_changes(&dirs, &commit, "rustdoc", true) {
                 let precompiled_rustdoc = builder
                     .config
                     .ci_rustc_dir()

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

@@ -2725,31 +2725,18 @@ impl Config {
             crate::exit!(1);
         }
 
-        // Warn if there were changes to the compiler or standard library since the ancestor commit.
-        let has_changes = !t!(helpers::git(Some(&self.src))
-            .args(["diff-index", "--quiet", &commit])
-            .arg("--")
-            .args([self.src.join("compiler"), self.src.join("library")])
-            .as_command_mut()
-            .status())
-        .success();
-        if has_changes {
-            if if_unchanged {
-                if self.verbose > 0 {
-                    println!(
-                        "WARNING: saw changes to compiler/ or library/ since {commit}; \
-                            ignoring `download-rustc`"
-                    );
-                }
-                return None;
+        let dirs = vec![PathBuf::from("compiler"), PathBuf::from("library")];
+        match self.check_for_changes(&dirs, &commit, "download-rustc", if_unchanged) {
+            Some(true) => {
+                println!(
+                    "WARNING: `download-rustc` is enabled, but there are changes to \
+                compiler/ or library/"
+                );
+                Some(commit.to_string())
             }
-            println!(
-                "WARNING: `download-rustc` is enabled, but there are changes to \
-                    compiler/ or library/"
-            );
+            Some(false) => Some(commit.to_string()),
+            None => None,
         }
-
-        Some(commit.to_string())
     }
 
     fn parse_download_ci_llvm(
@@ -2847,6 +2834,42 @@ impl Config {
 
         Some(commit.to_string())
     }
+
+    /// Check for changes in specified directories since a given commit.
+    /// Returns Some(true) if changes exist, Some(false) if no changes, None if check should be ignored.
+    pub fn check_for_changes(
+        &self,
+        dirs: &[PathBuf],
+        commit: &str,
+        option_name: &str,
+        if_unchanged: bool,
+    ) -> Option<bool> {
+        let mut git = helpers::git(Some(&self.src));
+        git.args(["diff-index", "--quiet", commit, "--"]);
+
+        for dir in dirs {
+            git.arg(self.src.join(dir));
+        }
+
+        let has_changes = !t!(git.as_command_mut().status()).success();
+
+        if has_changes {
+            if if_unchanged {
+                if self.verbose > 0 {
+                    println!(
+                        "WARNING: saw changes to one of {:?} since {}; \
+                        ignoring `{}`",
+                        dirs, commit, option_name
+                    );
+                }
+                None
+            } else {
+                Some(true)
+            }
+        } else {
+            Some(false)
+        }
+    }
 }
 
 /// Compares the current `Llvm` options against those in the CI LLVM builder and detects any incompatible options.