Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

extend the "if-unchanged" logic for compiler builds #131831

Merged
merged 7 commits into from
Nov 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
resolver = "2"
members = [
"compiler/rustc",
"src/build_helper",
"src/etc/test-float-parse",
"src/rustc-std-workspace/rustc-std-workspace-core",
"src/rustc-std-workspace/rustc-std-workspace-alloc",
"src/rustc-std-workspace/rustc-std-workspace-std",
"src/rustdoc-json-types",
"src/tools/build_helper",
"src/tools/cargotest",
"src/tools/clippy",
"src/tools/clippy/clippy_dev",
Expand Down
5 changes: 3 additions & 2 deletions config.example.toml
Original file line number Diff line number Diff line change
Expand Up @@ -500,8 +500,9 @@
# This is useful if you are working on tools, doc-comments, or library (you will be able to build
# the standard library without needing to build the compiler).
#
# Set this to "if-unchanged" to only download if the compiler (and library if running on CI) have
# not been modified.
# Set this to "if-unchanged" if you are working on `src/tools`, `tests` or `library` (on CI, `library`
# changes triggers in-tree compiler build) to speed up the build process.
#
# Set this to `true` to download unconditionally.
#download-rustc = false

Expand Down
2 changes: 1 addition & 1 deletion src/bootstrap/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ test = false
cc = "=1.1.22"
cmake = "=0.1.48"

build_helper = { path = "../tools/build_helper" }
build_helper = { path = "../build_helper" }
clap = { version = "4.4", default-features = false, features = ["std", "usage", "help", "derive", "error-context"] }
clap_complete = "4.4"
fd-lock = "4.0"
Expand Down
2 changes: 1 addition & 1 deletion src/bootstrap/src/core/build_steps/clippy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ macro_rules! lint_any {

lint_any!(
Bootstrap, "src/bootstrap", "bootstrap";
BuildHelper, "src/tools/build_helper", "build_helper";
BuildHelper, "src/build_helper", "build_helper";
BuildManifest, "src/tools/build-manifest", "build-manifest";
CargoMiri, "src/tools/miri/cargo-miri", "cargo-miri";
Clippy, "src/tools/clippy", "clippy";
Expand Down
2 changes: 1 addition & 1 deletion src/bootstrap/src/core/build_steps/doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1030,7 +1030,7 @@ macro_rules! tool_doc {
// NOTE: make sure to register these in `Builder::get_step_description`.
tool_doc!(
BuildHelper,
"src/tools/build_helper",
"src/build_helper",
rustc_tool = false,
is_library = true,
crates = ["build_helper"]
Expand Down
4 changes: 2 additions & 2 deletions src/bootstrap/src/core/build_steps/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1354,7 +1354,7 @@ impl Step for CrateBuildHelper {
const ONLY_HOSTS: bool = true;

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

fn make_run(run: RunConfig<'_>) {
Expand All @@ -1372,7 +1372,7 @@ impl Step for CrateBuildHelper {
Mode::ToolBootstrap,
host,
Kind::Test,
"src/tools/build_helper",
"src/build_helper",
SourceType::InTree,
&[],
);
Expand Down
53 changes: 36 additions & 17 deletions src/bootstrap/src/core/config/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,24 @@ use crate::utils::cache::{INTERNER, Interned};
use crate::utils::channel::{self, GitInfo};
use crate::utils::helpers::{self, exe, output, t};

/// Each path in this list is considered "allowed" in the `download-rustc="if-unchanged"` logic.
/// This means they can be modified and changes to these paths should never trigger a compiler build
/// when "if-unchanged" is set.
///
/// NOTE: Paths must have the ":!" prefix to tell git to ignore changes in those paths during
/// the diff check.
///
/// WARNING: Be cautious when adding paths to this list. If a path that influences the compiler build
/// is added here, it will cause bootstrap to skip necessary rebuilds, which may lead to risky results.
/// For example, "src/bootstrap" should never be included in this list as it plays a crucial role in the
/// final output/compiler, which can be significantly affected by changes made to the bootstrap sources.
#[rustfmt::skip] // We don't want rustfmt to oneline this list
pub(crate) const RUSTC_IF_UNCHANGED_ALLOWED_PATHS: &[&str] = &[
":!src/tools",
":!tests",
":!triagebot.toml",
];

macro_rules! check_ci_llvm {
($name:expr) => {
assert!(
Expand Down Expand Up @@ -2768,32 +2786,33 @@ impl Config {
}
};

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

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

// Look for a version to compare to based on the current commit.
// Only commits merged by bors will have CI artifacts.
let commit =
match self.last_modified_commit(&files_to_track, "download-rustc", if_unchanged) {
Some(commit) => commit,
None => {
if if_unchanged {
return None;
}
println!("ERROR: could not find commit hash for downloading rustc");
println!("HELP: maybe your repository history is too shallow?");
println!("HELP: consider disabling `download-rustc`");
println!("HELP: or fetch enough history to include one upstream commit");
crate::exit!(1);
let commit = match self.last_modified_commit(&allowed_paths, "download-rustc", if_unchanged)
{
Some(commit) => commit,
None => {
if if_unchanged {
return None;
}
};
println!("ERROR: could not find commit hash for downloading rustc");
println!("HELP: maybe your repository history is too shallow?");
println!("HELP: consider setting `rust.download-rustc=false` in config.toml");
println!("HELP: or fetch enough history to include one upstream commit");
crate::exit!(1);
}
};

if CiEnv::is_ci() && {
let head_sha =
Expand Down
17 changes: 16 additions & 1 deletion src/bootstrap/src/core/config/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use clap::CommandFactory;
use serde::Deserialize;

use super::flags::Flags;
use super::{ChangeIdWrapper, Config};
use super::{ChangeIdWrapper, Config, RUSTC_IF_UNCHANGED_ALLOWED_PATHS};
use crate::core::build_steps::clippy::get_clippy_rules_in_order;
use crate::core::build_steps::llvm;
use crate::core::config::{LldMode, Target, TargetSelection, TomlConfig};
Expand Down Expand Up @@ -410,3 +410,18 @@ fn jobs_precedence() {
);
assert_eq!(config.jobs, Some(123));
}

#[test]
fn check_rustc_if_unchanged_paths() {
let config = parse("");
let normalised_allowed_paths: Vec<_> = RUSTC_IF_UNCHANGED_ALLOWED_PATHS
.iter()
.map(|t| {
t.strip_prefix(":!").expect(&format!("{t} doesn't have ':!' prefix, but it should."))
})
.collect();

for p in normalised_allowed_paths {
assert!(config.src.join(p).exists(), "{p} doesn't exist.");
}
}
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pub struct Stage0Config {
}

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

let mut stage0 = Stage0::default();
for line in stage0_content.lines() {
Expand Down
File renamed without changes.
4 changes: 1 addition & 3 deletions src/ci/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -179,9 +179,7 @@ else
fi

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

Expand Down
2 changes: 1 addition & 1 deletion src/tools/bump-stage0/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ edition = "2021"

[dependencies]
anyhow = "1.0.34"
build_helper = { path = "../build_helper" }
build_helper = { path = "../../build_helper" }
curl = "0.4.38"
indexmap = { version = "2.0.0", features = ["serde"] }
serde = { version = "1.0.125", features = ["derive"] }
Expand Down
2 changes: 1 addition & 1 deletion src/tools/compiletest/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ unified-diff = "0.2.1"
getopts = "0.2"
indexmap = "2.0.0"
miropt-test-tools = { path = "../miropt-test-tools" }
build_helper = { path = "../build_helper" }
build_helper = { path = "../../build_helper" }
tracing = "0.1"
tracing-subscriber = { version = "0.3.3", default-features = false, features = ["fmt", "env-filter", "smallvec", "parking_lot", "ansi"] }
regex = "1.0"
Expand Down
2 changes: 1 addition & 1 deletion src/tools/opt-dist/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ version = "0.1.0"
edition = "2021"

[dependencies]
build_helper = { path = "../build_helper" }
build_helper = { path = "../../build_helper" }
env_logger = "0.11"
log = "0.4"
anyhow = { version = "1", features = ["backtrace"] }
Expand Down
2 changes: 1 addition & 1 deletion src/tools/run-make-support/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ similar = "2.5.0"
wasmparser = { version = "0.216", default-features = false, features = ["std"] }
regex = "1.8" # 1.8 to avoid memchr 2.6.0, as 2.5.0 is pinned in the workspace
gimli = "0.31.0"
build_helper = { path = "../build_helper" }
build_helper = { path = "../../build_helper" }
serde_json = "1.0"
libc = "0.2"

Expand Down
2 changes: 1 addition & 1 deletion src/tools/rustdoc-gui-test/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ version = "0.1.0"
edition = "2021"

[dependencies]
build_helper = { path = "../build_helper" }
build_helper = { path = "../../build_helper" }
compiletest = { path = "../compiletest" }
getopts = "0.2"
walkdir = "2"
2 changes: 1 addition & 1 deletion src/tools/suggest-tests/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@ edition = "2021"

[dependencies]
glob = "0.3.0"
build_helper = { version = "0.1.0", path = "../build_helper" }
build_helper = { version = "0.1.0", path = "../../build_helper" }
2 changes: 1 addition & 1 deletion src/tools/tidy/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ edition = "2021"
autobins = false

[dependencies]
build_helper = { path = "../build_helper" }
build_helper = { path = "../../build_helper" }
cargo_metadata = "0.18"
regex = "1"
miropt-test-tools = { path = "../miropt-test-tools" }
Expand Down
Loading