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