Mount path dependencies correctly

CHANGELOG.md

@@ -16,6 +16,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 - #543 - Added environment variables to control the UID and GID in the container
 - #524 - docker: Add Nix Store volume support
 - Added support for mounting volumes.
+- #684 - Enable cargo workspaces to work from any path in the workspace, and make path dependencies mount seamlessly.
 
 ### Changed
 

README.md

@@ -362,15 +362,6 @@ $ QEMU_STRACE=1 cross run --target aarch64-unknown-linux-gnu
 9 exit_group(0)
 ```
 
-## Caveats
-
-- path dependencies (in Cargo.toml) that point outside the Cargo project won't
-  work because `cross` use docker containers only mounts the Cargo project so
-  the container doesn't have access to the rest of the filesystem.
-  However, you may use Cargo's `--manifest-path` option to reference your
-  target crate, executed from a common root directory from which all your
-  dependencies are available.
-
 ## Minimum Supported Rust Version (MSRV)
 
 This crate is guaranteed to compile on stable Rust 1.58.1 and up. It *might*

ci/test.sh

@@ -135,6 +135,21 @@ EOF
                 popd
 
                 rm -rf "${td}"
+                td=$(mktemp -d)
+                git clone \
+                    --depth 1 \
+                    --recursive \
+                    https://github.com/cross-rs/test-workspace "${td}"
+
+                pushd "${td}"
+      cross run --target "${TARGET}" -p binary --manifest-path="./workspace/Cargo.toml"
+                pushd "workspace"
+                cross run --target "${TARGET}" -p binary
+                pushd "binary"
+                cross run --target "${TARGET}"
+                popd
+                popd
+                popd
             ;;
         esac
 

src/cargo.rs

@@ -1,12 +1,10 @@
 use serde::Deserialize;
-use std::io::Write;
 use std::path::{Path, PathBuf};
 use std::process::{Command, ExitStatus};
 
 use crate::cli::Args;
-use crate::config::Config;
 use crate::errors::*;
-use crate::extensions::{CommandExt, OutputExt};
+use crate::extensions::CommandExt;
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum Subcommand {
@@ -58,18 +56,48 @@ impl<'a> From<&'a str> for Subcommand {
 #[derive(Debug, Deserialize)]
 pub struct CargoMetadata {
     pub workspace_root: PathBuf,
+    pub target_directory: PathBuf,
+    pub packages: Vec<Package>,
+    pub workspace_members: Vec<String>,
 }
 
 impl CargoMetadata {
-    pub fn workspace_root(&self) -> &Path {
-        &self.workspace_root
+    fn non_workspace_members(&self) -> impl Iterator<Item = &Package> {
+        self.packages
+            .iter()
+            .filter(|p| !self.workspace_members.iter().any(|m| m == &p.id))
+    }
+
+    pub fn path_dependencies(&self) -> impl Iterator<Item = &Path> {
+        // TODO: Also filter out things that are in workspace, but not a workspace member
+        self.non_workspace_members().filter_map(|p| p.crate_path())
+    }
+}
+
+#[derive(Debug, Deserialize)]
+pub struct Package {
+    id: String,
+    manifest_path: PathBuf,
+    source: Option<String>,
+}
+
+impl Package {
+    /// Returns the absolute path to the packages manifest "folder"
+    fn crate_path(&self) -> Option<&Path> {
+        // when source is none, this package is a path dependency or a workspace member
+        if self.source.is_none() {
+            self.manifest_path.parent()
+        } else {
+            None
+        }
     }
 }
 
 /// Cargo metadata with specific invocation
 pub fn cargo_metadata_with_args(
     cd: Option<&Path>,
     args: Option<&Args>,
+    verbose: bool,
 ) -> Result<Option<CargoMetadata>> {
     let mut command = std::process::Command::new(
         std::env::var("CARGO")
@@ -87,29 +115,28 @@ pub fn cargo_metadata_with_args(
     } else {
         command.arg("--no-deps");
     }
-    #[derive(Deserialize)]
-    struct Manifest {
-        workspace_root: PathBuf,
+    if let Some(target) = args.and_then(|a| a.target.as_ref()) {
+        command.args(["--filter-platform", target.triple()]);
     }
-    if let Some(cd) = cd {
-        command.current_dir(cd);
+    if let Some(features) = args.map(|a| &a.features).filter(|v| !v.is_empty()) {
+        command.args([String::from("--features"), features.join(",")]);
     }
-    let output = command.run_and_get_output(false)?;
+    let output = command.run_and_get_output(verbose)?;
     if !output.status.success() {
-        let mut stderr = std::io::stderr();
-        stderr.write_all(&output.stderr)?;
-        stderr.flush()?;
-        std::process::exit(output.status.code().unwrap_or(1));
+        // TODO: logging
+        return Ok(None);
     }
-    let manifest: Option<Manifest> = serde_json::from_slice(&output.stdout)?;
-    Ok(manifest.map(|m| CargoMetadata {
-        workspace_root: m.workspace_root,
-    }))
-}
-
-/// Cargo metadata
-pub fn cargo_metadata(cd: Option<&Path>) -> Result<Option<CargoMetadata>> {
-    cargo_metadata_with_args(cd, None)
+    let manifest: Option<CargoMetadata> = serde_json::from_slice(&output.stdout)?;
+    manifest
+        .map(|m| -> Result<_> {
+            Ok(CargoMetadata {
+                target_directory: args
+                    .and_then(|a| a.target_dir.clone())
+                    .unwrap_or(m.target_directory),
+                ..m
+            })
+        })
+        .transpose()
 }
 
 /// Pass-through mode

src/cli.rs

@@ -12,6 +12,7 @@ pub struct Args {
     pub subcommand: Option<Subcommand>,
     pub channel: Option<String>,
     pub target: Option<Target>,
+    pub features: Vec<String>,
     pub target_dir: Option<PathBuf>,
     pub docker_in_docker: bool,
     pub enable_doctests: bool,
@@ -73,6 +74,7 @@ pub fn fmt_subcommands(stdout: &str) {
 pub fn parse(target_list: &TargetList) -> Result<Args> {
     let mut channel = None;
     let mut target = None;
+    let mut features = Vec::new();
     let mut manifest_path: Option<PathBuf> = None;
     let mut target_dir = None;
     let mut sc = None;
@@ -111,6 +113,15 @@ pub fn parse(target_list: &TargetList) -> Result<Args> {
                     .split_once('=')
                     .map(|(_, t)| Target::from(t, target_list));
                 all.push(arg);
+            } else if arg == "--features" {
+                all.push(arg);
+                if let Some(t) = args.next() {
+                    features.push(t.clone());
+                    all.push(t);
+                }
+            } else if arg.starts_with("--features=") {
+                features.extend(arg.split_once('=').map(|(_, t)| t.to_owned()));
+                all.push(arg);
             } else if arg == "--target-dir" {
                 all.push(arg);
                 if let Some(td) = args.next() {
@@ -144,6 +155,7 @@ pub fn parse(target_list: &TargetList) -> Result<Args> {
         subcommand: sc,
         channel,
         target,
+        features,
         target_dir,
         docker_in_docker,
         enable_doctests,

src/docker.rs

@@ -3,9 +3,9 @@ use std::process::{Command, ExitStatus};
 use std::{env, fs};
 
 use crate::cargo::CargoMetadata;
-use crate::errors::*;
 use crate::extensions::{CommandExt, SafeCommand};
 use crate::id;
+use crate::{errors::*, file};
 use crate::{Config, Target};
 use atty::Stream;
 use eyre::bail;
@@ -65,11 +65,31 @@ fn validate_env_var(var: &str) -> Result<(&str, Option<&str>)> {
     Ok((key, value))
 }
 
+#[allow(unused_variables)]
+pub fn mount(cmd: &mut Command, val: &Path, verbose: bool) -> Result<PathBuf> {
+    let host_path =
+        file::canonicalize(&val).wrap_err_with(|| format!("when canonicalizing path `{val:?}`"))?;
+    let mount_path: PathBuf;
+    #[cfg(target_os = "windows")]
+    {
+        // On Windows, we can not mount the directory name directly. Instead, we use wslpath to convert the path to a linux compatible path.
+        mount_path = wslpath(&host_path, verbose)?;
+    }
+    #[cfg(not(target_os = "windows"))]
+    {
+        mount_path = host_path.clone();
+    }
+    cmd.args(&[
+        "-v",
+        &format!("{}:{}", host_path.display(), mount_path.display()),
+    ]);
+    Ok(mount_path)
+}
+
 #[allow(clippy::too_many_arguments)] // TODO: refactor
 pub fn run(
     target: &Target,
     args: &[String],
-    target_dir: &Option<PathBuf>,
     metadata: &CargoMetadata,
     config: &Config,
     uses_xargo: bool,
@@ -90,9 +110,7 @@ pub fn run(
         .map(PathBuf::from)
         .unwrap_or_else(|| home_dir.join(".xargo"));
     let nix_store_dir = env::var_os("NIX_STORE").map(PathBuf::from);
-    let target_dir = target_dir
-        .clone()
-        .unwrap_or_else(|| metadata.workspace_root().join("target"));
+    let target_dir = &metadata.target_directory;
 
     // create the directories we are going to mount before we mount them,
     // otherwise `docker` will create them but they will be owned by `root`
@@ -105,10 +123,10 @@ pub fn run(
     let xargo_dir = mount_finder.find_mount_path(xargo_dir);
     let target_dir = mount_finder.find_mount_path(target_dir);
     // root is either workspace_root, or, if we're outside the workspace root, the current directory
-    let host_root = mount_finder.find_mount_path(if metadata.workspace_root().starts_with(cwd) {
+    let host_root = mount_finder.find_mount_path(if metadata.workspace_root.starts_with(cwd) {
         cwd
     } else {
-        metadata.workspace_root()
+        &metadata.workspace_root
     });
     let mount_root: PathBuf;
     #[cfg(target_os = "windows")]
@@ -124,7 +142,7 @@ pub fn run(
     #[cfg(target_os = "windows")]
     {
         // On Windows, we can not mount the directory name directly. Instead, we use wslpath to convert the path to a linux compatible path.
-        mount_cwd = wslpath(&cwd, verbose)?;
+        mount_cwd = wslpath(cwd, verbose)?;
     }
     #[cfg(not(target_os = "windows"))]
     {
@@ -151,11 +169,11 @@ pub fn run(
         // flag forwards the value from the parent shell
         docker.args(&["-e", var]);
     }
-    let mut env_volumes = false;
-    // FIXME(emilgardis 2022-04-07): This is a fallback so that if it's hard for use to do mounting logic, make it simple(r)
+    let mut mount_volumes = false;
+    // FIXME(emilgardis 2022-04-07): This is a fallback so that if it's hard for us to do mounting logic, make it simple(r)
     // Preferably we would not have to do this.
-    if cwd.strip_prefix(metadata.workspace_root()).is_err() {
-        env_volumes = true;
+    if cwd.strip_prefix(&metadata.workspace_root).is_err() {
+        mount_volumes = true;
     }
 
     for ref var in config.env_volumes(target)? {
@@ -188,11 +206,17 @@ pub fn run(
                 "-v",
                 &format!("{}:{}", host_path.display(), mount_path.display()),
             ]);
+            let mount_path = mount(&mut docker, val.as_ref(), verbose)?;
             docker.args(&["-e", &format!("{}={}", var, mount_path.display())]);
-            env_volumes = true;
+            mount_volumes = true;
         }
     }
 
+    for path in metadata.path_dependencies() {
+        mount(&mut docker, path, verbose)?;
+        mount_volumes = true;
+    }
+
     docker.args(&["-e", "PKG_CONFIG_ALLOW_CROSS=1"]);
 
     docker.arg("--rm");
@@ -246,7 +270,7 @@ pub fn run(
         .args(&["-v", &format!("{}:/cargo:Z", cargo_dir.display())])
         // Prevent `bin` from being mounted inside the Docker container.
         .args(&["-v", "/cargo/bin"]);
-    if env_volumes {
+    if mount_volumes {
         docker.args(&[
             "-v",
             &format!("{}:{}:Z", host_root.display(), mount_root.display()),
@@ -258,14 +282,14 @@ pub fn run(
         .args(&["-v", &format!("{}:/rust:Z,ro", sysroot.display())])
         .args(&["-v", &format!("{}:/target:Z", target_dir.display())]);
 
-    if env_volumes {
+    if mount_volumes {
         docker.args(&["-w".as_ref(), mount_cwd.as_os_str()]);
-    } else if mount_cwd == metadata.workspace_root() {
+    } else if mount_cwd == metadata.workspace_root {
         docker.args(&["-w", "/project"]);
     } else {
         // We do this to avoid clashes with path separators. Windows uses `\` as a path separator on Path::join
         let cwd = &cwd;
-        let working_dir = Path::new("project").join(cwd.strip_prefix(metadata.workspace_root())?);
+        let working_dir = Path::new("project").join(cwd.strip_prefix(&metadata.workspace_root)?);
         // No [T].join for OsStr
         let mut mount_wd = std::ffi::OsString::new();
         for part in working_dir.iter() {

src/file.rs

@@ -1,6 +1,6 @@
 use std::fs::File;
 use std::io::Read;
-use std::path::Path;
+use std::path::{Path, PathBuf};
 
 use crate::errors::*;
 
@@ -19,3 +19,19 @@ fn read_(path: &Path) -> Result<String> {
         .wrap_err_with(|| format!("couldn't read {}", path.display()))?;
     Ok(s)
 }
+
+pub fn canonicalize(path: impl AsRef<Path>) -> Result<PathBuf> {
+    _canonicalize(path.as_ref())
+}
+
+fn _canonicalize(path: &Path) -> Result<PathBuf> {
+    #[cfg(target_os = "windows")]
+    {
+        // Docker does not support UNC paths, this will try to not use UNC paths
+        dunce::canonicalize(&path).map_err(Into::into)
+    }
+    #[cfg(not(target_os = "windows"))]
+    {
+        Path::new(&path).canonicalize().map_err(Into::into)
+    }
+}