From 0eecee18b2e5d816130ca2af78cf4d9adfa30979 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/4] make it possible to run from anywhere in workspace --- src/cargo.rs | 44 +++++++++++++++++++++++--------------------- src/docker.rs | 27 ++++++++++++++++++++++++--- src/main.rs | 2 +- src/tests.rs | 21 ++++----------------- 4 files changed, 52 insertions(+), 42 deletions(-) diff --git a/src/cargo.rs b/src/cargo.rs index 9c976afcb..654ed580a 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 6f9085696..bccb1edf1 100644 --- a/src/docker.rs +++ b/src/docker.rs @@ -111,7 +111,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); @@ -236,9 +246,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 04e24e26d..0012ffa6c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -282,7 +282,7 @@ fn run() -> Result { let host_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 = host_version_meta.host(); let toml = toml(&root)?; let config = Config::new(toml); diff --git a/src/tests.rs b/src/tests.rs index 90c4278ba..6970ff077 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -15,23 +15,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 09e5d0eeaa15ddf05930ba6aab73ef589ef65a48 Mon Sep 17 00:00:00 2001 From: wngr Date: Sat, 19 Mar 2022 20:02:00 +0100 Subject: [PATCH 2/4] 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 | 64 +++++++++++++++++++++++++++++++++++---------------- src/cli.rs | 19 ++++++++++++++- src/docker.rs | 33 ++++++++++++++++++-------- src/main.rs | 17 ++++++++------ src/tests.rs | 5 ++-- 7 files changed, 101 insertions(+), 41 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a59ae19b..0f132a4d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,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 a6adcdb76..becb8b9d6 100644 --- a/README.md +++ b/README.md @@ -367,6 +367,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 654ed580a..0a288add2 100644 --- a/src/cargo.rs +++ b/src/cargo.rs @@ -1,9 +1,12 @@ 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; +use crate::extensions::{CommandExt, OutputExt}; #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum Subcommand { @@ -52,42 +55,63 @@ 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>) -> Result> { - #[derive(Deserialize)] - struct Manifest { - workspace_root: PathBuf, - } +/// 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") - .arg("--no-deps"); + 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"); + } + #[derive(Deserialize)] + struct Manifest { + workspace_root: PathBuf, + } if let Some(cd) = cd { command.current_dir(cd); } - let output = command.output()?; + let output = command.run_and_get_output(false)?; + 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)); + } let manifest: Option = serde_json::from_slice(&output.stdout)?; - Ok(manifest.map(|m| Root { - path: m.workspace_root, + 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 pub fn run(args: &[String], verbose: bool) -> Result { Command::new("cargo").args(args).run_and_get_status(verbose) diff --git a/src/cli.rs b/src/cli.rs index 64f78e47f..596b6a46d 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -15,6 +15,7 @@ pub struct Args { pub target_dir: Option, pub docker_in_docker: bool, pub enable_doctests: bool, + pub manifest_path: Option, } // Fix for issue #581. target_dir must be absolute. @@ -72,6 +73,7 @@ pub fn fmt_subcommands(stdout: &str) { pub fn parse(target_list: &TargetList) -> Result { 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(); @@ -82,7 +84,21 @@ pub fn parse(target_list: &TargetList) -> Result { 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); @@ -131,5 +147,6 @@ pub fn parse(target_list: &TargetList) -> Result { target_dir, docker_in_docker, enable_doctests, + manifest_path, }) } diff --git a/src/docker.rs b/src/docker.rs index bccb1edf1..40d9b1732 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; @@ -70,12 +70,13 @@ pub fn run( target: &Target, args: &[String], target_dir: &Option, - root: &Root, + 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()?) @@ -83,14 +84,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` @@ -102,7 +104,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")] { @@ -117,11 +124,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 +152,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)? { let (var, value) = validate_env_var(var)?; let value = match value { @@ -247,12 +260,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(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 0012ffa6c..d470001c8 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 { let host_version_meta = rustc_version::version_meta().wrap_err("couldn't fetch the `rustc` version")?; - if let Some(root) = cargo::root(None)? { + let cwd = std::env::current_dir()?; + if let Some(metadata) = cargo::cargo_metadata_with_args(Some(&cwd), Some(&args))? { let host = host_version_meta.host(); - let toml = toml(&root)?; + let toml = toml(&metadata)?; let config = Config::new(toml); let target = args .target @@ -407,16 +408,18 @@ fn run() -> Result { docker::register(&target, verbose)? } + let docker_root = env::current_dir()?; return docker::run( &target, &filtered_args, &args.target_dir, - &root, + &metadata, &config, uses_xargo, &sysroot, verbose, args.docker_in_docker, + &cwd, ); } } @@ -489,10 +492,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> { +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() { @@ -505,7 +508,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 6970ff077..565935da2 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 = OnceCell::new(); @@ -15,10 +14,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 4232a2a24e4cdbb20bdbb5b52af0dba557f9cb99 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 3/4] Mount path dependencies correctly --- CHANGELOG.md | 1 + README.md | 9 ------- ci/test.sh | 15 +++++++++++ src/cargo.rs | 75 ++++++++++++++++++++++++++++++++++----------------- src/cli.rs | 12 +++++++++ src/docker.rs | 58 +++++++++++++++++++++++++++------------ src/file.rs | 18 ++++++++++++- src/main.rs | 10 +++---- src/tests.rs | 2 +- 9 files changed, 142 insertions(+), 58 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f132a4d3..a4d24baf6 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 diff --git a/README.md b/README.md index becb8b9d6..35cc9e484 100644 --- a/README.md +++ b/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* 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 diff --git a/src/cargo.rs b/src/cargo.rs index 0a288add2..544dabc31 100644 --- a/src/cargo.rs +++ b/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,11 +56,40 @@ impl<'a> From<&'a str> for Subcommand { #[derive(Debug, Deserialize)] pub struct CargoMetadata { pub workspace_root: PathBuf, + pub target_directory: PathBuf, + pub packages: Vec, + pub workspace_members: Vec, } impl CargoMetadata { - pub fn workspace_root(&self) -> &Path { - &self.workspace_root + 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 + } } } @@ -70,6 +97,7 @@ impl CargoMetadata { pub fn cargo_metadata_with_args( cd: Option<&Path>, args: Option<&Args>, + verbose: bool, ) -> Result> { 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 = 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> { - cargo_metadata_with_args(cd, None) + let manifest: Option = 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 596b6a46d..eeac52420 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -12,6 +12,7 @@ pub struct Args { pub subcommand: Option, pub channel: Option, pub target: Option, + pub features: Vec, pub target_dir: Option, 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 { let mut channel = None; let mut target = None; + let mut features = Vec::new(); let mut manifest_path: Option = None; let mut target_dir = None; let mut sc = None; @@ -111,6 +113,15 @@ pub fn parse(target_list: &TargetList) -> Result { .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 { subcommand: sc, channel, target, + features, target_dir, docker_in_docker, enable_doctests, diff --git a/src/docker.rs b/src/docker.rs index 40d9b1732..df6e7520c 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; @@ -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 { + 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, 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() { 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 d470001c8..9bbacca5a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -283,7 +283,7 @@ fn run() -> Result { let host_version_meta = rustc_version::version_meta().wrap_err("couldn't fetch the `rustc` version")?; let cwd = std::env::current_dir()?; - if let Some(metadata) = cargo::cargo_metadata_with_args(Some(&cwd), Some(&args))? { + if let Some(metadata) = cargo::cargo_metadata_with_args(None, Some(&args), verbose)? { let host = host_version_meta.host(); let toml = toml(&metadata)?; let config = Config::new(toml); @@ -408,11 +408,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, @@ -492,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: &CargoMetadata) -> Result> { +fn toml(metadata: &CargoMetadata) -> Result> { let path = match env::var("CROSS_CONFIG") { Ok(var) => PathBuf::from(var), - Err(_) => root.workspace_root().join("Cross.toml"), + Err(_) => metadata.workspace_root.join("Cross.toml"), }; if path.exists() { @@ -508,7 +506,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 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) diff --git a/src/tests.rs b/src/tests.rs index 565935da2..42f0aadba 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -14,7 +14,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, true) .unwrap() .unwrap() .workspace_root From 12b47276428c5f07000ac2bf40162df6b4934216 Mon Sep 17 00:00:00 2001 From: Alexander Huszagh Date: Tue, 7 Jun 2022 11:15:44 -0500 Subject: [PATCH 4/4] Update workspace tests Ensure we build the entire workspace using the `--workspace` flag both with and without the manifest path, and also ensure we build the binary with all our workspace dependencies. --- ci/test.sh | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/ci/test.sh b/ci/test.sh index 137830a60..623d5fe15 100755 --- a/ci/test.sh +++ b/ci/test.sh @@ -25,6 +25,13 @@ function retry { return ${exit_code} } +workspace_test() { + cross build --target "${TARGET}" --workspace "${@}" + cross run --target "${TARGET}" -p binary "${@}" + cross run --target "${TARGET}" --bin dependencies \ + --features=dependencies "${@}" +} + main() { local td= @@ -142,9 +149,9 @@ EOF https://github.com/cross-rs/test-workspace "${td}" pushd "${td}" - cross run --target "${TARGET}" -p binary --manifest-path="./workspace/Cargo.toml" + TARGET="${TARGET}" workspace_test --manifest-path="./workspace/Cargo.toml" pushd "workspace" - cross run --target "${TARGET}" -p binary + TARGET="${TARGET}" workspace_test pushd "binary" cross run --target "${TARGET}" popd