Skip to content

Commit 4caa60c

Browse files
authored
Rollup merge of #131043 - liwagu:unify, r=albertlarsan68,onur-ozkan
Refactor change detection for rustdoc and download-rustc This pull request refactors the change detection logic in the build process by consolidating redundant code into a new helper method. The key changes include the removal of duplicate logic for checking changes in directories and the addition of a new method to handle this functionality. Refactoring and code simplification: * [`src/bootstrap/src/core/build_steps/tool.rs`](diffhunk://#diff-dc86e288bcf7b3ca3f8c127d3568fbafc785704883bc7fc336bd185910aed5daL588-R593): Removed redundant change detection logic and replaced it with a call to the new `check_for_changes` method. * [`src/bootstrap/src/core/config/config.rs`](diffhunk://#diff-5f5330cfcdb0a89b85ac3547b761c3a45c2534a85c4aaae8fea88c711a7a65b2R2837-R2872): Added a new method `check_for_changes` to centralize the logic for detecting changes in specified directories since a given commit. * [`src/bootstrap/src/core/config/config.rs`](diffhunk://#diff-5f5330cfcdb0a89b85ac3547b761c3a45c2534a85c4aaae8fea88c711a7a65b2L2728-R2740): Updated the existing change detection code to use the new `check_for_changes` method. Cleanup: * [`src/bootstrap/src/core/build_steps/tool.rs`](diffhunk://#diff-dc86e288bcf7b3ca3f8c127d3568fbafc785704883bc7fc336bd185910aed5daL13-R13): Removed the unused import `git` from the helpers module. r? ``@AlbertLarsan68``
2 parents be01dab + 1b59216 commit 4caa60c

File tree

2 files changed

+23
-66
lines changed

2 files changed

+23
-66
lines changed

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

+6-25
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,14 @@
11
use std::path::PathBuf;
22
use std::{env, fs};
33

4-
use build_helper::git::get_closest_merge_commit;
5-
64
use crate::core::build_steps::compile;
75
use crate::core::build_steps::toolstate::ToolState;
86
use crate::core::builder;
97
use crate::core::builder::{Builder, Cargo as CargoCommand, RunConfig, ShouldRun, Step};
108
use crate::core::config::TargetSelection;
119
use crate::utils::channel::GitInfo;
1210
use crate::utils::exec::{BootstrapCommand, command};
13-
use crate::utils::helpers::{add_dylib_path, exe, git, t};
11+
use crate::utils::helpers::{add_dylib_path, exe, t};
1412
use crate::{Compiler, Kind, Mode, gha};
1513

1614
#[derive(Debug, Clone, Hash, PartialEq, Eq)]
@@ -596,28 +594,11 @@ impl Step for Rustdoc {
596594
&& target_compiler.stage > 0
597595
&& builder.rust_info().is_managed_git_subrepository()
598596
{
599-
let commit = get_closest_merge_commit(
600-
Some(&builder.config.src),
601-
&builder.config.git_config(),
602-
&[],
603-
)
604-
.unwrap();
605-
606-
let librustdoc_src = builder.config.src.join("src/librustdoc");
607-
let rustdoc_src = builder.config.src.join("src/tools/rustdoc");
608-
609-
// FIXME: The change detection logic here is quite similar to `Config::download_ci_rustc_commit`.
610-
// It would be better to unify them.
611-
let has_changes = !git(Some(&builder.config.src))
612-
.allow_failure()
613-
.run_always()
614-
.args(["diff-index", "--quiet", &commit])
615-
.arg("--")
616-
.arg(librustdoc_src)
617-
.arg(rustdoc_src)
618-
.run(builder);
619-
620-
if !has_changes {
597+
let files_to_track = &["src/librustdoc", "src/tools/rustdoc"];
598+
599+
// Check if unchanged
600+
if builder.config.last_modified_commit(files_to_track, "download-rustc", true).is_some()
601+
{
621602
let precompiled_rustdoc = builder
622603
.config
623604
.ci_rustc_dir()

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

+17-41
Original file line numberDiff line numberDiff line change
@@ -2754,25 +2754,25 @@ impl Config {
27542754
}
27552755
};
27562756

2757-
let files_to_track = &[
2758-
self.src.join("compiler"),
2759-
self.src.join("library"),
2760-
self.src.join("src/version"),
2761-
self.src.join("src/stage0"),
2762-
self.src.join("src/ci/channel"),
2763-
];
2757+
let files_to_track =
2758+
&["compiler", "library", "src/version", "src/stage0", "src/ci/channel"];
27642759

27652760
// Look for a version to compare to based on the current commit.
27662761
// Only commits merged by bors will have CI artifacts.
2767-
let commit =
2768-
get_closest_merge_commit(Some(&self.src), &self.git_config(), files_to_track).unwrap();
2769-
if commit.is_empty() {
2770-
println!("ERROR: could not find commit hash for downloading rustc");
2771-
println!("HELP: maybe your repository history is too shallow?");
2772-
println!("HELP: consider disabling `download-rustc`");
2773-
println!("HELP: or fetch enough history to include one upstream commit");
2774-
crate::exit!(1);
2775-
}
2762+
let commit = match self.last_modified_commit(files_to_track, "download-rustc", if_unchanged)
2763+
{
2764+
Some(commit) => commit,
2765+
None => {
2766+
if if_unchanged {
2767+
return None;
2768+
}
2769+
println!("ERROR: could not find commit hash for downloading rustc");
2770+
println!("HELP: maybe your repository history is too shallow?");
2771+
println!("HELP: consider disabling `download-rustc`");
2772+
println!("HELP: or fetch enough history to include one upstream commit");
2773+
crate::exit!(1);
2774+
}
2775+
};
27762776

27772777
if CiEnv::is_ci() && {
27782778
let head_sha =
@@ -2787,31 +2787,7 @@ impl Config {
27872787
return None;
27882788
}
27892789

2790-
// Warn if there were changes to the compiler or standard library since the ancestor commit.
2791-
let has_changes = !t!(helpers::git(Some(&self.src))
2792-
.args(["diff-index", "--quiet", &commit])
2793-
.arg("--")
2794-
.args(files_to_track)
2795-
.as_command_mut()
2796-
.status())
2797-
.success();
2798-
if has_changes {
2799-
if if_unchanged {
2800-
if self.is_verbose() {
2801-
println!(
2802-
"WARNING: saw changes to compiler/ or library/ since {commit}; \
2803-
ignoring `download-rustc`"
2804-
);
2805-
}
2806-
return None;
2807-
}
2808-
println!(
2809-
"WARNING: `download-rustc` is enabled, but there are changes to \
2810-
compiler/ or library/"
2811-
);
2812-
}
2813-
2814-
Some(commit.to_string())
2790+
Some(commit)
28152791
}
28162792

28172793
fn parse_download_ci_llvm(

0 commit comments

Comments
 (0)