From 40c2ca96411caaeab1563ff9041879f742d1d71b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 12 Jun 2025 14:34:33 +0200 Subject: [PATCH 1/5] Move `shared_helpers` test to a dedicated module --- src/bootstrap/src/utils/shared_helpers.rs | 3 +++ .../shared_helpers_tests.rs => shared_helpers/tests.rs} | 7 ------- src/bootstrap/src/utils/tests/mod.rs | 1 - 3 files changed, 3 insertions(+), 8 deletions(-) rename src/bootstrap/src/utils/{tests/shared_helpers_tests.rs => shared_helpers/tests.rs} (73%) diff --git a/src/bootstrap/src/utils/shared_helpers.rs b/src/bootstrap/src/utils/shared_helpers.rs index 561af34a4478b..9c6b4a7615d81 100644 --- a/src/bootstrap/src/utils/shared_helpers.rs +++ b/src/bootstrap/src/utils/shared_helpers.rs @@ -6,6 +6,9 @@ #![allow(dead_code)] +#[cfg(test)] +mod tests; + use std::env; use std::ffi::OsString; use std::fs::OpenOptions; diff --git a/src/bootstrap/src/utils/tests/shared_helpers_tests.rs b/src/bootstrap/src/utils/shared_helpers/tests.rs similarity index 73% rename from src/bootstrap/src/utils/tests/shared_helpers_tests.rs rename to src/bootstrap/src/utils/shared_helpers/tests.rs index 6c47e7f24387e..559e9f70abd2f 100644 --- a/src/bootstrap/src/utils/tests/shared_helpers_tests.rs +++ b/src/bootstrap/src/utils/shared_helpers/tests.rs @@ -1,10 +1,3 @@ -//! The `shared_helpers` module can't have its own tests submodule, because -//! that would cause problems for the shim binaries that include it via -//! `#[path]`, so instead those unit tests live here. -//! -//! To prevent tidy from complaining about this file not being named `tests.rs`, -//! it lives inside a submodule directory named `tests`. - use crate::utils::shared_helpers::parse_value_from_args; #[test] diff --git a/src/bootstrap/src/utils/tests/mod.rs b/src/bootstrap/src/utils/tests/mod.rs index 73d55db994cc0..c2bf1c3ee8a82 100644 --- a/src/bootstrap/src/utils/tests/mod.rs +++ b/src/bootstrap/src/utils/tests/mod.rs @@ -1,2 +1 @@ pub mod git; -mod shared_helpers_tests; From 259157126328fb7bf84b1d5ecc64b28a2ca06997 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 12 Jun 2025 14:36:59 +0200 Subject: [PATCH 2/5] Stop using Config for `tempdir-as-a-service` in `build_stamp` tests --- src/bootstrap/src/utils/build_stamp/tests.rs | 24 ++++++++------------ 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/src/bootstrap/src/utils/build_stamp/tests.rs b/src/bootstrap/src/utils/build_stamp/tests.rs index 2d7d1f7124655..f150266159a25 100644 --- a/src/bootstrap/src/utils/build_stamp/tests.rs +++ b/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"); From 85401882d23f8edc946ef5e08e8c254da4ac1936 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 12 Jun 2025 15:18:21 +0200 Subject: [PATCH 3/5] Remove `RefCell` from `cc/cxx/ar/ranlib` It wasn't really needed there. --- src/bootstrap/src/core/build_steps/test.rs | 5 ++- src/bootstrap/src/core/build_steps/tool.rs | 2 +- src/bootstrap/src/core/builder/cargo.rs | 4 +-- src/bootstrap/src/lib.rs | 38 +++++++++++----------- src/bootstrap/src/utils/cc_detect.rs | 16 ++++----- src/bootstrap/src/utils/cc_detect/tests.rs | 26 +++++++-------- src/bootstrap/src/utils/tests/mod.rs | 2 ++ 7 files changed, 44 insertions(+), 49 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index ebb926d81cef4..8e21da0050f33 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/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)) diff --git a/src/bootstrap/src/core/build_steps/tool.rs b/src/bootstrap/src/core/build_steps/tool.rs index 680718f055299..3c7c42f43dba5 100644 --- a/src/bootstrap/src/core/build_steps/tool.rs +++ b/src/bootstrap/src/core/build_steps/tool.rs @@ -1321,7 +1321,7 @@ impl Builder<'_> { if compiler.host.is_msvc() { let curpaths = env::var_os("PATH").unwrap_or_default(); let curpaths = env::split_paths(&curpaths).collect::>(); - for (k, v) in self.cc.borrow()[&compiler.host].env() { + for (k, v) in self.cc[&compiler.host].env() { if k != "PATH" { continue; } diff --git a/src/bootstrap/src/core/builder/cargo.rs b/src/bootstrap/src/core/builder/cargo.rs index c6cfdb69356c5..0e3c3aaee0ff7 100644 --- a/src/bootstrap/src/core/builder/cargo.rs +++ b/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"); } diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index 25e59bfe3a889..5303e9a91b6fa 100644 --- a/src/bootstrap/src/lib.rs +++ b/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}; @@ -190,10 +190,10 @@ pub struct Build { // Runtime state filled in later on // C/C++ compilers and archiver for all targets - cc: RefCell>, - cxx: RefCell>, - ar: RefCell>, - ranlib: RefCell>, + cc: HashMap, + cxx: HashMap, + ar: HashMap, + ranlib: HashMap, // Miscellaneous // allow bidirectional lookups: both name -> path and path -> name crates: HashMap, @@ -464,10 +464,10 @@ 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(), crates: HashMap::new(), crate_paths: HashMap::new(), is_sudo, @@ -493,7 +493,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. @@ -1133,17 +1133,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 @@ -1153,8 +1153,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 @@ -1207,7 +1207,7 @@ 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. @@ -1215,7 +1215,7 @@ impl Build { 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. @@ -1223,7 +1223,7 @@ impl Build { 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")), } @@ -1240,7 +1240,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() diff --git a/src/bootstrap/src/utils/cc_detect.rs b/src/bootstrap/src/utils/cc_detect.rs index 851bb38074ef7..2af30c16f8fd9 100644 --- a/src/bootstrap/src/utils/cc_detect.rs +++ b/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); } } diff --git a/src/bootstrap/src/utils/cc_detect/tests.rs b/src/bootstrap/src/utils/cc_detect/tests.rs index 1f50738959fbb..9ddf91fcc8588 100644 --- a/src/bootstrap/src/utils/cc_detect/tests.rs +++ b/src/bootstrap/src/utils/cc_detect/tests.rs @@ -119,18 +119,14 @@ fn test_find_target_with_config() { target_config.ar = Some(PathBuf::from("dummy-ar")); target_config.ranlib = Some(PathBuf::from("dummy-ranlib")); build.config.target_config.insert(target.clone(), target_config); - find_target(&build, target.clone()); - let binding = build.cc.borrow(); - let cc_tool = binding.get(&target).unwrap(); + fill_target_compiler(&mut build, target.clone()); + let cc_tool = build.cc.get(&target).unwrap(); assert_eq!(cc_tool.path(), &PathBuf::from("dummy-cc")); - let binding = build.cxx.borrow(); - let cxx_tool = binding.get(&target).unwrap(); + let cxx_tool = build.cxx.get(&target).unwrap(); assert_eq!(cxx_tool.path(), &PathBuf::from("dummy-cxx")); - let binding = build.ar.borrow(); - let ar = binding.get(&target).unwrap(); + let ar = build.ar.get(&target).unwrap(); assert_eq!(ar, &PathBuf::from("dummy-ar")); - let binding = build.ranlib.borrow(); - let ranlib = binding.get(&target).unwrap(); + let ranlib = build.ranlib.get(&target).unwrap(); assert_eq!(ranlib, &PathBuf::from("dummy-ranlib")); } @@ -139,12 +135,12 @@ fn test_find_target_without_config() { let mut build = Build::new(Config { ..Config::parse(Flags::parse(&["build".to_owned()])) }); let target = TargetSelection::from_user("x86_64-unknown-linux-gnu"); build.config.target_config.clear(); - find_target(&build, target.clone()); - assert!(build.cc.borrow().contains_key(&target)); + fill_target_compiler(&mut build, target.clone()); + assert!(build.cc.contains_key(&target)); if !target.triple.contains("vxworks") { - assert!(build.cxx.borrow().contains_key(&target)); + assert!(build.cxx.contains_key(&target)); } - assert!(build.ar.borrow().contains_key(&target)); + assert!(build.ar.contains_key(&target)); } #[test] @@ -154,8 +150,8 @@ fn test_find() { let target2 = TargetSelection::from_user("x86_64-unknown-openbsd"); build.targets.push(target1.clone()); build.hosts.push(target2.clone()); - find(&build); + fill_compilers(&mut build); for t in build.hosts.iter().chain(build.targets.iter()).chain(iter::once(&build.host_target)) { - assert!(build.cc.borrow().contains_key(t), "CC not set for target {}", t.triple); + assert!(build.cc.contains_key(t), "CC not set for target {}", t.triple); } } diff --git a/src/bootstrap/src/utils/tests/mod.rs b/src/bootstrap/src/utils/tests/mod.rs index c2bf1c3ee8a82..73c500f6e3691 100644 --- a/src/bootstrap/src/utils/tests/mod.rs +++ b/src/bootstrap/src/utils/tests/mod.rs @@ -1 +1,3 @@ +//! This module contains shared utilities for bootstrap tests. + pub mod git; From dbe15e3e94ac206d212a5f3370edb6f9ac2c4463 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 12 Jun 2025 15:27:48 +0200 Subject: [PATCH 4/5] Remove environment variable modification in `test_default_compiler_wasi` --- src/bootstrap/src/lib.rs | 3 +++ src/bootstrap/src/utils/cc_detect.rs | 5 ++++- src/bootstrap/src/utils/cc_detect/tests.rs | 10 +++------- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index 5303e9a91b6fa..215586b32f099 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -194,6 +194,8 @@ pub struct Build { cxx: HashMap, ar: HashMap, ranlib: HashMap, + wasi_sdk_path: Option, + // Miscellaneous // allow bidirectional lookups: both name -> path and path -> name crates: HashMap, @@ -468,6 +470,7 @@ impl Build { 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, diff --git a/src/bootstrap/src/utils/cc_detect.rs b/src/bootstrap/src/utils/cc_detect.rs index 2af30c16f8fd9..dcafeb80f90ca 100644 --- a/src/bootstrap/src/utils/cc_detect.rs +++ b/src/bootstrap/src/utils/cc_detect.rs @@ -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++"), diff --git a/src/bootstrap/src/utils/cc_detect/tests.rs b/src/bootstrap/src/utils/cc_detect/tests.rs index 9ddf91fcc8588..bed03c18aaad4 100644 --- a/src/bootstrap/src/utils/cc_detect/tests.rs +++ b/src/bootstrap/src/utils/cc_detect/tests.rs @@ -77,11 +77,11 @@ fn test_new_cc_build() { #[test] fn test_default_compiler_wasi() { - let build = Build::new(Config { ..Config::parse(Flags::parse(&["build".to_owned()])) }); + let mut build = Build::new(Config { ..Config::parse(Flags::parse(&["build".to_owned()])) }); let target = TargetSelection::from_user("wasm32-wasi"); let wasi_sdk = PathBuf::from("/wasi-sdk"); - // SAFETY: bootstrap tests run on a single thread - unsafe { env::set_var("WASI_SDK_PATH", &wasi_sdk) }; + build.wasi_sdk_path = Some(wasi_sdk.clone()); + let mut cfg = cc::Build::new(); if let Some(result) = default_compiler(&mut cfg, Language::C, target.clone(), &build) { let expected = { @@ -94,10 +94,6 @@ fn test_default_compiler_wasi() { "default_compiler should return a compiler path for wasi target when WASI_SDK_PATH is set" ); } - // SAFETY: bootstrap tests run on a single thread - unsafe { - env::remove_var("WASI_SDK_PATH"); - } } #[test] From a71b463d2b3ae896db91b6e5ba116c77f1ce7d48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 12 Jun 2025 15:41:02 +0200 Subject: [PATCH 5/5] Move submodule path cache from `parse_gitmodules` to `Builder` It would not be correct if multiple values of `target_dir` were ever passed to the function in the same process. --- src/bootstrap/src/core/builder/mod.rs | 21 +++++++++++++++++++-- src/bootstrap/src/lib.rs | 8 -------- src/bootstrap/src/utils/helpers.rs | 2 +- src/build_helper/src/util.rs | 26 ++++++++++---------------- 4 files changed, 30 insertions(+), 27 deletions(-) diff --git a/src/bootstrap/src/core/builder/mod.rs b/src/bootstrap/src/core/builder/mod.rs index 887db683f7899..7433f0b0f3b44 100644 --- a/src/bootstrap/src/core/builder/mod.rs +++ b/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, + + /// Cached list of submodules from self.build.src. + submodule_paths_cache: OnceLock>, } 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. diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index 215586b32f099..fd874604bbe8a 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -591,14 +591,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) { diff --git a/src/bootstrap/src/utils/helpers.rs b/src/bootstrap/src/utils/helpers.rs index f2c3e8c0df464..4f361af4bf71f 100644 --- a/src/bootstrap/src/utils/helpers.rs +++ b/src/bootstrap/src/utils/helpers.rs @@ -98,7 +98,7 @@ pub fn is_dylib(path: &Path) -> bool { /// Return the path to the containing submodule if available. pub fn submodule_path_of(builder: &Builder<'_>, path: &str) -> Option { - let submodule_paths = build_helper::util::parse_gitmodules(&builder.src); + let submodule_paths = builder.submodule_paths(); submodule_paths.iter().find_map(|submodule_path| { if path.starts_with(submodule_path) { Some(submodule_path.to_string()) } else { None } }) diff --git a/src/build_helper/src/util.rs b/src/build_helper/src/util.rs index 72c05c4c48abf..80dd6813d136e 100644 --- a/src/build_helper/src/util.rs +++ b/src/build_helper/src/util.rs @@ -2,7 +2,6 @@ use std::fs::File; use std::io::{BufRead, BufReader}; use std::path::Path; use std::process::Command; -use std::sync::OnceLock; /// Invokes `build_helper::util::detail_exit` with `cfg!(test)` /// @@ -51,25 +50,20 @@ pub fn try_run(cmd: &mut Command, print_cmd_on_fail: bool) -> Result<(), ()> { } /// Returns the submodule paths from the `.gitmodules` file in the given directory. -pub fn parse_gitmodules(target_dir: &Path) -> &[String] { - static SUBMODULES_PATHS: OnceLock> = OnceLock::new(); +pub fn parse_gitmodules(target_dir: &Path) -> Vec { let gitmodules = target_dir.join(".gitmodules"); assert!(gitmodules.exists(), "'{}' file is missing.", gitmodules.display()); - let init_submodules_paths = || { - let file = File::open(gitmodules).unwrap(); + let file = File::open(gitmodules).unwrap(); - let mut submodules_paths = vec![]; - for line in BufReader::new(file).lines().map_while(Result::ok) { - let line = line.trim(); - if line.starts_with("path") { - let actual_path = line.split(' ').last().expect("Couldn't get value of path"); - submodules_paths.push(actual_path.to_owned()); - } + let mut submodules_paths = vec![]; + for line in BufReader::new(file).lines().map_while(Result::ok) { + let line = line.trim(); + if line.starts_with("path") { + let actual_path = line.split(' ').last().expect("Couldn't get value of path"); + submodules_paths.push(actual_path.to_owned()); } + } - submodules_paths - }; - - SUBMODULES_PATHS.get_or_init(|| init_submodules_paths()) + submodules_paths }