Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

miri-script: use --remap-path-prefix to print errors relative to the right root #3798

Merged
merged 5 commits into from
Aug 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .cargo/config.toml
Original file line number Diff line number Diff line change
@@ -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"]
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/setup/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment on lines +38 to +40
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we now need this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose more than anything I'm confused that we didn't need this before.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We now need this because miri-script needs nightly because it picks up the .config/cargo.toml which uses nightly features.

Previously we didn't need this because miri-script was built with stable, and then it used rustup-toolchain-install-master to install the toolchain used for Miri.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah.


- name: Install "master" toolchain
run: |
if [[ ${{ github.event_name }} == 'schedule' ]]; then
Expand Down
9 changes: 5 additions & 4 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
}
```
Expand Down Expand Up @@ -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.
Expand Down
4 changes: 0 additions & 4 deletions cargo-miri/miri

This file was deleted.

7 changes: 2 additions & 5 deletions cargo-miri/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
13 changes: 10 additions & 3 deletions miri
Original file line number Diff line number Diff line change
@@ -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
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
# 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 "$@"
4 changes: 0 additions & 4 deletions miri-script/miri

This file was deleted.

80 changes: 37 additions & 43 deletions miri-script/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -34,12 +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");
let Self { toolchain, cargo_extra_flags, .. } = &self;

// 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()]
Expand All @@ -56,10 +55,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("cargo-miri", "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);
Expand Down Expand Up @@ -151,8 +152,8 @@ impl Command {
| Command::Test { .. }
| Command::Run { .. }
| Command::Fmt { .. }
| Command::Clippy { .. }
| Command::Cargo { .. } => Self::auto_actions()?,
| Command::Doc { .. }
| Command::Clippy { .. } => Self::auto_actions()?,
| Command::Toolchain { .. }
| Command::Bench { .. }
| Command::RustcPull { .. }
Expand All @@ -166,9 +167,9 @@ 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::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()),
Expand Down Expand Up @@ -427,39 +428,37 @@ impl Command {

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)?;
e.build(".", &flags, /* quiet */ false)?;
e.build("cargo-miri", &flags, /* quiet */ false)?;
Ok(())
}

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)?;
e.check(".", &flags)?;
e.check("cargo-miri", &flags)?;
Ok(())
}

fn clippy(flags: Vec<String>) -> Result<()> {
fn doc(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)?;
e.clippy(path!(e.miri_dir / "miri-script" / "Cargo.toml"), &flags)?;
e.doc(".", &flags)?;
e.doc("cargo-miri", &flags)?;
Ok(())
}

fn cargo(flags: Vec<String>) -> Result<()> {
fn clippy(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
// current working dir*, not on the main Miri workspace. That is exactly what RA needs.
cmd!(e.sh, "cargo +{toolchain} {flags...}").run()?;
e.clippy(".", &flags)?;
e.clippy("cargo-miri", &flags)?;
e.clippy("miri-script", &flags)?;
Ok(())
}

fn test(bless: bool, mut flags: Vec<String>, target: Option<String>) -> 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.
Expand All @@ -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(())
}

Expand Down Expand Up @@ -504,32 +503,27 @@ 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());

// 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 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<String>| -> Result<()> {
let run_miri = |e: &MiriEnv, seed_flag: Option<String>| -> 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(".", "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(".", "run").args(quiet_flag).arg("--")
};
cmd.set_quiet(!verbose);
// Add Miri flags
Expand All @@ -545,14 +539,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(())
}
Expand All @@ -579,6 +573,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)
}
}
10 changes: 6 additions & 4 deletions miri-script/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ pub enum Command {
/// Flags that are passed through to `miri`.
flags: Vec<String>,
},
/// Build documentation
Doc {
/// Flags that are passed through to `cargo doc`.
flags: Vec<String>,
},
/// Format all sources and tests.
Fmt {
/// Flags that are passed through to `rustfmt`.
Expand All @@ -58,9 +63,6 @@ pub enum Command {
/// Flags that are passed through to `cargo clippy`.
flags: Vec<String>,
},
/// Runs just `cargo <flags>` with the Miri-specific environment variables.
/// Mainly meant to be invoked by rust-analyzer.
Cargo { flags: Vec<String> },
/// Runs the benchmarks from bench-cargo-miri in hyperfine. hyperfine needs to be installed.
Bench {
target: Option<String>,
Expand Down Expand Up @@ -151,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;
Expand Down Expand Up @@ -205,7 +208,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;
Expand Down
Loading