about summary refs log tree commit diff
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
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.
-rw-r--r--src/bootstrap/src/core/build_steps/gcc.rs4
-rw-r--r--src/bootstrap/src/core/config/config.rs6
-rw-r--r--src/bootstrap/src/core/download.rs4
-rw-r--r--src/build_helper/src/git.rs42
-rw-r--r--src/build_helper/src/git/tests.rs32
5 files changed, 70 insertions, 18 deletions
diff --git a/src/bootstrap/src/core/build_steps/gcc.rs b/src/bootstrap/src/core/build_steps/gcc.rs
index 867874fc7cc..94abbb35ac5 100644
--- a/src/bootstrap/src/core/build_steps/gcc.rs
+++ b/src/bootstrap/src/core/build_steps/gcc.rs
@@ -129,6 +129,10 @@ fn try_download_gcc(builder: &Builder<'_>, target: TargetSelection) -> Option<Pa
             eprintln!("Found local GCC modifications, GCC will *not* be downloaded");
             None
         }
+        PathFreshness::MissingUpstream => {
+            eprintln!("No upstream commit found, GCC will *not* be downloaded");
+            None
+        }
     }
 }
 
diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs
index 1406ebeb621..a2d13cffbff 100644
--- a/src/bootstrap/src/core/config/config.rs
+++ b/src/bootstrap/src/core/config/config.rs
@@ -3210,6 +3210,10 @@ impl Config {
 
                     upstream
                 }
+                PathFreshness::MissingUpstream => {
+                    eprintln!("No upstream commit found");
+                    return None;
+                }
             }
         } else {
             channel::read_commit_info_file(&self.src)
@@ -3289,7 +3293,7 @@ impl Config {
     pub fn has_changes_from_upstream(&self, paths: &[&str]) -> bool {
         match self.check_modifications(paths) {
             PathFreshness::LastModifiedUpstream { .. } => false,
-            PathFreshness::HasLocalModifications { .. } => true,
+            PathFreshness::HasLocalModifications { .. } | PathFreshness::MissingUpstream => true,
         }
     }
 
diff --git a/src/bootstrap/src/core/download.rs b/src/bootstrap/src/core/download.rs
index 3c9cfead5a9..0051b874729 100644
--- a/src/bootstrap/src/core/download.rs
+++ b/src/bootstrap/src/core/download.rs
@@ -734,6 +734,10 @@ download-rustc = false
             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 stamp_key = format!("{}{}", llvm_sha, self.llvm_assertions);
         let llvm_stamp = BuildStamp::new(&llvm_root).with_prefix("llvm").add_stamp(stamp_key);
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");