-
Notifications
You must be signed in to change notification settings - Fork 406
miri-script: use --remap-path-prefix to print errors relative to the right root #3798
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
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
19de1a7 to
9e21767
Compare
|
All right, this seems to be working now. @rust-lang/miri what do you think? |
|
Oh, looking at the Miri thing again we could also use a totally different approach... namely, to set this as a per-package variable in the profile, which will also avoid applying it to dependencies. |
d8e7771 to
116d2de
Compare
…h to the toml file
45ec272 to
d3fb2b8
Compare
0a33b8d to
b9ebd52
Compare
| - name: Install nightly toolchain | ||
| run: rustup toolchain install nightly --profile minimal | ||
| shell: bash |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah.
No objections. I trip over the path prefixes more in the rust-lang/rust tree than this repo, but still a nice change. |
|
Okay let's land this then. :) |
|
☀️ Test successful - checks-actions |
Inspired by rust-lang/rust-clippy#13232, this makes it so that when cargo-miri fails to build,
./miri checkwill print errors with paths likecargo-miri/src/setup.rs. That means we can get rid of the miri-symlink-hacks and instead tell RA to just always invoke the./miri clippyscript just once, in the root.This means that we can no longer share a target dir between cargo-miri and miri as the RUSTFLAGS are different to crates that are shared in the dependency tree need to be built twice with two different flags.
miri-scripthence now has to set the MIRI environment variable to tell thecargo miri setupinvocation where to find Miri.I also made it so that errors in miri-script itself are properly shown in RA, for which the
./mirishell wrapper needs to set the right flags.