Use $PWD and --manifest-path to select a default package in works…

…paces

This restores original, prevalent `--manifest-path` usage to select a
package containing the APK crate in a `[workspace]`, instead of using
`-p` for that.

src/error.rs

@@ -11,10 +11,11 @@ pub enum Error {
     ManifestNotFound,
     RustcNotFound,
     ManifestPathNotFound,
-    MultiplePackagesNotSupported,
     GlobPatternError(&'static str),
     Glob(GlobError),
     UnexpectedWorkspace(PathBuf),
+    NoPackageInManifest(PathBuf),
+    PackageNotFound(PathBuf, String),
     Io(PathBuf, IoError),
     Toml(PathBuf, TomlError),
 }
@@ -25,16 +26,32 @@ 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::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 => {
-                "Multiple packages are not yet supported (ie. in a `[workspace]`). Use `-p` to select a specific package."
-            }
             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::UnexpectedWorkspace(path) => {
+                return write!(f, "Did not expect a `[workspace]` at {}", path.display())
+            }
+            Self::NoPackageInManifest(manifest) => {
+                return write!(
+                    f,
+                    "Failed to parse manifest at `{}`: virtual manifests must be configured with `[workspace]`",
+                    manifest.display()
+                )
+            }
+            Self::PackageNotFound(workspace, name) => {
+                return write!(
+                    f,
+                    "package `{}` not found in workspace `{}`",
+                    name,
+                    workspace.display()
+                )
+            }
             Self::Io(path, error) => return write!(f, "{}: {}", path.display(), error),
             Self::Toml(file, error) => return write!(f, "{}: {}", file.display(), error),
         })

src/manifest.rs

@@ -16,7 +16,11 @@ impl Manifest {
 }
 
 #[derive(Clone, Debug, Deserialize)]
+#[serde(rename_all = "kebab-case")]
 pub struct Workspace {
+    #[serde(default)]
+    pub default_members: Vec<String>,
+    #[serde(default)]
     pub members: Vec<String>,
 }
 

src/subcommand.rs

@@ -54,19 +54,28 @@ impl Subcommand {
         );
 
         // 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, manifest) = if let Some((workspace_manifest_path, workspace)) =
-            &workspace_manifest
-        {
-            // If a workspace was found, find packages relative to it
-            utils::find_package_manifest_in_workspace(workspace_manifest_path, workspace, package)?
-        } else {
-            // Otherwise scan up the directories
-            utils::find_package_manifest(&search_path, package)?
-        };
+        let (manifest_path, manifest) =
+            if let (Some(package), Some((workspace_manifest_path, workspace))) =
+                (package, &workspace_manifest)
+            {
+                // If a workspace was found, and the user chose a package with `-p`, find packages relative to it
+                // TODO: What if we call `cargo apk run` in the workspace root, and detect a workspace? It should
+                // then use the `[package]` defined in the workspace (will be found below, though, but currently)
+                // fails with UnexpectedWorkspace)
+                utils::find_package_manifest_in_workspace(
+                    workspace_manifest_path,
+                    workspace,
+                    package,
+                )?
+            } else {
+                // Otherwise scan up the directories based on --manifest-path and the working directory.
+                // TODO: When we're in a workspace but the user didn't select a package by name, this
+                // is the right logic to use as long as we _also_ validate that the Cargo.toml we found
+                // was a member of this workspace?
+                utils::find_package_manifest(&search_path, package)?
+            };
 
         // The manifest is known to contain a package at this point
         let package = &manifest.package.as_ref().unwrap().name;

src/utils.rs

@@ -1,6 +1,6 @@
 use crate::config::Config;
 use crate::error::{Error, Result};
-use crate::manifest::Manifest;
+use crate::manifest::{Manifest, Package};
 use std::ffi::OsStr;
 use std::path::{Path, PathBuf};
 
@@ -19,20 +19,6 @@ pub fn list_rust_files(dir: &Path) -> Result<Vec<String>> {
     Ok(files)
 }
 
-fn match_package_name(manifest: &Manifest, name: Option<&str>) -> bool {
-    if let Some(p) = &manifest.package {
-        if let Some(name) = name {
-            if name == p.name {
-                return true;
-            }
-        } else {
-            return true;
-        }
-    }
-
-    false
-}
-
 /// Tries to find a package by the given `name` in the [workspace root] or member
 /// of the given [workspace] [`Manifest`].
 ///
@@ -43,25 +29,21 @@ fn match_package_name(manifest: &Manifest, name: Option<&str>) -> bool {
 pub fn find_package_manifest_in_workspace(
     workspace_manifest_path: &Path,
     workspace_manifest: &Manifest,
-    name: Option<&str>,
+    name: &str,
 ) -> Result<(PathBuf, Manifest)> {
     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 match_package_name(workspace_manifest, Some(name)) {
-        return Ok((
-            workspace_manifest_path.to_owned(),
-            workspace_manifest.clone(),
-        ));
+    if let Some(package) = &workspace_manifest.package {
+        if package.name == name {
+            return Ok((
+                workspace_manifest_path.to_owned(),
+                workspace_manifest.clone(),
+            ));
+        }
     }
 
     // Check all member packages inside the workspace
@@ -76,13 +58,20 @@ pub fn find_package_manifest_in_workspace(
                 return Err(Error::UnexpectedWorkspace(manifest_path));
             }
 
-            if match_package_name(&manifest, Some(name)) {
-                return Ok((manifest_path, manifest));
+            if let Some(package) = &manifest.package {
+                if package.name == name {
+                    return Ok((manifest_path, manifest));
+                }
+            } else {
+                return Err(Error::NoPackageInManifest(manifest_path));
             }
         }
     }
 
-    Err(Error::ManifestNotFound)
+    Err(Error::PackageNotFound(
+        workspace_manifest_path.into(),
+        name.into(),
+    ))
 }
 
 /// Recursively walk up the directories until finding a `Cargo.toml`
@@ -91,31 +80,40 @@ pub fn find_package_manifest_in_workspace(
 /// instead (that are members of the given workspace).
 pub fn find_package_manifest(path: &Path, name: Option<&str>) -> Result<(PathBuf, Manifest)> {
     let path = dunce::canonicalize(path).map_err(|e| Error::Io(path.to_owned(), e))?;
-    for manifest_path in path
+    let manifest_path = path
         .ancestors()
         .map(|dir| dir.join("Cargo.toml"))
-        .filter(|dir| dir.exists())
-    {
-        let manifest = Manifest::parse_from_toml(&manifest_path)?;
+        .find(|dir| dir.exists())
+        .ok_or(Error::ManifestNotFound)?;
 
-        // This function shouldn't be called when a workspace exists.
-        if manifest.workspace.is_some() {
-            return Err(Error::UnexpectedWorkspace(manifest_path));
-        }
+    let manifest = Manifest::parse_from_toml(&manifest_path)?;
 
-        if match_package_name(&manifest, name) {
-            return Ok((manifest_path, manifest));
+    // This function shouldn't be called when a workspace exists.
+    if manifest.workspace.is_some() {
+        return Err(Error::UnexpectedWorkspace(manifest_path));
+    }
+
+    if let Some(package) = &manifest.package {
+        if let Some(name) = name {
+            if package.name == name {
+                Ok((manifest_path, manifest))
+            } else {
+                Err(Error::PackageNotFound(manifest_path, name.into()))
+            }
+        } else {
+            Ok((manifest_path, manifest))
         }
+    } else {
+        Err(Error::NoPackageInManifest(manifest_path))
     }
-    Err(Error::ManifestNotFound)
 }
 
 /// Find the first `Cargo.toml` that contains a `[workspace]`
 pub fn find_workspace(potential_root: &Path) -> Result<Option<(PathBuf, Manifest)>> {
     for manifest_path in potential_root
         .ancestors()
         .map(|dir| dir.join("Cargo.toml"))
-        .filter(|dir| dir.exists())
+        .filter(|manifest| manifest.exists())
     {
         let manifest = Manifest::parse_from_toml(&manifest_path)?;
         if manifest.workspace.is_some() {