diff --git a/src/error.rs b/src/error.rs index 66ef5bb..4bab10c 100644 --- a/src/error.rs +++ b/src/error.rs @@ -7,12 +7,14 @@ use toml::de::Error as TomlError; #[derive(Debug)] pub enum Error { InvalidArgs, + ManifestNotAWorkspace, ManifestNotFound, RustcNotFound, ManifestPathNotFound, MultiplePackagesNotSupported, GlobPatternError(&'static str), Glob(GlobError), + UnexpectedWorkspace(PathBuf), Io(PathBuf, IoError), Toml(PathBuf, TomlError), } @@ -23,6 +25,7 @@ impl Display for Error { fn fmt(&self, f: &mut Formatter) -> FmtResult { f.write_str(match self { Self::InvalidArgs => "Invalid args.", + Self::ManifestNotAWorkspace => "The provided Cargo.toml does not contain a `[workspace]`", Self::ManifestNotFound => "Didn't find Cargo.toml.", Self::ManifestPathNotFound => "The manifest-path must be a path to a Cargo.toml file", Self::MultiplePackagesNotSupported => { @@ -31,6 +34,7 @@ impl Display for Error { Self::RustcNotFound => "Didn't find rustc.", Self::GlobPatternError(error) => error, Self::Glob(error) => return error.fmt(f), + Self::UnexpectedWorkspace(path) => return write!(f, "Did not expect a `[workspace]` at {}", path.display()), Self::Io(path, error) => return write!(f, "{}: {}", path.display(), error), Self::Toml(file, error) => return write!(f, "{}: {}", file.display(), error), }) diff --git a/src/subcommand.rs b/src/subcommand.rs index a9c6a76..9d21a30 100644 --- a/src/subcommand.rs +++ b/src/subcommand.rs @@ -25,6 +25,7 @@ impl Subcommand { args.package.len() < 2, "Multiple packages are not supported yet by `cargo-subcommand`" ); + let package = args.package.get(0).map(|s| s.as_str()); assert!( !args.workspace, "`--workspace` is not supported yet by `cargo-subcommand`" @@ -46,15 +47,30 @@ impl Subcommand { }) .transpose()?; - let (manifest_path, package) = utils::find_package( - &manifest_path.unwrap_or_else(|| std::env::current_dir().unwrap()), - args.package.get(0).map(|s| s.as_str()), - )?; + let search_path = manifest_path.map_or_else( + || std::env::current_dir().unwrap(), + |manifest_path| manifest_path.parent().unwrap().to_owned(), + ); + + // Scan the given and all parent directories for a Cargo.toml containing a workspace + // TODO: If set, validate that the found workspace is either the given `--manifest-path`, + // or contains the given `--manifest-path` as member. + let workspace_manifest = utils::find_workspace(&search_path)?; + + let (manifest_path, package) = + if let Some((workspace_manifest_path, workspace)) = &workspace_manifest { + // If a workspace was found, find packages relative to it + utils::find_package_in_workspace(workspace_manifest_path, workspace, package)? + } else { + // Otherwise scan up the directories + utils::find_package(&search_path, package)? + }; + let root_dir = manifest_path.parent().unwrap(); // TODO: Find, parse, and merge _all_ config files following the hierarchical structure: // https://doc.rust-lang.org/cargo/reference/config.html#hierarchical-structure - let config = LocalizedConfig::find_cargo_config_for_workspace(&root_dir)?; + let config = LocalizedConfig::find_cargo_config_for_workspace(root_dir)?; if let Some(config) = &config { config.set_env_vars().unwrap(); } @@ -76,8 +92,8 @@ impl Subcommand { }); let target_dir = target_dir.unwrap_or_else(|| { - utils::find_workspace(&manifest_path, &package) - .unwrap() + workspace_manifest + .map(|(path, _)| path) .unwrap_or_else(|| manifest_path.clone()) .parent() .unwrap() diff --git a/src/utils.rs b/src/utils.rs index 6767a81..d487432 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -19,22 +19,73 @@ pub fn list_rust_files(dir: &Path) -> Result> { Ok(files) } -fn member(manifest: &Path, members: &[String], package: &str) -> Result> { - let workspace_dir = manifest.parent().unwrap(); - for member in members { - for manifest_dir in glob::glob(workspace_dir.join(member).to_str().unwrap())? { +fn match_package_name(manifest: &Manifest, name: Option<&str>) -> Option { + if let Some(p) = &manifest.package { + if let Some(name) = name { + if name == p.name { + return Some(p.name.clone()); + } + } else { + return Some(p.name.clone()); + } + } + + None +} + +/// Tries to find a package by the given `name` in the [workspace root] or member +/// of the given [workspace] [`Manifest`]. +/// +/// When a workspace is not detected, call [`find_package()`] instead. +/// +/// [workspace root]: https://doc.rust-lang.org/cargo/reference/workspaces.html#root-package +/// [workspace]: https://doc.rust-lang.org/cargo/reference/workspaces.html#workspaces +pub fn find_package_in_workspace( + workspace_manifest_path: &Path, + workspace_manifest: &Manifest, + name: Option<&str>, +) -> Result<(PathBuf, String)> { + let workspace = workspace_manifest + .workspace + .as_ref() + .ok_or(Error::ManifestNotAWorkspace)?; + + // When building inside a workspace, require a package to be selected on the cmdline + // as selecting the right package is non-trivial and selecting multiple packages + // isn't currently supported yet. See the selection mechanism: + // https://doc.rust-lang.org/cargo/reference/workspaces.html#package-selection + let name = name.ok_or(Error::MultiplePackagesNotSupported)?; + + // Check if the workspace manifest also contains a [package] + if let Some(package) = match_package_name(workspace_manifest, Some(name)) { + return Ok((workspace_manifest_path.to_owned(), package)); + } + + // Check all member packages inside the workspace + let workspace_root = workspace_manifest_path.parent().unwrap(); + for member in &workspace.members { + for manifest_dir in glob::glob(workspace_root.join(member).to_str().unwrap())? { let manifest_path = manifest_dir?.join("Cargo.toml"); let manifest = Manifest::parse_from_toml(&manifest_path)?; - if let Some(p) = manifest.package.as_ref() { - if p.name == package { - return Ok(Some(manifest_path)); - } + + // Workspace members cannot themselves be/contain a new workspace + if manifest.workspace.is_some() { + return Err(Error::UnexpectedWorkspace(manifest_path)); + } + + if let Some(package) = match_package_name(&manifest, Some(name)) { + return Ok((manifest_path, package)); } } } - Ok(None) + + Err(Error::ManifestNotFound) } +/// Recursively walk up the directories until finding a `Cargo.toml` +/// +/// When a workspace has been detected, [`find_package_in_workspace()`] to find packages +/// instead (that are members of the given workspace). pub fn find_package(path: &Path, name: Option<&str>) -> Result<(PathBuf, String)> { let path = dunce::canonicalize(path).map_err(|e| Error::Io(path.to_owned(), e))?; for manifest_path in path @@ -43,38 +94,29 @@ pub fn find_package(path: &Path, name: Option<&str>) -> Result<(PathBuf, String) .filter(|dir| dir.exists()) { let manifest = Manifest::parse_from_toml(&manifest_path)?; - if let Some(p) = manifest.package.as_ref() { - if let Some(name) = name { - if name == p.name { - return Ok((manifest_path, p.name.clone())); - } - } else { - return Ok((manifest_path, p.name.clone())); - } + + // This function shouldn't be called when a workspace exists. + if manifest.workspace.is_some() { + return Err(Error::UnexpectedWorkspace(manifest_path)); } - if let Some(w) = manifest.workspace.as_ref() { - // TODO: This should also work if name is None - then all packages should simply be returned - let name = name.ok_or(Error::MultiplePackagesNotSupported)?; - if let Some(manifest_path) = member(&manifest_path, &w.members, name)? { - return Ok((manifest_path, name.to_string())); - } + + if let Some(package) = match_package_name(&manifest, name) { + return Ok((manifest_path, package)); } } Err(Error::ManifestNotFound) } -pub fn find_workspace(manifest: &Path, name: &str) -> Result> { - let dir = manifest.parent().unwrap(); - for manifest_path in dir +/// Find the first `Cargo.toml` that contains a `[workspace]` +pub fn find_workspace(potential_root: &Path) -> Result> { + for manifest_path in potential_root .ancestors() .map(|dir| dir.join("Cargo.toml")) .filter(|dir| dir.exists()) { let manifest = Manifest::parse_from_toml(&manifest_path)?; - if let Some(w) = manifest.workspace.as_ref() { - if member(&manifest_path, &w.members, name)?.is_some() { - return Ok(Some(manifest_path)); - } + if manifest.workspace.is_some() { + return Ok(Some((manifest_path, manifest))); } } Ok(None)