Explicitly model missing upstream

It shouldn't really happen, but if it does, at least we will have an explicit record of it.
This commit is contained in:
Jakub Beránek 2025-03-18 12:42:28 +01:00
parent 22280337f6
commit 1dabb76f40
5 changed files with 70 additions and 18 deletions

View file

@ -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
}
}
}

View file

@ -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,
}
}

View file

@ -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);

View file

@ -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 {

View file

@ -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");