Skip to content

Commit 820064c

Browse files
committed
Explicitly model missing upstream
It shouldn't really happen, but if it does, at least we will have an explicit record of it.
1 parent deea908 commit 820064c

File tree

5 files changed

+70
-18
lines changed

5 files changed

+70
-18
lines changed

src/bootstrap/src/core/build_steps/gcc.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,10 @@ fn try_download_gcc(builder: &Builder<'_>, target: TargetSelection) -> Option<Pa
129129
eprintln!("Found local GCC modifications, GCC will *not* be downloaded");
130130
None
131131
}
132+
PathFreshness::MissingUpstream => {
133+
eprintln!("No upstream commit found, GCC will *not* be downloaded");
134+
None
135+
}
132136
}
133137
}
134138

src/bootstrap/src/core/config/config.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3068,6 +3068,10 @@ impl Config {
30683068

30693069
upstream
30703070
}
3071+
PathFreshness::MissingUpstream => {
3072+
eprintln!("No upstream commit found");
3073+
return None;
3074+
}
30713075
}
30723076
} else {
30733077
channel::read_commit_info_file(&self.src)
@@ -3147,7 +3151,7 @@ impl Config {
31473151
pub fn has_changes_from_upstream(&self, paths: &[&str]) -> bool {
31483152
match self.check_modifications(paths) {
31493153
PathFreshness::LastModifiedUpstream { .. } => false,
3150-
PathFreshness::HasLocalModifications { .. } => true,
3154+
PathFreshness::HasLocalModifications { .. } | PathFreshness::MissingUpstream => true,
31513155
}
31523156
}
31533157

src/bootstrap/src/core/download.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -734,6 +734,10 @@ download-rustc = false
734734
match detect_llvm_freshness(self, self.rust_info.is_managed_git_subrepository()) {
735735
PathFreshness::LastModifiedUpstream { upstream } => upstream,
736736
PathFreshness::HasLocalModifications { upstream } => upstream,
737+
PathFreshness::MissingUpstream => {
738+
eprintln!("No upstream commit for downloading LLVM found");
739+
crate::exit!(1);
740+
}
737741
};
738742
let stamp_key = format!("{}{}", llvm_sha, self.llvm_assertions);
739743
let llvm_stamp = BuildStamp::new(&llvm_root).with_prefix("llvm").add_stamp(stamp_key);

src/build_helper/src/git.rs

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,9 @@ pub enum PathFreshness {
113113
/// `upstream` is the latest upstream merge commit that made modifications to the
114114
/// set of paths.
115115
HasLocalModifications { upstream: String },
116+
/// No upstream commit was found.
117+
/// This should not happen in most reasonable circumstances, but one never knows.
118+
MissingUpstream,
116119
}
117120

118121
/// This function figures out if a set of paths was last modified upstream or
@@ -173,21 +176,29 @@ pub fn check_path_modifications(
173176
// artifacts.
174177

175178
// Do not include HEAD, as it is never an upstream commit
176-
get_closest_upstream_commit(git_dir, config, ci_env)?
179+
// If we do not find an upstream commit in CI, something is seriously wrong.
180+
Some(
181+
get_closest_upstream_commit(git_dir, config, ci_env)?
182+
.expect("No upstream commit was found on CI"),
183+
)
177184
} else {
178-
// Outside CI, we have to find the most recent upstream commit that
179-
// modified the set of paths, to have an upstream reference.
180-
let upstream_sha = get_latest_commit_that_modified_files(
185+
// Outside CI, we want to find the most recent upstream commit that
186+
// modified the set of paths, to have an upstream reference that does not change
187+
// unnecessarily often.
188+
// However, if such commit is not found, we can fall back to the latest upstream commit
189+
let upstream_with_modifications = get_latest_commit_that_modified_files(
181190
git_dir,
182191
target_paths,
183192
config.git_merge_commit_email,
184193
)?;
185-
let Some(upstream_sha) = upstream_sha else {
186-
eprintln!("No upstream commit that modified paths {target_paths:?} found.");
187-
eprintln!("Try to fetch more upstream history.");
188-
return Err("No upstream commit with modifications found".to_string());
189-
};
190-
upstream_sha
194+
match upstream_with_modifications {
195+
Some(sha) => Some(sha),
196+
None => get_closest_upstream_commit(git_dir, config, ci_env)?,
197+
}
198+
};
199+
200+
let Some(upstream_sha) = upstream_sha else {
201+
return Ok(PathFreshness::MissingUpstream);
191202
};
192203

193204
// For local environments, we want to find out if something has changed
@@ -249,7 +260,7 @@ fn get_closest_upstream_commit(
249260
git_dir: Option<&Path>,
250261
config: &GitConfig<'_>,
251262
env: CiEnv,
252-
) -> Result<String, String> {
263+
) -> Result<Option<String>, String> {
253264
let mut git = Command::new("git");
254265

255266
if let Some(git_dir) = git_dir {
@@ -260,7 +271,7 @@ fn get_closest_upstream_commit(
260271
CiEnv::None => "HEAD",
261272
CiEnv::GitHubActions => {
262273
// On CI, we always have a merge commit at the tip.
263-
// We thus skip it, because although it can be creatd by
274+
// We thus skip it, because although it can be created by
264275
// `config.git_merge_commit_email`, it should not be upstream.
265276
"HEAD^1"
266277
}
@@ -273,7 +284,8 @@ fn get_closest_upstream_commit(
273284
&base,
274285
]);
275286

276-
Ok(output_result(&mut git)?.trim().to_owned())
287+
let output = output_result(&mut git)?.trim().to_owned();
288+
if output.is_empty() { Ok(None) } else { Ok(Some(output)) }
277289
}
278290

279291
/// Returns the files that have been modified in the current branch compared to the master branch.
@@ -287,7 +299,9 @@ pub fn get_git_modified_files(
287299
git_dir: Option<&Path>,
288300
extensions: &[&str],
289301
) -> Result<Vec<String>, String> {
290-
let merge_base = get_closest_upstream_commit(git_dir, config, CiEnv::None)?;
302+
let Some(merge_base) = get_closest_upstream_commit(git_dir, config, CiEnv::None)? else {
303+
return Err("No upstream commit was found".to_string());
304+
};
291305

292306
let mut git = Command::new("git");
293307
if let Some(git_dir) = git_dir {

src/build_helper/src/git/tests.rs

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ fn test_local_committed_modifications_subdirectory() {
9494
}
9595

9696
#[test]
97-
fn test_changes_in_head_upstream() {
97+
fn test_local_changes_in_head_upstream() {
9898
git_test(|ctx| {
9999
// We want to resolve to the upstream commit that made modifications to a,
100100
// even if it is currently HEAD
@@ -107,7 +107,7 @@ fn test_changes_in_head_upstream() {
107107
}
108108

109109
#[test]
110-
fn test_changes_in_previous_upstream() {
110+
fn test_local_changes_in_previous_upstream() {
111111
git_test(|ctx| {
112112
// We want to resolve to this commit, which modified a
113113
let sha = ctx.create_upstream_merge(&["a", "e"]);
@@ -124,7 +124,33 @@ fn test_changes_in_previous_upstream() {
124124
}
125125

126126
#[test]
127-
fn test_changes_negative_path() {
127+
fn test_local_no_upstream_commit_with_changes() {
128+
git_test(|ctx| {
129+
ctx.create_upstream_merge(&["a", "e"]);
130+
ctx.create_upstream_merge(&["a", "e"]);
131+
// We want to fall back to this commit, because there are no commits
132+
// that modified `x`.
133+
let sha = ctx.create_upstream_merge(&["a", "e"]);
134+
ctx.create_branch("feature");
135+
ctx.modify("d");
136+
ctx.commit();
137+
assert_eq!(
138+
ctx.get_source(&["x"], CiEnv::None),
139+
PathFreshness::LastModifiedUpstream { upstream: sha }
140+
);
141+
});
142+
}
143+
144+
#[test]
145+
fn test_local_no_upstream_commit() {
146+
git_test(|ctx| {
147+
let src = ctx.get_source(&["c", "d"], CiEnv::None);
148+
assert_eq!(src, PathFreshness::MissingUpstream);
149+
});
150+
}
151+
152+
#[test]
153+
fn test_local_changes_negative_path() {
128154
git_test(|ctx| {
129155
let upstream = ctx.create_upstream_merge(&["a"]);
130156
ctx.create_branch("feature");

0 commit comments

Comments
 (0)