Rollup merge of #142416 - Kobzol:bootstrap-cleanup-2, r=jieyouxu

Assorted bootstrap cleanups (step 2)

Very small improvements designed towards making bootstrap tests less hacky/special, and towards making it possible to run bootstrap tests in parallel.

Best reviewed commit by commit.

r? `@jieyouxu`

Author: tgross35

src/bootstrap/src/core/build_steps/test.rs

@@ -2009,7 +2009,7 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the
             // Note that if we encounter `PATH` we make sure to append to our own `PATH`
             // rather than stomp over it.
             if !builder.config.dry_run() && target.is_msvc() {
-                for (k, v) in builder.cc.borrow()[&target].env() {
+                for (k, v) in builder.cc[&target].env() {
                     if k != "PATH" {
                         cmd.env(k, v);
                     }
@@ -2026,8 +2026,7 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the
             // address sanitizer enabled (e.g., ntdll.dll).
             cmd.env("ASAN_WIN_CONTINUE_ON_INTERCEPTION_FAILURE", "1");
             // Add the address sanitizer runtime to the PATH - it is located next to cl.exe.
-            let asan_runtime_path =
-                builder.cc.borrow()[&target].path().parent().unwrap().to_path_buf();
+            let asan_runtime_path = builder.cc[&target].path().parent().unwrap().to_path_buf();
             let old_path = cmd
                 .get_envs()
                 .find_map(|(k, v)| (k == "PATH").then_some(v))

src/bootstrap/src/core/build_steps/tool.rs

@@ -1334,7 +1334,7 @@ impl Builder<'_> {
         if compiler.host.is_msvc() {
             let curpaths = env::var_os("PATH").unwrap_or_default();
             let curpaths = env::split_paths(&curpaths).collect::<Vec<_>>();
-            for (k, v) in self.cc.borrow()[&compiler.host].env() {
+            for (k, v) in self.cc[&compiler.host].env() {
                 if k != "PATH" {
                     continue;
                 }

src/bootstrap/src/core/builder/cargo.rs

@@ -278,9 +278,7 @@ impl Cargo {
             self.rustdocflags.arg(&arg);
         }
 
-        if !builder.config.dry_run()
-            && builder.cc.borrow()[&target].args().iter().any(|arg| arg == "-gz")
-        {
+        if !builder.config.dry_run() && builder.cc[&target].args().iter().any(|arg| arg == "-gz") {
             self.rustflags.arg("-Clink-arg=-gz");
         }
 

src/bootstrap/src/core/builder/mod.rs

@@ -5,7 +5,7 @@ use std::fmt::{self, Debug, Write};
 use std::hash::Hash;
 use std::ops::Deref;
 use std::path::{Path, PathBuf};
-use std::sync::LazyLock;
+use std::sync::{LazyLock, OnceLock};
 use std::time::{Duration, Instant};
 use std::{env, fs};
 
@@ -60,6 +60,9 @@ pub struct Builder<'a> {
     /// to do. For example: with `./x check foo bar` we get `paths=["foo",
     /// "bar"]`.
     pub paths: Vec<PathBuf>,
+
+    /// Cached list of submodules from self.build.src.
+    submodule_paths_cache: OnceLock<Vec<String>>,
 }
 
 impl Deref for Builder<'_> {
@@ -687,7 +690,7 @@ impl<'a> ShouldRun<'a> {
     ///
     /// [`path`]: ShouldRun::path
     pub fn paths(mut self, paths: &[&str]) -> Self {
-        let submodules_paths = build_helper::util::parse_gitmodules(&self.builder.src);
+        let submodules_paths = self.builder.submodule_paths();
 
         self.paths.insert(PathSet::Set(
             paths
@@ -1180,6 +1183,7 @@ impl<'a> Builder<'a> {
             stack: RefCell::new(Vec::new()),
             time_spent_on_dependencies: Cell::new(Duration::new(0, 0)),
             paths,
+            submodule_paths_cache: Default::default(),
         }
     }
 
@@ -1510,6 +1514,19 @@ impl<'a> Builder<'a> {
         None
     }
 
+    /// Updates all submodules, and exits with an error if submodule
+    /// management is disabled and the submodule does not exist.
+    pub fn require_and_update_all_submodules(&self) {
+        for submodule in self.submodule_paths() {
+            self.require_submodule(submodule, None);
+        }
+    }
+
+    /// Get all submodules from the src directory.
+    pub fn submodule_paths(&self) -> &[String] {
+        self.submodule_paths_cache.get_or_init(|| build_helper::util::parse_gitmodules(&self.src))
+    }
+
     /// Ensure that a given step is built, returning its output. This will
     /// cache the step, so it is safe (and good!) to call this as often as
     /// needed to ensure that all dependencies are built.

src/bootstrap/src/lib.rs

@@ -17,7 +17,7 @@
 //! also check out the `src/bootstrap/README.md` file for more information.
 #![cfg_attr(test, allow(unused))]
 
-use std::cell::{Cell, RefCell};
+use std::cell::Cell;
 use std::collections::{BTreeSet, HashMap, HashSet};
 use std::fmt::Display;
 use std::path::{Path, PathBuf};
@@ -189,10 +189,12 @@ pub struct Build {
 
     // Runtime state filled in later on
     // C/C++ compilers and archiver for all targets
-    cc: RefCell<HashMap<TargetSelection, cc::Tool>>,
-    cxx: RefCell<HashMap<TargetSelection, cc::Tool>>,
-    ar: RefCell<HashMap<TargetSelection, PathBuf>>,
-    ranlib: RefCell<HashMap<TargetSelection, PathBuf>>,
+    cc: HashMap<TargetSelection, cc::Tool>,
+    cxx: HashMap<TargetSelection, cc::Tool>,
+    ar: HashMap<TargetSelection, PathBuf>,
+    ranlib: HashMap<TargetSelection, PathBuf>,
+    wasi_sdk_path: Option<PathBuf>,
+
     // Miscellaneous
     // allow bidirectional lookups: both name -> path and path -> name
     crates: HashMap<String, Crate>,
@@ -466,10 +468,11 @@ impl Build {
             enzyme_info,
             in_tree_llvm_info,
             in_tree_gcc_info,
-            cc: RefCell::new(HashMap::new()),
-            cxx: RefCell::new(HashMap::new()),
-            ar: RefCell::new(HashMap::new()),
-            ranlib: RefCell::new(HashMap::new()),
+            cc: HashMap::new(),
+            cxx: HashMap::new(),
+            ar: HashMap::new(),
+            ranlib: HashMap::new(),
+            wasi_sdk_path: env::var_os("WASI_SDK_PATH").map(PathBuf::from),
             crates: HashMap::new(),
             crate_paths: HashMap::new(),
             is_sudo,
@@ -498,7 +501,7 @@ impl Build {
         }
 
         build.verbose(|| println!("finding compilers"));
-        utils::cc_detect::find(&build);
+        utils::cc_detect::fill_compilers(&mut build);
         // When running `setup`, the profile is about to change, so any requirements we have now may
         // be different on the next invocation. Don't check for them until the next time x.py is
         // run. This is ok because `setup` never runs any build commands, so it won't fail if commands are missing.
@@ -593,14 +596,6 @@ impl Build {
         }
     }
 
-    /// Updates all submodules, and exits with an error if submodule
-    /// management is disabled and the submodule does not exist.
-    pub fn require_and_update_all_submodules(&self) {
-        for submodule in build_helper::util::parse_gitmodules(&self.src) {
-            self.require_submodule(submodule, None);
-        }
-    }
-
     /// If any submodule has been initialized already, sync it unconditionally.
     /// This avoids contributors checking in a submodule change by accident.
     fn update_existing_submodules(&self) {
@@ -1143,17 +1138,17 @@ impl Build {
         if self.config.dry_run() {
             return PathBuf::new();
         }
-        self.cc.borrow()[&target].path().into()
+        self.cc[&target].path().into()
     }
 
     /// Returns the internal `cc::Tool` for the C compiler.
     fn cc_tool(&self, target: TargetSelection) -> Tool {
-        self.cc.borrow()[&target].clone()
+        self.cc[&target].clone()
     }
 
     /// Returns the internal `cc::Tool` for the C++ compiler.
     fn cxx_tool(&self, target: TargetSelection) -> Tool {
-        self.cxx.borrow()[&target].clone()
+        self.cxx[&target].clone()
     }
 
     /// Returns C flags that `cc-rs` thinks should be enabled for the
@@ -1163,8 +1158,8 @@ impl Build {
             return Vec::new();
         }
         let base = match c {
-            CLang::C => self.cc.borrow()[&target].clone(),
-            CLang::Cxx => self.cxx.borrow()[&target].clone(),
+            CLang::C => self.cc[&target].clone(),
+            CLang::Cxx => self.cxx[&target].clone(),
         };
 
         // Filter out -O and /O (the optimization flags) that we picked up
@@ -1217,23 +1212,23 @@ impl Build {
         if self.config.dry_run() {
             return None;
         }
-        self.ar.borrow().get(&target).cloned()
+        self.ar.get(&target).cloned()
     }
 
     /// Returns the path to the `ranlib` utility for the target specified.
     fn ranlib(&self, target: TargetSelection) -> Option<PathBuf> {
         if self.config.dry_run() {
             return None;
         }
-        self.ranlib.borrow().get(&target).cloned()
+        self.ranlib.get(&target).cloned()
     }
 
     /// Returns the path to the C++ compiler for the target specified.
     fn cxx(&self, target: TargetSelection) -> Result<PathBuf, String> {
         if self.config.dry_run() {
             return Ok(PathBuf::new());
         }
-        match self.cxx.borrow().get(&target) {
+        match self.cxx.get(&target) {
             Some(p) => Ok(p.path().into()),
             None => Err(format!("target `{target}` is not configured as a host, only as a target")),
         }
@@ -1250,7 +1245,7 @@ impl Build {
         } else if target.contains("vxworks") {
             // need to use CXX compiler as linker to resolve the exception functions
             // that are only existed in CXX libraries
-            Some(self.cxx.borrow()[&target].path().into())
+            Some(self.cxx[&target].path().into())
         } else if !self.config.is_host_target(target)
             && helpers::use_host_linker(target)
             && !target.is_msvc()

src/bootstrap/src/utils/build_stamp/tests.rs

@@ -1,32 +1,28 @@
 use std::path::PathBuf;
 
-use crate::{BuildStamp, Config, Flags};
+use tempfile::TempDir;
 
-fn temp_dir() -> PathBuf {
-    let config =
-        Config::parse(Flags::parse(&["check".to_owned(), "--config=/does/not/exist".to_owned()]));
-    config.tempdir()
-}
+use crate::{BuildStamp, Config, Flags};
 
 #[test]
 #[should_panic(expected = "prefix can not start or end with '.'")]
 fn test_with_invalid_prefix() {
-    let dir = temp_dir();
-    BuildStamp::new(&dir).with_prefix(".invalid");
+    let dir = TempDir::new().unwrap();
+    BuildStamp::new(dir.path()).with_prefix(".invalid");
 }
 
 #[test]
 #[should_panic(expected = "prefix can not start or end with '.'")]
 fn test_with_invalid_prefix2() {
-    let dir = temp_dir();
-    BuildStamp::new(&dir).with_prefix("invalid.");
+    let dir = TempDir::new().unwrap();
+    BuildStamp::new(dir.path()).with_prefix("invalid.");
 }
 
 #[test]
 fn test_is_up_to_date() {
-    let dir = temp_dir();
+    let dir = TempDir::new().unwrap();
 
-    let mut build_stamp = BuildStamp::new(&dir).add_stamp("v1.0.0");
+    let mut build_stamp = BuildStamp::new(dir.path()).add_stamp("v1.0.0");
     build_stamp.write().unwrap();
 
     assert!(
@@ -45,9 +41,9 @@ fn test_is_up_to_date() {
 
 #[test]
 fn test_with_prefix() {
-    let dir = temp_dir();
+    let dir = TempDir::new().unwrap();
 
-    let stamp = BuildStamp::new(&dir).add_stamp("v1.0.0");
+    let stamp = BuildStamp::new(dir.path()).add_stamp("v1.0.0");
     assert_eq!(stamp.path.file_name().unwrap(), ".stamp");
 
     let stamp = stamp.with_prefix("test");

src/bootstrap/src/utils/cc_detect.rs

@@ -61,8 +61,8 @@ fn new_cc_build(build: &Build, target: TargetSelection) -> cc::Build {
 ///
 /// This function determines which targets need a C compiler (and, if needed, a C++ compiler)
 /// by combining the primary build target, host targets, and any additional targets. For
-/// each target, it calls [`find_target`] to configure the necessary compiler tools.
-pub fn find(build: &Build) {
+/// each target, it calls [`fill_target_compiler`] to configure the necessary compiler tools.
+pub fn fill_compilers(build: &mut Build) {
     let targets: HashSet<_> = match build.config.cmd {
         // We don't need to check cross targets for these commands.
         crate::Subcommand::Clean { .. }
@@ -87,7 +87,7 @@ pub fn find(build: &Build) {
     };
 
     for target in targets.into_iter() {
-        find_target(build, target);
+        fill_target_compiler(build, target);
     }
 }
 
@@ -96,7 +96,7 @@ pub fn find(build: &Build) {
 /// This function uses both user-specified configuration (from `bootstrap.toml`) and auto-detection
 /// logic to determine the correct C/C++ compilers for the target. It also determines the appropriate
 /// archiver (`ar`) and sets up additional compilation flags (both handled and unhandled).
-pub fn find_target(build: &Build, target: TargetSelection) {
+pub fn fill_target_compiler(build: &mut Build, target: TargetSelection) {
     let mut cfg = new_cc_build(build, target);
     let config = build.config.target_config.get(&target);
     if let Some(cc) = config
@@ -113,7 +113,7 @@ pub fn find_target(build: &Build, target: TargetSelection) {
         cfg.try_get_archiver().map(|c| PathBuf::from(c.get_program())).ok()
     };
 
-    build.cc.borrow_mut().insert(target, compiler.clone());
+    build.cc.insert(target, compiler.clone());
     let mut cflags = build.cc_handled_clags(target, CLang::C);
     cflags.extend(build.cc_unhandled_cflags(target, GitRepo::Rustc, CLang::C));
 
@@ -135,7 +135,7 @@ pub fn find_target(build: &Build, target: TargetSelection) {
     // for VxWorks, record CXX compiler which will be used in lib.rs:linker()
     if cxx_configured || target.contains("vxworks") {
         let compiler = cfg.get_compiler();
-        build.cxx.borrow_mut().insert(target, compiler);
+        build.cxx.insert(target, compiler);
     }
 
     build.verbose(|| println!("CC_{} = {:?}", target.triple, build.cc(target)));
@@ -148,11 +148,11 @@ pub fn find_target(build: &Build, target: TargetSelection) {
     }
     if let Some(ar) = ar {
         build.verbose(|| println!("AR_{} = {ar:?}", target.triple));
-        build.ar.borrow_mut().insert(target, ar);
+        build.ar.insert(target, ar);
     }
 
     if let Some(ranlib) = config.and_then(|c| c.ranlib.clone()) {
-        build.ranlib.borrow_mut().insert(target, ranlib);
+        build.ranlib.insert(target, ranlib);
     }
 }
 
@@ -221,7 +221,10 @@ fn default_compiler(
         }
 
         t if t.contains("-wasi") => {
-            let root = PathBuf::from(std::env::var_os("WASI_SDK_PATH")?);
+            let root = build
+                .wasi_sdk_path
+                .as_ref()
+                .expect("WASI_SDK_PATH mut be configured for a -wasi target");
             let compiler = match compiler {
                 Language::C => format!("{t}-clang"),
                 Language::CPlusPlus => format!("{t}-clang++"),