From 56a229f0826cb38e6a072c3158e18e469bef92d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emil=20Gardstr=C3=B6m?= Date: Thu, 7 Apr 2022 18:27:46 +0200 Subject: [PATCH] correctly handle target-dir --- src/cargo.rs | 34 +++++++++++++++++++--------------- src/docker.rs | 27 +++++++++------------------ src/file.rs | 18 +++++++++++++++++- src/main.rs | 6 ++---- src/tests.rs | 2 +- 5 files changed, 48 insertions(+), 39 deletions(-) diff --git a/src/cargo.rs b/src/cargo.rs index f62c61d62..e22b132b8 100644 --- a/src/cargo.rs +++ b/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> { - 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 diff --git a/src/docker.rs b/src/docker.rs index 8e8ab3044..60ecc7fc0 100644 --- a/src/docker.rs +++ b/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, 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,7 +150,7 @@ 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; } @@ -161,22 +158,16 @@ pub fn run( 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() { diff --git a/src/file.rs b/src/file.rs index 2386cef53..f8a0181ef 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 { .wrap_err_with(|| format!("couldn't read {}", path.display()))?; Ok(s) } + +pub fn canonicalize(path: impl AsRef) -> Result { + _canonicalize(path.as_ref()) +} + +fn _canonicalize(path: &Path) -> Result { + #[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 dceab030b..db37f0b9d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -395,11 +395,9 @@ fn run() -> Result { 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> { 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> { 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) diff --git a/src/tests.rs b/src/tests.rs index 3217f3ede..4554db9f9 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -15,7 +15,7 @@ static WORKSPACE: OnceCell = 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