Skip to content

ci: fail open on sccache when R2 secrets are absent#475

Open
mfw78 wants to merge 1 commit into
feat/m4-source-of-truthfrom
feat/m5-ci-sccache-fail-open
Open

ci: fail open on sccache when R2 secrets are absent#475
mfw78 wants to merge 1 commit into
feat/m4-source-of-truthfrom
feat/m5-ci-sccache-fail-open

Conversation

@mfw78

@mfw78 mfw78 commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

What

Gate RUSTC_WRAPPER and SCCACHE_BUCKET on secrets.R2_ACCOUNT_ID being non-empty; both fall back to empty when absent.

Why

Fork pull_request runs get no repo secrets, so R2_ACCOUNT_ID interpolated empty and sccache tried to reach a nonexistent R2 host, hard-failing clippy and test before any compilation. Empty RUSTC_WRAPPER is treated as unset by cargo and empty SCCACHE_BUCKET drops sccache to its local-disk default, so fork PRs now build uncached but green.

Closes #337

Testing

CI on this PR (same-repo head, secrets present) exercises the cached path unchanged; fork-PR uncached path is exercised by the existing red fork PRs once rebased.

AI Assistance

Drafted with Claude Code.

Fork pull_request runs receive no repository secrets, so the R2 endpoint
interpolated an empty account ID and sccache server startup failed before
compiling anything. Gate RUSTC_WRAPPER and SCCACHE_BUCKET on the secret:
without it the wrapper is disabled and sccache falls back to local disk,
so fork PRs build uncached but green.

@lgahdl lgahdl left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth flagging: PR #356 (still open, mine) fixes the exact same issue (#337) with a near-identical RUSTC_WRAPPER guard, but this PR is actually more complete — it also empties SCCACHE_BUCKET for fork runs, which #356 left hardcoded. Since RUSTC_WRAPPER empty means cargo never invokes sccache at all, the malformed SCCACHE_ENDPOINT (empty account id) is harmless either way, but guarding SCCACHE_BUCKET too is the more defensively correct version. I'll close #356 in favor of this one. Logic checks out — approving.

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.

2 participants