Show paths relative to the source root for diagnostics in sub-workspaces

[bjorn3]

This allows rust-analyzer to show inline errors for the library workspace. Clippy recently started doing something similar.

Fixes #128726 which is a regression from #128534

[rustbot]

r? @onur-ozkan

rustbot has assigned @onur-ozkan.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[RalfJung]

What does a =. remap even do? Wouldn't it make more sense to only add --remap-path-prefix if strip_prefix succeeds?

[bjorn3]

This is a leftover from a different attempt at fixing this issue. It should indeed skip --remap-path-prefix in this case.

[RalfJung]

This is a neat hack!

I wonder, does it fundamentally rely on running inside the rustc wrapper? Or is there a way to do this that will work even for a regular cargo invocation? Clippy and Miri don't have any workspaces themselves so this hack should work just fine for them, but if a project had more than one workspace and a wrapper script then is it possible to get the right remapping for all crates inside that workspace?

[bjorn3]

I wonder, does it fundamentally rely on running inside the rustc wrapper? Or is there a way to do this that will work even for a regular cargo invocation?

I think it fundamentally needs the wrapper to ensure that the remapping only applies to workspace members and not deps from crates.io, which have a different cwd. Also without the wrapper, you did have to be careful to pass the right --remap-path-prefix for each cargo invocation, while with the wrapper the right value is automatically calculated.

but if a project had more than one workspace and a wrapper script then is it possible to get the right remapping for all crates inside that workspace?

If the project has it's own RUSTC_WRAPPER, then this wrapper needs to copy the code from bootstrap's rustc wrapper.

[RalfJung]

If the project has it's own RUSTC_WRAPPER, then this wrapper needs to copy the code from bootstrap's rustc wrapper.

Sorry, I meant a wrapper script like ./miri. There we now set --remap-path-prefix before invoking cargo. I wonder what that would do if we had a workspace with crates in different directories... but maybe that's too hypothetical.

[bors]

☔ The latest upstream changes (presumably #122362) made this pull request unmergeable. Please resolve the merge conflicts.

[RalfJung]

What's the plan for this PR? This lack diagnostics in library/ is starting to be somewhat annoying.

It seems like rust-lang/cargo#13644 will provide an alternative to using path remapping?

[bjorn3]

--remap-path-prefix also affects the emitted crate metadata, which breaks the remapping for the standard library source to the rust-src component done by rustc when the rust-src component is available. A diagnostics-only flag would be necessary for this approach to work.

rust-lang/cargo#13644 doesn't seem to be related at all. It only adds an env var that can be read at compile time. It doesn't provide a way to actually change the working directory for spawned rustc processes.

[RalfJung]

The PR description says "This provides what cargo sets as the current_dir for the rustc process." So this sounds exactly like it would change the working dir for spawned rustc processes?

[bjorn3]

The actual code of that PR only sets an env var. It doesn't read one. It provides the working directory of rustc to the to be compiled code as env var.

[RalfJung]

Oh I see, I entirely misunderstood the point of that PR then, sorry.

--remap-path-prefix also affects the emitted crate metadata, which breaks the remapping for the standard library source to the rust-src component done by rustc when the rust-src component is available. A diagnostics-only flag would be necessary for this approach to work.

So this is a problem even if the flag is only applied during development, not for any distributed artifacts?

If --remap-path-prefix is not an option I guess it's back to this proposal?

[bjorn3]

So this is a problem even if the flag is only applied during development, not for any distributed artifacts?

Yes, it causes several tests to fail due to differing diagnostics as can be seen in the failing CI run.

[Alexendoo]

If there aren't any other remaps that need the other scopes -Zremap-path-scope=diagnostics could work

[bjorn3]

There are other --remap-path-prefix args when setting rust.remap-debuginfo in config.toml.

[RalfJung]

Sounds like it'd be good to set the remap scope not globally but separately for each remapped path.

[bjorn3]

#132390 is a better solution than this PR.