correctly handle target-dir

src/cargo.rs

@@ -3,7 +3,6 @@ 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;
 
@@ -55,12 +54,7 @@ impl<'a> From<&'a str> for Subcommand {
 #[derive(Debug, Deserialize)]
 pub struct CargoMetadata {
     pub workspace_root: PathBuf,
-}
-
-impl CargoMetadata {
-    pub fn workspace_root(&self) -> &Path {
-        &self.workspace_root
-    }
+    pub target_directory: PathBuf,
 }
 
 /// Cargo metadata with specific invocation
@@ -92,14 +86,24 @@ pub fn cargo_metadata_with_args(
                 String::from_utf8(output.stderr)
             )
         })?;
-    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)
+    manifest
+        .map(|m| -> Result<_> {
+            Ok(CargoMetadata {
+                target_directory: args
+                    .and_then(|a| a.target_dir.clone())
+                    .map(|p| {
+                        if p.is_relative() {
+                            cd.expect("this is a bug, working directory should be provided here")
+                                .join(p)
+                        } else {
+                            p
+                        }
+                    })
+                    .unwrap_or(m.target_directory),
+                ..m
+            })
+        })
+        .transpose()
 }
 
 /// Pass-through mode

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;
@@ -56,7 +56,6 @@ pub fn register(target: &Target, verbose: bool) -> Result<()> {
 pub fn run(
     target: &Target,
     args: &[String],
-    target_dir: &Option<PathBuf>,
     metadata: &CargoMetadata,
     config: &Config,
     uses_xargo: bool,
@@ -77,9 +76,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`
@@ -92,10 +89,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")]
@@ -153,30 +150,24 @@ pub fn run(
     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)
     // Preferably we would not have to do this.
-    if cwd.strip_prefix(metadata.workspace_root()).is_err() {
+    if cwd.strip_prefix(&metadata.workspace_root).is_err() {
         env_volumes = true;
     }
 
     for ref var in config.env_volumes(target)? {
         validate_env_var(var)?;
 
         if let Ok(val) = env::var(var) {
-            let host_path: PathBuf;
+            let host_path = file::canonicalize(&val)
+                .wrap_err_with(|| format!("when canonicalizing path `{val}`"))?;
             let mount_path: PathBuf;
-
             #[cfg(target_os = "windows")]
             {
-                // Docker does not support UNC paths, this will try to not use UNC paths
-                host_path = dunce::canonicalize(&val)
-                    .wrap_err_with(|| format!("when canonicalizing path `{val}`"))?;
                 // 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"))]
             {
-                host_path = Path::new(&val)
-                    .canonicalize()
-                    .wrap_err_with(|| format!("when canonicalizing path `{val}`"))?;
                 mount_path = host_path.clone();
             }
             docker.args(&[
@@ -255,12 +246,12 @@ pub fn run(
 
     if env_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)
+    }
+}

src/main.rs

@@ -395,11 +395,9 @@ fn run() -> Result<ExitStatus> {
                     docker::register(&target, verbose)?
                 }
 
-                let docker_root = env::current_dir()?;
                 return docker::run(
                     &target,
                     &filtered_args,
-                    &args.target_dir,
                     &metadata,
                     &config,
                     uses_xargo,
@@ -468,7 +466,7 @@ pub(crate) fn warn_host_version_mismatch(
 fn toml(root: &CargoMetadata) -> Result<Option<CrossToml>> {
     let path = match env::var("CROSS_CONFIG") {
         Ok(var) => PathBuf::from(var),
-        Err(_) => root.workspace_root().join("Cross.toml"),
+        Err(_) => root.workspace_root.join("Cross.toml"),
     };
 
     if path.exists() {
@@ -481,7 +479,7 @@ fn toml(root: &CargoMetadata) -> Result<Option<CrossToml>> {
         Ok(Some(config))
     } else {
         // Checks if there is a lowercase version of this file
-        if root.workspace_root().join("cross.toml").exists() {
+        if root.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)

src/tests.rs

@@ -15,7 +15,7 @@ static WORKSPACE: OnceCell<PathBuf> = OnceCell::new();
 pub fn get_cargo_workspace() -> &'static Path {
     let manifest_dir = env!("CARGO_MANIFEST_DIR");
     WORKSPACE.get_or_init(|| {
-        crate::cargo::cargo_metadata(Some(manifest_dir.as_ref()))
+        crate::cargo::cargo_metadata_with_args(Some(manifest_dir.as_ref()), None)
             .unwrap()
             .unwrap()
             .workspace_root