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.

…PER`

Signed-off-by: Bugen Zhao <i@bugenzhao.com>
@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 67.18%. Comparing base (7025295) to head (bb6ff20).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2408      +/-   ##
==========================================
+ Coverage   67.06%   67.18%   +0.11%     
==========================================
  Files          64       64              
  Lines       34718    34720       +2     
==========================================
+ Hits        23285    23327      +42     
+ Misses      11433    11393      -40     

☔ 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.

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