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

Add path prefixes back when compiling clippy_dev and lintcheck #13232

Merged
merged 1 commit into from
Aug 7, 2024

Conversation

Alexendoo
Copy link
Member

cargo dev and cargo lintcheck use --manifest-path to select the package to compile, with this Cargo changes the CWD to the package's containing directory meaning paths in diagnostics start from e.g. src/ instead of clippy_dev/src/

Lintcheck uses a --remap-path-prefix trick when linting crates to re-add the directory name in diagnostics:

// `src/lib.rs` -> `target/lintcheck/sources/crate-1.2.3/src/lib.rs`
let remap_relative = format!("={}", self.path.display());

"--remap-path-prefix",
&remap_relative,

It works well as far as I can tell, when absolute paths appear they stay absolute and do not have the prefix added

profile-rustflags allows us to set per package RUSTFLAGS in order to use the same trick on the clippy_dev and lintcheck packages themselves

Now when you run into warnings/errors the filename will be correctly clickable

Before

error[E0425]: cannot find value `moo` in this scope
  --> src/lib.rs:41:5
   |
41 |     moo;
   |     ^^^ not found in this scope

After

error[E0425]: cannot find value `moo` in this scope
  --> clippy_dev/src/lib.rs:41:5
   |
41 |     moo;
   |     ^^^ not found in this scope

r? @flip1995

changelog: none

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Aug 7, 2024
@flip1995
Copy link
Member

flip1995 commented Aug 7, 2024

Makes sense. Thanks!

@bors r+

@bors
Copy link
Collaborator

bors commented Aug 7, 2024

📌 Commit 8225f2a has been approved by flip1995

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Aug 7, 2024

⌛ Testing commit 8225f2a with merge f2deab3...

@bors
Copy link
Collaborator

bors commented Aug 7, 2024

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: flip1995
Pushing f2deab3 to master...

@bors bors merged commit f2deab3 into rust-lang:master Aug 7, 2024
5 checks passed
@Alexendoo Alexendoo deleted the path-prefix branch August 7, 2024 19:07
bors added a commit to rust-lang/miri that referenced this pull request Aug 12, 2024
miri-script: use --remap-path-prefix to print errors relative to the right root

Inspired by rust-lang/rust-clippy#13232, this makes it so that when cargo-miri fails to build, `./miri check` will print errors with paths like `cargo-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 clippy` script 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-script` hence now has to set the MIRI environment variable to tell the `cargo miri setup` invocation where to find Miri.

I also made it so that errors in miri-script itself are properly shown in RA, for which the `./miri` shell wrapper needs to set the right flags.
RalfJung pushed a commit to RalfJung/rust that referenced this pull request Aug 18, 2024
…, r=saethlin

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

Inspired by rust-lang/rust-clippy#13232, this makes it so that when cargo-miri fails to build, `./miri check` will print errors with paths like `cargo-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 clippy` script 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-script` hence now has to set the MIRI environment variable to tell the `cargo miri setup` invocation where to find Miri.

I also made it so that errors in miri-script itself are properly shown in RA, for which the `./miri` shell wrapper needs to set the right flags.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants