From 9c05758ed47166d14bf073e6941dc95020ff7713 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Wed, 19 Mar 2025 14:39:16 +0100 Subject: [PATCH] Remove duplicated check for LLVM modifications and disable `download-ci-llvm=true` on CI --- src/bootstrap/src/core/build_steps/llvm.rs | 40 ++-------------------- src/bootstrap/src/core/config/config.rs | 11 ++++-- src/bootstrap/src/core/config/tests.rs | 14 ++++++-- 3 files changed, 23 insertions(+), 42 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/llvm.rs b/src/bootstrap/src/core/build_steps/llvm.rs index efecca6ebddd..28a8afd9c04a 100644 --- a/src/bootstrap/src/core/build_steps/llvm.rs +++ b/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 --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index dfd64c3246d2..b8068373c73f 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/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 --git a/src/bootstrap/src/core/config/tests.rs b/src/bootstrap/src/core/config/tests.rs index 6f21016ea744..068e237c2cdc 100644 --- a/src/bootstrap/src/core/config/tests.rs +++ b/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); }