Auto merge of #2474 - RalfJung:cargo-metadata-args, r=RalfJung

also forward --manifest-path to 'cargo metadata'

and then I went on to refactor the argument flag splitting thing a bit, and, uh, that snowballed...
This commit is contained in:
bors 2022-08-07 02:49:33 +00:00
commit a522442521
6 changed files with 171 additions and 101 deletions

134
cargo-miri/src/arg.rs Normal file
View File

@ -0,0 +1,134 @@
//! Utilities for dealing with argument flags
use std::borrow::Cow;
use std::env;
/// Determines whether a `--flag` is present.
pub fn has_arg_flag(name: &str) -> bool {
num_arg_flag(name) > 0
}
/// Determines how many times a `--flag` is present.
pub fn num_arg_flag(name: &str) -> usize {
env::args().take_while(|val| val != "--").filter(|val| val == name).count()
}
/// Yields all values of command line flag `name` as `Ok(arg)`, and all other arguments except
/// the flag as `Err(arg)`. (The flag `name` itself is not yielded at all, only its values are.)
pub struct ArgSplitFlagValue<'a, I> {
args: Option<I>,
name: &'a str,
}
impl<'a, I: Iterator> ArgSplitFlagValue<'a, I> {
fn new(args: I, name: &'a str) -> Self {
Self { args: Some(args), name }
}
}
impl<'s, I: Iterator<Item = Cow<'s, str>>> Iterator for ArgSplitFlagValue<'_, I> {
// If the original iterator was all `Owned`, then we will only ever yield `Owned`
// (so `into_owned()` is cheap).
type Item = Result<Cow<'s, str>, Cow<'s, str>>;
fn next(&mut self) -> Option<Self::Item> {
let Some(args) = self.args.as_mut() else {
// We already canceled this iterator.
return None;
};
let arg = args.next()?;
if arg == "--" {
// Stop searching at `--`.
self.args = None;
return None;
}
// These branches cannot be merged if we want to avoid the allocation in the `Borrowed` branch.
match &arg {
Cow::Borrowed(arg) =>
if let Some(suffix) = arg.strip_prefix(self.name) {
// Strip leading `name`.
if suffix.is_empty() {
// This argument is exactly `name`; the next one is the value.
return args.next().map(Ok);
} else if let Some(suffix) = suffix.strip_prefix('=') {
// This argument is `name=value`; get the value.
return Some(Ok(Cow::Borrowed(suffix)));
}
},
Cow::Owned(arg) =>
if let Some(suffix) = arg.strip_prefix(self.name) {
// Strip leading `name`.
if suffix.is_empty() {
// This argument is exactly `name`; the next one is the value.
return args.next().map(Ok);
} else if let Some(suffix) = suffix.strip_prefix('=') {
// This argument is `name=value`; get the value. We need to do an allocation
// here as a `String` cannot be subsliced (what would the lifetime be?).
return Some(Ok(Cow::Owned(suffix.to_owned())));
}
},
}
Some(Err(arg))
}
}
impl<'a, I: Iterator<Item = String> + 'a> ArgSplitFlagValue<'a, I> {
pub fn from_string_iter(
args: I,
name: &'a str,
) -> impl Iterator<Item = Result<String, String>> + 'a {
ArgSplitFlagValue::new(args.map(Cow::Owned), name).map(|x| {
match x {
Ok(Cow::Owned(s)) => Ok(s),
Err(Cow::Owned(s)) => Err(s),
_ => panic!("iterator converted owned to borrowed"),
}
})
}
}
impl<'x: 'a, 'a, I: Iterator<Item = &'x str> + 'a> ArgSplitFlagValue<'a, I> {
pub fn from_str_iter(
args: I,
name: &'a str,
) -> impl Iterator<Item = Result<&'x str, &'x str>> + 'a {
ArgSplitFlagValue::new(args.map(Cow::Borrowed), name).map(|x| {
match x {
Ok(Cow::Borrowed(s)) => Ok(s),
Err(Cow::Borrowed(s)) => Err(s),
_ => panic!("iterator converted borrowed to owned"),
}
})
}
}
/// Yields all values of command line flag `name`.
pub struct ArgFlagValueIter;
impl ArgFlagValueIter {
pub fn from_string_iter<'a, I: Iterator<Item = String> + 'a>(
args: I,
name: &'a str,
) -> impl Iterator<Item = String> + 'a {
ArgSplitFlagValue::from_string_iter(args, name).filter_map(Result::ok)
}
}
impl ArgFlagValueIter {
pub fn from_str_iter<'x: 'a, 'a, I: Iterator<Item = &'x str> + 'a>(
args: I,
name: &'a str,
) -> impl Iterator<Item = &'x str> + 'a {
ArgSplitFlagValue::from_str_iter(args, name).filter_map(Result::ok)
}
}
/// Gets the values of a `--flag`.
pub fn get_arg_flag_values(name: &str) -> impl Iterator<Item = String> + '_ {
ArgFlagValueIter::from_string_iter(env::args(), name)
}
/// Gets the value of a `--flag`.
pub fn get_arg_flag_value(name: &str) -> Option<String> {
get_arg_flag_values(name).next()
}

View File

@ -1,6 +1,7 @@
#![feature(let_else)]
#![allow(clippy::useless_format, clippy::derive_partial_eq_without_eq)]
mod arg;
mod phases;
mod setup;
mod util;

View File

@ -117,8 +117,9 @@ pub fn phase_cargo_miri(mut args: impl Iterator<Item = String>) {
cmd.arg(cargo_cmd);
// Forward all arguments before `--` other than `--target-dir` and its value to Cargo.
// (We want to *change* the target-dir value, so we must not forward it.)
let mut target_dir = None;
for arg in ArgSplitFlagValue::new(&mut args, "--target-dir") {
for arg in ArgSplitFlagValue::from_string_iter(&mut args, "--target-dir") {
match arg {
Ok(value) => {
if target_dir.is_some() {
@ -309,17 +310,18 @@ pub fn phase_rustc(mut args: impl Iterator<Item = String>, phase: RustcPhase) {
let mut cmd = miri();
// Ensure --emit argument for a check-only build is present.
// We cannot use the usual helpers since we need to check specifically in `env.args`.
if let Some(i) = env.args.iter().position(|arg| arg.starts_with("--emit=")) {
if let Some(val) =
ArgFlagValueIter::from_str_iter(env.args.iter().map(|s| s as &str), "--emit").next()
{
// For `no_run` tests, rustdoc passes a `--emit` flag; make sure it has the right shape.
assert_eq!(env.args[i], "--emit=metadata");
assert_eq!(val, "metadata");
} else {
// For all other kinds of tests, we can just add our flag.
cmd.arg("--emit=metadata");
}
// Alter the `-o` parameter so that it does not overwrite the JSON file we stored above.
let mut args = env.args.clone();
let mut args = env.args;
for i in 0..args.len() {
if args[i] == "-o" {
args[i + 1].push_str(".miri");
@ -343,7 +345,7 @@ pub fn phase_rustc(mut args: impl Iterator<Item = String>, phase: RustcPhase) {
return;
}
if runnable_crate && ArgFlagValueIter::new("--extern").any(|krate| krate == "proc_macro") {
if runnable_crate && get_arg_flag_values("--extern").any(|krate| krate == "proc_macro") {
// This is a "runnable" `proc-macro` crate (unit tests). We do not support
// interpreting that under Miri now, so we write a JSON file to (display a
// helpful message and) skip it in the runner phase.
@ -567,7 +569,7 @@ pub fn phase_rustdoc(mut args: impl Iterator<Item = String>) {
// Doctests of `proc-macro` crates (and their dependencies) are always built for the host,
// so we are not able to run them in Miri.
if ArgFlagValueIter::new("--crate-type").any(|crate_type| crate_type == "proc-macro") {
if get_arg_flag_values("--crate-type").any(|crate_type| crate_type == "proc-macro") {
eprintln!("Running doctests of `proc-macro` crates is not currently supported by Miri.");
return;
}

View File

@ -4,7 +4,6 @@ use std::ffi::OsString;
use std::fmt::Write as _;
use std::fs::{self, File};
use std::io::{self, BufWriter, Read, Write};
use std::iter::TakeWhile;
use std::ops::Not;
use std::path::{Path, PathBuf};
use std::process::Command;
@ -13,6 +12,8 @@ use cargo_metadata::{Metadata, MetadataCommand};
use rustc_version::VersionMeta;
use serde::{Deserialize, Serialize};
pub use crate::arg::*;
/// The information to run a crate with the given environment.
#[derive(Clone, Serialize, Deserialize)]
pub struct CrateRunEnv {
@ -74,78 +75,6 @@ pub fn show_error(msg: String) -> ! {
std::process::exit(1)
}
/// Determines whether a `--flag` is present.
pub fn has_arg_flag(name: &str) -> bool {
num_arg_flag(name) > 0
}
/// Determines how many times a `--flag` is present.
pub fn num_arg_flag(name: &str) -> usize {
std::env::args().take_while(|val| val != "--").filter(|val| val == name).count()
}
/// Yields all values of command line flag `name` as `Ok(arg)`, and all other arguments except
/// the flag as `Err(arg)`. (The flag `name` itself is not yielded at all, only its values are.)
pub struct ArgSplitFlagValue<'a, I> {
args: TakeWhile<I, fn(&String) -> bool>,
name: &'a str,
}
impl<'a, I: Iterator<Item = String>> ArgSplitFlagValue<'a, I> {
pub fn new(args: I, name: &'a str) -> Self {
Self {
// Stop searching at `--`.
args: args.take_while(|val| val != "--"),
name,
}
}
}
impl<I: Iterator<Item = String>> Iterator for ArgSplitFlagValue<'_, I> {
type Item = Result<String, String>;
fn next(&mut self) -> Option<Self::Item> {
let arg = self.args.next()?;
if let Some(suffix) = arg.strip_prefix(self.name) {
// Strip leading `name`.
if suffix.is_empty() {
// This argument is exactly `name`; the next one is the value.
return self.args.next().map(Ok);
} else if let Some(suffix) = suffix.strip_prefix('=') {
// This argument is `name=value`; get the value.
return Some(Ok(suffix.to_owned()));
}
}
Some(Err(arg))
}
}
/// Yields all values of command line flag `name`.
pub struct ArgFlagValueIter<'a>(ArgSplitFlagValue<'a, env::Args>);
impl<'a> ArgFlagValueIter<'a> {
pub fn new(name: &'a str) -> Self {
Self(ArgSplitFlagValue::new(env::args(), name))
}
}
impl Iterator for ArgFlagValueIter<'_> {
type Item = String;
fn next(&mut self) -> Option<Self::Item> {
loop {
if let Ok(value) = self.0.next()? {
return Some(value);
}
}
}
}
/// Gets the value of a `--flag`.
pub fn get_arg_flag_value(name: &str) -> Option<String> {
ArgFlagValueIter::new(name).next()
}
/// Escapes `s` in a way that is suitable for using it as a string literal in TOML syntax.
pub fn escape_for_toml(s: &str) -> String {
// We want to surround this string in quotes `"`. So we first escape all quotes,
@ -287,29 +216,33 @@ pub fn write_to_file(filename: &Path, content: &str) {
fs::rename(temp_filename, filename).unwrap();
}
pub fn get_cargo_metadata() -> Metadata {
// The `build.target-dir` config can be passed by `--config` flags, so forward them to
// `cargo metadata`.
let mut additional_options = Vec::new();
// Computes the extra flags that need to be passed to cargo to make it behave like the current
// cargo invocation.
fn cargo_extra_flags() -> Vec<String> {
let mut flags = Vec::new();
// `-Zunstable-options` is required by `--config`.
additional_options.push("-Zunstable-options".to_string());
flags.push("-Zunstable-options".to_string());
// Forward `--config` flags.
let config_flag = "--config";
for arg in ArgSplitFlagValue::new(
env::args().skip(3), // skip the program name, "miri" and "run" / "test"
config_flag,
)
// Only look at `Ok`
.flatten()
{
additional_options.push(config_flag.to_string());
additional_options.push(arg);
for arg in get_arg_flag_values(config_flag) {
flags.push(config_flag.to_string());
flags.push(arg);
}
let metadata =
MetadataCommand::new().no_deps().other_options(additional_options).exec().unwrap();
// Forward `--manifest-path`.
let manifest_flag = "--manifest-path";
if let Some(manifest) = get_arg_flag_value(manifest_flag) {
flags.push(manifest_flag.to_string());
flags.push(manifest);
}
metadata
flags
}
pub fn get_cargo_metadata() -> Metadata {
// This will honor the `CARGO` env var the same way our `cargo()` does.
MetadataCommand::new().no_deps().other_options(cargo_extra_flags()).exec().unwrap()
}
/// Pulls all the crates in this workspace from the cargo metadata.

4
ci.sh
View File

@ -54,8 +54,8 @@ function run_tests {
unset RUSTC MIRI
rm -rf .cargo
# Ensure that our benchmarks all work, on the host at least.
if [ -z "${MIRI_TEST_TARGET+exists}" ]; then
# Ensure that our benchmarks all work, but only on Linux hosts.
if [ -z "${MIRI_TEST_TARGET+exists}" ] && [ "$HOST_TARGET" = x86_64-unknown-linux-gnu ] ; then
for BENCH in $(ls "bench-cargo-miri"); do
cargo miri run --manifest-path bench-cargo-miri/$BENCH/Cargo.toml
done

4
miri
View File

@ -90,13 +90,13 @@ bench)
# Make sure we have an up-to-date Miri installed
"$0" install
# Run the requested benchmarks
if [ -z "$@" ]; then
if [ -z "${1+exists}" ]; then
BENCHES=( $(ls "$MIRIDIR/bench-cargo-miri" ) )
else
BENCHES=("$@")
fi
for BENCH in "${BENCHES[@]}"; do
hyperfine -w 1 -m 5 --shell=none "cargo +$TOOLCHAIN miri run --manifest-path bench-cargo-miri/$BENCH/Cargo.toml"
hyperfine -w 1 -m 5 --shell=none "cargo +$TOOLCHAIN miri run --manifest-path $MIRIDIR/bench-cargo-miri/$BENCH/Cargo.toml"
done
exit 0
;;