Unify usages of path modifications and log them in verbose mode

Kobzol · Kobzol · commit 1f5320e57bae · 2025-04-20T09:13:55.000+02:00
diff --git a/src/bootstrap/src/core/build_steps/gcc.rs b/src/bootstrap/src/core/build_steps/gcc.rs
@@ -110,6 +110,9 @@ fn try_download_gcc(builder: &Builder<'_>, target: TargetSelection) -> Option<Pa
         &builder.config,
         builder.config.rust_info.is_managed_git_subrepository(),
     );
+    builder.verbose(|| {
+        eprintln!("GCC freshness: {source:?}");
+    });
     match source {
         PathFreshness::LastModifiedUpstream { upstream } => {
             // Download from upstream CI
@@ -285,31 +288,17 @@ fn ci_gcc_root(config: &crate::Config) -> PathBuf {
 /// Detect whether GCC sources have been modified locally or not.
 #[cfg(not(test))]
 fn detect_gcc_freshness(config: &crate::Config, is_git: bool) -> build_helper::git::PathFreshness {
-    use build_helper::git::{PathFreshness, check_path_modifications};
-
-    let freshness = if is_git {
-        Some(
-            check_path_modifications(
-                Some(&config.src),
-                &config.git_config(),
-                &["src/gcc", "src/bootstrap/download-ci-gcc-stamp"],
-                config.ci_env(),
-            )
-            .unwrap(),
-        )
+    use build_helper::git::PathFreshness;
+
+    if is_git {
+        config.check_path_modifications(&["src/gcc", "src/bootstrap/download-ci-gcc-stamp"])
     } else if let Some(info) = crate::utils::channel::read_commit_info_file(&config.src) {
-        Some(PathFreshness::LastModifiedUpstream { upstream: info.sha.trim().to_owned() })
+        PathFreshness::LastModifiedUpstream { upstream: info.sha.trim().to_owned() }
     } else {
-        None
-    };
-
-    let Some(freshness) = freshness else {
         eprintln!("error: could not find commit hash for downloading GCC");
         eprintln!("HELP: maybe your repository history is too shallow?");
         eprintln!("HELP: consider disabling `download-ci-gcc`");
         eprintln!("HELP: or fetch enough history to include one upstream commit");
-        panic!();
-    };
-
-    freshness
+        crate::exit!(1);
+    }
 }
diff --git a/src/bootstrap/src/core/build_steps/llvm.rs b/src/bootstrap/src/core/build_steps/llvm.rs
@@ -14,7 +14,7 @@ use std::path::{Path, PathBuf};
 use std::sync::OnceLock;
 use std::{env, fs};
 
-use build_helper::git::{PathFreshness, check_path_modifications};
+use build_helper::git::PathFreshness;
 #[cfg(feature = "tracing")]
 use tracing::instrument;
 
@@ -183,31 +183,17 @@ pub const LLVM_INVALIDATION_PATHS: &[&str] = &[
 
 /// Detect whether LLVM sources have been modified locally or not.
 pub(crate) fn detect_llvm_freshness(config: &Config, is_git: bool) -> PathFreshness {
-    let freshness = if is_git {
-        Some(
-            check_path_modifications(
-                Some(&config.src),
-                &config.git_config(),
-                LLVM_INVALIDATION_PATHS,
-                config.ci_env(),
-            )
-            .unwrap(),
-        )
+    if is_git {
+        config.check_path_modifications(LLVM_INVALIDATION_PATHS)
     } else if let Some(info) = crate::utils::channel::read_commit_info_file(&config.src) {
-        Some(PathFreshness::LastModifiedUpstream { upstream: info.sha.trim().to_owned() })
+        PathFreshness::LastModifiedUpstream { upstream: info.sha.trim().to_owned() }
     } else {
-        None
-    };
-
-    let Some(freshness) = freshness else {
         eprintln!("error: could not find commit hash for downloading LLVM");
         eprintln!("HELP: maybe your repository history is too shallow?");
         eprintln!("HELP: consider disabling `download-ci-llvm`");
         eprintln!("HELP: or fetch enough history to include one upstream commit");
-        panic!();
-    };
-
-    freshness
+        crate::exit!(1);
+    }
 }
 
 /// Returns whether the CI-found LLVM is currently usable.
diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs
@@ -3193,7 +3193,11 @@ impl Config {
         let commit = if self.rust_info.is_managed_git_subrepository() {
             // Look for a version to compare to based on the current commit.
             // Only commits merged by bors will have CI artifacts.
-            match self.check_modifications(&allowed_paths) {
+            let freshness = self.check_path_modifications(&allowed_paths);
+            self.verbose(|| {
+                eprintln!("rustc freshness: {freshness:?}");
+            });
+            match freshness {
                 PathFreshness::LastModifiedUpstream { upstream } => upstream,
                 PathFreshness::HasLocalModifications { upstream } => {
                     if if_unchanged {
@@ -3291,13 +3295,14 @@ impl Config {
 
     /// Returns true if any of the `paths` have been modified locally.
     pub fn has_changes_from_upstream(&self, paths: &[&str]) -> bool {
-        match self.check_modifications(paths) {
+        match self.check_path_modifications(paths) {
             PathFreshness::LastModifiedUpstream { .. } => false,
             PathFreshness::HasLocalModifications { .. } | PathFreshness::MissingUpstream => true,
         }
     }
 
-    fn check_modifications(&self, paths: &[&str]) -> PathFreshness {
+    /// Checks whether any of the given paths have been modified w.r.t. upstream.
+    pub fn check_path_modifications(&self, paths: &[&str]) -> PathFreshness {
         check_path_modifications(Some(&self.src), &self.git_config(), paths, CiEnv::current())
             .unwrap()
     }
diff --git a/src/bootstrap/src/core/config/tests.rs b/src/bootstrap/src/core/config/tests.rs
@@ -51,7 +51,7 @@ fn download_ci_llvm() {
 
     let if_unchanged_config = parse("llvm.download-ci-llvm = \"if-unchanged\"");
     if if_unchanged_config.llvm_from_ci && if_unchanged_config.is_running_on_ci {
-        let has_changes = if_unchanged_config.has_changes_from_upstream(&["src/llvm-project"]);
+        let has_changes = if_unchanged_config.has_changes_from_upstream(LLVM_INVALIDATION_PATHS);
 
         assert!(
             !has_changes,
diff --git a/src/bootstrap/src/core/download.rs b/src/bootstrap/src/core/download.rs
@@ -730,15 +730,19 @@ download-rustc = false
         }
 
         let llvm_root = self.ci_llvm_root();
-        let llvm_sha =
-            match detect_llvm_freshness(self, self.rust_info.is_managed_git_subrepository()) {
-                PathFreshness::LastModifiedUpstream { upstream } => upstream,
-                PathFreshness::HasLocalModifications { upstream } => upstream,
-                PathFreshness::MissingUpstream => {
-                    eprintln!("No upstream commit for downloading LLVM found");
-                    crate::exit!(1);
-                }
-            };
+        let llvm_freshness =
+            detect_llvm_freshness(self, self.rust_info.is_managed_git_subrepository());
+        self.verbose(|| {
+            eprintln!("LLVM freshness: {llvm_freshness:?}");
+        });
+        let llvm_sha = match llvm_freshness {
+            PathFreshness::LastModifiedUpstream { upstream } => upstream,
+            PathFreshness::HasLocalModifications { upstream } => upstream,
+            PathFreshness::MissingUpstream => {
+                eprintln!("No upstream commit for downloading LLVM found");
+                crate::exit!(1);
+            }
+        };
         let stamp_key = format!("{}{}", llvm_sha, self.llvm_assertions);
         let llvm_stamp = BuildStamp::new(&llvm_root).with_prefix("llvm").add_stamp(stamp_key);
         if !llvm_stamp.is_up_to_date() && !self.dry_run() {
diff --git a/src/build_helper/src/git/tests.rs b/src/build_helper/src/git/tests.rs
@@ -10,7 +10,7 @@ fn test_pr_ci_unchanged_anywhere() {
     git_test(|ctx| {
         let sha = ctx.create_upstream_merge(&["a"]);
         ctx.create_nonupstream_merge(&["b"]);
-        let src = ctx.get_source(&["c"], CiEnv::GitHubActions);
+        let src = ctx.check_modifications(&["c"], CiEnv::GitHubActions);
         assert_eq!(src, PathFreshness::LastModifiedUpstream { upstream: sha });
     });
 }
@@ -20,7 +20,7 @@ fn test_pr_ci_changed_in_pr() {
     git_test(|ctx| {
         let sha = ctx.create_upstream_merge(&["a"]);
         ctx.create_nonupstream_merge(&["b"]);
-        let src = ctx.get_source(&["b"], CiEnv::GitHubActions);
+        let src = ctx.check_modifications(&["b"], CiEnv::GitHubActions);
         assert_eq!(src, PathFreshness::HasLocalModifications { upstream: sha });
     });
 }
@@ -30,7 +30,7 @@ fn test_auto_ci_unchanged_anywhere_select_parent() {
     git_test(|ctx| {
         let sha = ctx.create_upstream_merge(&["a"]);
         ctx.create_upstream_merge(&["b"]);
-        let src = ctx.get_source(&["c"], CiEnv::GitHubActions);
+        let src = ctx.check_modifications(&["c"], CiEnv::GitHubActions);
         assert_eq!(src, PathFreshness::LastModifiedUpstream { upstream: sha });
     });
 }
@@ -40,7 +40,7 @@ fn test_auto_ci_changed_in_pr() {
     git_test(|ctx| {
         let sha = ctx.create_upstream_merge(&["a"]);
         ctx.create_upstream_merge(&["b", "c"]);
-        let src = ctx.get_source(&["c", "d"], CiEnv::GitHubActions);
+        let src = ctx.check_modifications(&["c", "d"], CiEnv::GitHubActions);
         assert_eq!(src, PathFreshness::HasLocalModifications { upstream: sha });
     });
 }
@@ -53,7 +53,7 @@ fn test_local_uncommitted_modifications() {
         ctx.modify("a");
 
         assert_eq!(
-            ctx.get_source(&["a", "d"], CiEnv::None),
+            ctx.check_modifications(&["a", "d"], CiEnv::None),
             PathFreshness::HasLocalModifications { upstream: sha }
         );
     });
@@ -71,7 +71,7 @@ fn test_local_committed_modifications() {
         ctx.commit();
 
         assert_eq!(
-            ctx.get_source(&["a", "d"], CiEnv::None),
+            ctx.check_modifications(&["a", "d"], CiEnv::None),
             PathFreshness::HasLocalModifications { upstream: sha }
         );
     });
@@ -87,7 +87,7 @@ fn test_local_committed_modifications_subdirectory() {
         ctx.commit();
 
         assert_eq!(
-            ctx.get_source(&["a/b"], CiEnv::None),
+            ctx.check_modifications(&["a/b"], CiEnv::None),
             PathFreshness::HasLocalModifications { upstream: sha }
         );
     });
@@ -100,7 +100,7 @@ fn test_local_changes_in_head_upstream() {
         // even if it is currently HEAD
         let sha = ctx.create_upstream_merge(&["a"]);
         assert_eq!(
-            ctx.get_source(&["a", "d"], CiEnv::None),
+            ctx.check_modifications(&["a", "d"], CiEnv::None),
             PathFreshness::LastModifiedUpstream { upstream: sha }
         );
     });
@@ -117,7 +117,7 @@ fn test_local_changes_in_previous_upstream() {
         ctx.modify("d");
         ctx.commit();
         assert_eq!(
-            ctx.get_source(&["a"], CiEnv::None),
+            ctx.check_modifications(&["a"], CiEnv::None),
             PathFreshness::LastModifiedUpstream { upstream: sha }
         );
     });
@@ -135,7 +135,7 @@ fn test_local_no_upstream_commit_with_changes() {
         ctx.modify("d");
         ctx.commit();
         assert_eq!(
-            ctx.get_source(&["x"], CiEnv::None),
+            ctx.check_modifications(&["x"], CiEnv::None),
             PathFreshness::LastModifiedUpstream { upstream: sha }
         );
     });
@@ -144,7 +144,7 @@ fn test_local_no_upstream_commit_with_changes() {
 #[test]
 fn test_local_no_upstream_commit() {
     git_test(|ctx| {
-        let src = ctx.get_source(&["c", "d"], CiEnv::None);
+        let src = ctx.check_modifications(&["c", "d"], CiEnv::None);
         assert_eq!(src, PathFreshness::MissingUpstream);
     });
 }
@@ -159,15 +159,15 @@ fn test_local_changes_negative_path() {
         ctx.commit();
 
         assert_eq!(
-            ctx.get_source(&[":!b", ":!d"], CiEnv::None),
+            ctx.check_modifications(&[":!b", ":!d"], CiEnv::None),
             PathFreshness::LastModifiedUpstream { upstream: upstream.clone() }
         );
         assert_eq!(
-            ctx.get_source(&[":!c"], CiEnv::None),
+            ctx.check_modifications(&[":!c"], CiEnv::None),
             PathFreshness::HasLocalModifications { upstream: upstream.clone() }
         );
         assert_eq!(
-            ctx.get_source(&[":!d", ":!x"], CiEnv::None),
+            ctx.check_modifications(&[":!d", ":!x"], CiEnv::None),
             PathFreshness::HasLocalModifications { upstream }
         );
     });
@@ -198,7 +198,7 @@ impl GitCtx {
         ctx
     }
 
-    fn get_source(&self, target_paths: &[&str], ci_env: CiEnv) -> PathFreshness {
+    fn check_modifications(&self, target_paths: &[&str], ci_env: CiEnv) -> PathFreshness {
         check_path_modifications(Some(self.dir.path()), &self.git_config(), target_paths, ci_env)
             .unwrap()
     }
