Skip to content

Commit

Permalink
Auto merge of #10846 - EmbarkStudios:fix-workspace-resolution, r=epage
Browse files Browse the repository at this point in the history
Fix nested workspace resolution

This fixes a bug that was introduced in #10776 with nested workspaces.

As an example, say we have two workspaces:

`/code/example/Cargo.toml` and `/code/example/sub/Cargo.toml`, and a crate within the `sub` workspace `/code/example/sub/test-crate/Cargo.toml`.

Since the `ws_roots` is a HashMap with randomized ordering, this code will _sometimes_ cause the workspace at `/code/example/Cargo.toml` to be discovered and used _before_ `/code/example/sub/Cargo.toml`,

https://github.com/rust-lang/cargo/blob/b1dd22e668af5279e13a071ad4b17435bd6bfa4c/src/cargo/core/workspace.rs#L1704-L1710

This will then cause the `validate_members` method to fail as the member thinks it is a member of a different workspace than it should be.

https://github.com/rust-lang/cargo/blob/b1dd22e668af5279e13a071ad4b17435bd6bfa4c/src/cargo/core/workspace.rs#L874-L891

This change just makes it so that the input manifest path is walked up to find the (presumably) most appropriate workspace so that the ordering of the `HashMap` doesn't matter.

If you encounter this bug by running cargo nightly, you can workaround it by adding the crate(s) to the `excluded` field in the workspace they don't belong to.
  • Loading branch information
bors committed Jul 13, 2022
2 parents 82a8fb4 + fb56406 commit 92ff479
Showing 1 changed file with 11 additions and 4 deletions.
15 changes: 11 additions & 4 deletions src/cargo/core/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1702,10 +1702,17 @@ fn find_workspace_root_with_loader(
mut loader: impl FnMut(&Path) -> CargoResult<Option<PathBuf>>,
) -> CargoResult<Option<PathBuf>> {
// Check if there are any workspace roots that have already been found that would work
for (ws_root, ws_root_config) in config.ws_roots.borrow().iter() {
if manifest_path.starts_with(ws_root) && !ws_root_config.is_excluded(manifest_path) {
// Add `Cargo.toml` since ws_root is the root and not the file
return Ok(Some(ws_root.join("Cargo.toml").clone()));
{
let roots = config.ws_roots.borrow();
// Iterate through the manifests parent directories until we find a workspace
// root. Note we skip the first item since that is just the path itself
for current in manifest_path.ancestors().skip(1) {
if let Some(ws_config) = roots.get(current) {
if !ws_config.is_excluded(manifest_path) {
// Add `Cargo.toml` since ws_root is the root and not the file
return Ok(Some(current.join("Cargo.toml")));
}
}
}
}

Expand Down

0 comments on commit 92ff479

Please sign in to comment.