Migrate LLVM CI change detection to check_path_modifications

Author: Kobzol

src/bootstrap/src/core/build_steps/gcc.rs

@@ -12,12 +12,13 @@ use std::fs;
 use std::path::{Path, PathBuf};
 use std::sync::OnceLock;
 
+use build_helper::ci::CiEnv;
+
 use crate::core::builder::{Builder, Cargo, Kind, RunConfig, ShouldRun, Step};
 use crate::core::config::TargetSelection;
 use crate::utils::build_stamp::{BuildStamp, generate_smart_stamp_hash};
 use crate::utils::exec::command;
 use crate::utils::helpers::{self, t};
-use build_helper::ci::CiEnv;
 
 #[derive(Debug, Clone, Hash, PartialEq, Eq)]
 pub struct Gcc {
@@ -97,6 +98,8 @@ pub enum GccBuildStatus {
 /// Returns a path to the libgccjit.so file.
 #[cfg(not(test))]
 fn try_download_gcc(builder: &Builder<'_>, target: TargetSelection) -> Option<PathBuf> {
+    use build_helper::git::PathFreshness;
+
     // Try to download GCC from CI if configured and available
     if !matches!(builder.config.gcc_ci_mode, crate::core::config::GccCiMode::DownloadFromCi) {
         return None;

src/bootstrap/src/core/build_steps/llvm.rs

@@ -15,7 +15,7 @@ use std::sync::OnceLock;
 use std::{env, fs};
 
 use build_helper::ci::CiEnv;
-use build_helper::git::get_closest_merge_commit;
+use build_helper::git::{PathFreshness, check_path_modifications};
 #[cfg(feature = "tracing")]
 use tracing::instrument;
 
@@ -174,35 +174,38 @@ pub fn prebuilt_llvm_config(
     LlvmBuildStatus::ShouldBuild(Meta { stamp, res, out_dir, root: root.into() })
 }
 
-/// This retrieves the LLVM sha we *want* to use, according to git history.
-pub(crate) fn detect_llvm_sha(config: &Config, is_git: bool) -> String {
-    let llvm_sha = if is_git {
-        get_closest_merge_commit(
-            Some(&config.src),
-            &config.git_config(),
-            &[
-                config.src.join("src/llvm-project"),
-                config.src.join("src/bootstrap/download-ci-llvm-stamp"),
-                // the LLVM shared object file is named `LLVM-12-rust-{version}-nightly`
-                config.src.join("src/version"),
-            ],
+/// 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(),
+                &[
+                    "src/llvm-project",
+                    "src/bootstrap/download-ci-llvm-stamp",
+                    // the LLVM shared object file is named `LLVM-12-rust-{version}-nightly`
+                    "src/version",
+                ],
+                CiEnv::current(),
+            )
+            .unwrap(),
         )
-        .unwrap()
     } else if let Some(info) = crate::utils::channel::read_commit_info_file(&config.src) {
-        info.sha.trim().to_owned()
+        Some(PathFreshness::LastModifiedUpstream { upstream: info.sha.trim().to_owned() })
     } else {
-        "".to_owned()
+        None
     };
 
-    if llvm_sha.is_empty() {
+    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!();
-    }
+    };
 
-    llvm_sha
+    freshness
 }
 
 /// Returns whether the CI-found LLVM is currently usable.
@@ -282,12 +285,7 @@ pub(crate) fn is_ci_llvm_modified(config: &Config) -> bool {
         return false;
     }
 
-    let llvm_sha = detect_llvm_sha(config, true);
-    let head_sha = crate::output(
-        helpers::git(Some(&config.src)).arg("rev-parse").arg("HEAD").as_command_mut(),
-    );
-    let head_sha = head_sha.trim();
-    llvm_sha == head_sha
+    matches!(detect_llvm_freshness(config, true), PathFreshness::HasLocalModifications { .. })
 }
 
 #[derive(Debug, Clone, Hash, PartialEq, Eq)]

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

@@ -15,7 +15,9 @@ use std::{cmp, env, fs};
 
 use build_helper::ci::CiEnv;
 use build_helper::exit;
-use build_helper::git::{GitConfig, get_closest_merge_commit, output_result};
+use build_helper::git::{
+    GitConfig, PathFreshness, check_path_modifications, get_closest_merge_commit, output_result,
+};
 use serde::{Deserialize, Deserializer};
 use serde_derive::Deserialize;
 #[cfg(feature = "tracing")]
@@ -3107,9 +3109,7 @@ impl Config {
             self.update_submodule("src/llvm-project");
 
             // Check for untracked changes in `src/llvm-project`.
-            let has_changes = self
-                .last_modified_commit(&["src/llvm-project"], "download-ci-llvm", true)
-                .is_none();
+            let has_changes = self.has_changes_from_upstream(&["src/llvm-project"]);
 
             // Return false if there are untracked changes, otherwise check if CI LLVM is available.
             if has_changes { false } else { llvm::is_ci_llvm_available(self, asserts) }
@@ -3133,6 +3133,17 @@ impl Config {
         }
     }
 
+    /// Returns true if any of the `paths` have been modified locally.
+    fn has_changes_from_upstream(&self, paths: &[&str]) -> bool {
+        let freshness =
+            check_path_modifications(Some(&self.src), &self.git_config(), paths, CiEnv::current())
+                .unwrap();
+        match freshness {
+            PathFreshness::LastModifiedUpstream { .. } => false,
+            PathFreshness::HasLocalModifications { .. } => true,
+        }
+    }
+
     /// Returns the last commit in which any of `modified_paths` were changed,
     /// or `None` if there are untracked changes in the working directory and `if_unchanged` is true.
     pub fn last_modified_commit(

src/bootstrap/src/core/download.rs

@@ -721,16 +721,21 @@ download-rustc = false
     #[cfg(not(test))]
     pub(crate) fn maybe_download_ci_llvm(&self) {
         use build_helper::exit;
+        use build_helper::git::PathFreshness;
 
-        use crate::core::build_steps::llvm::detect_llvm_sha;
+        use crate::core::build_steps::llvm::detect_llvm_freshness;
         use crate::core::config::check_incompatible_options_for_ci_llvm;
 
         if !self.llvm_from_ci {
             return;
         }
 
         let llvm_root = self.ci_llvm_root();
-        let llvm_sha = detect_llvm_sha(self, self.rust_info.is_managed_git_subrepository());
+        let llvm_sha =
+            match detect_llvm_freshness(self, self.rust_info.is_managed_git_subrepository()) {
+                PathFreshness::LastModifiedUpstream { upstream } => upstream,
+                PathFreshness::HasLocalModifications { upstream } => upstream,
+            };
         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() {

src/build_helper/src/git.rs

@@ -253,6 +253,11 @@ pub fn check_path_modifications(
         upstream_sha
     };
 
+    // For local environments, we want to find out if something has changed
+    // from the latest upstream commit.
+    // However, that should be equivalent to checking if something has changed
+    // from the latest upstream commit *that modified `target_paths`*, and
+    // with this approach we do not need to invoke git an additional time.
     if has_changed_since(git_dir, &upstream_sha, target_paths) {
         Ok(PathFreshness::HasLocalModifications { upstream: upstream_sha })
     } else {
@@ -261,7 +266,7 @@ pub fn check_path_modifications(
 }
 
 /// Returns true if any of the passed `paths` have changed since the `base` commit.
-pub fn has_changed_since(git_dir: Option<&Path>, base: &str, paths: &[&Path]) -> bool {
+fn has_changed_since(git_dir: Option<&Path>, base: &str, paths: &[&str]) -> bool {
     let mut git = Command::new("git");
 
     if let Some(git_dir) = git_dir {

src/build_helper/src/git/tests.rs

@@ -1,9 +1,10 @@
-use crate::ci::CiEnv;
-use crate::git::{GitConfig, PathFreshness, check_path_modifications};
 use std::ffi::OsStr;
 use std::fs::OpenOptions;
 use std::process::Command;
 
+use crate::ci::CiEnv;
+use crate::git::{GitConfig, PathFreshness, check_path_modifications};
+
 #[test]
 fn test_pr_ci_unchanged_anywhere() {
     git_test(|ctx| {