Skip to content

Commit 4960125

Browse files
authored
workspace: Pay attention to members (#3102)
Previously we were ignoring this field and always doing auto-discovery. Cargo only does auto-discovery if this field is not set. Fixes #3090
1 parent a29089b commit 4960125

File tree

32 files changed

+360
-25
lines changed

32 files changed

+360
-25
lines changed

MODULE.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ use_repo(
119119
"cui__cfg-expr-0.17.2",
120120
"cui__clap-4.3.11",
121121
"cui__crates-index-3.3.0",
122+
"cui__glob-0.3.1",
122123
"cui__hex-0.4.3",
123124
"cui__indoc-2.0.5",
124125
"cui__itertools-0.13.0",

crate_universe/3rdparty/crates/BUILD.bazel

Lines changed: 6 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crate_universe/3rdparty/crates/BUILD.glob-0.3.1.bazel

Lines changed: 81 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crate_universe/3rdparty/crates/defs.bzl

Lines changed: 12 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crate_universe/Cargo.lock

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crate_universe/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ tracing = "0.1.40"
9393
tracing-subscriber = "0.3.18"
9494
url = "2.5.2"
9595
walkdir = "2.5.0"
96+
glob = "0.3.1"
9697

9798
[dev-dependencies]
9899
maplit = "1.0.2"

crate_universe/src/metadata/workspace_discoverer.rs

Lines changed: 132 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -50,14 +50,15 @@ fn discover_workspaces_with_cache(
5050
};
5151

5252
// First pass: Discover workspace parents.
53-
for cargo_toml_path in cargo_toml_paths {
54-
if let Some(workspace_parent) = discover_workspace_parent(&cargo_toml_path, manifest_cache)
55-
{
53+
for cargo_toml_path in &cargo_toml_paths {
54+
if let Some(workspace_parent) = discover_workspace_parent(cargo_toml_path, manifest_cache) {
5655
discovered_workspaces
5756
.workspaces_to_members
5857
.insert(workspace_parent, BTreeSet::new());
5958
} else {
60-
discovered_workspaces.non_workspaces.insert(cargo_toml_path);
59+
discovered_workspaces
60+
.non_workspaces
61+
.insert(cargo_toml_path.clone());
6162
}
6263
}
6364

@@ -69,6 +70,24 @@ fn discover_workspaces_with_cache(
6970
.collect::<BTreeSet<_>>()
7071
{
7172
let workspace_manifest = manifest_cache.get(&workspace_path).unwrap();
73+
74+
let workspace_explicit_members = workspace_manifest
75+
.workspace
76+
.as_ref()
77+
.and_then(|workspace| {
78+
// Unfortunately cargo_toml doesn't preserve presence/absence of this field, so we infer empty means absent.
79+
if workspace.members.is_empty() {
80+
return None;
81+
}
82+
let members = workspace
83+
.members
84+
.iter()
85+
.map(|member| glob::Pattern::new(member).map_err(anyhow::Error::from))
86+
.collect::<Result<BTreeSet<_>>>();
87+
Some(members)
88+
})
89+
.transpose()?;
90+
7291
'per_child: for entry in walkdir::WalkDir::new(workspace_path.parent().unwrap())
7392
.follow_links(true)
7493
.follow_root_links(true)
@@ -141,19 +160,54 @@ fn discover_workspaces_with_cache(
141160
bail!("Found manifest at {} which is a member of the workspace at {} which isn't included in the crates_universe", child_path, actual_workspace_path);
142161
}
143162

144-
for exclude in &workspace_manifest.workspace.as_ref().unwrap().exclude {
145-
let exclude_path = workspace_path.parent().unwrap().join(exclude);
146-
if exclude_path == child_path.parent().unwrap() {
147-
discovered_workspaces.non_workspaces.insert(child_path);
163+
let dir_relative_to_workspace_dir = child_path
164+
.parent()
165+
.unwrap()
166+
.strip_prefix(workspace_path.parent().unwrap());
167+
168+
if let Ok(dir_relative_to_workspace_dir) = dir_relative_to_workspace_dir {
169+
use itertools::Itertools;
170+
if workspace_manifest
171+
.workspace
172+
.as_ref()
173+
.unwrap()
174+
.exclude
175+
.contains(&dir_relative_to_workspace_dir.components().join("/"))
176+
{
148177
continue 'per_child;
149178
}
150179
}
151180

152-
discovered_workspaces
153-
.workspaces_to_members
154-
.get_mut(&actual_workspace_path)
155-
.unwrap()
156-
.insert(child_path.clone());
181+
if let (Ok(dir_relative_to_workspace_dir), Some(workspace_explicit_members)) = (
182+
dir_relative_to_workspace_dir,
183+
workspace_explicit_members.as_ref(),
184+
) {
185+
if workspace_explicit_members
186+
.iter()
187+
.any(|glob| glob.matches(dir_relative_to_workspace_dir.as_str()))
188+
{
189+
discovered_workspaces
190+
.workspaces_to_members
191+
.get_mut(&actual_workspace_path)
192+
.unwrap()
193+
.insert(child_path.clone());
194+
}
195+
} else {
196+
discovered_workspaces
197+
.workspaces_to_members
198+
.get_mut(&actual_workspace_path)
199+
.unwrap()
200+
.insert(child_path.clone());
201+
}
202+
}
203+
}
204+
205+
for cargo_toml_path in cargo_toml_paths {
206+
if !discovered_workspaces
207+
.all_workspaces_and_members()
208+
.contains(&cargo_toml_path)
209+
{
210+
discovered_workspaces.non_workspaces.insert(cargo_toml_path);
157211
}
158212
}
159213

@@ -247,18 +301,14 @@ mod test {
247301
expected.non_workspaces.extend([
248302
root_dir.join("non-ws").join("Cargo.toml"),
249303
root_dir.join("ws2").join("ws2excluded").join("Cargo.toml"),
250-
root_dir
251-
.join("ws2")
252-
.join("ws2excluded")
253-
.join("ws2excluded2")
254-
.join("Cargo.toml"),
255304
]);
256305

257306
let actual = discover_workspaces(
258307
vec![
259-
root_dir.join("ws1/ws1c1/Cargo.toml"),
260-
root_dir.join("ws2/Cargo.toml"),
261-
root_dir.join("non-ws/Cargo.toml"),
308+
root_dir.join("ws1").join("ws1c1").join("Cargo.toml"),
309+
root_dir.join("ws2").join("Cargo.toml"),
310+
root_dir.join("ws2").join("ws2excluded").join("Cargo.toml"),
311+
root_dir.join("non-ws").join("Cargo.toml"),
262312
]
263313
.into_iter()
264314
.collect(),
@@ -294,7 +344,7 @@ mod test {
294344
let expected = ws1_discovered_workspaces(&root_dir);
295345

296346
let actual = discover_workspaces(
297-
vec![root_dir.join("ws1/ws1c1/Cargo.toml")]
347+
vec![root_dir.join("ws1").join("ws1c1").join("Cargo.toml")]
298348
.into_iter()
299349
.collect(),
300350
&BTreeMap::new(),
@@ -323,7 +373,7 @@ mod test {
323373
let expected = ws1_discovered_workspaces(&root_dir);
324374

325375
let actual = discover_workspaces(
326-
vec![root_dir.join("ws1/ws1c1/Cargo.toml")]
376+
vec![root_dir.join("ws1").join("ws1c1").join("Cargo.toml")]
327377
.into_iter()
328378
.collect(),
329379
&BTreeMap::new(),
@@ -334,6 +384,65 @@ mod test {
334384
assert_eq!(expected, actual);
335385
}
336386

387+
#[test]
388+
fn test_discover_explicit_members() {
389+
let r = runfiles::Runfiles::create().unwrap();
390+
let root_dir =
391+
runfiles::rlocation!(r, "rules_rust/crate_universe/test_data/workspace_examples")
392+
.unwrap();
393+
let root_dir = Utf8PathBuf::from_path_buf(root_dir).unwrap();
394+
395+
let mut workspaces_to_members = BTreeMap::<Utf8PathBuf, BTreeSet<Utf8PathBuf>>::new();
396+
397+
let mut includes_members = BTreeSet::new();
398+
let includes_root = root_dir.join("includes");
399+
for child in [
400+
vec!["explicit-child"],
401+
vec!["glob-char1"],
402+
vec!["glob-char2"],
403+
vec!["glob-direct-children", "grandchild1"],
404+
vec!["glob-direct-children", "grandchild2"],
405+
vec!["glob-transitive-children", "level1", "anchor"],
406+
vec!["glob-transitive-children", "level1", "level2", "anchor"],
407+
vec![
408+
"glob-transitive-children",
409+
"level1",
410+
"level2",
411+
"level3",
412+
"anchor",
413+
],
414+
] {
415+
let mut path = includes_root.clone();
416+
for path_part in child {
417+
path.push(path_part);
418+
}
419+
path.push("Cargo.toml");
420+
includes_members.insert(path);
421+
}
422+
423+
workspaces_to_members.insert(includes_root.join("Cargo.toml"), includes_members);
424+
let non_workspaces = BTreeSet::<Utf8PathBuf>::new();
425+
426+
let expected = DiscoveredWorkspaces {
427+
workspaces_to_members,
428+
non_workspaces,
429+
};
430+
431+
let actual = discover_workspaces(
432+
vec![root_dir
433+
.join("includes")
434+
.join("explicit-child")
435+
.join("Cargo.toml")]
436+
.into_iter()
437+
.collect(),
438+
&BTreeMap::new(),
439+
root_dir.as_std_path(),
440+
)
441+
.unwrap();
442+
443+
assert_eq!(expected, actual);
444+
}
445+
337446
fn ws1_discovered_workspaces(root_dir: &Utf8Path) -> DiscoveredWorkspaces {
338447
let mut workspaces_to_members = BTreeMap::new();
339448
workspaces_to_members.insert(
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
[workspace]
2+
members = [
3+
"explicit-child",
4+
"glob-direct-children/*",
5+
"glob-transitive-children/**/anchor",
6+
"glob-char?",
7+
]
8+
exclude = [
9+
"glob-char3",
10+
"glob-direct-children/excluded",
11+
]
12+
13+
[package]
14+
name = "includes"
15+
version = "0.1.0"
16+
edition = "2021"
17+
18+
[dependencies]

0 commit comments

Comments
 (0)