Skip to content

Commit

Permalink
Print globs when workspace members can't be found (#15093)
Browse files Browse the repository at this point in the history
Cargo expands globs when loading workspace members. If the glob happens
to match some non-crate directory, it causes an error that may be tricky
to understand, because it makes Cargo complain that it failed to read a
`Cargo.toml` from a specific directory, but that directory name won't be
listed explicitly anywhere in `Cargo.toml`, so it may seem that Cargo is
trying to read some made-up phantom manifest.

I've made the error messages mention which glob has been used to select
the missing workspace member.
  • Loading branch information
epage authored Jan 23, 2025
2 parents 53a2fdf + 9b31464 commit 91d8140
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 16 deletions.
38 changes: 25 additions & 13 deletions src/cargo/core/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -742,8 +742,8 @@ impl<'gctx> Workspace<'gctx> {
// self.root_manifest must be Some to have retrieved workspace_config
let root_manifest_path = self.root_manifest.clone().unwrap();

let members_paths =
workspace_config.members_paths(workspace_config.members.as_ref().unwrap_or(&vec![]))?;
let members_paths = workspace_config
.members_paths(workspace_config.members.as_deref().unwrap_or_default())?;
let default_members_paths = if root_manifest_path == self.current_manifest {
if let Some(ref default) = workspace_config.default_members {
Some(workspace_config.members_paths(default)?)
Expand All @@ -754,22 +754,23 @@ impl<'gctx> Workspace<'gctx> {
None
};

for path in &members_paths {
for (path, glob) in &members_paths {
self.find_path_deps(&path.join("Cargo.toml"), &root_manifest_path, false)
.with_context(|| {
format!(
"failed to load manifest for workspace member `{}`\n\
referenced by workspace at `{}`",
referenced{} by workspace at `{}`",
path.display(),
root_manifest_path.display()
glob.map(|g| format!(" via `{g}`")).unwrap_or_default(),
root_manifest_path.display(),
)
})?;
}

self.find_path_deps(&root_manifest_path, &root_manifest_path, false)?;

if let Some(default) = default_members_paths {
for path in default {
for (path, default_member_glob) in default {
let normalized_path = paths::normalize_path(&path);
let manifest_path = normalized_path.join("Cargo.toml");
if !self.members.contains(&manifest_path) {
Expand All @@ -779,16 +780,19 @@ impl<'gctx> Workspace<'gctx> {
// manifest path, both because `members_paths` doesn't
// include `/Cargo.toml`, and because excluded paths may not
// be crates.
let exclude = members_paths.contains(&normalized_path)
let exclude = members_paths.iter().any(|(m, _)| *m == normalized_path)
&& workspace_config.is_excluded(&normalized_path);
if exclude {
continue;
}
bail!(
"package `{}` is listed in default-members but is not a member\n\
for workspace at {}.",
"package `{}` is listed in default-members{} but is not a member\n\
for workspace at `{}`.",
path.display(),
root_manifest_path.display()
default_member_glob
.map(|g| format!(" via `{g}`"))
.unwrap_or_default(),
root_manifest_path.display(),
)
}
self.default_members.push(manifest_path)
Expand Down Expand Up @@ -1872,8 +1876,13 @@ impl WorkspaceRootConfig {
self.members.is_some()
}

/// Returns expanded paths along with the glob that they were expanded from.
/// The glob is `None` if the path matched exactly.
#[tracing::instrument(skip_all)]
fn members_paths(&self, globs: &[String]) -> CargoResult<Vec<PathBuf>> {
fn members_paths<'g>(
&self,
globs: &'g [String],
) -> CargoResult<Vec<(PathBuf, Option<&'g str>)>> {
let mut expanded_list = Vec::new();

for glob in globs {
Expand All @@ -1883,16 +1892,19 @@ impl WorkspaceRootConfig {
// If glob does not find any valid paths, then put the original
// path in the expanded list to maintain backwards compatibility.
if expanded_paths.is_empty() {
expanded_list.push(pathbuf);
expanded_list.push((pathbuf, None));
} else {
let used_glob_pattern = expanded_paths.len() > 1 || expanded_paths[0] != pathbuf;
let glob = used_glob_pattern.then_some(glob.as_str());

// Some OS can create system support files anywhere.
// (e.g. macOS creates `.DS_Store` file if you visit a directory using Finder.)
// Such files can be reported as a member path unexpectedly.
// Check and filter out non-directory paths to prevent pushing such accidental unwanted path
// as a member.
for expanded_path in expanded_paths {
if expanded_path.is_dir() {
expanded_list.push(expanded_path);
expanded_list.push((expanded_path, glob));
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions tests/testsuite/workspaces.rs
Original file line number Diff line number Diff line change
Expand Up @@ -948,7 +948,7 @@ fn virtual_default_member_is_not_a_member() {
.with_status(101)
.with_stderr_data(str![[r#"
[ERROR] package `[ROOT]/foo/something-else` is listed in default-members but is not a member
for workspace at [ROOT]/foo/Cargo.toml.
for workspace at `[ROOT]/foo/Cargo.toml`.
"#]])
.run();
Expand Down Expand Up @@ -1731,7 +1731,7 @@ fn excluded_default_members_still_must_be_members() {
.with_status(101)
.with_stderr_data(str![[r#"
[ERROR] package `[ROOT]/foo/bar` is listed in default-members but is not a member
for workspace at [ROOT]/foo/Cargo.toml.
for workspace at `[ROOT]/foo/Cargo.toml`.
"#]])
.run();
Expand Down Expand Up @@ -1968,7 +1968,7 @@ fn glob_syntax_invalid_members() {
.with_status(101)
.with_stderr_data(str![[r#"
[ERROR] failed to load manifest for workspace member `[ROOT]/foo/crates/bar`
referenced by workspace at `[ROOT]/foo/Cargo.toml`
referenced via `crates/*` by workspace at `[ROOT]/foo/Cargo.toml`
Caused by:
failed to read `[ROOT]/foo/crates/bar/Cargo.toml`
Expand Down

0 comments on commit 91d8140

Please sign in to comment.