Auto merge of #3621 - RalfJung:argparse, r=RalfJung

use a little arg-parsing helper for miri-script
This commit is contained in:
bors 2024-05-19 16:58:51 +00:00
commit a32423c28e
5 changed files with 261 additions and 146 deletions

View file

@ -0,0 +1,136 @@
use std::env;
use std::iter;
use anyhow::{bail, Result};
pub struct Args {
args: iter::Peekable<env::Args>,
/// Set to `true` once we saw a `--`.
terminated: bool,
}
impl Args {
pub fn new() -> Self {
let mut args = Args { args: env::args().peekable(), terminated: false };
args.args.next().unwrap(); // skip program name
args
}
/// Get the next argument without any interpretation.
pub fn next_raw(&mut self) -> Option<String> {
self.args.next()
}
/// Consume a `-$f` flag if present.
pub fn get_short_flag(&mut self, flag: char) -> Result<bool> {
if self.terminated {
return Ok(false);
}
if let Some(next) = self.args.peek() {
if let Some(next) = next.strip_prefix("-") {
if let Some(next) = next.strip_prefix(flag) {
if next.is_empty() {
self.args.next().unwrap(); // consume this argument
return Ok(true);
} else {
bail!("`-{flag}` followed by value");
}
}
}
}
Ok(false)
}
/// Consume a `--$name` flag if present.
pub fn get_long_flag(&mut self, name: &str) -> Result<bool> {
if self.terminated {
return Ok(false);
}
if let Some(next) = self.args.peek() {
if let Some(next) = next.strip_prefix("--") {
if next == name {
self.args.next().unwrap(); // consume this argument
return Ok(true);
}
}
}
Ok(false)
}
/// Consume a `--$name val` or `--$name=val` option if present.
pub fn get_long_opt(&mut self, name: &str) -> Result<Option<String>> {
assert!(!name.is_empty());
if self.terminated {
return Ok(None);
}
let Some(next) = self.args.peek() else { return Ok(None) };
let Some(next) = next.strip_prefix("--") else { return Ok(None) };
let Some(next) = next.strip_prefix(name) else { return Ok(None) };
// Starts with `--flag`.
Ok(if let Some(val) = next.strip_prefix("=") {
// `--flag=val` form
let val = val.into();
self.args.next().unwrap(); // consume this argument
Some(val)
} else if next.is_empty() {
// `--flag val` form
self.args.next().unwrap(); // consume this argument
let Some(val) = self.args.next() else { bail!("`--{name}` not followed by value") };
Some(val)
} else {
// Some unrelated flag, like `--flag-more` or so.
None
})
}
/// Consume a `--$name=val` or `--$name` option if present; the latter
/// produces a default value. (`--$name val` is *not* accepted for this form
/// of argument, it understands `val` already as the next argument!)
pub fn get_long_opt_with_default(
&mut self,
name: &str,
default: &str,
) -> Result<Option<String>> {
assert!(!name.is_empty());
if self.terminated {
return Ok(None);
}
let Some(next) = self.args.peek() else { return Ok(None) };
let Some(next) = next.strip_prefix("--") else { return Ok(None) };
let Some(next) = next.strip_prefix(name) else { return Ok(None) };
// Starts with `--flag`.
Ok(if let Some(val) = next.strip_prefix("=") {
// `--flag=val` form
let val = val.into();
self.args.next().unwrap(); // consume this argument
Some(val)
} else if next.is_empty() {
// `--flag` form
self.args.next().unwrap(); // consume this argument
Some(default.into())
} else {
// Some unrelated flag, like `--flag-more` or so.
None
})
}
/// Returns the next free argument or uninterpreted flag, or `None` if there are no more
/// arguments left. `--` is returned as well, but it is interpreted in the sense that no more
/// flags will be parsed after this.
pub fn get_other(&mut self) -> Option<String> {
if self.terminated {
return self.args.next();
}
let next = self.args.next()?;
if next == "--" {
self.terminated = true; // don't parse any more flags
// This is where our parser is special, we do yield the `--`.
}
Some(next)
}
/// Return the rest of the aguments entirely unparsed.
pub fn remainder(self) -> Vec<String> {
self.args.collect()
}
}

View file

@ -25,7 +25,11 @@ impl MiriEnv {
/// Returns the location of the sysroot.
///
/// If the target is None the sysroot will be built for the host machine.
fn build_miri_sysroot(&mut self, quiet: bool, target: Option<&OsStr>) -> Result<PathBuf> {
fn build_miri_sysroot(
&mut self,
quiet: bool,
target: Option<impl AsRef<OsStr>>,
) -> Result<PathBuf> {
if let Some(miri_sysroot) = self.sh.var_os("MIRI_SYSROOT") {
// Sysroot already set, use that.
return Ok(miri_sysroot.into());
@ -37,14 +41,17 @@ impl MiriEnv {
self.build(path!(self.miri_dir / "Cargo.toml"), &[], quiet)?;
self.build(&manifest_path, &[], quiet)?;
let target_flag =
if let Some(target) = target { vec![OsStr::new("--target"), target] } else { vec![] };
let target_flag = if let Some(target) = &target {
vec![OsStr::new("--target"), target.as_ref()]
} else {
vec![]
};
let target_flag = &target_flag;
if !quiet {
eprint!("$ cargo miri setup");
if let Some(target) = target {
eprint!(" --target {target}", target = target.to_string_lossy());
if let Some(target) = &target {
eprint!(" --target {target}", target = target.as_ref().to_string_lossy());
}
eprintln!();
}
@ -157,8 +164,8 @@ impl Command {
Command::Build { flags } => Self::build(flags),
Command::Check { flags } => Self::check(flags),
Command::Test { bless, flags, target } => Self::test(bless, flags, target),
Command::Run { dep, verbose, many_seeds, flags } =>
Self::run(dep, verbose, many_seeds, flags),
Command::Run { dep, verbose, many_seeds, target, edition, flags } =>
Self::run(dep, verbose, many_seeds, target, edition, flags),
Command::Fmt { flags } => Self::fmt(flags),
Command::Clippy { flags } => Self::clippy(flags),
Command::Cargo { flags } => Self::cargo(flags),
@ -169,7 +176,7 @@ impl Command {
}
}
fn toolchain(flags: Vec<OsString>) -> Result<()> {
fn toolchain(flags: Vec<String>) -> Result<()> {
// Make sure rustup-toolchain-install-master is installed.
which::which("rustup-toolchain-install-master")
.context("Please install rustup-toolchain-install-master by running 'cargo install rustup-toolchain-install-master'")?;
@ -364,7 +371,7 @@ impl Command {
Ok(())
}
fn bench(target: Option<OsString>, benches: Vec<OsString>) -> Result<()> {
fn bench(target: Option<String>, benches: Vec<String>) -> Result<()> {
// The hyperfine to use
let hyperfine = env::var("HYPERFINE");
let hyperfine = hyperfine.as_deref().unwrap_or("hyperfine -w 1 -m 5 --shell=none");
@ -378,14 +385,14 @@ impl Command {
let sh = Shell::new()?;
sh.change_dir(miri_dir()?);
let benches_dir = "bench-cargo-miri";
let benches = if benches.is_empty() {
let benches: Vec<OsString> = if benches.is_empty() {
sh.read_dir(benches_dir)?
.into_iter()
.filter(|path| path.is_dir())
.map(Into::into)
.collect()
} else {
benches.to_owned()
benches.into_iter().map(Into::into).collect()
};
let target_flag = if let Some(target) = target {
let mut flag = OsString::from("--target=");
@ -409,28 +416,28 @@ impl Command {
Ok(())
}
fn install(flags: Vec<OsString>) -> Result<()> {
fn install(flags: Vec<String>) -> Result<()> {
let e = MiriEnv::new()?;
e.install_to_sysroot(e.miri_dir.clone(), &flags)?;
e.install_to_sysroot(path!(e.miri_dir / "cargo-miri"), &flags)?;
Ok(())
}
fn build(flags: Vec<OsString>) -> Result<()> {
fn build(flags: Vec<String>) -> Result<()> {
let e = MiriEnv::new()?;
e.build(path!(e.miri_dir / "Cargo.toml"), &flags, /* quiet */ false)?;
e.build(path!(e.miri_dir / "cargo-miri" / "Cargo.toml"), &flags, /* quiet */ false)?;
Ok(())
}
fn check(flags: Vec<OsString>) -> Result<()> {
fn check(flags: Vec<String>) -> Result<()> {
let e = MiriEnv::new()?;
e.check(path!(e.miri_dir / "Cargo.toml"), &flags)?;
e.check(path!(e.miri_dir / "cargo-miri" / "Cargo.toml"), &flags)?;
Ok(())
}
fn clippy(flags: Vec<OsString>) -> Result<()> {
fn clippy(flags: Vec<String>) -> Result<()> {
let e = MiriEnv::new()?;
e.clippy(path!(e.miri_dir / "Cargo.toml"), &flags)?;
e.clippy(path!(e.miri_dir / "cargo-miri" / "Cargo.toml"), &flags)?;
@ -438,7 +445,7 @@ impl Command {
Ok(())
}
fn cargo(flags: Vec<OsString>) -> Result<()> {
fn cargo(flags: Vec<String>) -> Result<()> {
let e = MiriEnv::new()?;
let toolchain = &e.toolchain;
// We carefully kept the working dir intact, so this will run cargo *on the workspace in the
@ -447,7 +454,7 @@ impl Command {
Ok(())
}
fn test(bless: bool, mut flags: Vec<OsString>, target: Option<OsString>) -> Result<()> {
fn test(bless: bool, mut flags: Vec<String>, target: Option<String>) -> Result<()> {
let mut e = MiriEnv::new()?;
// Prepare a sysroot.
@ -475,21 +482,30 @@ impl Command {
dep: bool,
verbose: bool,
many_seeds: Option<Range<u32>>,
mut flags: Vec<OsString>,
target: Option<String>,
edition: Option<String>,
flags: Vec<String>,
) -> Result<()> {
let mut e = MiriEnv::new()?;
let target = arg_flag_value(&flags, "--target");
// More flags that we will pass before `flags`
// (because `flags` may contain `--`).
let mut early_flags = Vec::<OsString>::new();
// Scan for "--edition", set one ourselves if that flag is not present.
let have_edition = arg_flag_value(&flags, "--edition").is_some();
if !have_edition {
flags.push("--edition=2021".into()); // keep in sync with `tests/ui.rs`.`
// Add target, edition to flags.
if let Some(target) = &target {
early_flags.push("--target".into());
early_flags.push(target.into());
}
if verbose {
early_flags.push("--verbose".into());
}
early_flags.push("--edition".into());
early_flags.push(edition.as_deref().unwrap_or("2021").into());
// Prepare a sysroot, and add it to the flags.
// Prepare a sysroot, add it to the flags.
let miri_sysroot = e.build_miri_sysroot(/* quiet */ !verbose, target.as_deref())?;
flags.push("--sysroot".into());
flags.push(miri_sysroot.into());
early_flags.push("--sysroot".into());
early_flags.push(miri_sysroot.into());
// Compute everything needed to run the actual command. Also add MIRIFLAGS.
let miri_manifest = path!(e.miri_dir / "Cargo.toml");
@ -515,7 +531,7 @@ impl Command {
};
cmd.set_quiet(!verbose);
// Add Miri flags
let cmd = cmd.args(&miri_flags).args(seed_flag).args(&flags);
let cmd = cmd.args(&miri_flags).args(&seed_flag).args(&early_flags).args(&flags);
// And run the thing.
Ok(cmd.run()?)
};
@ -534,7 +550,7 @@ impl Command {
Ok(())
}
fn fmt(flags: Vec<OsString>) -> Result<()> {
fn fmt(flags: Vec<String>) -> Result<()> {
use itertools::Itertools;
let e = MiriEnv::new()?;
@ -556,6 +572,6 @@ impl Command {
.filter_ok(|item| item.file_type().is_file())
.map_ok(|item| item.into_path());
e.format_files(files, &e.toolchain[..], &config_path, &flags[..])
e.format_files(files, &e.toolchain[..], &config_path, &flags)
}
}

View file

@ -1,10 +1,10 @@
#![allow(clippy::needless_question_mark)]
mod args;
mod commands;
mod util;
use std::ffi::OsString;
use std::{env, ops::Range};
use std::ops::Range;
use anyhow::{anyhow, bail, Context, Result};
@ -16,26 +16,26 @@ pub enum Command {
/// sysroot, to prevent conflicts with other toolchains.
Install {
/// Flags that are passed through to `cargo install`.
flags: Vec<OsString>,
flags: Vec<String>,
},
/// Just build miri.
Build {
/// Flags that are passed through to `cargo build`.
flags: Vec<OsString>,
flags: Vec<String>,
},
/// Just check miri.
Check {
/// Flags that are passed through to `cargo check`.
flags: Vec<OsString>,
flags: Vec<String>,
},
/// Build miri, set up a sysroot and then run the test suite.
Test {
bless: bool,
/// The cross-interpretation target.
/// If none then the host is the target.
target: Option<OsString>,
target: Option<String>,
/// Flags that are passed through to the test harness.
flags: Vec<OsString>,
flags: Vec<String>,
},
/// Build miri, set up a sysroot and then run the driver with the given <flags>.
/// (Also respects MIRIFLAGS environment variable.)
@ -43,33 +43,35 @@ pub enum Command {
dep: bool,
verbose: bool,
many_seeds: Option<Range<u32>>,
target: Option<String>,
edition: Option<String>,
/// Flags that are passed through to `miri`.
flags: Vec<OsString>,
flags: Vec<String>,
},
/// Format all sources and tests.
Fmt {
/// Flags that are passed through to `rustfmt`.
flags: Vec<OsString>,
flags: Vec<String>,
},
/// Runs clippy on all sources.
Clippy {
/// Flags that are passed through to `cargo clippy`.
flags: Vec<OsString>,
flags: Vec<String>,
},
/// Runs just `cargo <flags>` with the Miri-specific environment variables.
/// Mainly meant to be invoked by rust-analyzer.
Cargo { flags: Vec<OsString> },
Cargo { flags: Vec<String> },
/// Runs the benchmarks from bench-cargo-miri in hyperfine. hyperfine needs to be installed.
Bench {
target: Option<OsString>,
target: Option<String>,
/// List of benchmarks to run. By default all benchmarks are run.
benches: Vec<OsString>,
benches: Vec<String>,
},
/// Update and activate the rustup toolchain 'miri' to the commit given in the
/// `rust-version` file.
/// `rustup-toolchain-install-master` must be installed for this to work. Any extra
/// flags are passed to `rustup-toolchain-install-master`.
Toolchain { flags: Vec<OsString> },
Toolchain { flags: Vec<String> },
/// Pull and merge Miri changes from the rustc repo. Defaults to fetching the latest
/// rustc commit. The fetched commit is stored in the `rust-version` file, so the
/// next `./miri toolchain` will install the rustc that just got pulled.
@ -145,113 +147,95 @@ Pass extra flags to all cargo invocations. (Ignored by `./miri cargo`.)"#;
fn main() -> Result<()> {
// We are hand-rolling our own argument parser, since `clap` can't express what we need
// (https://github.com/clap-rs/clap/issues/5055).
let mut args = env::args_os().peekable();
args.next().unwrap(); // skip program name
let command = match args.next().and_then(|s| s.into_string().ok()).as_deref() {
Some("build") => Command::Build { flags: args.collect() },
Some("check") => Command::Check { flags: args.collect() },
let mut args = args::Args::new();
let command = match args.next_raw().as_deref() {
Some("build") => Command::Build { flags: args.remainder() },
Some("check") => Command::Check { flags: args.remainder() },
Some("test") => {
let mut target = None;
let mut bless = false;
while let Some(arg) = args.peek().and_then(|s| s.to_str()) {
match arg {
"--bless" => bless = true,
"--target" => {
// Skip "--target"
args.next().unwrap();
// Next argument is the target triple.
let val = args.peek().ok_or_else(|| {
anyhow!("`--target` must be followed by target triple")
})?;
target = Some(val.to_owned());
}
// Only parse the leading flags.
_ => break,
let mut flags = Vec::new();
loop {
if args.get_long_flag("bless")? {
bless = true;
} else if let Some(val) = args.get_long_opt("target")? {
target = Some(val);
} else if let Some(flag) = args.get_other() {
flags.push(flag);
} else {
break;
}
// Consume the flag, look at the next one.
args.next().unwrap();
}
Command::Test { bless, flags: args.collect(), target }
Command::Test { bless, flags, target }
}
Some("run") => {
let mut dep = false;
let mut verbose = false;
let mut many_seeds = None;
while let Some(arg) = args.peek().and_then(|s| s.to_str()) {
if arg == "--dep" {
let mut target = None;
let mut edition = None;
let mut flags = Vec::new();
loop {
if args.get_long_flag("dep")? {
dep = true;
} else if arg == "-v" || arg == "--verbose" {
} else if args.get_long_flag("verbose")? || args.get_short_flag('v')? {
verbose = true;
} else if arg == "--many-seeds" {
many_seeds = Some(0..256);
} else if let Some(val) = arg.strip_prefix("--many-seeds=") {
} else if let Some(val) = args.get_long_opt_with_default("many-seeds", "0..256")? {
let (from, to) = val.split_once("..").ok_or_else(|| {
anyhow!("invalid format for `--many-seeds`: expected `from..to`")
anyhow!("invalid format for `--many-seeds-range`: expected `from..to`")
})?;
let from: u32 = if from.is_empty() {
0
} else {
from.parse().context("invalid `from` in `--many-seeds=from..to")?
from.parse().context("invalid `from` in `--many-seeds-range=from..to")?
};
let to: u32 = to.parse().context("invalid `to` in `--many-seeds=from..to")?;
let to: u32 =
to.parse().context("invalid `to` in `--many-seeds-range=from..to")?;
many_seeds = Some(from..to);
} else if let Some(val) = args.get_long_opt("target")? {
target = Some(val);
} else if let Some(val) = args.get_long_opt("edition")? {
edition = Some(val);
} else if let Some(flag) = args.get_other() {
flags.push(flag);
} else {
break; // not for us
break;
}
// Consume the flag, look at the next one.
args.next().unwrap();
}
Command::Run { dep, verbose, many_seeds, flags: args.collect() }
Command::Run { dep, verbose, many_seeds, target, edition, flags }
}
Some("fmt") => Command::Fmt { flags: args.collect() },
Some("clippy") => Command::Clippy { flags: args.collect() },
Some("cargo") => Command::Cargo { flags: args.collect() },
Some("install") => Command::Install { flags: args.collect() },
Some("fmt") => Command::Fmt { flags: args.remainder() },
Some("clippy") => Command::Clippy { flags: args.remainder() },
Some("cargo") => Command::Cargo { flags: args.remainder() },
Some("install") => Command::Install { flags: args.remainder() },
Some("bench") => {
let mut target = None;
while let Some(arg) = args.peek().and_then(|s| s.to_str()) {
match arg {
"--target" => {
// Skip "--target"
args.next().unwrap();
// Next argument is the target triple.
let val = args.peek().ok_or_else(|| {
anyhow!("`--target` must be followed by target triple")
})?;
target = Some(val.to_owned());
}
// Only parse the leading flags.
_ => break,
let mut benches = Vec::new();
loop {
if let Some(val) = args.get_long_opt("target")? {
target = Some(val);
} else if let Some(flag) = args.get_other() {
benches.push(flag);
} else {
break;
}
// Consume the flag, look at the next one.
args.next().unwrap();
}
Command::Bench { target, benches: args.collect() }
Command::Bench { target, benches }
}
Some("toolchain") => Command::Toolchain { flags: args.collect() },
Some("toolchain") => Command::Toolchain { flags: args.remainder() },
Some("rustc-pull") => {
let commit = args.next().map(|a| a.to_string_lossy().into_owned());
if args.next().is_some() {
let commit = args.next_raw();
if args.next_raw().is_some() {
bail!("Too many arguments for `./miri rustc-pull`");
}
Command::RustcPull { commit }
}
Some("rustc-push") => {
let github_user = args
.next()
.ok_or_else(|| {
anyhow!("Missing first argument for `./miri rustc-push GITHUB_USER [BRANCH]`")
})?
.to_string_lossy()
.into_owned();
let branch =
args.next().unwrap_or_else(|| "miri-sync".into()).to_string_lossy().into_owned();
if args.next().is_some() {
let github_user = args.next_raw().ok_or_else(|| {
anyhow!("Missing first argument for `./miri rustc-push GITHUB_USER [BRANCH]`")
})?;
let branch = args.next_raw().unwrap_or_else(|| "miri-sync".into());
if args.next_raw().is_some() {
bail!("Too many arguments for `./miri rustc-push GITHUB_USER BRANCH`");
}
Command::RustcPush { github_user, branch }

View file

@ -27,30 +27,6 @@ pub fn flagsplit(flags: &str) -> Vec<String> {
flags.split(' ').map(str::trim).filter(|s| !s.is_empty()).map(str::to_string).collect()
}
pub fn arg_flag_value(
args: impl IntoIterator<Item = impl AsRef<OsStr>>,
flag: &str,
) -> Option<OsString> {
let mut args = args.into_iter();
while let Some(arg) = args.next() {
let arg = arg.as_ref();
if arg == "--" {
return None;
}
let Some(arg) = arg.to_str() else {
// Skip non-UTF-8 arguments.
continue;
};
if arg == flag {
// Next one is the value.
return Some(args.next()?.as_ref().to_owned());
} else if let Some(val) = arg.strip_prefix(flag).and_then(|s| s.strip_prefix("=")) {
return Some(val.to_owned().into());
}
}
None
}
/// Some extra state we track for building Miri, such as the right RUSTFLAGS.
pub struct MiriEnv {
/// miri_dir is the root of the miri repository checkout we are working in.
@ -133,7 +109,7 @@ impl MiriEnv {
pub fn build(
&self,
manifest_path: impl AsRef<OsStr>,
args: &[OsString],
args: &[String],
quiet: bool,
) -> Result<()> {
let MiriEnv { toolchain, cargo_extra_flags, .. } = self;
@ -149,21 +125,21 @@ impl MiriEnv {
Ok(())
}
pub fn check(&self, manifest_path: impl AsRef<OsStr>, args: &[OsString]) -> Result<()> {
pub fn check(&self, manifest_path: impl AsRef<OsStr>, args: &[String]) -> Result<()> {
let MiriEnv { toolchain, cargo_extra_flags, .. } = self;
cmd!(self.sh, "cargo +{toolchain} check {cargo_extra_flags...} --manifest-path {manifest_path} --all-targets {args...}")
.run()?;
Ok(())
}
pub fn clippy(&self, manifest_path: impl AsRef<OsStr>, args: &[OsString]) -> Result<()> {
pub fn clippy(&self, manifest_path: impl AsRef<OsStr>, args: &[String]) -> Result<()> {
let MiriEnv { toolchain, cargo_extra_flags, .. } = self;
cmd!(self.sh, "cargo +{toolchain} clippy {cargo_extra_flags...} --manifest-path {manifest_path} --all-targets {args...}")
.run()?;
Ok(())
}
pub fn test(&self, manifest_path: impl AsRef<OsStr>, args: &[OsString]) -> Result<()> {
pub fn test(&self, manifest_path: impl AsRef<OsStr>, args: &[String]) -> Result<()> {
let MiriEnv { toolchain, cargo_extra_flags, .. } = self;
cmd!(
self.sh,
@ -181,7 +157,7 @@ impl MiriEnv {
files: impl Iterator<Item = Result<PathBuf, walkdir::Error>>,
toolchain: &str,
config_path: &Path,
flags: &[OsString],
flags: &[String],
) -> anyhow::Result<()> {
use itertools::Itertools;

View file

@ -405,9 +405,12 @@ fn main() {
let mut rustc_args = vec![];
let mut after_dashdash = false;
// If user has explicitly enabled/disabled isolation
let mut isolation_enabled: Option<bool> = None;
// Note that we require values to be given with `=`, not with a space.
// This matches how rustc parses `-Z`.
// However, unlike rustc we do not accept a space after `-Z`.
for arg in args {
if rustc_args.is_empty() {
// Very first arg: binary name.