Skip to content
This repository was archived by the owner on May 28, 2025. It is now read-only.

Commit 3f61da7

Browse files
committed
Port download-ci-rustc to check_path_modifications
And get rid of `get_closest_merge_commit`.
1 parent 2b4e34f commit 3f61da7

File tree

6 files changed

+27
-156
lines changed

6 files changed

+27
-156
lines changed

src/bootstrap/src/core/build_steps/compile.rs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -111,14 +111,10 @@ impl Step for Std {
111111
// the `rust.download-rustc=true` option.
112112
let force_recompile = builder.rust_info().is_managed_git_subrepository()
113113
&& builder.download_rustc()
114-
&& builder.config.last_modified_commit(&["library"], "download-rustc", true).is_none();
114+
&& builder.config.has_changes_from_upstream(&["library"]);
115115

116116
trace!("is managed git repo: {}", builder.rust_info().is_managed_git_subrepository());
117117
trace!("download_rustc: {}", builder.download_rustc());
118-
trace!(
119-
"last modified commit: {:?}",
120-
builder.config.last_modified_commit(&["library"], "download-rustc", true)
121-
);
122118
trace!(force_recompile);
123119

124120
run.builder.ensure(Std {

src/bootstrap/src/core/build_steps/tool.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -687,8 +687,7 @@ impl Step for Rustdoc {
687687
let files_to_track = &["src/librustdoc", "src/tools/rustdoc"];
688688

689689
// Check if unchanged
690-
if builder.config.last_modified_commit(files_to_track, "download-rustc", true).is_some()
691-
{
690+
if !builder.config.has_changes_from_upstream(files_to_track) {
692691
let precompiled_rustdoc = builder
693692
.config
694693
.ci_rustc_dir()

src/bootstrap/src/core/builder/tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ fn ci_rustc_if_unchanged_logic() {
268268
paths.push("library");
269269
}
270270

271-
let has_changes = config.last_modified_commit(&paths, "download-rustc", true).is_none();
271+
let has_changes = config.has_changes_from_upstream(&paths);
272272

273273
assert!(
274274
!has_changes,

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

Lines changed: 19 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,7 @@ use std::{cmp, env, fs};
1515

1616
use build_helper::ci::CiEnv;
1717
use build_helper::exit;
18-
use build_helper::git::{
19-
GitConfig, PathFreshness, check_path_modifications, get_closest_merge_commit, output_result,
20-
};
18+
use build_helper::git::{GitConfig, PathFreshness, check_path_modifications, output_result};
2119
use serde::{Deserialize, Deserializer};
2220
use serde_derive::Deserialize;
2321
#[cfg(feature = "tracing")]
@@ -3045,19 +3043,22 @@ impl Config {
30453043
let commit = if self.rust_info.is_managed_git_subrepository() {
30463044
// Look for a version to compare to based on the current commit.
30473045
// Only commits merged by bors will have CI artifacts.
3048-
match self.last_modified_commit(&allowed_paths, "download-rustc", if_unchanged) {
3049-
Some(commit) => commit,
3050-
None => {
3046+
match self.check_modifications(&allowed_paths) {
3047+
PathFreshness::LastModifiedUpstream { upstream } => upstream,
3048+
PathFreshness::HasLocalModifications { upstream } => {
30513049
if if_unchanged {
30523050
return None;
30533051
}
3054-
println!("ERROR: could not find commit hash for downloading rustc");
3055-
println!("HELP: maybe your repository history is too shallow?");
3056-
println!(
3057-
"HELP: consider setting `rust.download-rustc=false` in bootstrap.toml"
3058-
);
3059-
println!("HELP: or fetch enough history to include one upstream commit");
3060-
crate::exit!(1);
3052+
3053+
if CiEnv::is_ci() {
3054+
eprintln!("CI rustc commit matches with HEAD and we are in CI.");
3055+
eprintln!(
3056+
"`rustc.download-ci` functionality will be skipped as artifacts are not available."
3057+
);
3058+
return None;
3059+
}
3060+
3061+
upstream
30613062
}
30623063
}
30633064
} else {
@@ -3066,19 +3067,6 @@ impl Config {
30663067
.expect("git-commit-info is missing in the project root")
30673068
};
30683069

3069-
if CiEnv::is_ci() && {
3070-
let head_sha =
3071-
output(helpers::git(Some(&self.src)).arg("rev-parse").arg("HEAD").as_command_mut());
3072-
let head_sha = head_sha.trim();
3073-
commit == head_sha
3074-
} {
3075-
eprintln!("CI rustc commit matches with HEAD and we are in CI.");
3076-
eprintln!(
3077-
"`rustc.download-ci` functionality will be skipped as artifacts are not available."
3078-
);
3079-
return None;
3080-
}
3081-
30823070
if debug_assertions_requested {
30833071
eprintln!(
30843072
"WARN: `rust.debug-assertions = true` will prevent downloading CI rustc as alt CI \
@@ -3134,61 +3122,16 @@ impl Config {
31343122
}
31353123

31363124
/// Returns true if any of the `paths` have been modified locally.
3137-
fn has_changes_from_upstream(&self, paths: &[&str]) -> bool {
3138-
let freshness =
3139-
check_path_modifications(Some(&self.src), &self.git_config(), paths, CiEnv::current())
3140-
.unwrap();
3141-
match freshness {
3125+
pub fn has_changes_from_upstream(&self, paths: &[&str]) -> bool {
3126+
match self.check_modifications(paths) {
31423127
PathFreshness::LastModifiedUpstream { .. } => false,
31433128
PathFreshness::HasLocalModifications { .. } => true,
31443129
}
31453130
}
31463131

3147-
/// Returns the last commit in which any of `modified_paths` were changed,
3148-
/// or `None` if there are untracked changes in the working directory and `if_unchanged` is true.
3149-
pub fn last_modified_commit(
3150-
&self,
3151-
modified_paths: &[&str],
3152-
option_name: &str,
3153-
if_unchanged: bool,
3154-
) -> Option<String> {
3155-
assert!(
3156-
self.rust_info.is_managed_git_subrepository(),
3157-
"Can't run `Config::last_modified_commit` on a non-git source."
3158-
);
3159-
3160-
// Look for a version to compare to based on the current commit.
3161-
// Only commits merged by bors will have CI artifacts.
3162-
let commit = get_closest_merge_commit(Some(&self.src), &self.git_config(), &[]).unwrap();
3163-
if commit.is_empty() {
3164-
println!("error: could not find commit hash for downloading components from CI");
3165-
println!("help: maybe your repository history is too shallow?");
3166-
println!("help: consider disabling `{option_name}`");
3167-
println!("help: or fetch enough history to include one upstream commit");
3168-
crate::exit!(1);
3169-
}
3170-
3171-
// Warn if there were changes to the compiler or standard library since the ancestor commit.
3172-
let mut git = helpers::git(Some(&self.src));
3173-
git.args(["diff-index", "--quiet", &commit, "--"]).args(modified_paths);
3174-
3175-
let has_changes = !t!(git.as_command_mut().status()).success();
3176-
if has_changes {
3177-
if if_unchanged {
3178-
if self.is_verbose() {
3179-
println!(
3180-
"warning: saw changes to one of {modified_paths:?} since {commit}; \
3181-
ignoring `{option_name}`"
3182-
);
3183-
}
3184-
return None;
3185-
}
3186-
println!(
3187-
"warning: `{option_name}` is enabled, but there are changes to one of {modified_paths:?}"
3188-
);
3189-
}
3190-
3191-
Some(commit.to_string())
3132+
fn check_modifications(&self, paths: &[&str]) -> PathFreshness {
3133+
check_path_modifications(Some(&self.src), &self.git_config(), paths, CiEnv::current())
3134+
.unwrap()
31923135
}
31933136
}
31943137

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,7 @@ fn download_ci_llvm() {
3939

4040
let if_unchanged_config = parse("llvm.download-ci-llvm = \"if-unchanged\"");
4141
if if_unchanged_config.llvm_from_ci {
42-
let has_changes = if_unchanged_config
43-
.last_modified_commit(&["src/llvm-project"], "download-ci-llvm", true)
44-
.is_none();
42+
let has_changes = if_unchanged_config.has_changes_from_upstream(&["src/llvm-project"]);
4543

4644
assert!(
4745
!has_changes,

src/build_helper/src/git.rs

Lines changed: 4 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
#[cfg(test)]
22
mod tests;
33

4-
use std::path::{Path, PathBuf};
4+
use std::path::Path;
55
use std::process::{Command, Stdio};
66

77
use crate::ci::CiEnv;
@@ -101,73 +101,6 @@ pub fn updated_master_branch(
101101
Err("Cannot find any suitable upstream master branch".to_owned())
102102
}
103103

104-
/// Finds the nearest merge commit by comparing the local `HEAD` with the upstream branch's state.
105-
/// To work correctly, the upstream remote must be properly configured using `git remote add <name> <url>`.
106-
/// In most cases `get_closest_merge_commit` is the function you are looking for as it doesn't require remote
107-
/// to be configured.
108-
fn git_upstream_merge_base(
109-
config: &GitConfig<'_>,
110-
git_dir: Option<&Path>,
111-
) -> Result<String, String> {
112-
let updated_master = updated_master_branch(config, git_dir)?;
113-
let mut git = Command::new("git");
114-
if let Some(git_dir) = git_dir {
115-
git.current_dir(git_dir);
116-
}
117-
Ok(output_result(git.arg("merge-base").arg(&updated_master).arg("HEAD"))?.trim().to_owned())
118-
}
119-
120-
/// Searches for the nearest merge commit in the repository that also exists upstream.
121-
///
122-
/// It looks for the most recent commit made by the merge bot by matching the author's email
123-
/// address with the merge bot's email.
124-
pub fn get_closest_merge_commit(
125-
git_dir: Option<&Path>,
126-
config: &GitConfig<'_>,
127-
target_paths: &[PathBuf],
128-
) -> Result<String, String> {
129-
let mut git = Command::new("git");
130-
131-
if let Some(git_dir) = git_dir {
132-
git.current_dir(git_dir);
133-
}
134-
135-
let channel = include_str!("../../ci/channel").trim();
136-
137-
let merge_base = {
138-
if CiEnv::is_ci() &&
139-
// FIXME: When running on rust-lang managed CI and it's not a nightly build,
140-
// `git_upstream_merge_base` fails with an error message similar to this:
141-
// ```
142-
// called `Result::unwrap()` on an `Err` value: "command did not execute successfully:
143-
// cd \"/checkout\" && \"git\" \"merge-base\" \"origin/master\" \"HEAD\"\nexpected success, got: exit status: 1\n"
144-
// ```
145-
// Investigate and resolve this issue instead of skipping it like this.
146-
(channel == "nightly" || !CiEnv::is_rust_lang_managed_ci_job())
147-
{
148-
git_upstream_merge_base(config, git_dir).unwrap()
149-
} else {
150-
// For non-CI environments, ignore rust-lang/rust upstream as it usually gets
151-
// outdated very quickly.
152-
"HEAD".to_string()
153-
}
154-
};
155-
156-
git.args([
157-
"rev-list",
158-
&format!("--author={}", config.git_merge_commit_email),
159-
"-n1",
160-
"--first-parent",
161-
&merge_base,
162-
]);
163-
164-
if !target_paths.is_empty() {
165-
git.arg("--").args(target_paths);
166-
}
167-
168-
Ok(output_result(&mut git)?.trim().to_owned())
169-
}
170-
171104
/// Represents the result of checking whether a set of paths
172105
/// have been modified locally or not.
173106
#[derive(PartialEq, Debug)]
@@ -184,10 +117,12 @@ pub enum PathFreshness {
184117

185118
/// This function figures out if a set of paths was last modified upstream or
186119
/// if there are some local modifications made to them.
187-
///
188120
/// It can be used to figure out if we should download artifacts from CI or rather
189121
/// build them locally.
190122
///
123+
/// The function assumes that at least a single upstream bors merge commit is in the
124+
/// local git history.
125+
///
191126
/// `target_paths` should be a non-empty slice of paths (relative to `git_dir` or the
192127
/// current working directory) whose modifications would invalidate the artifact.
193128
/// Each path can also be a negative match, i.e. `:!foo`. This matches changes outside

0 commit comments

Comments
 (0)