From 16afe1a2343de58c29480ab0d90490b58eee00d8 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 6 Sep 2020 19:28:58 +0200 Subject: [PATCH] towards letting cargo do binary selection: wrappers and runners set up --- cargo-miri/Cargo.lock | 24 +-- cargo-miri/Cargo.toml | 1 - cargo-miri/bin.rs | 334 +++++++++++++----------------------------- 3 files changed, 104 insertions(+), 255 deletions(-) diff --git a/cargo-miri/Cargo.lock b/cargo-miri/Cargo.lock index 0052bfa183d..bb3b05db03a 100644 --- a/cargo-miri/Cargo.lock +++ b/cargo-miri/Cargo.lock @@ -45,7 +45,6 @@ dependencies = [ name = "cargo-miri" version = "0.1.0" dependencies = [ - "cargo_metadata", "directories", "rustc-workspace-hack", "rustc_version", @@ -54,17 +53,6 @@ dependencies = [ "vergen", ] -[[package]] -name = "cargo_metadata" -version = "0.11.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "89fec17b16f1ac67908af82e47d0a90a7afd0e1827b181cd77504323d3263d35" -dependencies = [ - "semver 0.10.0", - "serde", - "serde_json", -] - [[package]] name = "cfg-if" version = "0.1.10" @@ -228,7 +216,7 @@ version = "0.2.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "138e3e0acb6c9fb258b19b67cb8abd63c00679d2851805ea151465464fe9030a" dependencies = [ - "semver 0.9.0", + "semver", ] [[package]] @@ -246,16 +234,6 @@ dependencies = [ "semver-parser", ] -[[package]] -name = "semver" -version = "0.10.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "394cec28fa623e00903caf7ba4fa6fb9a0e260280bb8cdbbba029611108a0190" -dependencies = [ - "semver-parser", - "serde", -] - [[package]] name = "semver-parser" version = "0.7.0" diff --git a/cargo-miri/Cargo.toml b/cargo-miri/Cargo.toml index 91c47836948..2de581c1c2e 100644 --- a/cargo-miri/Cargo.toml +++ b/cargo-miri/Cargo.toml @@ -14,7 +14,6 @@ test = false # we have no unit tests doctest = false # and no doc tests [dependencies] -cargo_metadata = "0.11" directories = "2.0" rustc_version = "0.2.3" serde_json = "1.0.40" diff --git a/cargo-miri/bin.rs b/cargo-miri/bin.rs index 12c5d1c3255..f3a2a511517 100644 --- a/cargo-miri/bin.rs +++ b/cargo-miri/bin.rs @@ -116,50 +116,6 @@ fn xargo_check() -> Command { Command::new(env::var_os("XARGO_CHECK").unwrap_or_else(|| OsString::from("xargo-check"))) } -fn list_targets() -> impl Iterator { - // 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()); - - let mut cmd = cargo_metadata::MetadataCommand::new(); - if let Some(manifest_path) = &manifest_path { - cmd.manifest_path(manifest_path); - } - let mut metadata = if let Ok(metadata) = cmd.exec() { - metadata - } else { - show_error(format!("Could not obtain Cargo metadata; likely an ill-formed manifest")); - }; - - let current_dir = std::env::current_dir(); - - let package_index = metadata - .packages - .iter() - .position(|package| { - let package_manifest_path = Path::new(&package.manifest_path); - if let Some(manifest_path) = &manifest_path { - package_manifest_path == manifest_path - } else { - let current_dir = current_dir.as_ref().expect("could not read current directory"); - let package_manifest_directory = package_manifest_path - .parent() - .expect("could not find parent directory of package manifest"); - package_manifest_directory == current_dir - } - }) - .unwrap_or_else(|| { - show_error(format!( - "this seems to be a workspace, which is not supported by `cargo miri`.\n\ - Try to `cd` into the crate you want to test, and re-run `cargo miri` there." - )) - }); - let package = metadata.packages.remove(package_index); - - // Finally we got the list of targets to build - package.targets.into_iter() -} - fn xargo_version() -> Option<(u32, u32, u32)> { let out = xargo_check().arg("--version").output().ok()?; if !out.status.success() { @@ -381,173 +337,77 @@ path = "lib.rs" } } -enum CargoTargets { - All, - Filtered { lib: bool, bin: Vec, test: Vec }, -} - -impl CargoTargets { - fn matches(&self, kind: &str, name: &str) -> bool { - match self { - CargoTargets::All => true, - CargoTargets::Filtered { lib, bin, test } => match kind { - "lib" => *lib, - "bin" => bin.iter().any(|n| n == name), - "test" => test.iter().any(|n| n == name), - _ => false, - }, - } - } -} - -fn parse_cargo_miri_args( - mut args: impl Iterator, -) -> (CargoTargets, Vec, Vec) { - let mut lib_present = false; - let mut bin_targets = Vec::new(); - let mut test_targets = Vec::new(); - let mut additional_args = Vec::new(); - while let Some(arg) = args.next() { - match arg { - arg if arg == "--" => { - // Miri arguments begin after the first "--". - break; - } - arg if arg == "--lib" => lib_present = true, - arg if arg == "--bin" => { - if let Some(binary) = args.next() { - if binary == "--" { - show_error(format!("\"--bin\" takes one argument.")); - } else { - bin_targets.push(binary) - } - } else { - show_error(format!("\"--bin\" takes one argument.")); - } - } - arg if arg.starts_with("--bin=") => bin_targets.push((&arg["--bin=".len()..]).to_string()), - arg if arg == "--test" => { - if let Some(test) = args.next() { - if test == "--" { - show_error(format!("\"--test\" takes one argument.")); - } else { - test_targets.push(test) - } - } else { - show_error(format!("\"--test\" takes one argument.")); - } - } - arg if arg.starts_with("--test=") => test_targets.push((&arg["--test=".len()..]).to_string()), - other => additional_args.push(other), - } - } - let targets = if !lib_present && bin_targets.len() == 0 && test_targets.len() == 0 { - CargoTargets::All - } else { - CargoTargets::Filtered { lib: lib_present, bin: bin_targets, test: test_targets } - }; - (targets, additional_args, args.collect()) -} - -fn in_cargo_miri() { - let (subcommand, skip) = match std::env::args().nth(2).as_deref() { - Some("test") => (MiriCommand::Test, 3), - Some("run") => (MiriCommand::Run, 3), - Some("setup") => (MiriCommand::Setup, 3), - // Default command, if there is an option or nothing. - Some(s) if s.starts_with("-") => (MiriCommand::Run, 2), - None => (MiriCommand::Run, 2), +fn phase_cargo_miri(mut args: env::Args) { + // Require a subcommand before any flags. + // We cannot know which of those flags take arguments and which do not, + // so we cannot detect subcommands later. + let subcommand = match args.next().as_deref() { + Some("test") => MiriCommand::Test, + Some("run") => MiriCommand::Run, + Some("setup") => MiriCommand::Setup, // Invalid command. - Some(s) => show_error(format!("Unknown command `{}`", s)), + None => show_error(format!("`cargo miri` must be immediately followed by `test`, `run`, or `setup`.")), + Some(s) => show_error(format!("unknown command `{}`", s)), }; let verbose = has_arg_flag("-v"); // We always setup. setup(subcommand); - if subcommand == MiriCommand::Setup { - // Stop here. - return; + + // Invoke actual cargo for the job, but with different flags. + let miri_path = std::env::current_exe().expect("current executable path invalid"); + let cargo_cmd = match subcommand { + MiriCommand::Test => "test", + MiriCommand::Run => "run", + MiriCommand::Setup => return, // `cargo miri setup` stops here. + }; + let mut cmd = cargo(); + cmd.arg(cargo_cmd); + + // Make sure we know the build target, and cargo does, too. + // This is needed to make the `CARGO_TARGET_*_RUNNER` env var do something, + // and it later helps us detect which crates are proc-macro/build-script + // (host crates) and which crates are needed for the program itself. + let target = if let Some(target) = get_arg_flag_value("--target") { + target + } else { + // No target given. Pick default and tell cargo about it. + let host = version_info().host; + cmd.arg("--target"); + cmd.arg(&host); + host + }; + + // Forward all further arguments. + cmd.args(args); + + // Set `RUSTC_WRAPPER` to ourselves. Cargo will prepend that binary to its usual invocation, + // i.e., the first argument is `rustc` -- which is what we use in `main` to distinguish + // the two codepaths. (That extra argument is why we prefer this over setting `RUSTC`.) + if env::var_os("RUSTC_WRAPPER").is_some() { + println!("WARNING: Ignoring `RUSTC_WRAPPER` environment variable, Miri does not support wrapping."); + } + cmd.env("RUSTC_WRAPPER", &miri_path); + if verbose { + eprintln!("+ RUSTC_WRAPPER={:?}", miri_path); } - // FIXME: this accepts --test, --lib, and multiple --bin for `cargo miri run`. - let (target_filters, cargo_args, miri_args) = - parse_cargo_miri_args(std::env::args().skip(skip)); + // Set the runner for the current target to us as well, so we can interpret the binaries. + let runner_env_name = format!("CARGO_TARGET_{}_RUNNER", target.to_uppercase().replace('-', "_")); + cmd.env(runner_env_name, &miri_path); - // Now run the command. - for target in list_targets() { - let kind = target - .kind - .get(0) - .expect("badly formatted cargo metadata: target::kind is an empty array"); - if !target_filters.matches(kind, &target.name) { - continue; - } - // Now we run `cargo check $FLAGS $ARGS`, giving the user the - // 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("check"); - match (subcommand, kind.as_str()) { - (MiriCommand::Run, "bin") => { - // FIXME: we default to running all binaries here. - cmd.arg("--bin").arg(target.name); - } - (MiriCommand::Test, "test") => { - cmd.arg("--test").arg(target.name); - } - (MiriCommand::Test, "lib") => { - // There can be only one lib. - cmd.arg("--lib").arg("--profile").arg("test"); - } - (MiriCommand::Test, "bin") => { - cmd.arg("--bin").arg(target.name).arg("--profile").arg("test"); - } - // The remaining targets we do not even want to build. - _ => continue, - } - // Forward further `cargo` args. - for arg in cargo_args.iter() { - cmd.arg(arg); - } - // We want to always run `cargo` with `--target`. This later helps us detect - // which crates are proc-macro/build-script (host crates) and which crates are - // needed for the program itself. - if get_arg_flag_value("--target").is_none() { - // When no `--target` is given, default to the host. - cmd.arg("--target"); - cmd.arg(version_info().host); - } - - // Serialize the remaining args into a special environemt variable. - // This will be read by `inside_cargo_rustc` when we go to invoke - // our actual target crate (the binary or the test we are running). - // Since we're using "cargo check", we have no other way of passing - // these arguments. - cmd.env("MIRI_ARGS", serde_json::to_string(&miri_args).expect("failed to serialize args")); - - // Set `RUSTC_WRAPPER` to ourselves. Cargo will prepend that binary to its usual invocation, - // i.e., the first argument is `rustc` -- which is what we use in `main` to distinguish - // the two codepaths. (That extra argument is why we prefer this over setting `RUSTC`.) - if env::var_os("RUSTC_WRAPPER").is_some() { - println!("WARNING: Ignoring existing `RUSTC_WRAPPER` environment variable, Miri does not support wrapping."); - } - let path = std::env::current_exe().expect("current executable path invalid"); - cmd.env("RUSTC_WRAPPER", path); - if verbose { - cmd.env("MIRI_VERBOSE", ""); // this makes `inside_cargo_rustc` verbose. - eprintln!("+ {:?}", cmd); - } - - let exit_status = - cmd.spawn().expect("could not run cargo").wait().expect("failed to wait for cargo?"); - - if !exit_status.success() { - std::process::exit(exit_status.code().unwrap_or(-1)) - } + // Run cargo. + if verbose { + cmd.env("MIRI_VERBOSE", ""); // this makes `inside_cargo_rustc` verbose. + eprintln!("+ {:?}", cmd); } + let exit_status = + cmd.spawn().expect("could not run cargo").wait().expect("failed to wait for cargo?"); + + std::process::exit(exit_status.code().unwrap_or(-1)) } -fn inside_cargo_rustc() { +fn phase_cargo_rustc(mut args: env::Args) { /// Determines if we are being invoked (as rustc) to build a crate for /// the "target" architecture, in contrast to the "host" architecture. /// Host crates are for build scripts and proc macros and still need to @@ -569,15 +429,35 @@ fn inside_cargo_rustc() { fn is_runnable_crate() -> bool { let is_bin = get_arg_flag_value("--crate-type").as_deref() == Some("bin"); let is_test = has_arg_flag("--test"); - is_bin || is_test + let print = get_arg_flag_value("--print").is_some(); + (is_bin || is_test) && !print } let verbose = std::env::var_os("MIRI_VERBOSE").is_some(); let target_crate = is_target_crate(); + if target_crate && is_runnable_crate() { + // This is the binary or test crate that we want to interpret under Miri. + // But we cannot run it here, as cargo invoked us as a compiler -- our stdin and stdout are not + // like we want them. + // Instead of compiling, we write JSON into the output file with all the relevant command-line flags + // and environment variables; this is sued alter when cargo calls us again in the CARGO_TARGET_RUNNER phase. + let filename = format!( + "{}/{}{}", + get_arg_flag_value("--out-dir").unwrap(), + get_arg_flag_value("--crate-name").unwrap(), + // This is technically a `-C` flag but the prefix seems unique enough... + // (and cargo passes this before the filename so it should be unique) + get_arg_flag_value("extra-filename").unwrap_or(String::new()), + ); + eprintln!("Miri is supposed to run {}", filename); + return; + } + let mut cmd = miri(); // Forward arguments. - cmd.args(std::env::args().skip(2)); // skip `cargo-miri rustc` + cmd.args(args); + // FIXME: Make the build check-only! // We make sure to only specify our custom Xargo sysroot for target crates - that is, // crates which are needed for interpretation by Miri. proc-macros and build scripts @@ -589,23 +469,9 @@ fn inside_cargo_rustc() { cmd.arg(sysroot); } - // If this is a runnable target crate, we want Miri to start interpretation; - // otherwise we want Miri to behave like rustc and build the crate as usual. - if target_crate && is_runnable_crate() { - // This is the binary or test crate that we want to interpret under Miri. - // (Testing `target_crate` is needed to exclude build scripts.) - // We deserialize the arguments that are meant for Miri from the special environment - // variable "MIRI_ARGS", and feed them to the 'miri' binary. - // - // `env::var` is okay here, well-formed JSON is always UTF-8. - let magic = std::env::var("MIRI_ARGS").expect("missing MIRI_ARGS"); - let miri_args: Vec = - serde_json::from_str(&magic).expect("failed to deserialize MIRI_ARGS"); - cmd.args(miri_args); - } else { - // We want to compile, not interpret. - cmd.env("MIRI_BE_RUSTC", "1"); - }; + // We want to compile, not interpret. We still use Miri to make sure the compiler version etc + // are the exact same as what is used for interpretation. + cmd.env("MIRI_BE_RUSTC", "1"); // Run it. if verbose { @@ -620,6 +486,10 @@ fn inside_cargo_rustc() { } } +fn phase_cargo_runner(binary: &str, args: env::Args) { + eprintln!("Asked to execute {}, args: {:?}", binary, args.collect::>()); +} + fn main() { // Check for version and help flags even when invoked as `cargo-miri`. if has_arg_flag("--help") || has_arg_flag("-h") { @@ -631,18 +501,20 @@ fn main() { return; } - if let Some("miri") = std::env::args().nth(1).as_deref() { - // This arm is for when `cargo miri` is called. We call `cargo check` for each applicable target, - // but with the `RUSTC` env var set to the `cargo-miri` binary so that we come back in the other branch, - // and dispatch the invocations to `rustc` and `miri`, respectively. - in_cargo_miri(); - } else if let Some("rustc") = std::env::args().nth(1).as_deref() { - // This arm is executed when `cargo-miri` runs `cargo check` with the `RUSTC_WRAPPER` env var set to itself: - // dependencies get dispatched to `rustc`, the final test/binary to `miri`. - inside_cargo_rustc(); - } else { - show_error(format!( - "`cargo-miri` must be called with either `miri` or `rustc` as first argument." - )) + let mut args = std::env::args(); + // Skip binary name. + args.next().unwrap(); + + // Dispatch to `cargo-miri` phase. There are three phases: + // - When we are called via `cargo miri`, we run as the frontend and invoke the underlying + // cargo. We set RUSTC_WRAPPER and CARGO_TARGET_RUNNER to ourselves. + // - When we are executed due to RUSTC_WRAPPER, we build crates or store the flags of + // binary crates for later interpretation. + // - When we are executed due to CARGO_TARGET_RUNNER, we start interpretation based on the + // flags that were stored earlier. + match &*args.next().unwrap() { + "miri" => phase_cargo_miri(args), + "rustc" => phase_cargo_rustc(args), + binary => phase_cargo_runner(binary, args), } }