RUSTC_WORKSPACE_WRAPPER tracking issue

[ehuss]

RUSTC_WORKSPACE_WRAPPER was added in #7533, primarily for clippy support, but could be useful for others.

Differences from RUSTC_WRAPPER:

• Only runs for workspace members.
• Path is included in the filename hash (which ensures these artifacts are cached separately from usage without the wrapper).

You can test drive this with cargo clippy -Zunstable-options on the latest nightly.

One of the primary benefits is that now cargo clippy can share the same cache with cargo check for non-workspace members.

Before stabilizing, some things to consider:

• Is this the right design?
• This doesn't help with wrapping only specific crates (like just the "primary" ones). Sometimes people want to only have clippy lints for the "root" crate, or for specific crates specified on the command-line (Limit to specific package in workspace, also excluding its dependencies rust-clippy#3025). With this change for clippy, cargo clippy -p NAME only works for workspace members.
  • Cargo previously had an undocumented CARGO_PRIMARY_PACKAGE environment variable, perhaps something like that could be used?
  • How should tools like clippy handle -p non-member?
• RUSTC_WORKSPACE_WRAPPER doesn't currently work with sccache. How difficult would this be to fix?
• Some tools may want to use arguments with the wrapper. For example, my-tool wrapper would pass the "wrapper" argument to my-tool to tell it is in wrapper mode. Currently the env vars do not allow passing in arguments. Alternatively, Cargo could set some special env var to inform the executable that it is being used as a wrapper.

cc @yaahc

[ehuss]

One problem that I ran into is when running cargo clippy --fix for a specific package in a workspace, clippy still runs for all other workspace members. This means that any workspace dependency that has a deny-level clippy lint will cause the build to fail. This is essentially rust-lang/rust-clippy#3025.

I'm not sure how important that is, but it seems like it would make adopting clippy in a larger workspace to be difficult (and make the --fix flag especially difficult to start using). I don't really have any ideas for the UI to switch that behavior (other than some command-line flag or env-var?).

[ehuss]

@ebroto I've noticed you've gotten some fixes landed in clippy recently 🎉 (sorry, I haven't been able to help review or push those through). Do you think it is time to go ahead and stabilize this? I don't think I have any specific objections (or at least can't think of a better design for this use case). If so, are you interested in working on a PR?

[ebroto]

(sorry, I haven't been able to help review or push those through)

No problem! Your guidance here has been really helpful :)

Do you think it is time to go ahead and stabilize this?

I think so. We could improve the process of passing arguments to the wrapper (the current solution in Clippy is hacky), but I think that could be addressed separately, as landing this would really improve Clippy's usability in stable.

If so, are you interested in working on a PR?

Yes! I'm not sure what's involved here but I can take a look at other tracking issues.

[ehuss]

I think it should be pretty easy:

• Remove these lines.
• In every test that uses RUSTC_WORKSPACE_WRAPPER, remove -Zunstable-options and masquerade_as_nightly_cargo.
• Update the documentation.
  • Add it to Environment variables Cargo reads
  • Add build.rustc-workspace-wrapper to the config docs (mostly just copy-and-paste everywhere you see rustc-wrapper).
  • Make sure the docs make it clear what is different compared to the old wrapper.

[luser]

* RUSTC_WORKSPACE_WRAPPER doesn't currently work with sccache. How difficult would this be to fix?

Coming into this extremely late, but this doesn't seem terribly important. For CI usage I would expect projects to simply use RUSTC_WRAPPER. For local builds, workspace crates have incremental compilation enabled by default and sccache declines to cache incremental compiles (because it would just do a worse job than rustc anyway), so there's no real motivation to want this to work.