From 2ec0e3f2e1a0ea4139d507afda5d48d8507fd07f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emil=20Gardstr=C3=B6m?= Date: Wed, 6 Apr 2022 22:22:57 +0200 Subject: [PATCH 1/7] make it possible to run from anywhere in workspace --- src/cargo.rs | 44 +++++++++++++++++++++++--------------------- src/docker.rs | 27 ++++++++++++++++++++++++--- src/main.rs | 2 +- src/tests.rs | 22 ++++------------------ 4 files changed, 52 insertions(+), 43 deletions(-) diff --git a/src/cargo.rs b/src/cargo.rs index 14e9585e6..947fee36f 100644 --- a/src/cargo.rs +++ b/src/cargo.rs @@ -1,6 +1,6 @@ +use serde::Deserialize; use std::path::{Path, PathBuf}; use std::process::{Command, ExitStatus}; -use std::{env, fs}; use crate::errors::*; use crate::extensions::CommandExt; @@ -54,7 +54,7 @@ impl<'a> From<&'a str> for Subcommand { #[derive(Debug)] pub struct Root { - path: PathBuf, + pub path: PathBuf, } impl Root { @@ -64,26 +64,28 @@ impl Root { } /// Cargo project root -pub fn root() -> Result> { - let cd = env::current_dir().wrap_err("couldn't get current directory")?; - - let mut dir = &*cd; - loop { - let toml = dir.join("Cargo.toml"); - - if fs::metadata(&toml).is_ok() { - return Ok(Some(Root { - path: dir.to_owned(), - })); - } - - match dir.parent() { - Some(p) => dir = p, - None => break, - } +pub fn root(cd: Option<&Path>) -> Result> { + #[derive(Deserialize)] + struct Manifest { + workspace_root: PathBuf, } - - Ok(None) + 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") + .arg("--no-deps"); + if let Some(cd) = cd { + command.current_dir(cd); + } + let output = command.output()?; + let manifest: Option = serde_json::from_slice(&output.stdout)?; + Ok(manifest.map(|m| Root { + path: m.workspace_root, + })) } /// Pass-through mode diff --git a/src/docker.rs b/src/docker.rs index d9e0ba394..aa2da7348 100644 --- a/src/docker.rs +++ b/src/docker.rs @@ -98,7 +98,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(&std::env::current_dir()?, verbose)?; + } + #[cfg(not(target_os = "windows"))] + { + mount_cwd = mount_finder.find_mount_path(std::env::current_dir()?); } let sysroot = mount_finder.find_mount_path(sysroot); @@ -231,9 +241,20 @@ pub fn run( .args(&["-v", &format!("{}:/target:Z", target_dir.display())]); if env_volumes { - docker.args(&["-w", &mount_root.display().to_string()]); - } else { + docker.args(&["-w".as_ref(), mount_cwd.as_os_str()]); + } else if mount_cwd == 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 = &std::env::current_dir()?; + let working_dir = Path::new("project").join(cwd.strip_prefix(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/main.rs b/src/main.rs index 15df03aff..ffc9fc37f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -279,7 +279,7 @@ fn run() -> Result { let version_meta = rustc_version::version_meta().wrap_err("couldn't fetch the `rustc` version")?; - if let Some(root) = cargo::root()? { + if let Some(root) = cargo::root(None)? { let host = version_meta.host(); let toml = toml(&root)?; let config = Config::new(toml); diff --git a/src/tests.rs b/src/tests.rs index 5fa299105..46e4301d9 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -6,7 +6,6 @@ use std::{ }; use once_cell::sync::OnceCell; -use serde::Deserialize; static WORKSPACE: OnceCell = OnceCell::new(); @@ -14,23 +13,10 @@ static WORKSPACE: OnceCell = 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::root(Some(manifest_dir.as_ref())) + .unwrap() + .unwrap() + .path }) } From 8cab3b6a974d2f479cbb607a25a33dfe5667bf62 Mon Sep 17 00:00:00 2001 From: wngr Date: Sat, 19 Mar 2022 20:02:00 +0100 Subject: [PATCH 2/7] feat: Parse Cargo's --manifest-path option to determine mounted docker root MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commits adds support for parsing the `--manifest-path` option to cross. So far, the `Cargo.toml` manifest of the crate (or its Cargo workspace) to compile has been assumed to be in the current working directory. This means, that relative crate dependencies were not supported, because those paths were not mapped into the docker container. Take the following example structure, where `my-bin` depends on `my-lib`: . ├── my-bin │ ├── Cargo.toml │ └── src │ └── main.rs └── my-lib ├── Cargo.toml └── src └── lib.rs This commits enables such scenarios, by running cross from `.` like so: `cross build --manifest-path=my-lib/Cargo.toml --target x86_64-pc-windows-gnu`, as `.` is mapped as the container's root, and the options passed through to Cargo. Related cross-rs#388 cross-rs#139 cross-rs#277 cross-rs#78 Co-authored-by: Kviring Alexey --- CHANGELOG.md | 1 + README.md | 3 +++ src/cargo.rs | 54 +++++++++++++++++++++++++++++++-------------------- src/cli.rs | 19 +++++++++++++++++- src/docker.rs | 5 +++-- src/main.rs | 4 +++- 6 files changed, 61 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 29e881f3e..43c7bd17f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] +- #494 - Parse Cargo's --manifest-path option to determine mounted docker root - #629 - Update Android NDK version and API version - #681 - Warn on unknown fields and confusable targets - #665 - when not using [env.volumes](https://github.com/cross-rs/cross#mounting-volumes-into-the-build-environment), mount project in /project diff --git a/README.md b/README.md index 9ae5418a5..e3f4215b9 100644 --- a/README.md +++ b/README.md @@ -355,6 +355,9 @@ $ QEMU_STRACE=1 cross run --target aarch64-unknown-linux-gnu - 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) diff --git a/src/cargo.rs b/src/cargo.rs index 947fee36f..d32c08c1a 100644 --- a/src/cargo.rs +++ b/src/cargo.rs @@ -64,28 +64,40 @@ impl Root { } /// Cargo project root -pub fn root(cd: Option<&Path>) -> Result> { - #[derive(Deserialize)] - struct Manifest { - workspace_root: PathBuf, - } - 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") - .arg("--no-deps"); - if let Some(cd) = cd { - command.current_dir(cd); +pub fn root(cd: Option<&Path>, manifest_path: Option) -> Result> { + if let Some(manifest_path) = manifest_path { + if !manifest_path.is_file() { + eyre::bail!("No manifest found at \"{}\"", manifest_path.display()); + } + return Ok(Some(Root { + path: manifest_path + .parent() + .expect("File must have a parent") + .to_owned(), + })); + } else { + #[derive(Deserialize)] + struct Manifest { + workspace_root: PathBuf, + } + 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") + .arg("--no-deps"); + if let Some(cd) = cd { + command.current_dir(cd); + } + let output = command.output()?; + let manifest: Option = serde_json::from_slice(&output.stdout)?; + Ok(manifest.map(|m| Root { + path: m.workspace_root, + })) } - let output = command.output()?; - let manifest: Option = serde_json::from_slice(&output.stdout)?; - Ok(manifest.map(|m| Root { - path: m.workspace_root, - })) } /// Pass-through mode diff --git a/src/cli.rs b/src/cli.rs index f62ee82de..91c0c83f0 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -13,11 +13,13 @@ pub struct Args { pub target: Option, pub target_dir: Option, pub docker_in_docker: bool, + pub manifest_path: Option, } pub fn parse(target_list: &TargetList) -> Args { let mut channel = None; let mut target = None; + let mut manifest_path: Option = None; let mut target_dir = None; let mut sc = None; let mut all: Vec = Vec::new(); @@ -28,7 +30,21 @@ pub fn parse(target_list: &TargetList) -> 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); @@ -73,5 +89,6 @@ pub fn parse(target_list: &TargetList) -> Args { target, target_dir, docker_in_docker, + manifest_path, } } diff --git a/src/docker.rs b/src/docker.rs index aa2da7348..cac40162b 100644 --- a/src/docker.rs +++ b/src/docker.rs @@ -58,6 +58,7 @@ pub fn run( args: &[String], target_dir: &Option, root: &Root, + docker_root: &Path, config: &Config, uses_xargo: bool, sysroot: &Path, @@ -89,7 +90,7 @@ 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); + let host_root = mount_finder.find_mount_path(docker_root); let mount_root: PathBuf; #[cfg(target_os = "windows")] { @@ -247,7 +248,7 @@ pub fn run( } else { // We do this to avoid clashes with path separators. Windows uses `\` as a path separator on Path::join let cwd = &std::env::current_dir()?; - let working_dir = Path::new("project").join(cwd.strip_prefix(root)?); + let working_dir = Path::new("project").join(cwd.strip_prefix(docker_root)?); // No [T].join for OsStr let mut mount_wd = std::ffi::OsString::new(); for part in working_dir.iter() { diff --git a/src/main.rs b/src/main.rs index ffc9fc37f..ef93c4641 100644 --- a/src/main.rs +++ b/src/main.rs @@ -279,7 +279,7 @@ fn run() -> Result { let version_meta = rustc_version::version_meta().wrap_err("couldn't fetch the `rustc` version")?; - if let Some(root) = cargo::root(None)? { + if let Some(root) = cargo::root(None, args.manifest_path)? { let host = version_meta.host(); let toml = toml(&root)?; let config = Config::new(toml); @@ -386,11 +386,13 @@ fn run() -> Result { docker::register(&target, verbose)? } + let docker_root = env::current_dir()?; return docker::run( &target, &filtered_args, &args.target_dir, &root, + &docker_root, &config, uses_xargo, &sysroot, From dc930d30d2092c3d6c6b00ded302c650342a9a91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emil=20Gardstr=C3=B6m?= Date: Thu, 7 Apr 2022 17:34:24 +0200 Subject: [PATCH 3/7] make manifest path work with metadata contains a minor workaround for when logic is hard --- src/cargo.rs | 80 +++++++++++++++++++++++++++------------------------ src/docker.rs | 34 +++++++++++++++------- src/main.rs | 17 +++++------ src/tests.rs | 4 +-- 4 files changed, 76 insertions(+), 59 deletions(-) diff --git a/src/cargo.rs b/src/cargo.rs index d32c08c1a..fc0c6f99b 100644 --- a/src/cargo.rs +++ b/src/cargo.rs @@ -2,6 +2,8 @@ use serde::Deserialize; 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; @@ -52,52 +54,54 @@ impl<'a> From<&'a str> for Subcommand { } } -#[derive(Debug)] -pub struct Root { - pub path: PathBuf, +#[derive(Debug, Deserialize)] +pub struct CargoMetadata { + pub workspace_root: PathBuf, } -impl Root { - pub fn path(&self) -> &Path { - &self.path +impl CargoMetadata { + pub fn workspace_root(&self) -> &Path { + &self.workspace_root } } -/// Cargo project root -pub fn root(cd: Option<&Path>, manifest_path: Option) -> Result> { - if let Some(manifest_path) = manifest_path { - if !manifest_path.is_file() { - eyre::bail!("No manifest found at \"{}\"", manifest_path.display()); +/// Cargo metadata with specific invocation +pub fn cargo_metadata_with_args( + cd: Option<&Path>, + args: Option<&Args>, +) -> Result> { + 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()]); } - return Ok(Some(Root { - path: manifest_path - .parent() - .expect("File must have a parent") - .to_owned(), - })); } else { - #[derive(Deserialize)] - struct Manifest { - workspace_root: PathBuf, - } - 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") - .arg("--no-deps"); - if let Some(cd) = cd { - command.current_dir(cd); - } - let output = command.output()?; - let manifest: Option = serde_json::from_slice(&output.stdout)?; - Ok(manifest.map(|m| Root { - path: m.workspace_root, - })) + command.arg("--no-deps"); } + let output = command.output()?; + let manifest: Option = + serde_json::from_slice(&output.stdout).wrap_err_with(|| { + format!( + "{command:?} returned nothing. {:?}", + 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) } /// Pass-through mode diff --git a/src/docker.rs b/src/docker.rs index cac40162b..8e8ab3044 100644 --- a/src/docker.rs +++ b/src/docker.rs @@ -2,7 +2,7 @@ use std::path::{Path, PathBuf}; use std::process::{Command, ExitStatus}; use std::{env, fs}; -use crate::cargo::Root; +use crate::cargo::CargoMetadata; use crate::errors::*; use crate::extensions::{CommandExt, SafeCommand}; use crate::id; @@ -57,13 +57,13 @@ pub fn run( target: &Target, args: &[String], target_dir: &Option, - root: &Root, - docker_root: &Path, + metadata: &CargoMetadata, config: &Config, uses_xargo: bool, sysroot: &Path, verbose: bool, docker_in_docker: bool, + cwd: &Path, ) -> Result { let mount_finder = if docker_in_docker { MountFinder::new(docker_read_mount_paths()?) @@ -71,14 +71,15 @@ 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 = target_dir + .clone() + .unwrap_or_else(|| metadata.workspace_root().join("target")); // create the directories we are going to mount before we mount them, // otherwise `docker` will create them but they will be owned by `root` @@ -90,7 +91,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(docker_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")] { @@ -105,11 +111,11 @@ 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(&std::env::current_dir()?, verbose)?; + mount_cwd = wslpath(&cwd, verbose)?; } #[cfg(not(target_os = "windows"))] { - mount_cwd = mount_finder.find_mount_path(std::env::current_dir()?); + mount_cwd = mount_finder.find_mount_path(cwd); } let sysroot = mount_finder.find_mount_path(sysroot); @@ -145,6 +151,12 @@ pub fn run( 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) + // Preferably we would not have to do this. + if cwd.strip_prefix(metadata.workspace_root()).is_err() { + env_volumes = true; + } + for ref var in config.env_volumes(target)? { validate_env_var(var)?; @@ -243,12 +255,12 @@ pub fn run( if env_volumes { docker.args(&["-w".as_ref(), mount_cwd.as_os_str()]); - } else if mount_cwd == 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 = &std::env::current_dir()?; - let working_dir = Path::new("project").join(cwd.strip_prefix(docker_root)?); + 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() { diff --git a/src/main.rs b/src/main.rs index ef93c4641..923fd5c97 100644 --- a/src/main.rs +++ b/src/main.rs @@ -23,7 +23,7 @@ use std::process::ExitStatus; use config::Config; use serde::Deserialize; -use self::cargo::{Root, Subcommand}; +use self::cargo::{CargoMetadata, Subcommand}; use self::cross_toml::CrossToml; use self::errors::*; use self::rustc::{TargetList, VersionMetaExt}; @@ -279,9 +279,10 @@ fn run() -> Result { let version_meta = rustc_version::version_meta().wrap_err("couldn't fetch the `rustc` version")?; - if let Some(root) = cargo::root(None, args.manifest_path)? { + let cwd = std::env::current_dir()?; + if let Some(metadata) = cargo::cargo_metadata_with_args(Some(&cwd), Some(&args))? { let host = version_meta.host(); - let toml = toml(&root)?; + let toml = toml(&metadata)?; let config = Config::new(toml); let target = args .target @@ -391,13 +392,13 @@ fn run() -> Result { &target, &filtered_args, &args.target_dir, - &root, - &docker_root, + &metadata, &config, uses_xargo, &sysroot, verbose, args.docker_in_docker, + &cwd, ); } } @@ -408,10 +409,10 @@ fn run() -> Result { /// 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> { +fn toml(root: &CargoMetadata) -> Result> { let path = match env::var("CROSS_CONFIG") { Ok(var) => PathBuf::from(var), - Err(_) => root.path().join("Cross.toml"), + Err(_) => root.workspace_root().join("Cross.toml"), }; if path.exists() { @@ -424,7 +425,7 @@ fn toml(root: &Root) -> Result> { Ok(Some(config)) } else { // Checks if there is a lowercase version of this file - if root.path().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 46e4301d9..adab14fa2 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -13,10 +13,10 @@ 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::root(Some(manifest_dir.as_ref())) + crate::cargo::cargo_metadata(Some(manifest_dir.as_ref())) .unwrap() .unwrap() - .path + .workspace_root }) } From 9060245e22774a13102f7475c6900862cdafe507 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 4/7] 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 fc0c6f99b..523863785 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; @@ -57,12 +56,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 @@ -94,14 +88,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 923fd5c97..bb24dcf03 100644 --- a/src/main.rs +++ b/src/main.rs @@ -387,11 +387,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, @@ -412,7 +410,7 @@ fn run() -> Result { 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() { @@ -425,7 +423,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 adab14fa2..5c43f175d 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -13,7 +13,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 From 1a1948942436f174b69c15241bd086726457e4e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emil=20Gardstr=C3=B6m?= Date: Thu, 7 Apr 2022 19:24:30 +0200 Subject: [PATCH 5/7] resolve and mount path dependencies --- src/cargo.rs | 37 ++++++++++++++++++++++++++++++++++++ src/docker.rs | 52 ++++++++++++++++++++++++++++++--------------------- 2 files changed, 68 insertions(+), 21 deletions(-) diff --git a/src/cargo.rs b/src/cargo.rs index 523863785..9cd902880 100644 --- a/src/cargo.rs +++ b/src/cargo.rs @@ -57,6 +57,40 @@ impl<'a> From<&'a str> for Subcommand { pub struct CargoMetadata { pub workspace_root: PathBuf, pub target_directory: PathBuf, + pub packages: Vec, + pub workspace_members: Vec, +} + +impl CargoMetadata { + fn non_workspace_members(&self) -> impl Iterator { + self.packages + .iter() + .filter(|p| !self.workspace_members.iter().any(|m| m == &p.id)) + } + + pub fn path_dependencies(&self) -> impl Iterator { + // 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, +} + +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 @@ -80,6 +114,9 @@ pub fn cargo_metadata_with_args( } else { command.arg("--no-deps"); } + if let Some(target) = args.and_then(|a| a.target.as_ref()) { + command.args(["--filter-platform", target.triple()]); + } let output = command.output()?; let manifest: Option = serde_json::from_slice(&output.stdout).wrap_err_with(|| { diff --git a/src/docker.rs b/src/docker.rs index 60ecc7fc0..e2f1170ee 100644 --- a/src/docker.rs +++ b/src/docker.rs @@ -52,6 +52,26 @@ pub fn register(target: &Target, verbose: bool) -> Result<()> { .run(verbose) } +pub fn mount(cmd: &mut Command, val: &Path, verbose: bool) -> Result { + 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, @@ -147,38 +167,28 @@ 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 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() { - env_volumes = true; + mount_volumes = true; } for ref var in config.env_volumes(target)? { validate_env_var(var)?; if let Ok(val) = env::var(var) { - 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(); - } - docker.args(&[ - "-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"); @@ -232,7 +242,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()), @@ -244,7 +254,7 @@ 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 { docker.args(&["-w", "/project"]); From 4cc5af76f3fe2db0f1539846643981353ac0f54c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emil=20Gardstr=C3=B6m?= Date: Thu, 7 Apr 2022 19:48:18 +0200 Subject: [PATCH 6/7] add test for workspace support --- ci/test.sh | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/ci/test.sh b/ci/test.sh index 9931fdda5..137830a60 100755 --- a/ci/test.sh +++ b/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 From 7860d47b451be595c4bd00e6d20032533acfc044 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emil=20Gardstr=C3=B6m?= Date: Thu, 7 Apr 2022 19:52:19 +0200 Subject: [PATCH 7/7] satisfy clippy --- src/docker.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/docker.rs b/src/docker.rs index e2f1170ee..e1e90f965 100644 --- a/src/docker.rs +++ b/src/docker.rs @@ -52,6 +52,7 @@ pub fn register(target: &Target, verbose: bool) -> Result<()> { .run(verbose) } +#[allow(unused_variables)] pub fn mount(cmd: &mut Command, val: &Path, verbose: bool) -> Result { let host_path = file::canonicalize(&val).wrap_err_with(|| format!("when canonicalizing path `{val:?}`"))?; @@ -128,7 +129,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"))] {