Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Parse Cargo's --manifest-path option to determine mounted docker root #494

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
- Change rust edition to 2021 and bump MSRV for the cross binary to 1.58.1
- #654 - Use color-eyre for error reporting
- #658 - Upgrade dependencies
Expand Down
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,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)

Expand Down
41 changes: 25 additions & 16 deletions src/cargo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,26 +64,35 @@ impl Root {
}

/// Cargo project root
pub fn root() -> Result<Option<Root>> {
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(),
}));
pub fn root(manifest_path: Option<PathBuf>) -> Result<Option<Root>> {
if let Some(manifest_path) = manifest_path {
if !manifest_path.is_file() {
eyre::bail!("No manifest found at \"{}\"", manifest_path.display());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should be consistent about how we present paths

we currently have three ways of doing it

format!("{}", path) format!("`{}`", path) and format!(r#""{}""#, path)

don't know which one is best and it's not a blocker for this pr

}
return Ok(Some(Root {
path: manifest_path
.parent()
.expect("File must have a parent")
.to_owned(),
}));
} else {
let cd = env::current_dir().wrap_err("couldn't get current dir")?;
let mut dir = &*cd;
loop {
let toml = dir.join("Cargo.toml");

match dir.parent() {
Some(p) => dir = p,
None => break,
if fs::metadata(&toml).is_ok() {
return Ok(Some(Root {
path: dir.to_owned(),
}));
}
if let Some(p) = dir.parent() {
dir = p;
} else {
break Ok(None);
}
}
}

Ok(None)
}

/// Pass-through mode
Expand Down
19 changes: 18 additions & 1 deletion src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,13 @@ pub struct Args {
pub target: Option<Target>,
pub target_dir: Option<PathBuf>,
pub docker_in_docker: bool,
pub manifest_path: Option<PathBuf>,
}

pub fn parse(target_list: &TargetList) -> Args {
let mut channel = None;
let mut target = None;
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 @@ -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);
Expand Down Expand Up @@ -73,5 +89,6 @@ pub fn parse(target_list: &TargetList) -> Args {
target,
target_dir,
docker_in_docker,
manifest_path,
}
}
3 changes: 2 additions & 1 deletion src/docker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ pub fn run(
args: &[String],
target_dir: &Option<PathBuf>,
root: &Root,
docker_root: &Path,
config: &Config,
uses_xargo: bool,
sysroot: &Path,
Expand Down Expand Up @@ -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 mount_root = mount_finder.find_mount_path(root);
let mount_root = mount_finder.find_mount_path(docker_root);
let sysroot = mount_finder.find_mount_path(sysroot);

let mut cmd = if uses_xargo {
Expand Down
4 changes: 3 additions & 1 deletion src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ fn run() -> Result<ExitStatus> {

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(args.manifest_path)? {
let host = version_meta.host();

if host.is_supported(args.target.as_ref()) {
Expand Down Expand Up @@ -358,11 +358,13 @@ fn run() -> Result<ExitStatus> {
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,
Expand Down