xbuild/cargo: Linearize workspace detection (#83)

* xbuild/cargo: Linearize workspace detection

* utils: Support path canonicalization for empty strings

When calling `.parent()` on a filename the result is `""`, which should
be treated as PWD (`"."`) but `dunce::canonicalize()` ends up in a "No
such file or directory".

* Canonicalize `--manifest-path` so that a workspace is always found

`.ancestors()` only calls `.parent()` on a `Path` which walks up until
the string empty, but this isn't the root of the filesystem if the path
wasn't absolute.

* Use workspace members to select package by `--manifest-path` or `$PWD`

Instead of relying on non-workspace logic.

* Pre-parse all workspace manifests to check for errors

* Pre-parse all member manifests for `ByName` selection too

xbuild/src/cargo/config.rs

@@ -8,10 +8,22 @@ pub struct Config {
 }
 
 impl Config {
-    pub fn parse_from_toml(path: &Path) -> Result<Self> {
+    pub fn parse_from_toml(path: impl AsRef<Path>) -> Result<Self> {
         let contents = std::fs::read_to_string(path)?;
         Ok(toml::from_str(&contents)?)
     }
+
+    /// Search for and open `.cargo/config.toml` in any parent of the workspace root path.
+    pub fn find_cargo_config_for_workspace(workspace: impl AsRef<Path>) -> Result<Option<Self>> {
+        let workspace = workspace.as_ref();
+        let workspace = dunce::canonicalize(workspace)?;
+        workspace
+            .ancestors()
+            .map(|dir| dir.join(".cargo/config.toml"))
+            .find(|p| p.is_file())
+            .map(Config::parse_from_toml)
+            .transpose()
+    }
 }
 
 #[derive(Debug, Deserialize)]

xbuild/src/cargo/manifest.rs

@@ -2,7 +2,7 @@ use anyhow::Result;
 use serde::Deserialize;
 use std::path::Path;
 
-#[derive(Debug, Deserialize)]
+#[derive(Clone, Debug, Deserialize)]
 pub struct Manifest {
     pub workspace: Option<Workspace>,
     pub package: Option<Package>,
@@ -15,12 +15,16 @@ impl Manifest {
     }
 }
 
-#[derive(Debug, Deserialize)]
+#[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>,
 }
 
-#[derive(Debug, Deserialize)]
+#[derive(Clone, Debug, Deserialize)]
 pub struct Package {
     pub name: String,
     pub version: String,

xbuild/src/cargo/mod.rs

@@ -1,5 +1,5 @@
-use crate::{CompileTarget, Opt};
 use anyhow::{Context, Result};
+use std::ffi::OsStr;
 use std::path::{Path, PathBuf};
 use std::process::Command;
 
@@ -10,10 +10,16 @@ mod utils;
 
 pub use artifact::{Artifact, CrateType};
 
+use self::config::Config;
+use self::manifest::Manifest;
+use crate::{CompileTarget, Opt};
+
 pub struct Cargo {
     package: String,
     features: Vec<String>,
-    manifest: PathBuf,
+    _workspace_manifest: Option<Manifest>,
+    manifest: Manifest,
+    package_root: PathBuf,
     target_dir: PathBuf,
     offline: bool,
 }
@@ -26,11 +32,53 @@ impl Cargo {
         target_dir: Option<PathBuf>,
         offline: bool,
     ) -> Result<Self> {
-        let (manifest, package) = utils::find_package(
-            &manifest_path.unwrap_or_else(|| std::env::current_dir().unwrap()),
-            package,
+        let manifest_path = manifest_path
+            .map(|path| {
+                if path.file_name() != Some(OsStr::new("Cargo.toml")) || !path.is_file() {
+                    Err(anyhow::anyhow!(
+                        "The manifest-path must be a path to a Cargo.toml file"
+                    ))
+                } else {
+                    Ok(path)
+                }
+            })
+            .transpose()?;
+
+        let search_path = manifest_path.map_or_else(
+            || std::env::current_dir().context("Could not retrieve current directory"),
+            |manifest_path| utils::canonicalize(manifest_path.parent().unwrap()),
         )?;
-        let root_dir = manifest.parent().unwrap();
+
+        // Scan the given and all parent directories for a Cargo.toml containing a workspace
+        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
+            let selector = match package {
+                Some(name) => utils::PackageSelector::ByName(name),
+                None => utils::PackageSelector::ByPath(&search_path),
+            };
+            utils::find_package_manifest_in_workspace(workspace_manifest_path, workspace, selector)?
+        } else {
+            // Otherwise scan up the directories based on --manifest-path and the working directory.
+            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;
+
+        let package_root = 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 = Config::find_cargo_config_for_workspace(package_root)?;
+        // TODO: Import LocalizedConfig code from cargo-subcommand and propagate `[env]`
+        // if let Some(config) = &config {
+        //     config.set_env_vars().unwrap();
+        // }
+
         let target_dir = target_dir
             .or_else(|| {
                 std::env::var_os("CARGO_BUILD_TARGET_DIR")
@@ -44,18 +92,23 @@ impl Cargo {
                     target_dir
                 }
             });
+
         let target_dir = target_dir.unwrap_or_else(|| {
-            utils::find_workspace(&manifest, &package)
-                .unwrap()
-                .unwrap_or_else(|| manifest.clone())
+            workspace_manifest
+                .as_ref()
+                .map(|(path, _)| path)
+                .unwrap_or_else(|| &manifest_path)
                 .parent()
                 .unwrap()
-                .join(utils::get_target_dir_name(root_dir).unwrap())
+                .join(utils::get_target_dir_name(config.as_ref()).unwrap())
         });
+
         Ok(Self {
-            package,
+            package: package.clone(),
             features,
+            _workspace_manifest: workspace_manifest.map(|(_path, manifest)| manifest),
             manifest,
+            package_root: package_root.to_owned(),
             target_dir,
             offline,
         })
@@ -69,25 +122,25 @@ impl Cargo {
         &self.package
     }
 
-    pub fn manifest(&self) -> &Path {
+    pub fn manifest(&self) -> &Manifest {
         &self.manifest
     }
 
-    pub fn root_dir(&self) -> &Path {
-        self.manifest.parent().unwrap()
+    pub fn package_root(&self) -> &Path {
+        &self.package_root
     }
 
     pub fn examples(&self) -> Result<Vec<Artifact>> {
         let mut artifacts = vec![];
-        for file in utils::list_rust_files(&self.root_dir().join("examples"))? {
+        for file in utils::list_rust_files(&self.package_root().join("examples"))? {
             artifacts.push(Artifact::Example(file));
         }
         Ok(artifacts)
     }
 
     pub fn bins(&self) -> Result<Vec<Artifact>> {
         let mut artifacts = vec![];
-        for file in utils::list_rust_files(&self.root_dir().join("src").join("bin"))? {
+        for file in utils::list_rust_files(&self.package_root().join("src").join("bin"))? {
             artifacts.push(Artifact::Root(file));
         }
         Ok(artifacts)
@@ -97,7 +150,7 @@ impl Cargo {
         CargoBuild::new(
             target,
             &self.features,
-            self.root_dir(),
+            self.package_root(),
             target_dir,
             self.offline,
         )