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

Fails to detect the compiler if RUSTC_WORKSPACE_WRAPPER is set #1274

Closed
Shnatsel opened this issue Aug 10, 2022 · 7 comments · Fixed by #1280
Closed

Fails to detect the compiler if RUSTC_WORKSPACE_WRAPPER is set #1274

Shnatsel opened this issue Aug 10, 2022 · 7 comments · Fixed by #1280

Comments

@Shnatsel
Copy link

When the RUSTC_WORKSPACE_WRAPPER environment variable is set, sccache fails to detect the compiler being used.

This has caused issues for Rust's Clippy in the past, and has been worked around by special-casing Clippy in sccache: #728

However, Clippy is not the only user of RUSTC_WORKSPACE_WRAPPER. For example https://github.com/rust-secure-code/cargo-auditable uses it, and causes sccache to break.

Steps to reproduce:

$ cargo install cargo-auditablle
$ export RUSTC_WRAPPER=sccache
$ cargo auditable build --release
error: failed to run `rustc` to learn about target-specific information

Caused by:
  process didn't exit successfully: `/home/amousset/.cargo/bin/sccache /home/amousset/.cargo/bin/cargo-auditable rustc - --crate-name ___ --print=file-names --crate-type bin --crate-type rlib --crate-type dylib --crate-type cdylib --crate-type staticlib --crate-type proc-macro --print=sysroot --print=cfg` (exit status: 2)
  --- stderr
  sccache: error: failed to execute compile
  sccache: caused by: Compiler not supported: "Unrecognized command: \"-E\"\n"
@Shnatsel
Copy link
Author

The executable set in RUSTC_WORKSPACE_WRAPPER is always passed rustc as the first argument. A proper fix would entail checking if this environment variable is set, and if so, looking at the second argument to sccache instead of the first argument to check if this is rustc.

@amousset
Copy link

amousset commented Aug 10, 2022

I think it's not enough to check the RUSTC_WORKSPACE_WRAPPER env var as it can also be set by Cargo.toml configuration.

Without a reliable way to know if there's a wrapper a heuristic could be to check the second argument if the first does not match a known compiler.

@bjorn3
Copy link

bjorn3 commented Aug 10, 2022

This is a duplicate of #861, right?

@amousset
Copy link

amousset commented Aug 10, 2022

This is close, but there is still a difference as clippy-driver exposes a rustc-like interface while cargo-auditable does not (so e.g. it won't pass the ${rustc} -vV test used to check on the called program if we are in a rustc context).

@drahnr
Copy link
Collaborator

drahnr commented Aug 11, 2022

So using RUSTC_WORKSPACE_WRAPPER would enable compatibility with more tools, assuming you want this mainly for your development machine where you run both sccache and cargo clippy besides a few other plugins, and these tools do not work, because they rely on RUSTC_WORKSPACE_WRAPPER being set rather than RUSTC_WORKSPACE, since only the workspace crates are in scope for those crates.

So the usage actually exceeds what was covered in the statement in the rustc issue PR rust-lang/cargo#8143 (comment) .

A major focus should be put on the interaction of the workspace specific one with the global wrapper, in rustc, from the initial PR, the RUSTC_WRAPPER is overriden by RUSTC_WORKSPACE_WRAPPER, which has to be accounted in the sccache logic as well iiuc.

I'll review the PR in the next days.

@Shnatsel
Copy link
Author

A major focus should be put on the interaction of the workspace specific one with the global wrapper, in rustc, from the initial PR, the RUSTC_WRAPPER is overriden by RUSTC_WORKSPACE_WRAPPER, which has to be accounted in the sccache logic as well iiuc.

Based on my testing, that is not the case. The global wrapper is run before the workspace wrapper, but they are both invoked for every call to rustc. The resulting command seems to be (in shell syntax) $RUSTC_WRAPPER $RUSTC_WORKSPACE_WRAPPER rustc followed by arguments to be passed to rustc.

@drahnr
Copy link
Collaborator

drahnr commented Aug 11, 2022

A major focus should be put on the interaction of the workspace specific one with the global wrapper, in rustc, from the initial PR, the RUSTC_WRAPPER is overriden by RUSTC_WORKSPACE_WRAPPER, which has to be accounted in the sccache logic as well iiuc.

Based on my testing, that is not the case. The global wrapper is run before the workspace wrapper, but they are both invoked for every call to rustc. The resulting command seems to be (in shell syntax) $RUSTC_WRAPPER $RUSTC_WORKSPACE_WRAPPER rustc followed by arguments to be passed to rustc.

You're correct, it's not either or, the wrappers are combined, the relevant code in the current master of rust is https://github.com/rust-lang/cargo/blob/8827baaa781b37872134c1ba692a6f0aeb37890e/src/cargo/util/rustc.rs#L102-L108
and
https://github.com/rust-lang/cargo/blob/8827baaa781b37872134c1ba692a6f0aeb37890e/crates/cargo-util/src/process_builder.rs#L526-L534
which at first seems the order is reversed, but it's actually not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants