Remove duplicated check for LLVM modifications and disable download-ci-llvm=true on CI

Author: Kobzol

Diff for: src/bootstrap/src/core/build_steps/llvm.rs

@@ -14,7 +14,6 @@ use std::path::{Path, PathBuf};
 use std::sync::OnceLock;
 use std::{env, fs};
 
-use build_helper::ci::CiEnv;
 use build_helper::git::get_closest_merge_commit;
 #[cfg(feature = "tracing")]
 use tracing::instrument;
@@ -206,10 +205,9 @@ pub(crate) fn detect_llvm_sha(config: &Config, is_git: bool) -> String {
 
 /// Returns whether the CI-found LLVM is currently usable.
 ///
-/// This checks both the build triple platform to confirm we're usable at all,
-/// and then verifies if the current HEAD matches the detected LLVM SHA head,
-/// in which case LLVM is indicated as not available.
-pub(crate) fn is_ci_llvm_available(config: &Config, asserts: bool) -> bool {
+/// This checks the build triple platform to confirm we're usable at all, and if LLVM
+/// with/without assertions is available.
+pub(crate) fn is_ci_llvm_available_for_target(config: &Config, asserts: bool) -> bool {
     // This is currently all tier 1 targets and tier 2 targets with host tools
     // (since others may not have CI artifacts)
     // https://doc.rust-lang.org/rustc/platform-support.html#tier-1
@@ -254,41 +252,9 @@ pub(crate) fn is_ci_llvm_available(config: &Config, asserts: bool) -> bool {
         return false;
     }
 
-    if is_ci_llvm_modified(config) {
-        eprintln!("Detected LLVM as non-available: running in CI and modified LLVM in this change");
-        return false;
-    }
-
     true
 }
 
-/// Returns true if we're running in CI with modified LLVM (and thus can't download it)
-pub(crate) fn is_ci_llvm_modified(config: &Config) -> bool {
-    // If not running in a CI environment, return false.
-    if !config.is_running_on_ci {
-        return false;
-    }
-
-    // In rust-lang/rust managed CI, assert the existence of the LLVM submodule.
-    if CiEnv::is_rust_lang_managed_ci_job() {
-        assert!(
-            config.in_tree_llvm_info.is_managed_git_subrepository(),
-            "LLVM submodule must be fetched in rust-lang/rust managed CI builders."
-        );
-    }
-    // If LLVM submodule isn't present, skip the change check as it won't work.
-    else if !config.in_tree_llvm_info.is_managed_git_subrepository() {
-        return false;
-    }
-
-    let llvm_sha = detect_llvm_sha(config, true);
-    let head_sha = crate::output(
-        helpers::git(Some(&config.src)).arg("rev-parse").arg("HEAD").as_command_mut(),
-    );
-    let head_sha = head_sha.trim();
-    llvm_sha == head_sha
-}
-
 #[derive(Debug, Clone, Hash, PartialEq, Eq)]
 pub struct Llvm {
     pub target: TargetSelection,

Diff for: src/bootstrap/src/core/config/config.rs

@@ -3116,7 +3116,7 @@ impl Config {
                 .is_none();
 
             // Return false if there are untracked changes, otherwise check if CI LLVM is available.
-            if has_changes { false } else { llvm::is_ci_llvm_available(self, asserts) }
+            if has_changes { false } else { llvm::is_ci_llvm_available_for_target(self, asserts) }
         };
 
         match download_ci_llvm {
@@ -3127,8 +3127,15 @@ impl Config {
                     );
                 }
 
+                if b && self.is_running_on_ci {
+                    // On CI, we must always rebuild LLVM if there were any modifications to it
+                    panic!(
+                        "`llvm.download-ci-llvm` cannot be set to `true` on CI. Use `if-unchanged` instead."
+                    );
+                }
+
                 // If download-ci-llvm=true we also want to check that CI llvm is available
-                b && llvm::is_ci_llvm_available(self, asserts)
+                b && llvm::is_ci_llvm_available_for_target(self, asserts)
             }
             StringOrBool::String(s) if s == "if-unchanged" => if_unchanged(),
             StringOrBool::String(other) => {

Diff for: src/bootstrap/src/core/config/tests.rs

@@ -25,13 +25,21 @@ pub(crate) fn parse(config: &str) -> Config {
 #[test]
 fn download_ci_llvm() {
     let config = parse("");
-    let is_available = llvm::is_ci_llvm_available(&config, config.llvm_assertions);
+    let is_available = llvm::is_ci_llvm_available_for_target(&config, config.llvm_assertions);
     if is_available {
         assert!(config.llvm_from_ci);
     }
 
-    let config = parse("llvm.download-ci-llvm = true");
-    let is_available = llvm::is_ci_llvm_available(&config, config.llvm_assertions);
+    let config = Config::parse_inner(
+        Flags::parse(&[
+            "check".to_string(),
+            "--config=/does/not/exist".to_string(),
+            "--ci".to_string(),
+            "false".to_string(),
+        ]),
+        |&_| toml::from_str("llvm.download-ci-llvm = true"),
+    );
+    let is_available = llvm::is_ci_llvm_available_for_target(&config, config.llvm_assertions);
     if is_available {
         assert!(config.llvm_from_ci);
     }