Skip to content

Commit 134459e

Browse files
authored
Rollup merge of #131831 - onur-ozkan:improve-rustc-if-unchanged-logic, r=Mark-Simulacrum
extend the "if-unchanged" logic for compiler builds Implements the first item from [this tracking issue](#131744). In short, we want to make "if-unchanged" logic to check for changes outside of certain allowed directories, and this PR implements that. See #131658 for more context.
2 parents 583b25d + 2d143ab commit 134459e

File tree

26 files changed

+70
-37
lines changed

26 files changed

+70
-37
lines changed

Cargo.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,12 @@
22
resolver = "2"
33
members = [
44
"compiler/rustc",
5+
"src/build_helper",
56
"src/etc/test-float-parse",
67
"src/rustc-std-workspace/rustc-std-workspace-core",
78
"src/rustc-std-workspace/rustc-std-workspace-alloc",
89
"src/rustc-std-workspace/rustc-std-workspace-std",
910
"src/rustdoc-json-types",
10-
"src/tools/build_helper",
1111
"src/tools/cargotest",
1212
"src/tools/clippy",
1313
"src/tools/clippy/clippy_dev",

config.example.toml

+3-2
Original file line numberDiff line numberDiff line change
@@ -500,8 +500,9 @@
500500
# This is useful if you are working on tools, doc-comments, or library (you will be able to build
501501
# the standard library without needing to build the compiler).
502502
#
503-
# Set this to "if-unchanged" to only download if the compiler (and library if running on CI) have
504-
# not been modified.
503+
# Set this to "if-unchanged" if you are working on `src/tools`, `tests` or `library` (on CI, `library`
504+
# changes triggers in-tree compiler build) to speed up the build process.
505+
#
505506
# Set this to `true` to download unconditionally.
506507
#download-rustc = false
507508

src/bootstrap/Cargo.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ test = false
4040
cc = "=1.1.22"
4141
cmake = "=0.1.48"
4242

43-
build_helper = { path = "../tools/build_helper" }
43+
build_helper = { path = "../build_helper" }
4444
clap = { version = "4.4", default-features = false, features = ["std", "usage", "help", "derive", "error-context"] }
4545
clap_complete = "4.4"
4646
fd-lock = "4.0"

src/bootstrap/src/core/build_steps/clippy.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,7 @@ macro_rules! lint_any {
295295

296296
lint_any!(
297297
Bootstrap, "src/bootstrap", "bootstrap";
298-
BuildHelper, "src/tools/build_helper", "build_helper";
298+
BuildHelper, "src/build_helper", "build_helper";
299299
BuildManifest, "src/tools/build-manifest", "build-manifest";
300300
CargoMiri, "src/tools/miri/cargo-miri", "cargo-miri";
301301
Clippy, "src/tools/clippy", "clippy";

src/bootstrap/src/core/build_steps/doc.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1030,7 +1030,7 @@ macro_rules! tool_doc {
10301030
// NOTE: make sure to register these in `Builder::get_step_description`.
10311031
tool_doc!(
10321032
BuildHelper,
1033-
"src/tools/build_helper",
1033+
"src/build_helper",
10341034
rustc_tool = false,
10351035
is_library = true,
10361036
crates = ["build_helper"]

src/bootstrap/src/core/build_steps/test.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1354,7 +1354,7 @@ impl Step for CrateBuildHelper {
13541354
const ONLY_HOSTS: bool = true;
13551355

13561356
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
1357-
run.path("src/tools/build_helper")
1357+
run.path("src/build_helper")
13581358
}
13591359

13601360
fn make_run(run: RunConfig<'_>) {
@@ -1372,7 +1372,7 @@ impl Step for CrateBuildHelper {
13721372
Mode::ToolBootstrap,
13731373
host,
13741374
Kind::Test,
1375-
"src/tools/build_helper",
1375+
"src/build_helper",
13761376
SourceType::InTree,
13771377
&[],
13781378
);

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

+36-17
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,24 @@ use crate::utils::cache::{INTERNER, Interned};
2828
use crate::utils::channel::{self, GitInfo};
2929
use crate::utils::helpers::{self, exe, output, t};
3030

31+
/// Each path in this list is considered "allowed" in the `download-rustc="if-unchanged"` logic.
32+
/// This means they can be modified and changes to these paths should never trigger a compiler build
33+
/// when "if-unchanged" is set.
34+
///
35+
/// NOTE: Paths must have the ":!" prefix to tell git to ignore changes in those paths during
36+
/// the diff check.
37+
///
38+
/// WARNING: Be cautious when adding paths to this list. If a path that influences the compiler build
39+
/// is added here, it will cause bootstrap to skip necessary rebuilds, which may lead to risky results.
40+
/// For example, "src/bootstrap" should never be included in this list as it plays a crucial role in the
41+
/// final output/compiler, which can be significantly affected by changes made to the bootstrap sources.
42+
#[rustfmt::skip] // We don't want rustfmt to oneline this list
43+
pub(crate) const RUSTC_IF_UNCHANGED_ALLOWED_PATHS: &[&str] = &[
44+
":!src/tools",
45+
":!tests",
46+
":!triagebot.toml",
47+
];
48+
3149
macro_rules! check_ci_llvm {
3250
($name:expr) => {
3351
assert!(
@@ -2768,32 +2786,33 @@ impl Config {
27682786
}
27692787
};
27702788

2771-
let mut files_to_track = vec!["compiler", "src/version", "src/stage0", "src/ci/channel"];
2789+
// RUSTC_IF_UNCHANGED_ALLOWED_PATHS
2790+
let mut allowed_paths = RUSTC_IF_UNCHANGED_ALLOWED_PATHS.to_vec();
27722791

2773-
// In CI, disable ci-rustc if there are changes in the library tree. But for non-CI, ignore
2792+
// In CI, disable ci-rustc if there are changes in the library tree. But for non-CI, allow
27742793
// these changes to speed up the build process for library developers. This provides consistent
27752794
// functionality for library developers between `download-rustc=true` and `download-rustc="if-unchanged"`
27762795
// options.
2777-
if CiEnv::is_ci() {
2778-
files_to_track.push("library");
2796+
if !CiEnv::is_ci() {
2797+
allowed_paths.push(":!library");
27792798
}
27802799

27812800
// Look for a version to compare to based on the current commit.
27822801
// Only commits merged by bors will have CI artifacts.
2783-
let commit =
2784-
match self.last_modified_commit(&files_to_track, "download-rustc", if_unchanged) {
2785-
Some(commit) => commit,
2786-
None => {
2787-
if if_unchanged {
2788-
return None;
2789-
}
2790-
println!("ERROR: could not find commit hash for downloading rustc");
2791-
println!("HELP: maybe your repository history is too shallow?");
2792-
println!("HELP: consider disabling `download-rustc`");
2793-
println!("HELP: or fetch enough history to include one upstream commit");
2794-
crate::exit!(1);
2802+
let commit = match self.last_modified_commit(&allowed_paths, "download-rustc", if_unchanged)
2803+
{
2804+
Some(commit) => commit,
2805+
None => {
2806+
if if_unchanged {
2807+
return None;
27952808
}
2796-
};
2809+
println!("ERROR: could not find commit hash for downloading rustc");
2810+
println!("HELP: maybe your repository history is too shallow?");
2811+
println!("HELP: consider setting `rust.download-rustc=false` in config.toml");
2812+
println!("HELP: or fetch enough history to include one upstream commit");
2813+
crate::exit!(1);
2814+
}
2815+
};
27972816

27982817
if CiEnv::is_ci() && {
27992818
let head_sha =

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

+16-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use clap::CommandFactory;
88
use serde::Deserialize;
99

1010
use super::flags::Flags;
11-
use super::{ChangeIdWrapper, Config};
11+
use super::{ChangeIdWrapper, Config, RUSTC_IF_UNCHANGED_ALLOWED_PATHS};
1212
use crate::core::build_steps::clippy::get_clippy_rules_in_order;
1313
use crate::core::build_steps::llvm;
1414
use crate::core::config::{LldMode, Target, TargetSelection, TomlConfig};
@@ -427,3 +427,18 @@ fn jobs_precedence() {
427427
);
428428
assert_eq!(config.jobs, Some(123));
429429
}
430+
431+
#[test]
432+
fn check_rustc_if_unchanged_paths() {
433+
let config = parse("");
434+
let normalised_allowed_paths: Vec<_> = RUSTC_IF_UNCHANGED_ALLOWED_PATHS
435+
.iter()
436+
.map(|t| {
437+
t.strip_prefix(":!").expect(&format!("{t} doesn't have ':!' prefix, but it should."))
438+
})
439+
.collect();
440+
441+
for p in normalised_allowed_paths {
442+
assert!(config.src.join(p).exists(), "{p} doesn't exist.");
443+
}
444+
}
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.

src/tools/build_helper/src/stage0_parser.rs src/build_helper/src/stage0_parser.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ pub struct Stage0Config {
2525
}
2626

2727
pub fn parse_stage0_file() -> Stage0 {
28-
let stage0_content = include_str!("../../../stage0");
28+
let stage0_content = include_str!("../../stage0");
2929

3030
let mut stage0 = Stage0::default();
3131
for line in stage0_content.lines() {
File renamed without changes.

src/ci/run.sh

+1-3
Original file line numberDiff line numberDiff line change
@@ -179,9 +179,7 @@ else
179179
fi
180180

181181
if [ "$NO_DOWNLOAD_CI_RUSTC" = "" ]; then
182-
# disabled for now, see https://github.com/rust-lang/rust/issues/131658
183-
#RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --set rust.download-rustc=if-unchanged"
184-
true
182+
RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --set rust.download-rustc=if-unchanged"
185183
fi
186184
fi
187185

src/tools/bump-stage0/Cargo.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ edition = "2021"
77

88
[dependencies]
99
anyhow = "1.0.34"
10-
build_helper = { path = "../build_helper" }
10+
build_helper = { path = "../../build_helper" }
1111
curl = "0.4.38"
1212
indexmap = { version = "2.0.0", features = ["serde"] }
1313
serde = { version = "1.0.125", features = ["derive"] }

src/tools/compiletest/Cargo.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ unified-diff = "0.2.1"
1414
getopts = "0.2"
1515
indexmap = "2.0.0"
1616
miropt-test-tools = { path = "../miropt-test-tools" }
17-
build_helper = { path = "../build_helper" }
17+
build_helper = { path = "../../build_helper" }
1818
tracing = "0.1"
1919
tracing-subscriber = { version = "0.3.3", default-features = false, features = ["fmt", "env-filter", "smallvec", "parking_lot", "ansi"] }
2020
regex = "1.0"

src/tools/opt-dist/Cargo.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ version = "0.1.0"
44
edition = "2021"
55

66
[dependencies]
7-
build_helper = { path = "../build_helper" }
7+
build_helper = { path = "../../build_helper" }
88
env_logger = "0.11"
99
log = "0.4"
1010
anyhow = { version = "1", features = ["backtrace"] }

src/tools/run-make-support/Cargo.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ similar = "2.5.0"
1010
wasmparser = { version = "0.216", default-features = false, features = ["std"] }
1111
regex = "1.8" # 1.8 to avoid memchr 2.6.0, as 2.5.0 is pinned in the workspace
1212
gimli = "0.31.0"
13-
build_helper = { path = "../build_helper" }
13+
build_helper = { path = "../../build_helper" }
1414
serde_json = "1.0"
1515
libc = "0.2"
1616

src/tools/rustdoc-gui-test/Cargo.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ version = "0.1.0"
44
edition = "2021"
55

66
[dependencies]
7-
build_helper = { path = "../build_helper" }
7+
build_helper = { path = "../../build_helper" }
88
compiletest = { path = "../compiletest" }
99
getopts = "0.2"
1010
walkdir = "2"

src/tools/suggest-tests/Cargo.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,4 +5,4 @@ edition = "2021"
55

66
[dependencies]
77
glob = "0.3.0"
8-
build_helper = { version = "0.1.0", path = "../build_helper" }
8+
build_helper = { version = "0.1.0", path = "../../build_helper" }

src/tools/tidy/Cargo.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ edition = "2021"
55
autobins = false
66

77
[dependencies]
8-
build_helper = { path = "../build_helper" }
8+
build_helper = { path = "../../build_helper" }
99
cargo_metadata = "0.18"
1010
regex = "1"
1111
miropt-test-tools = { path = "../miropt-test-tools" }

0 commit comments

Comments
 (0)