From f7c938aaf0f497bb23e7338156435c3bf261e052 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 9 Aug 2024 23:54:02 +0200 Subject: [PATCH 1/5] miri-script: use --remap-path-prefix to print errors relative to the right root --- src/tools/miri/.cargo/config.toml | 9 +++ src/tools/miri/CONTRIBUTING.md | 1 + src/tools/miri/cargo-miri/miri | 4 -- src/tools/miri/cargo-miri/src/util.rs | 7 +- src/tools/miri/miri | 13 +++- src/tools/miri/miri-script/miri | 4 -- src/tools/miri/miri-script/src/commands.rs | 42 ++++++------ src/tools/miri/miri-script/src/util.rs | 75 +++++++++++++--------- src/tools/miri/tests/ui.rs | 12 ++-- 9 files changed, 92 insertions(+), 75 deletions(-) create mode 100644 src/tools/miri/.cargo/config.toml delete mode 100755 src/tools/miri/cargo-miri/miri delete mode 100755 src/tools/miri/miri-script/miri diff --git a/src/tools/miri/.cargo/config.toml b/src/tools/miri/.cargo/config.toml new file mode 100644 index 0000000000000..42e7c2c48189a --- /dev/null +++ b/src/tools/miri/.cargo/config.toml @@ -0,0 +1,9 @@ +[unstable] +profile-rustflags = true + +# Add back the containing directory of the packages we have to refer to using --manifest-path. +# Per-package profiles avoid adding this to build dependencies. +[profile.dev.package."cargo-miri"] +rustflags = ["--remap-path-prefix", "=cargo-miri"] +[profile.dev.package."miri-script"] +rustflags = ["--remap-path-prefix", "=miri-script"] diff --git a/src/tools/miri/CONTRIBUTING.md b/src/tools/miri/CONTRIBUTING.md index 9067cbc603261..8aca8d459ddca 100644 --- a/src/tools/miri/CONTRIBUTING.md +++ b/src/tools/miri/CONTRIBUTING.md @@ -309,6 +309,7 @@ anyone but Miri itself. They are used to communicate between different Miri binaries, and as such worth documenting: * `CARGO_EXTRA_FLAGS` is understood by `./miri` and passed to all host cargo invocations. + It is reserved for CI usage; setting the wrong flags this way can easily confuse the script. * `MIRI_BE_RUSTC` can be set to `host` or `target`. It tells the Miri driver to actually not interpret the code but compile it like rustc would. With `target`, Miri sets some compiler flags to prepare the code for interpretation; with `host`, this is not done. diff --git a/src/tools/miri/cargo-miri/miri b/src/tools/miri/cargo-miri/miri deleted file mode 100755 index cf3ad06788ab1..0000000000000 --- a/src/tools/miri/cargo-miri/miri +++ /dev/null @@ -1,4 +0,0 @@ -#!/bin/sh -# RA invokes `./miri cargo ...` for each workspace, so we need to forward that to the main `miri` -# script. See . -exec "$(dirname "$0")"/../miri "$@" diff --git a/src/tools/miri/cargo-miri/src/util.rs b/src/tools/miri/cargo-miri/src/util.rs index 5f2794e2244f4..56f38de8de620 100644 --- a/src/tools/miri/cargo-miri/src/util.rs +++ b/src/tools/miri/cargo-miri/src/util.rs @@ -93,12 +93,9 @@ pub fn find_miri() -> PathBuf { if let Some(path) = env::var_os("MIRI") { return path.into(); } + // Assume it is in the same directory as ourselves. let mut path = std::env::current_exe().expect("current executable path invalid"); - if cfg!(windows) { - path.set_file_name("miri.exe"); - } else { - path.set_file_name("miri"); - } + path.set_file_name(format!("miri{}", env::consts::EXE_SUFFIX)); path } diff --git a/src/tools/miri/miri b/src/tools/miri/miri index 07383bb59ebcc..5d07ad9e249d6 100755 --- a/src/tools/miri/miri +++ b/src/tools/miri/miri @@ -1,8 +1,15 @@ #!/usr/bin/env bash set -e +# We want to call the binary directly, so we need to know where it ends up. +MIRI_SCRIPT_TARGET_DIR="$(dirname "$0")"/miri-script/target +# If stdout is not a terminal and we are not on CI, assume that we are being invoked by RA, and use JSON output. +if ! [ -t 1 ] && [ -z "$CI" ]; then + MESSAGE_FORMAT="--message-format=json" +fi +# We need a nightly toolchain, for the `profile-rustflags` cargo feature. +cargo +nightly build $CARGO_EXTRA_FLAGS --manifest-path "$(dirname "$0")"/miri-script/Cargo.toml \ + -q --target-dir "$MIRI_SCRIPT_TARGET_DIR" $MESSAGE_FORMAT || \ + ( echo "Failed to build miri-script. Is the 'nightly' toolchain installed?"; exit 1 ) # Instead of doing just `cargo run --manifest-path .. $@`, we invoke miri-script binary directly. Invoking `cargo run` goes through # rustup (that sets it's own environmental variables), which is undesirable. -MIRI_SCRIPT_TARGET_DIR="$(dirname "$0")"/miri-script/target -cargo +stable build $CARGO_EXTRA_FLAGS -q --target-dir "$MIRI_SCRIPT_TARGET_DIR" --manifest-path "$(dirname "$0")"/miri-script/Cargo.toml || \ - ( echo "Failed to build miri-script. Is the 'stable' toolchain installed?"; exit 1 ) "$MIRI_SCRIPT_TARGET_DIR"/debug/miri-script "$@" diff --git a/src/tools/miri/miri-script/miri b/src/tools/miri/miri-script/miri deleted file mode 100755 index cf3ad06788ab1..0000000000000 --- a/src/tools/miri/miri-script/miri +++ /dev/null @@ -1,4 +0,0 @@ -#!/bin/sh -# RA invokes `./miri cargo ...` for each workspace, so we need to forward that to the main `miri` -# script. See . -exec "$(dirname "$0")"/../miri "$@" diff --git a/src/tools/miri/miri-script/src/commands.rs b/src/tools/miri/miri-script/src/commands.rs index fc205040baf57..705ddaa9eadef 100644 --- a/src/tools/miri/miri-script/src/commands.rs +++ b/src/tools/miri/miri-script/src/commands.rs @@ -22,6 +22,7 @@ const JOSH_FILTER: &str = const JOSH_PORT: &str = "42042"; impl MiriEnv { + /// Prepares the environment: builds miri and cargo-miri and a sysroot. /// Returns the location of the sysroot. /// /// If the target is None the sysroot will be built for the host machine. @@ -35,7 +36,6 @@ impl MiriEnv { return Ok(miri_sysroot.into()); } let manifest_path = path!(self.miri_dir / "cargo-miri" / "Cargo.toml"); - let Self { toolchain, cargo_extra_flags, .. } = &self; // Make sure everything is built. Also Miri itself. self.build(path!(self.miri_dir / "Cargo.toml"), &[], quiet)?; @@ -56,10 +56,12 @@ impl MiriEnv { eprintln!(); } - let mut cmd = cmd!(self.sh, - "cargo +{toolchain} --quiet run {cargo_extra_flags...} --manifest-path {manifest_path} -- - miri setup --print-sysroot {target_flag...}" - ); + let mut cmd = self + .cargo_cmd(&manifest_path, "run") + .arg("--quiet") + .arg("--") + .args(&["miri", "setup", "--print-sysroot"]) + .args(target_flag); cmd.set_quiet(quiet); let output = cmd.read()?; self.sh.set_var("MIRI_SYSROOT", &output); @@ -459,7 +461,7 @@ impl Command { fn test(bless: bool, mut flags: Vec, target: Option) -> Result<()> { let mut e = MiriEnv::new()?; - // Prepare a sysroot. + // Prepare a sysroot. (Also builds cargo-miri, which we need.) e.build_miri_sysroot(/* quiet */ false, target.as_deref())?; // Forward information to test harness. @@ -504,7 +506,7 @@ impl Command { early_flags.push("--edition".into()); early_flags.push(edition.as_deref().unwrap_or("2021").into()); - // Prepare a sysroot, add it to the flags. + // Prepare a sysroot, add it to the flags. (Also builds cargo-miri, which we need.) let miri_sysroot = e.build_miri_sysroot(/* quiet */ !verbose, target.as_deref())?; early_flags.push("--sysroot".into()); early_flags.push(miri_sysroot.into()); @@ -513,23 +515,19 @@ impl Command { let miri_manifest = path!(e.miri_dir / "Cargo.toml"); let miri_flags = e.sh.var("MIRIFLAGS").unwrap_or_default(); let miri_flags = flagsplit(&miri_flags); - let toolchain = &e.toolchain; - let extra_flags = &e.cargo_extra_flags; let quiet_flag = if verbose { None } else { Some("--quiet") }; // This closure runs the command with the given `seed_flag` added between the MIRIFLAGS and // the `flags` given on the command-line. - let run_miri = |sh: &Shell, seed_flag: Option| -> Result<()> { + let run_miri = |e: &MiriEnv, seed_flag: Option| -> Result<()> { // The basic command that executes the Miri driver. let mut cmd = if dep { - cmd!( - sh, - "cargo +{toolchain} {quiet_flag...} test {extra_flags...} --manifest-path {miri_manifest} --test ui -- --miri-run-dep-mode" - ) + e.cargo_cmd(&miri_manifest, "test") + .args(&["--test", "ui"]) + .args(quiet_flag) + .arg("--") + .args(&["--miri-run-dep-mode"]) } else { - cmd!( - sh, - "cargo +{toolchain} {quiet_flag...} run {extra_flags...} --manifest-path {miri_manifest} --" - ) + e.cargo_cmd(&miri_manifest, "run").args(quiet_flag).arg("--") }; cmd.set_quiet(!verbose); // Add Miri flags @@ -545,14 +543,14 @@ impl Command { }; // Run the closure once or many times. if let Some(seed_range) = many_seeds { - e.run_many_times(seed_range, |sh, seed| { + e.run_many_times(seed_range, |e, seed| { eprintln!("Trying seed: {seed}"); - run_miri(sh, Some(format!("-Zmiri-seed={seed}"))).inspect_err(|_| { + run_miri(e, Some(format!("-Zmiri-seed={seed}"))).inspect_err(|_| { eprintln!("FAILING SEED: {seed}"); }) })?; } else { - run_miri(&e.sh, None)?; + run_miri(&e, None)?; } Ok(()) } @@ -579,6 +577,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, &config_path, &flags) } } diff --git a/src/tools/miri/miri-script/src/util.rs b/src/tools/miri/miri-script/src/util.rs index e1b77be192e31..568dc1ec0b38c 100644 --- a/src/tools/miri/miri-script/src/util.rs +++ b/src/tools/miri/miri-script/src/util.rs @@ -7,7 +7,7 @@ use std::thread; use anyhow::{anyhow, Context, Result}; use dunce::canonicalize; use path_macro::path; -use xshell::{cmd, Shell}; +use xshell::{cmd, Cmd, Shell}; pub fn miri_dir() -> std::io::Result { const MIRI_SCRIPT_ROOT_DIR: &str = env!("CARGO_MANIFEST_DIR"); @@ -28,13 +28,14 @@ pub fn flagsplit(flags: &str) -> Vec { } /// Some extra state we track for building Miri, such as the right RUSTFLAGS. +#[derive(Clone)] pub struct MiriEnv { /// miri_dir is the root of the miri repository checkout we are working in. pub miri_dir: PathBuf, /// active_toolchain is passed as `+toolchain` argument to cargo/rustc invocations. pub toolchain: String, /// Extra flags to pass to cargo. - pub cargo_extra_flags: Vec, + cargo_extra_flags: Vec, /// The rustc sysroot pub sysroot: PathBuf, /// The shell we use. @@ -54,15 +55,14 @@ impl MiriEnv { // Determine some toolchain properties if !libdir.exists() { - println!("Something went wrong determining the library dir."); - println!("I got {} but that does not exist.", libdir.display()); - println!("Please report a bug at https://github.com/rust-lang/miri/issues."); + eprintln!("Something went wrong determining the library dir."); + eprintln!("I got {} but that does not exist.", libdir.display()); + eprintln!("Please report a bug at https://github.com/rust-lang/miri/issues."); std::process::exit(2); } - // Share target dir between `miri` and `cargo-miri`. - let target_dir = std::env::var_os("CARGO_TARGET_DIR") - .unwrap_or_else(|| path!(miri_dir / "target").into()); - sh.set_var("CARGO_TARGET_DIR", target_dir); + + // Hard-code the target dir, since we rely on all binaries ending up in the same spot. + sh.set_var("CARGO_TARGET_DIR", path!(miri_dir / "target")); // We configure dev builds to not be unusably slow. let devel_opt_level = @@ -91,10 +91,26 @@ impl MiriEnv { // Get extra flags for cargo. let cargo_extra_flags = std::env::var("CARGO_EXTRA_FLAGS").unwrap_or_default(); let cargo_extra_flags = flagsplit(&cargo_extra_flags); + if cargo_extra_flags.iter().any(|a| a == "--release" || a.starts_with("--profile")) { + // This makes binaries end up in different paths, let's not do that. + eprintln!( + "Passing `--release` or `--profile` in `CARGO_EXTRA_FLAGS` will totally confuse miri-script, please don't do that." + ); + std::process::exit(1); + } Ok(MiriEnv { miri_dir, toolchain, sh, sysroot, cargo_extra_flags }) } + pub fn cargo_cmd(&self, manifest_path: impl AsRef, cmd: &str) -> Cmd<'_> { + let MiriEnv { toolchain, cargo_extra_flags, .. } = self; + let manifest_path = Path::new(manifest_path.as_ref()); + cmd!( + self.sh, + "cargo +{toolchain} {cmd} {cargo_extra_flags...} --manifest-path {manifest_path}" + ) + } + pub fn install_to_sysroot( &self, path: impl AsRef, @@ -102,6 +118,7 @@ impl MiriEnv { ) -> Result<()> { let MiriEnv { sysroot, toolchain, cargo_extra_flags, .. } = self; // Install binaries to the miri toolchain's `sysroot` so they do not interact with other toolchains. + // (Not using `cargo_cmd` as `install` is special and doesn't use `--manifest-path`.) cmd!(self.sh, "cargo +{toolchain} install {cargo_extra_flags...} --path {path} --force --root {sysroot} {args...}").run()?; Ok(()) } @@ -112,40 +129,34 @@ impl MiriEnv { args: &[String], quiet: bool, ) -> Result<()> { - let MiriEnv { toolchain, cargo_extra_flags, .. } = self; let quiet_flag = if quiet { Some("--quiet") } else { None }; // We build the tests as well, (a) to avoid having rebuilds when building the tests later // and (b) to have more parallelism during the build of Miri and its tests. - let mut cmd = cmd!( - self.sh, - "cargo +{toolchain} build --bins --tests {cargo_extra_flags...} --manifest-path {manifest_path} {quiet_flag...} {args...}" - ); + // This means `./miri run` without `--dep` will build Miri twice (for the sysroot with + // dev-dependencies, and then for running without dev-dependencies), but the way more common + // `./miri test` will avoid building Miri twice. + let mut cmd = self + .cargo_cmd(manifest_path, "build") + .args(&["--bins", "--tests"]) + .args(quiet_flag) + .args(args); cmd.set_quiet(quiet); cmd.run()?; Ok(()) } pub fn check(&self, manifest_path: impl AsRef, 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()?; + self.cargo_cmd(manifest_path, "check").arg("--all-targets").args(args).run()?; Ok(()) } pub fn clippy(&self, manifest_path: impl AsRef, 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()?; + self.cargo_cmd(manifest_path, "clippy").arg("--all-targets").args(args).run()?; Ok(()) } pub fn test(&self, manifest_path: impl AsRef, args: &[String]) -> Result<()> { - let MiriEnv { toolchain, cargo_extra_flags, .. } = self; - cmd!( - self.sh, - "cargo +{toolchain} test {cargo_extra_flags...} --manifest-path {manifest_path} {args...}" - ) - .run()?; + self.cargo_cmd(manifest_path, "test").args(args).run()?; Ok(()) } @@ -155,7 +166,6 @@ impl MiriEnv { pub fn format_files( &self, files: impl Iterator>, - toolchain: &str, config_path: &Path, flags: &[String], ) -> anyhow::Result<()> { @@ -166,6 +176,7 @@ impl MiriEnv { // Format in batches as not all our files fit into Windows' command argument limit. for batch in &files.chunks(256) { // Build base command. + let toolchain = &self.toolchain; let mut cmd = cmd!( self.sh, "rustfmt +{toolchain} --edition=2021 --config-path {config_path} --unstable-features --skip-children {flags...}" @@ -197,7 +208,7 @@ impl MiriEnv { pub fn run_many_times( &self, range: Range, - run: impl Fn(&Shell, u32) -> Result<()> + Sync, + run: impl Fn(&Self, u32) -> Result<()> + Sync, ) -> Result<()> { // `next` is atomic so threads can concurrently fetch their next value to run. let next = AtomicU32::new(range.start); @@ -207,10 +218,10 @@ impl MiriEnv { let mut handles = Vec::new(); // Spawn one worker per core. for _ in 0..thread::available_parallelism()?.get() { - // Create a copy of the shell for this thread. - let local_shell = self.sh.clone(); + // Create a copy of the environment for this thread. + let local_miri = self.clone(); let handle = s.spawn(|| -> Result<()> { - let local_shell = local_shell; // move the copy into this thread. + let local_miri = local_miri; // move the copy into this thread. // Each worker thread keeps asking for numbers until we're all done. loop { let cur = next.fetch_add(1, Ordering::Relaxed); @@ -219,7 +230,7 @@ impl MiriEnv { break; } // Run the command with this seed. - run(&local_shell, cur).inspect_err(|_| { + run(&local_miri, cur).inspect_err(|_| { // If we failed, tell everyone about this. failed.store(true, Ordering::Relaxed); })?; diff --git a/src/tools/miri/tests/ui.rs b/src/tools/miri/tests/ui.rs index 95f8b05410293..9cbcf6e42a795 100644 --- a/src/tools/miri/tests/ui.rs +++ b/src/tools/miri/tests/ui.rs @@ -13,7 +13,7 @@ use ui_test::{ }; fn miri_path() -> PathBuf { - PathBuf::from(option_env!("MIRI").unwrap_or(env!("CARGO_BIN_EXE_miri"))) + PathBuf::from(env::var("MIRI").unwrap_or_else(|_| env!("CARGO_BIN_EXE_miri").into())) } fn get_host() -> String { @@ -29,7 +29,7 @@ pub fn flagsplit(flags: &str) -> Vec { // Build the shared object file for testing native function calls. fn build_native_lib() -> PathBuf { - let cc = option_env!("CC").unwrap_or("cc"); + let cc = env::var("CC").unwrap_or_else(|_| "cc".into()); // Target directory that we can write to. let so_target_dir = Path::new(&env::var_os("CARGO_TARGET_DIR").unwrap()).join("miri-native-lib"); @@ -84,9 +84,11 @@ fn miri_config(target: &str, path: &str, mode: Mode, with_dependencies: bool) -> if with_dependencies { // Set the `cargo-miri` binary, which we expect to be in the same folder as the `miri` binary. // (It's a separate crate, so we don't get an env var from cargo.) - let mut prog = miri_path(); - prog.set_file_name("cargo-miri"); - config.dependency_builder.program = prog; + config.dependency_builder.program = { + let mut prog = miri_path(); + prog.set_file_name(format!("cargo-miri{}", env::consts::EXE_SUFFIX)); + prog + }; let builder_args = ["miri", "run"]; // There is no `cargo miri build` so we just use `cargo miri run`. config.dependency_builder.args = builder_args.into_iter().map(Into::into).collect(); config.dependencies_crate_manifest_path = From d2e0970bde2c383ba75a44432db198d9a99e3808 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 9 Aug 2024 23:59:40 +0200 Subject: [PATCH 2/5] update suggested RA config; the './miri cargo' command is not needed any more --- src/tools/miri/CONTRIBUTING.md | 8 ++++---- src/tools/miri/miri-script/src/commands.rs | 13 +------------ src/tools/miri/miri-script/src/main.rs | 4 ---- src/tools/miri/miri-script/src/util.rs | 2 +- 4 files changed, 6 insertions(+), 21 deletions(-) diff --git a/src/tools/miri/CONTRIBUTING.md b/src/tools/miri/CONTRIBUTING.md index 8aca8d459ddca..1c76354eaeea4 100644 --- a/src/tools/miri/CONTRIBUTING.md +++ b/src/tools/miri/CONTRIBUTING.md @@ -173,24 +173,24 @@ to `.vscode/settings.json` in your local Miri clone: "cargo-miri/Cargo.toml", "miri-script/Cargo.toml", ], + "rust-analyzer.check.invocationLocation": "root", + "rust-analyzer.check.invocationStrategy": "once", "rust-analyzer.check.overrideCommand": [ "env", "MIRI_AUTO_OPS=no", "./miri", - "cargo", "clippy", // make this `check` when working with a locally built rustc "--message-format=json", - "--all-targets", ], // Contrary to what the name suggests, this also affects proc macros. + "rust-analyzer.cargo.buildScripts.invocationLocation": "root", + "rust-analyzer.cargo.buildScripts.invocationStrategy": "once", "rust-analyzer.cargo.buildScripts.overrideCommand": [ "env", "MIRI_AUTO_OPS=no", "./miri", - "cargo", "check", "--message-format=json", - "--all-targets", ], } ``` diff --git a/src/tools/miri/miri-script/src/commands.rs b/src/tools/miri/miri-script/src/commands.rs index 705ddaa9eadef..d4e86b05fe414 100644 --- a/src/tools/miri/miri-script/src/commands.rs +++ b/src/tools/miri/miri-script/src/commands.rs @@ -153,8 +153,7 @@ impl Command { | Command::Test { .. } | Command::Run { .. } | Command::Fmt { .. } - | Command::Clippy { .. } - | Command::Cargo { .. } => Self::auto_actions()?, + | Command::Clippy { .. } => Self::auto_actions()?, | Command::Toolchain { .. } | Command::Bench { .. } | Command::RustcPull { .. } @@ -170,7 +169,6 @@ impl Command { 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), Command::Bench { target, benches } => Self::bench(target, benches), Command::Toolchain { flags } => Self::toolchain(flags), Command::RustcPull { commit } => Self::rustc_pull(commit.clone()), @@ -449,15 +447,6 @@ impl Command { Ok(()) } - fn cargo(flags: Vec) -> 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 - // current working dir*, not on the main Miri workspace. That is exactly what RA needs. - cmd!(e.sh, "cargo +{toolchain} {flags...}").run()?; - Ok(()) - } - fn test(bless: bool, mut flags: Vec, target: Option) -> Result<()> { let mut e = MiriEnv::new()?; diff --git a/src/tools/miri/miri-script/src/main.rs b/src/tools/miri/miri-script/src/main.rs index c4f0d808d93e1..1e181cad0840c 100644 --- a/src/tools/miri/miri-script/src/main.rs +++ b/src/tools/miri/miri-script/src/main.rs @@ -58,9 +58,6 @@ pub enum Command { /// Flags that are passed through to `cargo clippy`. flags: Vec, }, - /// Runs just `cargo ` with the Miri-specific environment variables. - /// Mainly meant to be invoked by rust-analyzer. - Cargo { flags: Vec }, /// Runs the benchmarks from bench-cargo-miri in hyperfine. hyperfine needs to be installed. Bench { target: Option, @@ -205,7 +202,6 @@ fn main() -> Result<()> { } 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; diff --git a/src/tools/miri/miri-script/src/util.rs b/src/tools/miri/miri-script/src/util.rs index 568dc1ec0b38c..df7c206a5ba8f 100644 --- a/src/tools/miri/miri-script/src/util.rs +++ b/src/tools/miri/miri-script/src/util.rs @@ -33,7 +33,7 @@ pub struct MiriEnv { /// miri_dir is the root of the miri repository checkout we are working in. pub miri_dir: PathBuf, /// active_toolchain is passed as `+toolchain` argument to cargo/rustc invocations. - pub toolchain: String, + toolchain: String, /// Extra flags to pass to cargo. cargo_extra_flags: Vec, /// The rustc sysroot From 118be4182cdf8049baf86da7c161c2ba6a8dcf7e Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 10 Aug 2024 00:07:31 +0200 Subject: [PATCH 3/5] add './miri doc' command --- src/tools/miri/.github/workflows/ci.yml | 2 +- src/tools/miri/miri-script/src/commands.rs | 9 +++++++++ src/tools/miri/miri-script/src/main.rs | 6 ++++++ src/tools/miri/miri-script/src/util.rs | 5 +++++ 4 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/tools/miri/.github/workflows/ci.yml b/src/tools/miri/.github/workflows/ci.yml index fc4e484fa3898..efdeaeffe1592 100644 --- a/src/tools/miri/.github/workflows/ci.yml +++ b/src/tools/miri/.github/workflows/ci.yml @@ -60,7 +60,7 @@ jobs: - name: clippy (all features) run: ./miri clippy --all-features -- -D warnings - name: rustdoc - run: RUSTDOCFLAGS="-Dwarnings" ./miri cargo doc --document-private-items + run: RUSTDOCFLAGS="-Dwarnings" ./miri doc --document-private-items # These jobs doesn't actually test anything, but they're only used to tell # bors the build completed, as there is no practical way to detect when a diff --git a/src/tools/miri/miri-script/src/commands.rs b/src/tools/miri/miri-script/src/commands.rs index d4e86b05fe414..d29e3b1788231 100644 --- a/src/tools/miri/miri-script/src/commands.rs +++ b/src/tools/miri/miri-script/src/commands.rs @@ -153,6 +153,7 @@ impl Command { | Command::Test { .. } | Command::Run { .. } | Command::Fmt { .. } + | Command::Doc { .. } | Command::Clippy { .. } => Self::auto_actions()?, | Command::Toolchain { .. } | Command::Bench { .. } @@ -167,6 +168,7 @@ impl Command { Command::Test { bless, flags, target } => Self::test(bless, flags, target), Command::Run { dep, verbose, many_seeds, target, edition, flags } => Self::run(dep, verbose, many_seeds, target, edition, flags), + Command::Doc { flags } => Self::doc(flags), Command::Fmt { flags } => Self::fmt(flags), Command::Clippy { flags } => Self::clippy(flags), Command::Bench { target, benches } => Self::bench(target, benches), @@ -439,6 +441,13 @@ impl Command { Ok(()) } + fn doc(flags: Vec) -> Result<()> { + let e = MiriEnv::new()?; + e.doc(path!(e.miri_dir / "Cargo.toml"), &flags)?; + e.doc(path!(e.miri_dir / "cargo-miri" / "Cargo.toml"), &flags)?; + Ok(()) + } + fn clippy(flags: Vec) -> Result<()> { let e = MiriEnv::new()?; e.clippy(path!(e.miri_dir / "Cargo.toml"), &flags)?; diff --git a/src/tools/miri/miri-script/src/main.rs b/src/tools/miri/miri-script/src/main.rs index 1e181cad0840c..9214823710709 100644 --- a/src/tools/miri/miri-script/src/main.rs +++ b/src/tools/miri/miri-script/src/main.rs @@ -48,6 +48,11 @@ pub enum Command { /// Flags that are passed through to `miri`. flags: Vec, }, + /// Build documentation + Doc { + /// Flags that are passed through to `cargo doc`. + flags: Vec, + }, /// Format all sources and tests. Fmt { /// Flags that are passed through to `rustfmt`. @@ -148,6 +153,7 @@ fn main() -> Result<()> { let command = match args.next_raw().as_deref() { Some("build") => Command::Build { flags: args.remainder() }, Some("check") => Command::Check { flags: args.remainder() }, + Some("doc") => Command::Doc { flags: args.remainder() }, Some("test") => { let mut target = None; let mut bless = false; diff --git a/src/tools/miri/miri-script/src/util.rs b/src/tools/miri/miri-script/src/util.rs index df7c206a5ba8f..8fcf18e4a3875 100644 --- a/src/tools/miri/miri-script/src/util.rs +++ b/src/tools/miri/miri-script/src/util.rs @@ -150,6 +150,11 @@ impl MiriEnv { Ok(()) } + pub fn doc(&self, manifest_path: impl AsRef, args: &[String]) -> Result<()> { + self.cargo_cmd(manifest_path, "doc").args(args).run()?; + Ok(()) + } + pub fn clippy(&self, manifest_path: impl AsRef, args: &[String]) -> Result<()> { self.cargo_cmd(manifest_path, "clippy").arg("--all-targets").args(args).run()?; Ok(()) From 8197f07e0ec61b2a7c9d53ca8047ecc35d09c378 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 10 Aug 2024 12:57:52 +0200 Subject: [PATCH 4/5] miri-script: pass around the relative crate dir, not the absolute path to the toml file --- src/tools/miri/miri-script/src/commands.rs | 32 ++++++++++------------ src/tools/miri/miri-script/src/util.rs | 30 +++++++++----------- 2 files changed, 28 insertions(+), 34 deletions(-) diff --git a/src/tools/miri/miri-script/src/commands.rs b/src/tools/miri/miri-script/src/commands.rs index d29e3b1788231..d72c1eb62046d 100644 --- a/src/tools/miri/miri-script/src/commands.rs +++ b/src/tools/miri/miri-script/src/commands.rs @@ -35,11 +35,10 @@ impl MiriEnv { // Sysroot already set, use that. return Ok(miri_sysroot.into()); } - let manifest_path = path!(self.miri_dir / "cargo-miri" / "Cargo.toml"); // Make sure everything is built. Also Miri itself. - self.build(path!(self.miri_dir / "Cargo.toml"), &[], quiet)?; - self.build(&manifest_path, &[], quiet)?; + self.build(".", &[], quiet)?; + self.build("cargo-miri", &[], quiet)?; let target_flag = if let Some(target) = &target { vec![OsStr::new("--target"), target.as_ref()] @@ -57,7 +56,7 @@ impl MiriEnv { } let mut cmd = self - .cargo_cmd(&manifest_path, "run") + .cargo_cmd("cargo-miri", "run") .arg("--quiet") .arg("--") .args(&["miri", "setup", "--print-sysroot"]) @@ -429,30 +428,30 @@ impl Command { fn build(flags: Vec) -> 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)?; + e.build(".", &flags, /* quiet */ false)?; + e.build("cargo-miri", &flags, /* quiet */ false)?; Ok(()) } fn check(flags: Vec) -> 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)?; + e.check(".", &flags)?; + e.check("cargo-miri", &flags)?; Ok(()) } fn doc(flags: Vec) -> Result<()> { let e = MiriEnv::new()?; - e.doc(path!(e.miri_dir / "Cargo.toml"), &flags)?; - e.doc(path!(e.miri_dir / "cargo-miri" / "Cargo.toml"), &flags)?; + e.doc(".", &flags)?; + e.doc("cargo-miri", &flags)?; Ok(()) } fn clippy(flags: Vec) -> 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)?; - e.clippy(path!(e.miri_dir / "miri-script" / "Cargo.toml"), &flags)?; + e.clippy(".", &flags)?; + e.clippy("cargo-miri", &flags)?; + e.clippy("miri-script", &flags)?; Ok(()) } @@ -476,7 +475,7 @@ impl Command { // Then test, and let caller control flags. // Only in root project as `cargo-miri` has no tests. - e.test(path!(e.miri_dir / "Cargo.toml"), &flags)?; + e.test(".", &flags)?; Ok(()) } @@ -510,7 +509,6 @@ impl Command { 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"); let miri_flags = e.sh.var("MIRIFLAGS").unwrap_or_default(); let miri_flags = flagsplit(&miri_flags); let quiet_flag = if verbose { None } else { Some("--quiet") }; @@ -519,13 +517,13 @@ impl Command { let run_miri = |e: &MiriEnv, seed_flag: Option| -> Result<()> { // The basic command that executes the Miri driver. let mut cmd = if dep { - e.cargo_cmd(&miri_manifest, "test") + e.cargo_cmd(".", "test") .args(&["--test", "ui"]) .args(quiet_flag) .arg("--") .args(&["--miri-run-dep-mode"]) } else { - e.cargo_cmd(&miri_manifest, "run").args(quiet_flag).arg("--") + e.cargo_cmd(".", "run").args(quiet_flag).arg("--") }; cmd.set_quiet(!verbose); // Add Miri flags diff --git a/src/tools/miri/miri-script/src/util.rs b/src/tools/miri/miri-script/src/util.rs index 8fcf18e4a3875..35c604b407e1e 100644 --- a/src/tools/miri/miri-script/src/util.rs +++ b/src/tools/miri/miri-script/src/util.rs @@ -102,9 +102,9 @@ impl MiriEnv { Ok(MiriEnv { miri_dir, toolchain, sh, sysroot, cargo_extra_flags }) } - pub fn cargo_cmd(&self, manifest_path: impl AsRef, cmd: &str) -> Cmd<'_> { + pub fn cargo_cmd(&self, crate_dir: impl AsRef, cmd: &str) -> Cmd<'_> { let MiriEnv { toolchain, cargo_extra_flags, .. } = self; - let manifest_path = Path::new(manifest_path.as_ref()); + let manifest_path = path!(self.miri_dir / crate_dir.as_ref() / "Cargo.toml"); cmd!( self.sh, "cargo +{toolchain} {cmd} {cargo_extra_flags...} --manifest-path {manifest_path}" @@ -117,18 +117,14 @@ impl MiriEnv { args: impl IntoIterator>, ) -> Result<()> { let MiriEnv { sysroot, toolchain, cargo_extra_flags, .. } = self; + let path = path!(self.miri_dir / path.as_ref()); // Install binaries to the miri toolchain's `sysroot` so they do not interact with other toolchains. // (Not using `cargo_cmd` as `install` is special and doesn't use `--manifest-path`.) cmd!(self.sh, "cargo +{toolchain} install {cargo_extra_flags...} --path {path} --force --root {sysroot} {args...}").run()?; Ok(()) } - pub fn build( - &self, - manifest_path: impl AsRef, - args: &[String], - quiet: bool, - ) -> Result<()> { + pub fn build(&self, crate_dir: impl AsRef, args: &[String], quiet: bool) -> Result<()> { let quiet_flag = if quiet { Some("--quiet") } else { None }; // We build the tests as well, (a) to avoid having rebuilds when building the tests later // and (b) to have more parallelism during the build of Miri and its tests. @@ -136,7 +132,7 @@ impl MiriEnv { // dev-dependencies, and then for running without dev-dependencies), but the way more common // `./miri test` will avoid building Miri twice. let mut cmd = self - .cargo_cmd(manifest_path, "build") + .cargo_cmd(crate_dir, "build") .args(&["--bins", "--tests"]) .args(quiet_flag) .args(args); @@ -145,23 +141,23 @@ impl MiriEnv { Ok(()) } - pub fn check(&self, manifest_path: impl AsRef, args: &[String]) -> Result<()> { - self.cargo_cmd(manifest_path, "check").arg("--all-targets").args(args).run()?; + pub fn check(&self, crate_dir: impl AsRef, args: &[String]) -> Result<()> { + self.cargo_cmd(crate_dir, "check").arg("--all-targets").args(args).run()?; Ok(()) } - pub fn doc(&self, manifest_path: impl AsRef, args: &[String]) -> Result<()> { - self.cargo_cmd(manifest_path, "doc").args(args).run()?; + pub fn doc(&self, crate_dir: impl AsRef, args: &[String]) -> Result<()> { + self.cargo_cmd(crate_dir, "doc").args(args).run()?; Ok(()) } - pub fn clippy(&self, manifest_path: impl AsRef, args: &[String]) -> Result<()> { - self.cargo_cmd(manifest_path, "clippy").arg("--all-targets").args(args).run()?; + pub fn clippy(&self, crate_dir: impl AsRef, args: &[String]) -> Result<()> { + self.cargo_cmd(crate_dir, "clippy").arg("--all-targets").args(args).run()?; Ok(()) } - pub fn test(&self, manifest_path: impl AsRef, args: &[String]) -> Result<()> { - self.cargo_cmd(manifest_path, "test").args(args).run()?; + pub fn test(&self, crate_dir: impl AsRef, args: &[String]) -> Result<()> { + self.cargo_cmd(crate_dir, "test").args(args).run()?; Ok(()) } From feab3240857e568b25d3950b6e5697bb4b0b7e6e Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 10 Aug 2024 13:06:24 +0200 Subject: [PATCH 5/5] CI: we now need the nightly toolchain as well --- src/tools/miri/.github/workflows/setup/action.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/tools/miri/.github/workflows/setup/action.yml b/src/tools/miri/.github/workflows/setup/action.yml index 8f54b5b8d81a9..bf5749a7b17eb 100644 --- a/src/tools/miri/.github/workflows/setup/action.yml +++ b/src/tools/miri/.github/workflows/setup/action.yml @@ -35,6 +35,10 @@ runs: run: cargo install -f rustup-toolchain-install-master hyperfine shell: bash + - name: Install nightly toolchain + run: rustup toolchain install nightly --profile minimal + shell: bash + - name: Install "master" toolchain run: | if [[ ${{ github.event_name }} == 'schedule' ]]; then