Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 14 additions & 2 deletions src/bin/cargo/commands/locate_project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,27 @@ pub struct ProjectLocation<'a> {

pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {
let root_manifest;
let workspace_root;
let workspace;
let root = match WhatToFind::parse(args) {
WhatToFind::CurrentManifest => {
root_manifest = args.root_manifest(gctx)?;
&root_manifest
}
WhatToFind::Workspace => {
workspace = args.workspace(gctx)?;
workspace.root_manifest()
root_manifest = args.root_manifest(gctx)?;
// Try fast path first - only works when package is explicitly listed in members
if let Some(ws_root) =
cargo::core::find_workspace_root_with_membership_check(&root_manifest, gctx)?
{
workspace_root = ws_root;
&workspace_root
} else {
// Fallback to full workspace loading for path dependency membership.
// If loading fails, we must propagate the error to avoid false results.
workspace = args.workspace(gctx)?;
workspace.root_manifest()
}
}
};

Expand Down
2 changes: 1 addition & 1 deletion src/cargo/core/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ pub use self::source_id::SourceId;
pub use self::summary::{FeatureMap, FeatureValue, Summary};
pub use self::workspace::{
MaybePackage, Workspace, WorkspaceConfig, WorkspaceRootConfig, find_workspace_root,
resolve_relative_path,
find_workspace_root_with_membership_check, resolve_relative_path,
};
pub use cargo_util_schemas::core::{GitReference, PackageIdSpec, SourceKind};

Expand Down
100 changes: 100 additions & 0 deletions src/cargo/core/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2059,10 +2059,50 @@ impl WorkspaceRootConfig {
!explicit_member && excluded
}

/// Checks if the path is explicitly listed as a workspace member.
///
/// Returns `true` ONLY if:
/// - The path is the workspace root manifest itself, or
/// - The path matches one of the explicit `members` patterns
///
/// NOTE: This does NOT check for implicit path dependency membership.
/// A `false` return does NOT mean the package is definitely not a member -
/// it could still be a member via path dependencies. Callers should fallback
/// to full workspace loading when this returns `false`.
fn is_explicitly_listed_member(&self, manifest_path: &Path) -> bool {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// default-members are allowed to be excluded, but they
// still must be referred to by the original (unfiltered)
// members list. Note that we aren't testing against the
// manifest path, both because `members_paths` doesn't
// include `/Cargo.toml`, and because excluded paths may not
// be crates.

I found this interesting comment. Wonder what would happen with the current implementation in these situations:

  • A member is listed in workspace.default-members but not in workspace.members.
  • A member is listed in both workspace.default-members and workspace.exclude
  • A member is listed in both workspace.members and workspace.exclude

The new implementation should be aligned with the default args.workspace() behavior IMO.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked whether the manifest has a default member. If it does, I run the full check using args.workspace().

let root_manifest = self.root_dir.join("Cargo.toml");
if manifest_path == root_manifest {
return true;
}
match self.members {
Some(ref members) => {
// Use members_paths to properly expand glob patterns
let Ok(expanded_members) = self.members_paths(members) else {
return false;
};
// Normalize the manifest path for comparison
let normalized_manifest = paths::normalize_path(manifest_path);
expanded_members.iter().any(|(member_path, _)| {
// Normalize the member path as glob expansion may leave ".." components
let normalized_member = paths::normalize_path(member_path);
// Compare the manifest's parent directory with the member path exactly
// instead of using starts_with to avoid matching nested directories
normalized_manifest.parent() == Some(normalized_member.as_path())
})
}
None => false,
}
}

fn has_members_list(&self) -> bool {
self.members.is_some()
}

/// Returns true if this workspace config has default-members defined.
fn has_default_members(&self) -> bool {
self.default_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)]
Expand Down Expand Up @@ -2157,6 +2197,66 @@ pub fn find_workspace_root(
})
}

/// Finds the workspace root for a manifest, with minimal verification.
///
/// This is similar to `find_workspace_root`, but additionally verifies that the
/// package and workspace agree on each other:
/// - If the package has an explicit `package.workspace` pointer, it is trusted
/// - Otherwise, the workspace must include the package in its `members` list
pub fn find_workspace_root_with_membership_check(
manifest_path: &Path,
gctx: &GlobalContext,
) -> CargoResult<Option<PathBuf>> {
let source_id = SourceId::for_manifest_path(manifest_path)?;
let current_manifest = read_manifest(manifest_path, source_id, gctx)?;

match current_manifest.workspace_config() {
WorkspaceConfig::Root(root_config) => {
// This manifest is a workspace root itself
// If default-members are defined, fall back to full loading for proper validation
if root_config.has_default_members() {
Ok(None)
} else {
Ok(Some(manifest_path.to_path_buf()))
}
}
WorkspaceConfig::Member {
root: Some(path_to_root),
} => {
// Has explicit `package.workspace` pointer - verify the workspace agrees
let ws_manifest_path = read_root_pointer(manifest_path, path_to_root);
let ws_source_id = SourceId::for_manifest_path(&ws_manifest_path)?;
let ws_manifest = read_manifest(&ws_manifest_path, ws_source_id, gctx)?;

// Verify the workspace includes this package in its members
if let WorkspaceConfig::Root(ref root_config) = *ws_manifest.workspace_config() {
if root_config.is_explicitly_listed_member(manifest_path)
&& !root_config.is_excluded(manifest_path)
{
return Ok(Some(ws_manifest_path));
}
}
// Workspace doesn't agree with the pointer - not a valid workspace root
Ok(None)
}
WorkspaceConfig::Member { root: None } => {
// No explicit pointer, walk up with membership validation
find_workspace_root_with_loader(manifest_path, gctx, |candidate_manifest_path| {
let source_id = SourceId::for_manifest_path(candidate_manifest_path)?;
let manifest = read_manifest(candidate_manifest_path, source_id, gctx)?;
if let WorkspaceConfig::Root(ref root_config) = *manifest.workspace_config() {
if root_config.is_explicitly_listed_member(manifest_path)
&& !root_config.is_excluded(manifest_path)
{
return Ok(Some(candidate_manifest_path.to_path_buf()));
}
}
Ok(None)
})
}
}
}

/// Finds the path of the root of the workspace.
///
/// This uses a callback to determine if the given path tells us what the
Expand Down
Loading