Skip to content

Commit 64b0472

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 ab95d1b commit 64b0472

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
@@ -131,6 +131,10 @@ fn try_download_gcc(builder: &Builder<'_>, target: TargetSelection) -> Option<Pa
131131
eprintln!("Found local GCC modifications, GCC will *not* be downloaded");
132132
None
133133
}
134+
PathFreshness::MissingUpstream => {
135+
eprintln!("No upstream commit found, GCC will *not* be downloaded");
136+
None
137+
}
134138
}
135139
}
136140

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

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

30613061
upstream
30623062
}
3063+
PathFreshness::MissingUpstream => {
3064+
eprintln!("No upstream commit found");
3065+
return None;
3066+
}
30633067
}
30643068
} else {
30653069
channel::read_commit_info_file(&self.src)
@@ -3125,7 +3129,7 @@ impl Config {
31253129
pub fn has_changes_from_upstream(&self, paths: &[&str]) -> bool {
31263130
match self.check_modifications(paths) {
31273131
PathFreshness::LastModifiedUpstream { .. } => false,
3128-
PathFreshness::HasLocalModifications { .. } => true,
3132+
PathFreshness::HasLocalModifications { .. } | PathFreshness::MissingUpstream => true,
31293133
}
31303134
}
31313135

src/bootstrap/src/core/download.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -735,6 +735,10 @@ download-rustc = false
735735
match detect_llvm_freshness(self, self.rust_info.is_managed_git_subrepository()) {
736736
PathFreshness::LastModifiedUpstream { upstream } => upstream,
737737
PathFreshness::HasLocalModifications { upstream } => upstream,
738+
PathFreshness::MissingUpstream => {
739+
eprintln!("No upstream commit for downloading LLVM found");
740+
crate::exit!(1);
741+
}
738742
};
739743
let stamp_key = format!("{}{}", llvm_sha, self.llvm_assertions);
740744
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
@@ -42,6 +42,9 @@ pub enum PathFreshness {
4242
/// `upstream` is the latest upstream merge commit that made modifications to the
4343
/// set of paths.
4444
HasLocalModifications { upstream: String },
45+
/// No upstream commit was found.
46+
/// This should not happen in most reasonable circumstances, but one never knows.
47+
MissingUpstream,
4548
}
4649

4750
/// This function figures out if a set of paths was last modified upstream or
@@ -102,21 +105,29 @@ pub fn check_path_modifications(
102105
// artifacts.
103106

104107
// Do not include HEAD, as it is never an upstream commit
105-
get_closest_upstream_commit(git_dir, config, ci_env)?
108+
// If we do not find an upstream commit in CI, something is seriously wrong.
109+
Some(
110+
get_closest_upstream_commit(git_dir, config, ci_env)?
111+
.expect("No upstream commit was found on CI"),
112+
)
106113
} else {
107-
// Outside CI, we have to find the most recent upstream commit that
108-
// modified the set of paths, to have an upstream reference.
109-
let upstream_sha = get_latest_commit_that_modified_files(
114+
// Outside CI, we want to find the most recent upstream commit that
115+
// modified the set of paths, to have an upstream reference that does not change
116+
// unnecessarily often.
117+
// However, if such commit is not found, we can fall back to the latest upstream commit
118+
let upstream_with_modifications = get_latest_commit_that_modified_files(
110119
git_dir,
111120
target_paths,
112121
config.git_merge_commit_email,
113122
)?;
114-
let Some(upstream_sha) = upstream_sha else {
115-
eprintln!("No upstream commit that modified paths {target_paths:?} found.");
116-
eprintln!("Try to fetch more upstream history.");
117-
return Err("No upstream commit with modifications found".to_string());
118-
};
119-
upstream_sha
123+
match upstream_with_modifications {
124+
Some(sha) => Some(sha),
125+
None => get_closest_upstream_commit(git_dir, config, ci_env)?,
126+
}
127+
};
128+
129+
let Some(upstream_sha) = upstream_sha else {
130+
return Ok(PathFreshness::MissingUpstream);
120131
};
121132

122133
// For local environments, we want to find out if something has changed
@@ -178,7 +189,7 @@ fn get_closest_upstream_commit(
178189
git_dir: Option<&Path>,
179190
config: &GitConfig<'_>,
180191
env: CiEnv,
181-
) -> Result<String, String> {
192+
) -> Result<Option<String>, String> {
182193
let mut git = Command::new("git");
183194

184195
if let Some(git_dir) = git_dir {
@@ -189,7 +200,7 @@ fn get_closest_upstream_commit(
189200
CiEnv::None => "HEAD",
190201
CiEnv::GitHubActions => {
191202
// On CI, we always have a merge commit at the tip.
192-
// We thus skip it, because although it can be creatd by
203+
// We thus skip it, because although it can be created by
193204
// `config.git_merge_commit_email`, it should not be upstream.
194205
"HEAD^1"
195206
}
@@ -202,7 +213,8 @@ fn get_closest_upstream_commit(
202213
&base,
203214
]);
204215

205-
Ok(output_result(&mut git)?.trim().to_owned())
216+
let output = output_result(&mut git)?.trim().to_owned();
217+
if output.is_empty() { Ok(None) } else { Ok(Some(output)) }
206218
}
207219

208220
/// Returns the files that have been modified in the current branch compared to the master branch.
@@ -216,7 +228,9 @@ pub fn get_git_modified_files(
216228
git_dir: Option<&Path>,
217229
extensions: &[&str],
218230
) -> Result<Vec<String>, String> {
219-
let merge_base = get_closest_upstream_commit(git_dir, config, CiEnv::None)?;
231+
let Some(merge_base) = get_closest_upstream_commit(git_dir, config, CiEnv::None)? else {
232+
return Err("No upstream commit was found".to_string());
233+
};
220234

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