Skip to content

Commit bbbf184

Browse files
Kobzolhaenoe
authored andcommitted
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 dec9426 commit bbbf184

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
@@ -3210,6 +3210,10 @@ impl Config {
32103210

32113211
upstream
32123212
}
3213+
PathFreshness::MissingUpstream => {
3214+
eprintln!("No upstream commit found");
3215+
return None;
3216+
}
32133217
}
32143218
} else {
32153219
channel::read_commit_info_file(&self.src)
@@ -3289,7 +3293,7 @@ impl Config {
32893293
pub fn has_changes_from_upstream(&self, paths: &[&str]) -> bool {
32903294
match self.check_modifications(paths) {
32913295
PathFreshness::LastModifiedUpstream { .. } => false,
3292-
PathFreshness::HasLocalModifications { .. } => true,
3296+
PathFreshness::HasLocalModifications { .. } | PathFreshness::MissingUpstream => true,
32933297
}
32943298
}
32953299

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
@@ -177,21 +180,29 @@ pub fn check_path_modifications(
177180
// artifacts.
178181

179182
// Do not include HEAD, as it is never an upstream commit
180-
get_closest_upstream_commit(git_dir, config, ci_env)?
183+
// If we do not find an upstream commit in CI, something is seriously wrong.
184+
Some(
185+
get_closest_upstream_commit(git_dir, config, ci_env)?
186+
.expect("No upstream commit was found on CI"),
187+
)
181188
} else {
182-
// Outside CI, we have to find the most recent upstream commit that
183-
// modified the set of paths, to have an upstream reference.
184-
let upstream_sha = get_latest_commit_that_modified_files(
189+
// Outside CI, we want to find the most recent upstream commit that
190+
// modified the set of paths, to have an upstream reference that does not change
191+
// unnecessarily often.
192+
// However, if such commit is not found, we can fall back to the latest upstream commit
193+
let upstream_with_modifications = get_latest_commit_that_modified_files(
185194
git_dir,
186195
target_paths,
187196
config.git_merge_commit_email,
188197
)?;
189-
let Some(upstream_sha) = upstream_sha else {
190-
eprintln!("No upstream commit that modified paths {target_paths:?} found.");
191-
eprintln!("Try to fetch more upstream history.");
192-
return Err("No upstream commit with modifications found".to_string());
193-
};
194-
upstream_sha
198+
match upstream_with_modifications {
199+
Some(sha) => Some(sha),
200+
None => get_closest_upstream_commit(git_dir, config, ci_env)?,
201+
}
202+
};
203+
204+
let Some(upstream_sha) = upstream_sha else {
205+
return Ok(PathFreshness::MissingUpstream);
195206
};
196207

197208
// For local environments, we want to find out if something has changed
@@ -253,7 +264,7 @@ fn get_closest_upstream_commit(
253264
git_dir: Option<&Path>,
254265
config: &GitConfig<'_>,
255266
env: CiEnv,
256-
) -> Result<String, String> {
267+
) -> Result<Option<String>, String> {
257268
let mut git = Command::new("git");
258269

259270
if let Some(git_dir) = git_dir {
@@ -264,7 +275,7 @@ fn get_closest_upstream_commit(
264275
CiEnv::None => "HEAD",
265276
CiEnv::GitHubActions => {
266277
// On CI, we always have a merge commit at the tip.
267-
// We thus skip it, because although it can be creatd by
278+
// We thus skip it, because although it can be created by
268279
// `config.git_merge_commit_email`, it should not be upstream.
269280
"HEAD^1"
270281
}
@@ -277,7 +288,8 @@ fn get_closest_upstream_commit(
277288
&base,
278289
]);
279290

280-
Ok(output_result(&mut git)?.trim().to_owned())
291+
let output = output_result(&mut git)?.trim().to_owned();
292+
if output.is_empty() { Ok(None) } else { Ok(Some(output)) }
281293
}
282294

283295
/// 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(
291303
git_dir: Option<&Path>,
292304
extensions: &[&str],
293305
) -> Result<Vec<String>, String> {
294-
let merge_base = get_closest_upstream_commit(git_dir, config, CiEnv::None)?;
306+
let Some(merge_base) = get_closest_upstream_commit(git_dir, config, CiEnv::None)? else {
307+
return Err("No upstream commit was found".to_string());
308+
};
295309

296310
let mut git = Command::new("git");
297311
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)