Implement a new unified function for figuring out how if a set of paths have been modified locally

Also adds several git tests to make sure that the behavior works in common cases (PR CI, auto CI, local usage).

Author: Kobzol

src/bootstrap/Cargo.lock

@@ -225,12 +225,12 @@ dependencies = [
 
 [[package]]
 name = "errno"
-version = "0.3.9"
+version = "0.3.10"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "534c5cf6194dfab3db3242765c03bbe257cf92f22b38f6bc0c58d59108a820ba"
+checksum = "33d852cb9b869c2a9b3df2f71a3074817f01e1844f839a144f5fcef059a4eb5d"
 dependencies = [
  "libc",
- "windows-sys 0.52.0",
+ "windows-sys 0.59.0",
 ]
 
 [[package]]
@@ -334,9 +334,9 @@ checksum = "bbd2bcb4c963f2ddae06a2efc7e9f3591312473c50c6685e1f298068316e66fe"
 
 [[package]]
 name = "libc"
-version = "0.2.167"
+version = "0.2.171"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "09d6582e104315a817dff97f75133544b2e094ee22447d2acf4a74e189ba06fc"
+checksum = "c19937216e9d3aa9956d9bb8dfc0b0c8beb6058fc4f7a4dc4d850edf86a237d6"
 
 [[package]]
 name = "libredox"

src/build_helper/Cargo.toml

@@ -8,3 +8,6 @@ edition = "2021"
 [dependencies]
 serde = "1"
 serde_derive = "1"
+
+[dev-dependencies]
+tempfile = "3.19"

src/build_helper/src/git.rs

@@ -1,3 +1,6 @@
+#[cfg(test)]
+mod tests;
+
 use std::path::Path;
 use std::process::{Command, Stdio};
 
@@ -167,7 +170,181 @@ pub fn get_closest_merge_commit(
     Ok(output_result(&mut git)?.trim().to_owned())
 }
 
-/// Returns the files that have been modified in the current branch compared to the last merge.
+/// Represents the result of checking whether a set of paths
+/// have been modified locally or not.
+#[derive(PartialEq, Debug)]
+pub enum PathFreshness {
+    /// Artifacts should be downloaded from this upstream commit,
+    /// there are no local modifications.
+    LastModifiedUpstream { upstream: String },
+    /// There are local modifications to a certain set of paths.
+    /// "Local" essentially means "not-upstream" here.
+    /// `upstream` is the latest upstream merge commit that made modifications to the
+    /// set of paths.
+    HasLocalModifications { upstream: String },
+}
+
+/// This function figures out if a set of paths was last modified upstream or
+/// if there are some local modifications made to them.
+///
+/// It can be used to figure out if we should download artifacts from CI or rather
+/// build them locally.
+///
+/// `target_paths` should be a non-empty slice of paths (git `pathspec`s) relative to `git_dir`
+/// or the current working directory whose modifications would invalidate the artifact.
+/// Each pathspec can also be a negative match, i.e. `:!foo`. This matches changes outside
+/// the `foo` directory.
+/// See https://git-scm.com/docs/gitglossary#Documentation/gitglossary.txt-aiddefpathspecapathspec
+/// for how git `pathspec` works.
+///
+/// The function behaves differently in CI and outside CI.
+///
+/// - Outside CI, we want to find out if `target_paths` were modified in some local commit on
+/// top of the local master branch.
+/// If not, we try to find the most recent upstream commit (which we assume are commits
+/// made by bors) that modified `target_paths`.
+/// We don't want to simply take the latest master commit to avoid changing the output of
+/// this function frequently after rebasing on the latest master branch even if `target_paths`
+/// were not modified upstream in the meantime. In that case we would be redownloading CI
+/// artifacts unnecessarily.
+///
+/// - In CI, we always fetch only a single parent merge commit, so we do not have access
+/// to the full git history. Luckily, we only need to distinguish between two situations:
+/// 1) The current PR made modifications to `target_paths`.
+/// In that case, a build is typically necessary.
+/// 2) The current PR did not make modifications to `target_paths`.
+/// In that case we simply take the latest upstream commit, because on CI there is no need to avoid
+/// redownloading.
+pub fn check_path_modifications(
+    git_dir: Option<&Path>,
+    config: &GitConfig<'_>,
+    target_paths: &[&str],
+    ci_env: CiEnv,
+) -> Result<PathFreshness, String> {
+    assert!(!target_paths.is_empty());
+    for path in target_paths {
+        assert!(Path::new(path.trim_start_matches(":!")).is_relative());
+    }
+
+    let upstream_sha = if matches!(ci_env, CiEnv::GitHubActions) {
+        // Here the situation is different for PR CI and try/auto CI.
+        // For PR CI, we have the following history:
+        // <merge commit made by GitHub>
+        // 1-N PR commits
+        // upstream merge commit made by bors
+        //
+        // For try/auto CI, we have the following history:
+        // <**non-upstream** merge commit made by bors>
+        // 1-N PR commits
+        // upstream merge commit made by bors
+        //
+        // But on both cases, HEAD should be a merge commit.
+        // So if HEAD contains modifications of `target_paths`, our PR has modified
+        // them. If not, we can use the only available upstream commit for downloading
+        // artifacts.
+
+        // Do not include HEAD, as it is never an upstream commit
+        get_closest_upstream_commit(git_dir, config, ci_env)?
+    } else {
+        // Outside CI, we have to find the most recent upstream commit that
+        // modified the set of paths, to have an upstream reference.
+        let upstream_sha = get_latest_commit_that_modified_files(
+            git_dir,
+            target_paths,
+            config.git_merge_commit_email,
+        )?;
+        let Some(upstream_sha) = upstream_sha else {
+            eprintln!("No upstream commit that modified paths {target_paths:?} found.");
+            eprintln!("Try to fetch more upstream history.");
+            return Err("No upstream commit with modifications found".to_string());
+        };
+        upstream_sha
+    };
+
+    if has_changed_since(git_dir, &upstream_sha, target_paths) {
+        Ok(PathFreshness::HasLocalModifications { upstream: upstream_sha })
+    } else {
+        Ok(PathFreshness::LastModifiedUpstream { upstream: upstream_sha })
+    }
+}
+
+/// 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: &[&str]) -> bool {
+    let mut git = Command::new("git");
+
+    if let Some(git_dir) = git_dir {
+        git.current_dir(git_dir);
+    }
+
+    git.args(["diff-index", "--quiet", base, "--"]).args(paths);
+
+    // Exit code 0 => no changes
+    // Exit code 1 => some changes were detected
+    !git.status().expect("cannot run git diff-index").success()
+}
+
+/// Returns the latest commit that modified `target_paths`, or `None` if no such commit was found.
+/// If `author` is `Some`, only considers commits made by that author.
+fn get_latest_commit_that_modified_files(
+    git_dir: Option<&Path>,
+    target_paths: &[&str],
+    author: &str,
+) -> Result<Option<String>, String> {
+    let mut git = Command::new("git");
+
+    if let Some(git_dir) = git_dir {
+        git.current_dir(git_dir);
+    }
+
+    git.args(["rev-list", "-n1", "--first-parent", "HEAD", "--author", author]);
+
+    if !target_paths.is_empty() {
+        git.arg("--").args(target_paths);
+    }
+    let output = output_result(&mut git)?.trim().to_owned();
+    if output.is_empty() { Ok(None) } else { Ok(Some(output)) }
+}
+
+/// Returns the most recent commit found in the local history that should definitely
+/// exist upstream. We identify upstream commits by the e-mail of the commit author.
+///
+/// If `include_head` is false, the HEAD (current) commit will be ignored and only
+/// its parents will be searched. This is useful for try/auto CI, where HEAD is
+/// actually a commit made by bors, although it is not upstream yet.
+fn get_closest_upstream_commit(
+    git_dir: Option<&Path>,
+    config: &GitConfig<'_>,
+    env: CiEnv,
+) -> Result<String, String> {
+    let mut git = Command::new("git");
+
+    if let Some(git_dir) = git_dir {
+        git.current_dir(git_dir);
+    }
+
+    let base = match env {
+        CiEnv::None => "HEAD",
+        CiEnv::GitHubActions => {
+            // On CI, we always have a merge commit at the tip.
+            // We thus skip it, because although it can be creatd by
+            // `config.git_merge_commit_email`, it should not be upstream.
+            "HEAD^1"
+        }
+    };
+    git.args([
+        "rev-list",
+        &format!("--author={}", config.git_merge_commit_email),
+        "-n1",
+        "--first-parent",
+        &base,
+    ]);
+
+    Ok(output_result(&mut git)?.trim().to_owned())
+}
+
+/// Returns the files that have been modified in the current branch compared to the master branch.
+/// This includes committed changes, uncommitted changes, and changes that are not even staged.
+///
 /// The `extensions` parameter can be used to filter the files by their extension.
 /// Does not include removed files.
 /// If `extensions` is empty, all files will be returned.
@@ -176,7 +353,7 @@ pub fn get_git_modified_files(
     git_dir: Option<&Path>,
     extensions: &[&str],
 ) -> Result<Vec<String>, String> {
-    let merge_base = get_closest_merge_commit(git_dir, config, &[])?;
+    let merge_base = get_closest_upstream_commit(git_dir, config, CiEnv::None)?;
 
     let mut git = Command::new("git");
     if let Some(git_dir) = git_dir {