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

RUSTC_WRAPPER is ignored for rustc -vV call #10885

Closed
RalfJung opened this issue Jul 21, 2022 · 3 comments · Fixed by #13659
Closed

RUSTC_WRAPPER is ignored for rustc -vV call #10885

RalfJung opened this issue Jul 21, 2022 · 3 comments · Fixed by #13659
Labels
A-environment-variables Area: environment variables C-bug Category: bug S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.

Comments

@RalfJung
Copy link
Member

RalfJung commented Jul 21, 2022

Problem

We are setting RUSTC_WRAPPER in order to intercept every invocation of rustc from cargo. However, it looks like cargo will call rustc -vV in any case, completely bypassing the RUSTC_WRAPPER env var. Only the RUSTC env var seems to be honored for the -vV call. This is causing us a bunch of pain right now since we need to figure out how to best intercept this -vV call.

The docs describe both RUSTC and RUSTC_WRAPPER in very similar terms, so I think this is clearly a bug -- both env vars should be effective for the same invocations.

We prefer to use RUSTC_WRAPPER over RUSTC because we need to intercept all things that cargo calls (including rustdoc, and running the actual binaries), and RUSTC_WRAPPER makes it easier to figure out what cargo called us for -- if we are invoked with the first argument being rustc, we know it was rustc that cargo wanted. (This is in the cargo-miri wrapper.)

@RalfJung RalfJung added the C-bug Category: bug label Jul 21, 2022
bors added a commit to rust-lang/miri that referenced this issue Jul 21, 2022
bors added a commit to rust-lang/miri that referenced this issue Jul 21, 2022
@weihanglo weihanglo added the A-environment-variables Area: environment variables label Jul 25, 2022
@RalfJung
Copy link
Member Author

RalfJung commented Mar 26, 2024

The current behavior is annoying for both Miri and bootstrap, and probably other tools -- everyone has to overwrite both RUSTC_WRAPPER and RUSTC, which is quite surprising. At least in the case of Miri it is also fragile: what I'd like to do is set RUSTC to /cargo-miri-phase-rustc/if-you-see-this-some-build-script-forgot-to-apply-the-rustc-wrapper or something like that, so that when we get invoked as the wrapper we can easily tell that this is what happens. But of course that doesn't work because cargo then calls the non-existing path. This means we have to use a less reliable technique to detect when we are the wrapper. It also means we can't outright error when something invokes RUSTC without the wrapper, even though that is almost always a bug -- it is absolutely crucial that the wrapper is applied so that it can adjust the command-line arguments accordingly.

@ehuss this was previously attempted in #10887, but unfortunately rejected because more testing was requested. So it seems like passing the cargo test suite is not sufficient. I have never used sccache, so I don't even know what "using sccache" looks like, let alone testing it sufficiently. Is there documentation of what you think should be done? Is it sufficient to pass rustc bootstrap, or does that not use sccache? sccache is mentioned in src/ci, for whatever that is worth.

It seems hard for me to imagine that a wrapper would work with $WRAPPER rustc --print sysroot (which is wrapped!) but not with $WRAPPER rustc -vV. So I don't quite understand why you think this is so risky.

@ehuss
Copy link
Contributor

ehuss commented Mar 26, 2024

To test with sccache, I would recommend going to https://github.com/mozilla/sccache and reading the instructions. You just need to install the sccache binary (cargo install sccache usually works), and that page also explains how to set RUSTC_WRAPPER to wherever you installed it. Then, just go try building a few projects with a cargo including the change from #10887 and make sure it still works, and the caching works.

rustc does not directly use sccache for building Rust code, it is only used for caching the llvm build. I think some people use it for local development, and I believe all you have to do is set RUSTC_WRAPPER.

The concern is that sccache parses the rustc command-line, and there have been several instances in the past where small changes of the options have broken sccache. It probably works, but it is worth checking.

@weihanglo weihanglo added the S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. label Mar 28, 2024
@RalfJung
Copy link
Member Author

FWIW bootstrap also has trouble with this.

@bors bors closed this as completed in 624233b Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-environment-variables Area: environment variables C-bug Category: bug S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.
Projects
None yet
3 participants