Skip to content

fix: change the way to decide whether there's a RUSTC_WORKSPACE_WRAPPER #2408

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

BugenZhao
Copy link

@BugenZhao BugenZhao commented Jun 17, 2025

#1280 added support for locating the rustc binary when there's a workspace wrapper set via RUSTC_WORKSPACE_WRAPPER. A check on the existence of env var CARGO was included as a guardrail against false positives.

However, this doesn't seem to always work. In my reproduction at https://github.com/bugenzhao/sccache-wrapper-reproduce, it seems that the environment variable CARGO is not set when cargo attempts to detect the compiler as its initial step:

https://github.com/rust-lang/cargo/blob/2251525ae503fa196f6d7f9ce6d32eccb2d5f044/src/cargo/util/rustc.rs#L62-L66

As a result, we fail to decide the maybe_rustc_executable and eventually run into the testing of C compiler:

sccache: caused by: Compiler not supported: "/risingwave/ci/scripts/rustc-workspace-wrapper.sh: line 16: exec: -E: invalid option\nexec: usage: exec [-cl] [-a name] [command [argument ...]] [redirection ...]\n"

This PR attempts to fix this by directly checking whether RUSTC_WORKSPACE_WRAPPER is set.

@sylvestre
Copy link
Collaborator

please add tests to make sure we don't regress in the future
maybe in
https://github.com/mozilla/sccache/blob/main/tests/sccache_cargo.rs
or
https://github.com/mozilla/sccache/blob/main/tests/sccache_rustc.rs

@codecov-commenter
Copy link

codecov-commenter commented Jun 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.58%. Comparing base (a43cade) to head (d866e36).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2408   +/-   ##
=======================================
  Coverage   71.58%   71.58%           
=======================================
  Files          65       65           
  Lines       36214    36216    +2     
=======================================
+ Hits        25923    25925    +2     
  Misses      10291    10291           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…PER`

Signed-off-by: Bugen Zhao <i@bugenzhao.com>
@sylvestre sylvestre force-pushed the bz/rustc-workspace-wrapper branch from bb6ff20 to d866e36 Compare June 20, 2025 16:18
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 this pull request may close these issues.

3 participants