Skip to content

Commit

Permalink
Merge #684
Browse files Browse the repository at this point in the history
684: enable cargo workspace and path dependencies to work seamlessly r=Alexhuszagh a=Emilgardis

This will allow cargo workspaces to work seamlessly with cross, assuming they don't reference anything outside the current workspace root. This change also automatically mounts (if needed) path dependencies.

If they do reference things outside the workspace root, and that fact is not know to `cargo metadata` (e.g, it's not a workspace member or a path dependency), you'll have to mount it with

```toml
[build.env]
volumes = ["THING"]
```

and set `$THING="/path/to/thing"` (we need to work on accessibility for this feature too, a lot of reported issues are solved by it)

I'm sure there is an issue directly related to workspaces, but I can't find any.

fixes #388 

Co-authored-by: Emil Gardström <emil.gardstrom@gmail.com>
Co-authored-by: wngr <oliver@wngr.de>
Co-authored-by: Alexander Huszagh <ahuszagh@gmail.com>
  • Loading branch information
4 people authored Jun 7, 2022
2 parents 6b42263 + 3e59d85 commit 0c1d76d
Show file tree
Hide file tree
Showing 9 changed files with 233 additions and 71 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
6 changes: 0 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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*
Expand Down
23 changes: 23 additions & 0 deletions ci/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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=

Expand Down Expand Up @@ -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

Expand Down
99 changes: 76 additions & 23 deletions src/cargo.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -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<Package>,
pub workspace_members: Vec<String>,
}

impl Root {
pub fn path(&self) -> &Path {
&self.path
impl CargoMetadata {
fn non_workspace_members(&self) -> impl Iterator<Item = &Package> {
self.packages
.iter()
.filter(|p| !self.workspace_members.iter().any(|m| m == &p.id))
}
}

/// Cargo project root
pub fn root() -> Result<Option<Root>> {
let cd = env::current_dir().wrap_err("couldn't get current directory")?;
pub fn path_dependencies(&self) -> impl Iterator<Item = &Path> {
// 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<String>,
}

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<Option<CargoMetadata>> {
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<CargoMetadata> = 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
Expand Down
31 changes: 30 additions & 1 deletion src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@ pub struct Args {
pub subcommand: Option<Subcommand>,
pub channel: Option<String>,
pub target: Option<Target>,
pub features: Vec<String>,
pub target_dir: Option<PathBuf>,
pub docker_in_docker: bool,
pub enable_doctests: bool,
pub manifest_path: Option<PathBuf>,
}

// Fix for issue #581. target_dir must be absolute.
Expand Down Expand Up @@ -72,6 +74,8 @@ pub fn fmt_subcommands(stdout: &str) {
pub fn parse(target_list: &TargetList) -> Result<Args> {
let mut channel = None;
let mut target = None;
let mut features = Vec::new();
let mut manifest_path: Option<PathBuf> = None;
let mut target_dir = None;
let mut sc = None;
let mut all: Vec<String> = Vec::new();
Expand All @@ -82,7 +86,21 @@ pub fn parse(target_list: &TargetList) -> Result<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);
Expand All @@ -95,6 +113,15 @@ pub fn parse(target_list: &TargetList) -> Result<Args> {
.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() {
Expand Down Expand Up @@ -128,8 +155,10 @@ pub fn parse(target_list: &TargetList) -> Result<Args> {
subcommand: sc,
channel,
target,
features,
target_dir,
docker_in_docker,
enable_doctests,
manifest_path,
})
}
Loading

0 comments on commit 0c1d76d

Please sign in to comment.