bootstrap: Inhibit download-rustc in CI when tools are changed

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));
     });
 }

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

@@ -43,13 +43,32 @@ 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
+/// 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",
+    // Tool changes should inhibit download-rustc in CI, to avoid situations like
+    // <https://github.com/rust-lang/rust/pull/139998#issuecomment-2824674661>
+    // where download-rustc interferes with test metrics for delicate compiletest changes.
+    ":!src/tools",
+    // tidy-alphabetical-end
 ];
 
 macro_rules! check_ci_llvm {
@@ -3180,16 +3199,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() {

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."))
         })