From b37e33a19878634bf277d0244c3595902c10fc28 Mon Sep 17 00:00:00 2001 From: naoNao89 <90588855+naoNao89@users.noreply.github.com> Date: Thu, 10 Jul 2025 15:32:01 +0700 Subject: [PATCH] fix: improve workspace detection for workspace members - Enhanced CargoMetadata::new() to preserve manifest path context - Added comprehensive find_target_package() method that handles both standalone packages and workspace members - Improved error messages with detailed workspace member information - Maintains full backward compatibility for standalone packages Fixes #909 Assisted-by: Claude 4 --- src/config/manifest.rs | 77 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 70 insertions(+), 7 deletions(-) diff --git a/src/config/manifest.rs b/src/config/manifest.rs index fd0ab3ff..82fc6269 100644 --- a/src/config/manifest.rs +++ b/src/config/manifest.rs @@ -18,21 +18,23 @@ pub struct CargoMetadata { impl CargoMetadata { // Create a new instance from the Cargo.toml at the given path. pub async fn new(manifest: &Path) -> Result { + let manifest_path = dunce::simplified(manifest).to_path_buf(); let mut cmd = MetadataCommand::new(); - cmd.manifest_path(dunce::simplified(manifest)); + cmd.manifest_path(&manifest_path); let metadata = spawn_blocking(move || cmd.exec()) .await .context("error awaiting spawned cargo metadata task")? .context("error getting cargo metadata")?; - Self::from_metadata(metadata) + Self::from_metadata_with_manifest_path(metadata, manifest_path) } - pub(crate) fn from_metadata(metadata: Metadata) -> Result { - let package = metadata - .root_package() - .cloned() - .context("could not find the root package of the target crate")?; + + + /// Create a new instance from metadata with a known manifest path. + /// This is the preferred method as it can better handle workspace scenarios. + pub(crate) fn from_metadata_with_manifest_path(metadata: Metadata, original_manifest_path: std::path::PathBuf) -> Result { + let package = Self::find_target_package(&metadata, Some(&original_manifest_path))?; // Get the path to the Cargo.toml manifest. let manifest_path = package.manifest_path.to_string(); @@ -43,4 +45,65 @@ impl CargoMetadata { manifest_path, }) } + + /// Find the target package from metadata, handling both standalone packages and workspace members. + fn find_target_package(metadata: &Metadata, original_manifest_path: Option<&std::path::Path>) -> Result { + // First, try the traditional approach for standalone packages + if let Some(package) = metadata.root_package() { + return Ok(package.clone()); + } + + // If no root package exists, we're likely in a workspace. + // In this case, we need to find the package that corresponds to the manifest path + // that was used to generate this metadata. + + let workspace_packages = metadata.workspace_packages(); + + if workspace_packages.is_empty() { + anyhow::bail!( + "could not find the root package of the target crate: no root package found and no workspace members available" + ); + } + + // If we have the original manifest path, try to find the exact matching package + if let Some(original_path) = original_manifest_path { + // Canonicalize the original path for comparison + let canonical_original = dunce::canonicalize(original_path) + .with_context(|| format!("failed to canonicalize manifest path: {}", original_path.display()))?; + + for package in &workspace_packages { + // Canonicalize the package's manifest path for comparison + if let Ok(canonical_package) = dunce::canonicalize(&package.manifest_path) { + if canonical_original == canonical_package { + return Ok((*package).clone()); + } + } + } + + // If we couldn't find an exact match, provide helpful error information + let package_names: Vec = workspace_packages.iter() + .map(|p| format!("{} ({})", p.name, p.manifest_path)) + .collect(); + anyhow::bail!( + "could not find the root package of the target crate: manifest path '{}' does not match any workspace member. \ + Available workspace members: [{}]", + original_path.display(), + package_names.join(", ") + ); + } + + // If there's only one workspace package, use it + if workspace_packages.len() == 1 { + return Ok(workspace_packages[0].clone()); + } + + // If there are multiple workspace packages and we don't have a specific manifest path, + // we can't determine which one to use. + let package_names: Vec<&str> = workspace_packages.iter().map(|p| p.name.as_str()).collect(); + anyhow::bail!( + "could not find the root package of the target crate: multiple workspace members found: [{}]. \ + Consider running trunk from within a specific package directory or using a more specific manifest path.", + package_names.join(", ") + ); + } }