about summary refs log tree commit diff
path: root/src/build_helper
diff options
context:
space:
mode:
authorJakub Beránek <berykubik@gmail.com>2025-03-18 12:42:28 +0100
committerJakub Beránek <berykubik@gmail.com>2025-04-20 09:13:55 +0200
commit1dabb76f40a4186a68d98bb3d04a3b089608c656 (patch)
treef1838c194477d66a2075fe98f930d968643a3eb5 /src/build_helper
parent22280337f632a8f04b16a9d1edb40dac5b542513 (diff)
downloadrust-1dabb76f40a4186a68d98bb3d04a3b089608c656.tar.gz
rust-1dabb76f40a4186a68d98bb3d04a3b089608c656.zip
Explicitly model missing upstream
It shouldn't really happen, but if it does, at least we will have an explicit record of it.
Diffstat (limited to 'src/build_helper')
-rw-r--r--src/build_helper/src/git.rs42
-rw-r--r--src/build_helper/src/git/tests.rs32
2 files changed, 57 insertions, 17 deletions
diff --git a/src/build_helper/src/git.rs b/src/build_helper/src/git.rs
index bc8dc3472d3..39950568987 100644
--- a/src/build_helper/src/git.rs
+++ b/src/build_helper/src/git.rs
@@ -113,6 +113,9 @@ pub enum PathFreshness {
     /// `upstream` is the latest upstream merge commit that made modifications to the
     /// set of paths.
     HasLocalModifications { upstream: String },
+    /// No upstream commit was found.
+    /// This should not happen in most reasonable circumstances, but one never knows.
+    MissingUpstream,
 }
 
 /// This function figures out if a set of paths was last modified upstream or
@@ -177,21 +180,29 @@ pub fn check_path_modifications(
         // artifacts.
 
         // Do not include HEAD, as it is never an upstream commit
-        get_closest_upstream_commit(git_dir, config, ci_env)?
+        // If we do not find an upstream commit in CI, something is seriously wrong.
+        Some(
+            get_closest_upstream_commit(git_dir, config, ci_env)?
+                .expect("No upstream commit was found on CI"),
+        )
     } 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(
+        // Outside CI, we want to find the most recent upstream commit that
+        // modified the set of paths, to have an upstream reference that does not change
+        // unnecessarily often.
+        // However, if such commit is not found, we can fall back to the latest upstream commit
+        let upstream_with_modifications = 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
+        match upstream_with_modifications {
+            Some(sha) => Some(sha),
+            None => get_closest_upstream_commit(git_dir, config, ci_env)?,
+        }
+    };
+
+    let Some(upstream_sha) = upstream_sha else {
+        return Ok(PathFreshness::MissingUpstream);
     };
 
     // For local environments, we want to find out if something has changed
@@ -253,7 +264,7 @@ fn get_closest_upstream_commit(
     git_dir: Option<&Path>,
     config: &GitConfig<'_>,
     env: CiEnv,
-) -> Result<String, String> {
+) -> Result<Option<String>, String> {
     let mut git = Command::new("git");
 
     if let Some(git_dir) = git_dir {
@@ -264,7 +275,7 @@ fn get_closest_upstream_commit(
         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
+            // We thus skip it, because although it can be created by
             // `config.git_merge_commit_email`, it should not be upstream.
             "HEAD^1"
         }
@@ -277,7 +288,8 @@ fn get_closest_upstream_commit(
         &base,
     ]);
 
-    Ok(output_result(&mut git)?.trim().to_owned())
+    let output = output_result(&mut git)?.trim().to_owned();
+    if output.is_empty() { Ok(None) } else { Ok(Some(output)) }
 }
 
 /// Returns the files that have been modified in the current branch compared to the master branch.
@@ -291,7 +303,9 @@ pub fn get_git_modified_files(
     git_dir: Option<&Path>,
     extensions: &[&str],
 ) -> Result<Vec<String>, String> {
-    let merge_base = get_closest_upstream_commit(git_dir, config, CiEnv::None)?;
+    let Some(merge_base) = get_closest_upstream_commit(git_dir, config, CiEnv::None)? else {
+        return Err("No upstream commit was found".to_string());
+    };
 
     let mut git = Command::new("git");
     if let Some(git_dir) = git_dir {
diff --git a/src/build_helper/src/git/tests.rs b/src/build_helper/src/git/tests.rs
index cc502f08387..c6ad9de5a75 100644
--- a/src/build_helper/src/git/tests.rs
+++ b/src/build_helper/src/git/tests.rs
@@ -94,7 +94,7 @@ fn test_local_committed_modifications_subdirectory() {
 }
 
 #[test]
-fn test_changes_in_head_upstream() {
+fn test_local_changes_in_head_upstream() {
     git_test(|ctx| {
         // We want to resolve to the upstream commit that made modifications to a,
         // even if it is currently HEAD
@@ -107,7 +107,7 @@ fn test_changes_in_head_upstream() {
 }
 
 #[test]
-fn test_changes_in_previous_upstream() {
+fn test_local_changes_in_previous_upstream() {
     git_test(|ctx| {
         // We want to resolve to this commit, which modified a
         let sha = ctx.create_upstream_merge(&["a", "e"]);
@@ -124,7 +124,33 @@ fn test_changes_in_previous_upstream() {
 }
 
 #[test]
-fn test_changes_negative_path() {
+fn test_local_no_upstream_commit_with_changes() {
+    git_test(|ctx| {
+        ctx.create_upstream_merge(&["a", "e"]);
+        ctx.create_upstream_merge(&["a", "e"]);
+        // We want to fall back to this commit, because there are no commits
+        // that modified `x`.
+        let sha = ctx.create_upstream_merge(&["a", "e"]);
+        ctx.create_branch("feature");
+        ctx.modify("d");
+        ctx.commit();
+        assert_eq!(
+            ctx.get_source(&["x"], CiEnv::None),
+            PathFreshness::LastModifiedUpstream { upstream: sha }
+        );
+    });
+}
+
+#[test]
+fn test_local_no_upstream_commit() {
+    git_test(|ctx| {
+        let src = ctx.get_source(&["c", "d"], CiEnv::None);
+        assert_eq!(src, PathFreshness::MissingUpstream);
+    });
+}
+
+#[test]
+fn test_local_changes_negative_path() {
     git_test(|ctx| {
         let upstream = ctx.create_upstream_merge(&["a"]);
         ctx.create_branch("feature");