From 29b2fad82f4fe82d8ae575a89ac8b617ba8e1366 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Wed, 1 Jan 2020 04:12:27 -0500 Subject: [PATCH] Use 'cargo check' to build the sysroot and target crate Fixes #1057 I'm using my original approach from PR #1048. Ideally, we would distinguish between build-deps/dependencies/'final crate' via a different approach (e.g. the target directory). However, I haven't been able to get that to work just yet. However, everything should be working with the approach I'm using. At a minimum, we can use this PR to verify that everything works as expected when we don't actually produce native build outputs. --- Cargo.lock | 11 +++--- Cargo.toml | 1 + src/bin/cargo-miri.rs | 82 ++++++++++++++++++++++++++++++++----------- 3 files changed, 68 insertions(+), 26 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9550b2bfdf..01c3a9a3c8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -96,7 +96,7 @@ dependencies = [ "semver 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)", "serde 1.0.103 (registry+https://github.com/rust-lang/crates.io-index)", "serde_derive 1.0.103 (registry+https://github.com/rust-lang/crates.io-index)", - "serde_json 1.0.42 (registry+https://github.com/rust-lang/crates.io-index)", + "serde_json 1.0.44 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -152,7 +152,7 @@ dependencies = [ "rustfix 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)", "serde 1.0.103 (registry+https://github.com/rust-lang/crates.io-index)", "serde_derive 1.0.103 (registry+https://github.com/rust-lang/crates.io-index)", - "serde_json 1.0.42 (registry+https://github.com/rust-lang/crates.io-index)", + "serde_json 1.0.44 (registry+https://github.com/rust-lang/crates.io-index)", "tempfile 3.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "tester 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)", "winapi 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)", @@ -340,6 +340,7 @@ dependencies = [ "rustc-workspace-hack 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", "rustc_version 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)", "serde 1.0.103 (registry+https://github.com/rust-lang/crates.io-index)", + "serde_json 1.0.44 (registry+https://github.com/rust-lang/crates.io-index)", "shell-escape 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)", "vergen 3.0.4 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -534,7 +535,7 @@ dependencies = [ "failure 0.1.6 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)", "serde 1.0.103 (registry+https://github.com/rust-lang/crates.io-index)", - "serde_json 1.0.42 (registry+https://github.com/rust-lang/crates.io-index)", + "serde_json 1.0.44 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -576,7 +577,7 @@ dependencies = [ [[package]] name = "serde_json" -version = "1.0.42" +version = "1.0.44" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "itoa 0.4.4 (registry+https://github.com/rust-lang/crates.io-index)", @@ -810,7 +811,7 @@ dependencies = [ "checksum semver-parser 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)" = "388a1df253eca08550bef6c72392cfe7c30914bf41df5269b68cbd6ff8f570a3" "checksum serde 1.0.103 (registry+https://github.com/rust-lang/crates.io-index)" = "1217f97ab8e8904b57dd22eb61cde455fa7446a9c1cf43966066da047c1f3702" "checksum serde_derive 1.0.103 (registry+https://github.com/rust-lang/crates.io-index)" = "a8c6faef9a2e64b0064f48570289b4bf8823b7581f1d6157c1b52152306651d0" -"checksum serde_json 1.0.42 (registry+https://github.com/rust-lang/crates.io-index)" = "1a3351dcbc1f067e2c92ab7c3c1f288ad1a4cffc470b5aaddb4c2e0a3ae80043" +"checksum serde_json 1.0.44 (registry+https://github.com/rust-lang/crates.io-index)" = "48c575e0cc52bdd09b47f330f646cf59afc586e9c4e3ccd6fc1f625b8ea1dad7" "checksum shell-escape 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)" = "170a13e64f2a51b77a45702ba77287f5c6829375b04a69cf2222acd17d0cfab9" "checksum socket2 0.3.11 (registry+https://github.com/rust-lang/crates.io-index)" = "e8b74de517221a2cb01a53349cf54182acdc31a074727d3079068448c0676d85" "checksum syn 1.0.8 (registry+https://github.com/rust-lang/crates.io-index)" = "661641ea2aa15845cddeb97dad000d22070bb5c1fb456b96c1cba883ec691e92" diff --git a/Cargo.toml b/Cargo.toml index da9481cce5..236905b895 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -50,6 +50,7 @@ rustc-workspace-hack = "1.0.0" # between "cargo build" and "cargo intall". num-traits = "*" serde = { version = "*", features = ["derive"] } +serde_json = "1.0.44" [build-dependencies] vergen = "3" diff --git a/src/bin/cargo-miri.rs b/src/bin/cargo-miri.rs index 9c6eab1ec7..7c286b49ad 100644 --- a/src/bin/cargo-miri.rs +++ b/src/bin/cargo-miri.rs @@ -6,7 +6,7 @@ use std::ops::Not; use std::path::{Path, PathBuf}; use std::process::Command; -const XARGO_MIN_VERSION: (u32, u32, u32) = (0, 3, 17); +const XARGO_MIN_VERSION: (u32, u32, u32) = (0, 3, 19); const CARGO_MIRI_HELP: &str = r#"Interprets bin crates and tests in Miri @@ -84,7 +84,37 @@ fn get_arg_flag_value(name: &str) -> Option { } } -fn list_targets() -> impl Iterator { +fn is_build_dep(mut args: impl Iterator) -> bool { + args.any(|arg| arg.starts_with("--emit=") && arg.contains("link")) +} + +// Returns whether or not Cargo invoked the wrapper (this binary) to compile +// the final, target crate (either a test for 'cargo test', or a binary for 'cargo run') +// Right now, this is an awful hack that checks several different pieces of information +// to try to figure out if the crate being compiled is the right one. +// Ideally, Cargo would set en environment variable indicating whether or +// not the wrapper is being invoked on the target crate. +// For now, this is the best we can do +fn is_target_crate(is_build_script: bool) -> bool { + // Cargo sets this to the directory containing the manifest of the crate + // the wrapper is being invoekd to compile. This should be unique + // across the entire build (except for build scripts, which we handle below). + // We cannot check the crate name, since this may not be unique + // (e.g. if the build contains multiple versions of the same crate, + // or the same crate from multiple sources) + let manifest_dir = std::env::var("CARGO_MANIFEST_DIR").ok(); + + // The manifest directory for our target crate. This is set by `cargo-miri` + // (the original invoation by the user) by using `cargo_metadata` to locate + // the manifest. + let expected_dir = std::env::var("MIRI_MAGIC_DIR").expect("MIRI_MAGIC_DIR not set!"); + + manifest_dir == Some(expected_dir) && !is_build_script + +} + + +fn read_cargo_metadata() -> (impl Iterator, String) { // We need to get the manifest, and then the metadata, to enumerate targets. let manifest_path = get_arg_flag_value("--manifest-path").map(|m| Path::new(&m).canonicalize().unwrap()); @@ -124,7 +154,7 @@ fn list_targets() -> impl Iterator { let package = metadata.packages.remove(package_index); // Finally we got the list of targets to build - package.targets.into_iter() + (package.targets.into_iter(), metadata.workspace_root.to_string_lossy().to_string()) } /// Returns the path to the `miri` binary @@ -197,7 +227,7 @@ fn xargo() -> Command { // Bootstrap tells us where to find xargo Command::new(val) } else { - Command::new("xargo") + Command::new("xargo-check") } } @@ -458,8 +488,10 @@ fn in_cargo_miri() { return; } + let (targets, root_dir) = read_cargo_metadata(); + // Now run the command. - for target in list_targets() { + for target in targets { let mut args = std::env::args().skip(skip); let kind = target .kind @@ -469,7 +501,8 @@ fn in_cargo_miri() { // change to add additional arguments. `FLAGS` is set to identify // this target. The user gets to control what gets actually passed to Miri. let mut cmd = cargo(); - cmd.arg("rustc"); + cmd.arg("check"); + cmd.arg("--target").arg("x86_64-unknown-linux-gnu"); match (subcommand, kind.as_str()) { (MiriCommand::Run, "bin") => { // FIXME: we just run all the binaries here. @@ -496,10 +529,15 @@ fn in_cargo_miri() { } cmd.arg(arg); } + + let args_vec: Vec = args.collect(); + cmd.env("MIRI_MAGIC_DIR", root_dir.clone()); + cmd.env("MIRI_MAGIC_ARGS", serde_json::to_string(&args_vec).expect("failed to serialize args")); + // Add `--` (to end the `cargo` flags), and then the user flags. We add markers around the // user flags to be able to identify them later. "cargo rustc" adds more stuff after this, // so we have to mark both the beginning and the end. - cmd.arg("--").arg("cargo-miri-marker-begin").args(args).arg("cargo-miri-marker-end"); + //cmd.arg("--").arg("cargo-miri-marker-begin").args(args).arg("cargo-miri-marker-end"); let path = std::env::current_exe().expect("current executable path invalid"); cmd.env("RUSTC_WRAPPER", path); if verbose { @@ -519,25 +557,27 @@ fn inside_cargo_rustc() { let sysroot = std::env::var("MIRI_SYSROOT").expect("The wrapper should have set MIRI_SYSROOT"); let rustc_args = std::env::args().skip(2); // skip `cargo rustc` - let mut args: Vec = - rustc_args.chain(Some("--sysroot".to_owned())).chain(Some(sysroot)).collect(); - args.splice(0..0, miri::miri_default_args().iter().map(ToString::to_string)); + + let in_build_script = is_build_dep(std::env::args().skip(2)); + + let mut args = if in_build_script { + rustc_args.collect() + } else { + let mut args: Vec = rustc_args + .chain(Some("--sysroot".to_owned())) + .chain(Some(sysroot)) + .collect(); + args.splice(0..0, miri::miri_default_args().iter().map(ToString::to_string)); + args + }; // See if we can find the `cargo-miri` markers. Those only get added to the binary we want to // run. They also serve to mark the user-defined arguments, which we have to move all the way // to the end (they get added somewhere in the middle). let needs_miri = - if let Some(begin) = args.iter().position(|arg| arg == "cargo-miri-marker-begin") { - let end = args - .iter() - .position(|arg| arg == "cargo-miri-marker-end") - .expect("cannot find end marker"); - // These mark the user arguments. We remove the first and last as they are the markers. - let mut user_args = args.drain(begin..=end); - assert_eq!(user_args.next().unwrap(), "cargo-miri-marker-begin"); - assert_eq!(user_args.next_back().unwrap(), "cargo-miri-marker-end"); - // Collect the rest and add it back at the end. - let mut user_args = user_args.collect::>(); + if is_target_crate(in_build_script) { + let magic = std::env::var("MIRI_MAGIC_ARGS").expect("missing MIRI_MAGIC_ARGS"); + let mut user_args: Vec = serde_json::from_str(&magic).expect("failed to deserialize args"); args.append(&mut user_args); // Run this in Miri. true