Fix cargo locate-project --workspace performance issue #16423

moabo3li · 2025-12-20T12:04:08Z

Fix cargo locate-project --workspace performance issue

cargo locate-project --workspace was unnecessarily loading and validating all workspace members, causing slowdowns in large workspaces (>1s for 1500+ members).
Changed to use find_workspace_root() instead of args.workspace(), which only parses the minimal manifests needed to locate the workspace root path without loading all members.

Fixes #15107

rustbot · 2025-12-20T12:04:13Z

r? @ehuss

rustbot has assigned @ehuss.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

src/bin/cargo/commands/locate_project.rs

… in locate-project

weihanglo · 2025-12-20T15:14:11Z

tests/testsuite/locate_project.rs

We might also want to add some extra failing cases:

Run from a nested directory of a package. The package specifies package.workspace to the other workspace but the workspace in its parent directory

Run from a nested directory of a package. The outer workspace manifest doesn't have that package as a member.

The current implementation may forget to take this into account #15107 (comment).

weihanglo · 2025-12-20T21:36:51Z

tests/testsuite/locate_project.rs

+                name = "nested"
+                version = "0.0.0"
+                
+                [package.workspace]


This test setup is wrong.

package.workspace should be a valid workspace path

https://doc.rust-lang.org/cargo/reference/manifest.html#the-workspace-field

Done in what way?

I expected that point to a valid workspace rootz and that affects what this command returns

I’m sorry if I’m missing something, but I don’t understand why this is not considered valid.

nested/Cargo.toml has workspace = "..", which resolves to the parent directory.
The parent Cargo.toml contains a [workspace] section with members = ["nested"], which is a valid workspace root.

Therefore, workspace = ".." points to a valid workspace path according to the documentation.

Is there something I’m misunderstanding?

In this test:

workspace = ".." points to the parent directory

The parent workspace lists ["nested"] in members, which includes this package

Given this, the cargo metadata command returns [ROOT]/foo/Cargo.toml (the parent workspace root).

I was reading the wrong diff. Sorry.

#16423 (comment)

In that comment I was looking for failure cases, where package.workspace points to some other workspaces, not the parent one. With that we can validate when workspace manifest and package manifest disagree with each other, and should return the workspace the package manifest belongs to.

This test itself is nice to have to validate the package.workspace working, but not the case I was looking for.

BTW, by failing I didn't meant they really need to fail. They are edge cases where Cargo might want to point to the right workspace rather than the wrong workspace.

I added these tests, plus one more to validate cases like nested/*. If the tests and solution look good, I’ll add them along with the commits made before the fix, as usual.

weihanglo · 2025-12-20T21:38:30Z

tests/testsuite/locate_project.rs

+        .with_stdout_data(
+            str![[r#"
+{
+  "root": "[ROOT]/foo/Cargo.toml"


Shouldn't this point to the non-member package?

With find_workspace_root, it returns the parent workspace root, even though the package is not a member

before change it should fails with error "current package believes it's in a workspace when it's not"

With find_workspace_root, it returns the parent workspace root, even though the package is not a member

Is this the desired behavior?

Please see #15107 (comment) that we still want a minimal verification

tests/testsuite/locate_project.rs

…aces

… return appropriate error message

weihanglo · 2025-12-22T17:41:13Z

tests/testsuite/locate_project.rs

+        .with_stdout_data(
+            str![[r#"
+{
+  "root": "[ROOT]/foo/inner-workspace/Cargo.toml"


This test doesn't make much sense to me. The inner-workspace/pkg/Cargo.toml will always point to the inner-workspace/Cargo.toml because it is the immediate parent directory that contain a workspace manifest, regardless of having a outer workspace manifest or not.

To avoid the parent directory probiding in your code path, you might want to change workspace = ".." to point to a workspace manifest that is in a sibiling directory of the package manifest.

weihanglo · 2025-12-22T17:51:15Z

src/cargo/core/workspace.rs

+        }
+        match self.members {
+            Some(ref members) => members.iter().any(|mem| {
+                if mem.contains('*') {


This is not the correct way to handle glob syntax, see this function for the right approach (and we might want reuse it or refactor to reuse)

cargo/src/cargo/core/workspace.rs

Lines 2066 to 2101 in 1f5feec

/// 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<'g>(

&self,

globs: &'g [String],

) -> CargoResult<Vec<(PathBuf, Option<&'g str>)>> {

let mut expanded_list = Vec::new();

for glob in globs {

let pathbuf = self.root_dir.join(glob);

let expanded_paths = Self::expand_member_path(&pathbuf)?;

// 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, 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, glob));

}

}

}

}

Ok(expanded_list)

}

weihanglo · 2025-12-22T17:58:10Z

src/cargo/core/workspace.rs

+    /// - The path is the workspace root manifest itself, or
+    /// - No explicit `members` list is specified (implicit membership), or
+    /// - The path matches one of the `members` patterns
+    fn is_path_member(&self, manifest_path: &Path) -> bool {


I think this is missing an important check: path dependencies.

@epage what is your though on this, as it may provide a false result?

… function

rustbot assigned ehuss Dec 20, 2025

rustbot added A-cli Area: Command-line interface, option parsing, etc. Command-locate-project S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 20, 2025

weihanglo reviewed Dec 20, 2025

View reviewed changes

src/bin/cargo/commands/locate_project.rs Show resolved Hide resolved

test(locate_project): add test for handling missing workspace members…

34566e1

… in locate-project

moabo3li force-pushed the locate_project_workspace branch from af4174a to e6bd4cb Compare December 20, 2025 14:33

weihanglo reviewed Dec 20, 2025

View reviewed changes

moabo3li force-pushed the locate_project_workspace branch from e6bd4cb to 6667ca2 Compare December 20, 2025 20:09

moabo3li changed the title ~~refactor(locations): streamline workspace root retrieval in exec function~~ Fix cargo locate-project --workspace performance issue Dec 20, 2025

weihanglo reviewed Dec 20, 2025

View reviewed changes

moabo3li added 2 commits December 21, 2025 11:41

test(locate_project): add test to handle errors for non-member worksp…

b1f1884

…aces

test(locate_project): add error handling for non-member workspaces to…

9668c97

… return appropriate error message

moabo3li force-pushed the locate_project_workspace branch 2 times, most recently from f04c198 to f928e9e Compare December 22, 2025 14:32

rustbot added the A-workspaces Area: workspaces label Dec 22, 2025

weihanglo reviewed Dec 22, 2025

View reviewed changes

refactor(locate_project): streamline workspace root retrieval in exec…

ae37cf6

… function

moabo3li force-pushed the locate_project_workspace branch from f928e9e to ae37cf6 Compare December 22, 2025 18:31

	/// 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<'g>(
	&self,
	globs: &'g [String],
	) -> CargoResult<Vec<(PathBuf, Option<&'g str>)>> {
	let mut expanded_list = Vec::new();

	for glob in globs {
	let pathbuf = self.root_dir.join(glob);
	let expanded_paths = Self::expand_member_path(&pathbuf)?;

	// 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, 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, glob));
	}
	}
	}
	}

	Ok(expanded_list)
	}

Fix cargo locate-project --workspace performance issue #16423

Are you sure you want to change the base?

Fix cargo locate-project --workspace performance issue #16423

Conversation

moabo3li commented Dec 20, 2025

Uh oh!

rustbot commented Dec 20, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants