diff options
| author | Jakub Beránek <berykubik@gmail.com> | 2025-03-18 12:42:28 +0100 |
|---|---|---|
| committer | Jakub Beránek <berykubik@gmail.com> | 2025-04-20 09:13:55 +0200 |
| commit | 1dabb76f40a4186a68d98bb3d04a3b089608c656 (patch) | |
| tree | f1838c194477d66a2075fe98f930d968643a3eb5 /src/build_helper | |
| parent | 22280337f632a8f04b16a9d1edb40dac5b542513 (diff) | |
| download | rust-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.rs | 42 | ||||
| -rw-r--r-- | src/build_helper/src/git/tests.rs | 32 |
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"); |
