From f28ff13af549e26f4cd54cc2a2e0656ec6263c90 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sat, 26 Apr 2025 13:45:29 +1000 Subject: [PATCH 1/2] bootstrap: Extract a separate non-CI download-rustc allowlist --- src/bootstrap/src/core/config/config.rs | 25 ++++++++++++++++--------- src/bootstrap/src/core/config/tests.rs | 1 + 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 23b623d9bab23..75c4765b6d95a 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -52,6 +52,21 @@ pub(crate) const RUSTC_IF_UNCHANGED_ALLOWED_PATHS: &[&str] = &[ ":!triagebot.toml", ]; +/// Additional "allowed" paths for the `download-rustc="if-unchanged"` logic +/// that apply to local dev builds, but not to CI builds. +/// +/// When modifying this list, the corresponding tests in +/// `builder::tests::ci_rustc_if_unchanged_logic` should also be updated. +pub(crate) const RUSTC_IF_UNCHANGED_EXTRA_ALLOWED_PATHS_OUTSIDE_CI: &[&str] = &[ + // tidy-alphabetical-start + // 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. + ":!library", + // tidy-alphabetical-end +]; + macro_rules! check_ci_llvm { ($name:expr) => { assert!( @@ -3180,16 +3195,8 @@ impl Config { // 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, 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 you update "library" logic here, update `builder::tests::ci_rustc_if_unchanged_logic` test - // logic accordingly. if !self.is_running_on_ci { - allowed_paths.push(":!library"); + allowed_paths.extend_from_slice(RUSTC_IF_UNCHANGED_EXTRA_ALLOWED_PATHS_OUTSIDE_CI); } let commit = if self.rust_info.is_managed_git_subrepository() { diff --git a/src/bootstrap/src/core/config/tests.rs b/src/bootstrap/src/core/config/tests.rs index 96ac8a6d52fab..fd687108cfb8e 100644 --- a/src/bootstrap/src/core/config/tests.rs +++ b/src/bootstrap/src/core/config/tests.rs @@ -459,6 +459,7 @@ fn check_rustc_if_unchanged_paths() { let config = parse(""); let normalised_allowed_paths: Vec<_> = RUSTC_IF_UNCHANGED_ALLOWED_PATHS .iter() + .chain(super::RUSTC_IF_UNCHANGED_EXTRA_ALLOWED_PATHS_OUTSIDE_CI) .map(|t| { t.strip_prefix(":!").expect(&format!("{t} doesn't have ':!' prefix, but it should.")) }) From b05be3780646d86b8f5905c199603967582d1e92 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sat, 26 Apr 2025 13:48:08 +1000 Subject: [PATCH 2/2] bootstrap: Inhibit download-rustc in CI when tools are changed --- src/bootstrap/src/core/builder/tests.rs | 17 +++++++++++++++-- src/bootstrap/src/core/config/config.rs | 8 ++++++-- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/bootstrap/src/core/builder/tests.rs b/src/bootstrap/src/core/builder/tests.rs index 51852099dc398..98cab3796900a 100644 --- a/src/bootstrap/src/core/builder/tests.rs +++ b/src/bootstrap/src/core/builder/tests.rs @@ -281,14 +281,27 @@ fn ci_rustc_if_unchanged_do_not_invalidate_on_library_changes_outside_ci() { } #[test] -fn ci_rustc_if_unchanged_do_not_invalidate_on_tool_changes() { +fn ci_rustc_if_unchanged_do_not_invalidate_on_tool_changes_in_ci() { git_test(|ctx| { prepare_rustc_checkout(ctx); let sha = ctx.create_upstream_merge(&["compiler/bar"]); - // This change should not invalidate download-ci-rustc + // This change should invalidate download-ci-rustc ctx.create_nonupstream_merge(&["src/tools/foo"]); let config = parse_config_download_rustc_at(ctx.get_path(), "if-unchanged", true); + assert_eq!(config.download_rustc_commit, None); + }); +} + +#[test] +fn ci_rustc_if_unchanged_do_not_invalidate_on_tool_changes_outside_ci() { + git_test(|ctx| { + prepare_rustc_checkout(ctx); + let sha = ctx.create_upstream_merge(&["compiler/bar"]); + // This change should not invalidate download-ci-rustc + ctx.create_nonupstream_merge(&["src/tools/foo"]); + + let config = parse_config_download_rustc_at(ctx.get_path(), "if-unchanged", false); assert_eq!(config.download_rustc_commit, Some(sha)); }); } diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 75c4765b6d95a..4bd23cfce8f7d 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -43,13 +43,13 @@ use crate::utils::helpers::{self, exe, output, t}; /// 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", + // tidy-alphabetical-start ":!src/librustdoc", ":!src/rustdoc-json-types", ":!tests", ":!triagebot.toml", + // tidy-alphabetical-end ]; /// Additional "allowed" paths for the `download-rustc="if-unchanged"` logic @@ -64,6 +64,10 @@ pub(crate) const RUSTC_IF_UNCHANGED_EXTRA_ALLOWED_PATHS_OUTSIDE_CI: &[&str] = &[ // functionality for library developers between `download-rustc=true` and `download-rustc="if-unchanged"` // options. ":!library", + // Tool changes should inhibit download-rustc in CI, to avoid situations like + // + // where download-rustc interferes with test metrics for delicate compiletest changes. + ":!src/tools", // tidy-alphabetical-end ];