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

Author: Kobzol

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 !CiEnv::is_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,

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

@@ -3113,7 +3113,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 {
@@ -3124,8 +3124,15 @@ impl Config {
                     );
                 }
 
+                if b && CiEnv::is_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) => {

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

@@ -4,6 +4,7 @@ use std::fs::{File, remove_file};
 use std::io::Write;
 use std::path::Path;
 
+use build_helper::ci::CiEnv;
 use clap::CommandFactory;
 use serde::Deserialize;
 
@@ -24,15 +25,18 @@ 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);
-    if is_available {
-        assert!(config.llvm_from_ci);
+    // download-ci-llvm=true is not supported on CI
+    if !CiEnv::is_ci() {
+        let config = parse("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);
+        }
     }
 
     let config = parse("llvm.download-ci-llvm = false");