Skip to content

Commit a71b463

Browse files
committed
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.
1 parent dbe15e3 commit a71b463

File tree

4 files changed

+30
-27
lines changed

4 files changed

+30
-27
lines changed

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

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use std::fmt::{self, Debug, Write};
55
use std::hash::Hash;
66
use std::ops::Deref;
77
use std::path::{Path, PathBuf};
8-
use std::sync::LazyLock;
8+
use std::sync::{LazyLock, OnceLock};
99
use std::time::{Duration, Instant};
1010
use std::{env, fs};
1111

@@ -60,6 +60,9 @@ pub struct Builder<'a> {
6060
/// to do. For example: with `./x check foo bar` we get `paths=["foo",
6161
/// "bar"]`.
6262
pub paths: Vec<PathBuf>,
63+
64+
/// Cached list of submodules from self.build.src.
65+
submodule_paths_cache: OnceLock<Vec<String>>,
6366
}
6467

6568
impl Deref for Builder<'_> {
@@ -687,7 +690,7 @@ impl<'a> ShouldRun<'a> {
687690
///
688691
/// [`path`]: ShouldRun::path
689692
pub fn paths(mut self, paths: &[&str]) -> Self {
690-
let submodules_paths = build_helper::util::parse_gitmodules(&self.builder.src);
693+
let submodules_paths = self.builder.submodule_paths();
691694

692695
self.paths.insert(PathSet::Set(
693696
paths
@@ -1180,6 +1183,7 @@ impl<'a> Builder<'a> {
11801183
stack: RefCell::new(Vec::new()),
11811184
time_spent_on_dependencies: Cell::new(Duration::new(0, 0)),
11821185
paths,
1186+
submodule_paths_cache: Default::default(),
11831187
}
11841188
}
11851189

@@ -1510,6 +1514,19 @@ impl<'a> Builder<'a> {
15101514
None
15111515
}
15121516

1517+
/// Updates all submodules, and exits with an error if submodule
1518+
/// management is disabled and the submodule does not exist.
1519+
pub fn require_and_update_all_submodules(&self) {
1520+
for submodule in self.submodule_paths() {
1521+
self.require_submodule(submodule, None);
1522+
}
1523+
}
1524+
1525+
/// Get all submodules from the src directory.
1526+
pub fn submodule_paths(&self) -> &[String] {
1527+
self.submodule_paths_cache.get_or_init(|| build_helper::util::parse_gitmodules(&self.src))
1528+
}
1529+
15131530
/// Ensure that a given step is built, returning its output. This will
15141531
/// cache the step, so it is safe (and good!) to call this as often as
15151532
/// needed to ensure that all dependencies are built.

src/bootstrap/src/lib.rs

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -591,14 +591,6 @@ impl Build {
591591
}
592592
}
593593

594-
/// Updates all submodules, and exits with an error if submodule
595-
/// management is disabled and the submodule does not exist.
596-
pub fn require_and_update_all_submodules(&self) {
597-
for submodule in build_helper::util::parse_gitmodules(&self.src) {
598-
self.require_submodule(submodule, None);
599-
}
600-
}
601-
602594
/// If any submodule has been initialized already, sync it unconditionally.
603595
/// This avoids contributors checking in a submodule change by accident.
604596
fn update_existing_submodules(&self) {

src/bootstrap/src/utils/helpers.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ pub fn is_dylib(path: &Path) -> bool {
9898

9999
/// Return the path to the containing submodule if available.
100100
pub fn submodule_path_of(builder: &Builder<'_>, path: &str) -> Option<String> {
101-
let submodule_paths = build_helper::util::parse_gitmodules(&builder.src);
101+
let submodule_paths = builder.submodule_paths();
102102
submodule_paths.iter().find_map(|submodule_path| {
103103
if path.starts_with(submodule_path) { Some(submodule_path.to_string()) } else { None }
104104
})

src/build_helper/src/util.rs

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ use std::fs::File;
22
use std::io::{BufRead, BufReader};
33
use std::path::Path;
44
use std::process::Command;
5-
use std::sync::OnceLock;
65

76
/// Invokes `build_helper::util::detail_exit` with `cfg!(test)`
87
///
@@ -51,25 +50,20 @@ pub fn try_run(cmd: &mut Command, print_cmd_on_fail: bool) -> Result<(), ()> {
5150
}
5251

5352
/// Returns the submodule paths from the `.gitmodules` file in the given directory.
54-
pub fn parse_gitmodules(target_dir: &Path) -> &[String] {
55-
static SUBMODULES_PATHS: OnceLock<Vec<String>> = OnceLock::new();
53+
pub fn parse_gitmodules(target_dir: &Path) -> Vec<String> {
5654
let gitmodules = target_dir.join(".gitmodules");
5755
assert!(gitmodules.exists(), "'{}' file is missing.", gitmodules.display());
5856

59-
let init_submodules_paths = || {
60-
let file = File::open(gitmodules).unwrap();
57+
let file = File::open(gitmodules).unwrap();
6158

62-
let mut submodules_paths = vec![];
63-
for line in BufReader::new(file).lines().map_while(Result::ok) {
64-
let line = line.trim();
65-
if line.starts_with("path") {
66-
let actual_path = line.split(' ').last().expect("Couldn't get value of path");
67-
submodules_paths.push(actual_path.to_owned());
68-
}
59+
let mut submodules_paths = vec![];
60+
for line in BufReader::new(file).lines().map_while(Result::ok) {
61+
let line = line.trim();
62+
if line.starts_with("path") {
63+
let actual_path = line.split(' ').last().expect("Couldn't get value of path");
64+
submodules_paths.push(actual_path.to_owned());
6965
}
66+
}
7067

71-
submodules_paths
72-
};
73-
74-
SUBMODULES_PATHS.get_or_init(|| init_submodules_paths())
68+
submodules_paths
7569
}

0 commit comments

Comments
 (0)