Auto merge of rust-lang#138704 - Kobzol:ci-llvm-checks, r=onur-ozkan

Simplify CI LLVM checks in bootstrap

Extracted out of rust-lang#138591. Apart from simplifying the checks, it will make it easier to run E2E tests of bootstrap on a mostly empty directory without checking out LLVM on CI :)

The commits should be mostly self-explanatory.

r? `@onur-ozkan`

Author: bors

src/bootstrap/src/core/build_steps/gcc.rs

@@ -273,7 +273,7 @@ fn detect_gcc_sha(config: &crate::Config, is_git: bool) -> String {
         get_closest_merge_commit(
             Some(&config.src),
             &config.git_config(),
-            &[config.src.join("src/gcc"), config.src.join("src/bootstrap/download-ci-gcc-stamp")],
+            &["src/gcc", "src/bootstrap/download-ci-gcc-stamp"],
         )
         .unwrap()
     } else if let Some(info) = crate::utils::channel::read_commit_info_file(&config.src) {

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;
@@ -174,20 +173,19 @@ pub fn prebuilt_llvm_config(
     LlvmBuildStatus::ShouldBuild(Meta { stamp, res, out_dir, root: root.into() })
 }
 
+/// Paths whose changes invalidate LLVM downloads.
+pub const LLVM_INVALIDATION_PATHS: &[&str] = &[
+    "src/llvm-project",
+    "src/bootstrap/download-ci-llvm-stamp",
+    // the LLVM shared object file is named `LLVM-<LLVM-version>-rust-{version}-nightly`
+    "src/version",
+];
+
 /// This retrieves the LLVM sha we *want* to use, according to git history.
 pub(crate) fn detect_llvm_sha(config: &Config, is_git: bool) -> String {
     let llvm_sha = if is_git {
-        get_closest_merge_commit(
-            Some(&config.src),
-            &config.git_config(),
-            &[
-                config.src.join("src/llvm-project"),
-                config.src.join("src/bootstrap/download-ci-llvm-stamp"),
-                // the LLVM shared object file is named `LLVM-12-rust-{version}-nightly`
-                config.src.join("src/version"),
-            ],
-        )
-        .unwrap()
+        get_closest_merge_commit(Some(&config.src), &config.git_config(), LLVM_INVALIDATION_PATHS)
+            .unwrap()
     } else if let Some(info) = crate::utils::channel::read_commit_info_file(&config.src) {
         info.sha.trim().to_owned()
     } else {
@@ -207,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
@@ -255,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,

src/bootstrap/src/core/builder/tests.rs

@@ -1074,7 +1074,7 @@ fn test_prebuilt_llvm_config_path_resolution() {
     let config = configure(
         r#"
             [llvm]
-            download-ci-llvm = true
+            download-ci-llvm = "if-unchanged"
         "#,
     );
 

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

@@ -23,6 +23,7 @@ use tracing::{instrument, span};
 
 use crate::core::build_steps::compile::CODEGEN_BACKEND_PREFIX;
 use crate::core::build_steps::llvm;
+use crate::core::build_steps::llvm::LLVM_INVALIDATION_PATHS;
 pub use crate::core::config::flags::Subcommand;
 use crate::core::config::flags::{Color, Flags, Warnings};
 use crate::core::download::is_download_ci_available;
@@ -3096,7 +3097,14 @@ impl Config {
         download_ci_llvm: Option<StringOrBool>,
         asserts: bool,
     ) -> bool {
-        let download_ci_llvm = download_ci_llvm.unwrap_or(StringOrBool::Bool(true));
+        // We don't ever want to use `true` on CI, as we should not
+        // download upstream artifacts if there are any local modifications.
+        let default = if self.is_running_on_ci {
+            StringOrBool::String("if-unchanged".to_string())
+        } else {
+            StringOrBool::Bool(true)
+        };
+        let download_ci_llvm = download_ci_llvm.unwrap_or(default);
 
         let if_unchanged = || {
             if self.rust_info.is_from_tarball() {
@@ -3109,13 +3117,13 @@ impl Config {
             #[cfg(not(test))]
             self.update_submodule("src/llvm-project");
 
-            // Check for untracked changes in `src/llvm-project`.
+            // Check for untracked changes in `src/llvm-project` and other important places.
             let has_changes = self
-                .last_modified_commit(&["src/llvm-project"], "download-ci-llvm", true)
+                .last_modified_commit(LLVM_INVALIDATION_PATHS, "download-ci-llvm", true)
                 .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 {
@@ -3126,8 +3134,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) => {

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

@@ -12,6 +12,7 @@ use super::flags::Flags;
 use super::{ChangeIdWrapper, Config, RUSTC_IF_UNCHANGED_ALLOWED_PATHS};
 use crate::core::build_steps::clippy::{LintConfig, get_clippy_rules_in_order};
 use crate::core::build_steps::llvm;
+use crate::core::build_steps::llvm::LLVM_INVALIDATION_PATHS;
 use crate::core::config::{LldMode, Target, TargetSelection, TomlConfig};
 
 pub(crate) fn parse(config: &str) -> Config {
@@ -24,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);
     }
@@ -41,7 +50,7 @@ fn download_ci_llvm() {
     let if_unchanged_config = parse("llvm.download-ci-llvm = \"if-unchanged\"");
     if if_unchanged_config.llvm_from_ci {
         let has_changes = if_unchanged_config
-            .last_modified_commit(&["src/llvm-project"], "download-ci-llvm", true)
+            .last_modified_commit(LLVM_INVALIDATION_PATHS, "download-ci-llvm", true)
             .is_none();
 
         assert!(

src/bootstrap/src/lib.rs

@@ -15,6 +15,7 @@
 //!
 //! More documentation can be found in each respective module below, and you can
 //! also check out the `src/bootstrap/README.md` file for more information.
+#![cfg_attr(test, allow(unused))]
 
 use std::cell::{Cell, RefCell};
 use std::collections::{BTreeSet, HashMap, HashSet};

src/build_helper/src/git.rs

@@ -1,4 +1,4 @@
-use std::path::{Path, PathBuf};
+use std::path::Path;
 use std::process::{Command, Stdio};
 
 use crate::ci::CiEnv;
@@ -121,7 +121,7 @@ fn git_upstream_merge_base(
 pub fn get_closest_merge_commit(
     git_dir: Option<&Path>,
     config: &GitConfig<'_>,
-    target_paths: &[PathBuf],
+    target_paths: &[&str],
 ) -> Result<String, String> {
     let mut git = Command::new("git");
 

src/ci/docker/host-x86_64/mingw-check/check-default-config-profiles.sh

@@ -8,5 +8,6 @@ config_dir="../src/bootstrap/defaults"
 # Loop through each configuration file in the directory
 for config_file in "$config_dir"/*.toml;
 do
-    python3 ../x.py check --config $config_file --dry-run
+    # Disable CI mode, because it is not compatible with all profiles
+    python3 ../x.py check --config $config_file --dry-run --ci=false
 done