Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Print globs when workspace members can't be found #15093

Merged
merged 1 commit into from
Jan 23, 2025
Merged

Print globs when workspace members can't be found #15093

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 25 additions & 13 deletions src/cargo/core/workspace.rs
View file
Open in desktop
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
View file
Open in desktop
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
Loading