diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a59ae19b..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 @@ -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 a6adcdb76..35cc9e484 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/ci/test.sh b/ci/test.sh index 9931fdda5..207c95d59 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/cargo.rs b/src/cargo.rs index 9c976afcb..544dabc31 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, + pub workspace_members: Vec, } -impl Root { - pub fn path(&self) -> &Path { - &self.path +impl CargoMetadata { + fn non_workspace_members(&self) -> impl Iterator { + self.packages + .iter() + .filter(|p| !self.workspace_members.iter().any(|m| m == &p.id)) } -} -/// Cargo project root -pub fn root() -> Result> { - let cd = env::current_dir().wrap_err("couldn't get current directory")?; + 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()) + } +} - let mut dir = &*cd; - loop { - let toml = dir.join("Cargo.toml"); +#[derive(Debug, Deserialize)] +pub struct Package { + id: String, + manifest_path: PathBuf, + source: Option, +} - 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> { + 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 = 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 64f78e47f..eeac52420 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -12,9 +12,11 @@ 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, + pub manifest_path: Option, } // 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 { 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; let mut all: Vec = Vec::new(); @@ -82,7 +86,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); @@ -95,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() { @@ -128,8 +155,10 @@ pub fn parse(target_list: &TargetList) -> Result { 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 6f9085696..df6e7520c 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 { + 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, - 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 +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 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 04e24e26d..9bbacca5a 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()? { + 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 { 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> { +fn toml(metadata: &CargoMetadata) -> Result> { 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> { 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) diff --git a/src/tests.rs b/src/tests.rs index 90c4278ba..42f0aadba 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,23 +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(|| { - #[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 }) }