Repository: cross-rs/cross (rust)

Issue:
Building with local dependencies
It seems that when building with a local dependency cross fails to load the source for that local dependency. If this is my Cargo.toml:
```toml
[dependencies]
nasatopo = { path = "../nasatopo" }
```
The output of `cross build` is:
```
error: failed to load source for a dependency on `nasatopo`

Caused by:
  Unable to update /nasatopo

Caused by:
  failed to read `/nasatopo/Cargo.toml`

Caused by:
  No such file or directory (os error 2)
```
Which is technically correct as the is not /nasatopo, but it the Cargo.toml I specified a local path, not a global one, as cross seems to understand.
Furthermore if I try to replace the relative path with a global one, the same error still happens, this time pointing to a path which is actually valid and takes to the location of the nasatopo source folder.

I think this has to do with how a docker container interfaces with the file-system, but I am not familiar and cannot tell.

Pull request description:
enable cargo workspace and path dependencies to work seamlessly
This will allow cargo workspaces to work seamlessly with cross, assuming they don't reference anything outside the current workspace root. This change also automatically mounts (if needed) path dependencies.

If they do reference things outside the workspace root, and that fact is not know to `cargo metadata` (e.g, it's not a workspace member or a path dependency), you'll have to mount it with

```toml
[build.env]
volumes = ["THING"]
```

and set `$THING="/path/to/thing"` (we need to work on accessibility for this feature too, a lot of reported issues are solved by it)

I'm sure there is an issue directly related to workspaces, but I can't find any.

fixes #388 

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9a59ae1..a4d24ba 100644
--- a/CHANGELOG.md
+++ b/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
 
@@ -45,6 +46,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 - Re-enabled `sparc64-unknown-linux-gnu` image
 - #582 - Added `libprocstat.so` to FreeBSD images
 - #665 - when not using [env.volumes](https://github.com/cross-rs/cross#mounting-volumes-into-the-build-environment), mount project in /project
+- #494 - Parse Cargo's --manifest-path option to determine mounted docker root
 
 
 ### Removed
diff --git a/README.md b/README.md
index a6adcdb..35cc9e4 100644
--- a/README.md
+++ b/README.md
@@ -362,12 +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.
-
 ## Minimum Supported Rust Version (MSRV)
 
 This crate is guaranteed to compile on stable Rust 1.58.1 and up. It *might*
diff --git a/src/cargo.rs b/src/cargo.rs
index 9c976af..544dabc 100644
--- a/src/cargo.rs
+++ b/src/cargo.rs
@@ -1,7 +1,8 @@
+use serde::Deserialize;
 use std::path::{Path, PathBuf};
 use std::process::{Command, ExitStatus};
-use std::{env, fs};
 
+use crate::cli::Args;
 use crate::errors::*;
 use crate::extensions::CommandExt;
 
@@ -52,38 +53,90 @@ impl<'a> From<&'a str> for Subcommand {
     }
 }
 
-#[derive(Debug)]
-pub struct Root {
-    path: PathBuf,
+#[derive(Debug, Deserialize)]
+pub struct CargoMetadata {
+    pub workspace_root: PathBuf,
+    pub target_directory: PathBuf,
+    pub packages: Vec<Package>,
+    pub workspace_members: Vec<String>,
 }
 
-impl Root {
-    pub fn path(&self) -> &Path {
-        &self.path
+impl CargoMetadata {
+    fn non_workspace_members(&self) -> impl Iterator<Item = &Package> {
+        self.packages
+            .iter()
+            .filter(|p| !self.workspace_members.iter().any(|m| m == &p.id))
     }
-}
 
-/// Cargo project root
-pub fn root() -> Result<Option<Root>> {
-    let cd = env::current_dir().wrap_err("couldn't get current directory")?;
+    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())
+    }
+}
 
-    let mut dir = &*cd;
-    loop {
-        let toml = dir.join("Cargo.toml");
+#[derive(Debug, Deserialize)]
+pub struct Package {
+    id: String,
+    manifest_path: PathBuf,
+    source: Option<String>,
+}
 
-        if fs::metadata(&toml).is_ok() {
-            return Ok(Some(Root {
-                path: dir.to_owned(),
-            }));
+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
         }
+    }
+}
 
-        match dir.parent() {
-            Some(p) => dir = p,
-            None => break,
+/// 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")
+            .ok()
+            .unwrap_or_else(|| "cargo".to_string()),
+    );
+    command.arg("metadata").arg("--format-version=1");
+    if let Some(cd) = cd {
+        command.current_dir(cd);
+    }
+    if let Some(config) = args {
+        if let Some(ref manifest_path) = config.manifest_path {
+            command.args(["--manifest-path".as_ref(), manifest_path.as_os_str()]);
         }
+    } else {
+        command.arg("--no-deps");
     }
-
-    Ok(None)
+    if let Some(target) = args.and_then(|a| a.target.as_ref()) {
+        command.args(["--filter-platform", target.triple()]);
+    }
+    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(verbose)?;
+    if !output.status.success() {
+        // TODO: logging
+        return Ok(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
diff --git a/src/cli.rs b/src/cli.rs
index 64f78e4..eeac524 100644
--- a/src/cli.rs
+++ b/src/cli.rs
@@ -12,9 +12,11 @@ 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,
+    pub manifest_path: Option<PathBuf>,
 }
 
 // Fix for issue #581. target_dir must be absolute.
@@ -72,6 +74,8 @@ 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;
     let mut all: Vec<String> = Vec::new();
@@ -82,7 +86,21 @@ pub fn parse(target_list: &TargetList) -> Result<Args> {
             if arg.is_empty() {
                 continue;
             }
-            if let ("+", ch) = arg.split_at(1) {
+            if arg == "--manifest-path" {
+                all.push(arg);
+                if let Some(m) = args.next() {
+                    let p = PathBuf::from(&m);
+                    all.push(m);
+                    manifest_path = env::current_dir().ok().map(|cwd| cwd.join(p));
+                }
+            } else if arg.starts_with("--manifest-path=") {
+                manifest_path = arg
+                    .split_once('=')
+                    .map(|x| x.1)
+                    .map(PathBuf::from)
+                    .and_then(|p| env::current_dir().ok().map(|cwd| cwd.join(p)));
+                all.push(arg);
+            } else if let ("+", ch) = arg.split_at(1) {
                 channel = Some(ch.to_string());
             } else if arg == "--target" {
                 all.push(arg);
@@ -95,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() {
@@ -128,8 +155,10 @@ pub fn parse(target_list: &TargetList) -> Result<Args> {
         subcommand: sc,
         channel,
         target,
+        features,
         target_dir,
         docker_in_docker,
         enable_doctests,
+        manifest_path,
     })
 }
diff --git a/src/docker.rs b/src/docker.rs
index 6f90856..df6e752 100644
--- a/src/docker.rs
+++ b/src/docker.rs
@@ -2,10 +2,10 @@ use std::path::{Path, PathBuf};
 use std::process::{Command, ExitStatus};
 use std::{env, fs};
 
-use crate::cargo::Root;
-use crate::errors::*;
+use crate::cargo::CargoMetadata;
 use crate::extensions::{CommandExt, SafeCommand};
 use crate::id;
+use crate::{errors::*, file};
 use crate::{Config, Target};
 use atty::Stream;
 use eyre::bail;
@@ -65,17 +65,38 @@ 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>,
-    root: &Root,
+    metadata: &CargoMetadata,
     config: &Config,
     uses_xargo: bool,
     sysroot: &Path,
     verbose: bool,
     docker_in_docker: bool,
+    cwd: &Path,
 ) -> Result<ExitStatus> {
     let mount_finder = if docker_in_docker {
         MountFinder::new(docker_read_mount_paths()?)
@@ -83,14 +104,13 @@ pub fn run(
         MountFinder::default()
     };
 
-    let root = root.path();
     let home_dir = home::home_dir().ok_or_else(|| eyre::eyre!("could not find home directory"))?;
     let cargo_dir = home::cargo_home()?;
     let xargo_dir = env::var_os("XARGO_HOME")
         .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(|| 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`
@@ -102,7 +122,12 @@ pub fn run(
     let cargo_dir = mount_finder.find_mount_path(cargo_dir);
     let xargo_dir = mount_finder.find_mount_path(xargo_dir);
     let target_dir = mount_finder.find_mount_path(target_dir);
-    let host_root = mount_finder.find_mount_path(root);
+    // 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) {
+        cwd
+    } else {
+        &metadata.workspace_root
+    });
     let mount_root: PathBuf;
     #[cfg(target_os = "windows")]
     {
@@ -111,7 +136,17 @@ pub fn run(
     }
     #[cfg(not(target_os = "windows"))]
     {
-        mount_root = host_root.clone();
+        mount_root = mount_finder.find_mount_path(host_root.clone());
+    }
+    let mount_cwd: 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_cwd = wslpath(cwd, verbose)?;
+    }
+    #[cfg(not(target_os = "windows"))]
+    {
+        mount_cwd = mount_finder.find_mount_path(cwd);
     }
     let sysroot = mount_finder.find_mount_path(sysroot);
 
@@ -134,7 +169,13 @@ pub fn run(
         // flag forwards the value from the parent shell
         docker.args(&["-e", var]);
     }
-    let mut env_volumes = false;
+    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() {
+        mount_volumes = true;
+    }
+
     for ref var in config.env_volumes(target)? {
         let (var, value) = validate_env_var(var)?;
         let value = match value {
@@ -165,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");
@@ -223,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()),
@@ -235,10 +282,21 @@ pub fn run(
         .args(&["-v", &format!("{}:/rust:Z,ro", sysroot.display())])
         .args(&["-v", &format!("{}:/target:Z", target_dir.display())]);
 
-    if env_volumes {
-        docker.args(&["-w", &mount_root.display().to_string()]);
-    } else {
+    if mount_volumes {
+        docker.args(&["-w".as_ref(), mount_cwd.as_os_str()]);
+    } 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)?);
+        // No [T].join for OsStr
+        let mut mount_wd = std::ffi::OsString::new();
+        for part in working_dir.iter() {
+            mount_wd.push("/");
+            mount_wd.push(part);
+        }
+        docker.args(&["-w".as_ref(), mount_wd.as_os_str()]);
     }
 
     // When running inside NixOS or using Nix packaging we need to add the Nix
diff --git a/src/file.rs b/src/file.rs
index 2386cef..f8a0181 100644
--- a/src/file.rs
+++ b/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)
+    }
+}
diff --git a/src/main.rs b/src/main.rs
index 04e24e2..9bbacca 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -25,7 +25,7 @@ use config::Config;
 use rustc_version::Channel;
 use serde::Deserialize;
 
-use self::cargo::{Root, Subcommand};
+use self::cargo::{CargoMetadata, Subcommand};
 use self::cross_toml::CrossToml;
 use self::errors::*;
 use self::extensions::OutputExt;
@@ -282,9 +282,10 @@ fn run() -> Result<ExitStatus> {
 
     let host_version_meta =
         rustc_version::version_meta().wrap_err("couldn't fetch the `rustc` version")?;
-    if let Some(root) = cargo::root()? {
+    let cwd = std::env::current_dir()?;
+    if let Some(metadata) = cargo::cargo_metadata_with_args(None, Some(&args), verbose)? {
         let host = host_version_meta.host();
-        let toml = toml(&root)?;
+        let toml = toml(&metadata)?;
         let config = Config::new(toml);
         let target = args
             .target
@@ -410,13 +411,13 @@ fn run() -> Result<ExitStatus> {
                 return docker::run(
                     &target,
                     &filtered_args,
-                    &args.target_dir,
-                    &root,
+                    &metadata,
                     &config,
                     uses_xargo,
                     &sysroot,
                     verbose,
                     args.docker_in_docker,
+                    &cwd,
                 );
             }
         }
@@ -489,10 +490,10 @@ pub(crate) fn warn_host_version_mismatch(
 
 /// Parses the `Cross.toml` at the root of the Cargo project or from the
 /// `CROSS_CONFIG` environment variable (if any exist in either location).
-fn toml(root: &Root) -> Result<Option<CrossToml>> {
+fn toml(metadata: &CargoMetadata) -> Result<Option<CrossToml>> {
     let path = match env::var("CROSS_CONFIG") {
         Ok(var) => PathBuf::from(var),
-        Err(_) => root.path().join("Cross.toml"),
+        Err(_) => metadata.workspace_root.join("Cross.toml"),
     };
 
     if path.exists() {
@@ -505,7 +506,7 @@ fn toml(root: &Root) -> Result<Option<CrossToml>> {
         Ok(Some(config))
     } else {
         // Checks if there is a lowercase version of this file
-        if root.path().join("cross.toml").exists() {
+        if metadata.workspace_root.join("cross.toml").exists() {
             eprintln!("There's a file named cross.toml, instead of Cross.toml. You may want to rename it, or it won't be considered.");
         }
         Ok(None)


Test patch:
diff --git a/ci/test.sh b/ci/test.sh
index 9931fdd..207c95d 100755
--- a/ci/test.sh
+++ b/ci/test.sh
@@ -25,6 +25,14 @@ function retry {
   return ${exit_code}
 }
 
+workspace_test() {
+  # "${@}" is an unbound variable for bash 3.2, which is the installed version on macOS
+  cross build --target "${TARGET}" --workspace "$@"
+  cross run --target "${TARGET}" -p binary "$@"
+  cross run --target "${TARGET}" --bin dependencies \
+    --features=dependencies "$@"
+}
+
 main() {
     local td=
 
@@ -135,6 +143,21 @@ EOF
                 popd
 
                 rm -rf "${td}"
+                td=$(mktemp -d)
+                git clone \
+                    --depth 1 \
+                    --recursive \
+                    https://github.com/cross-rs/test-workspace "${td}"
+                
+                pushd "${td}"
+                TARGET="${TARGET}" workspace_test --manifest-path="./workspace/Cargo.toml"
+                pushd "workspace"
+                TARGET="${TARGET}" workspace_test
+                pushd "binary"
+                cross run --target "${TARGET}"
+                popd
+                popd
+                popd
             ;;
         esac
 
diff --git a/src/tests.rs b/src/tests.rs
index 90c4278..42f0aad 100644
--- a/src/tests.rs
+++ b/src/tests.rs
@@ -7,7 +7,6 @@ use std::{
 
 use once_cell::sync::OnceCell;
 use rustc_version::VersionMeta;
-use serde::Deserialize;
 
 static WORKSPACE: OnceCell<PathBuf> = OnceCell::new();
 
@@ -15,23 +14,10 @@ static WORKSPACE: OnceCell<PathBuf> = OnceCell::new();
 pub fn get_cargo_workspace() -> &'static Path {
     let manifest_dir = env!("CARGO_MANIFEST_DIR");
     WORKSPACE.get_or_init(|| {
-        #[derive(Deserialize)]
-        struct Manifest {
-            workspace_root: PathBuf,
-        }
-        let output = std::process::Command::new(
-            std::env::var("CARGO")
-                .ok()
-                .unwrap_or_else(|| "cargo".to_string()),
-        )
-        .arg("metadata")
-        .arg("--format-version=1")
-        .arg("--no-deps")
-        .current_dir(manifest_dir)
-        .output()
-        .unwrap();
-        let manifest: Manifest = serde_json::from_slice(&output.stdout).unwrap();
-        manifest.workspace_root
+        crate::cargo::cargo_metadata_with_args(Some(manifest_dir.as_ref()), None, true)
+            .unwrap()
+            .unwrap()
+            .workspace_root
     })
 }