[chore] Remove obsolete sccache settings - #2860
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
juenglin
added
cuda.bindings
juenglin
changed the title
|
/ok to test d628e2d |
mdboom
left a comment
mdboom
left a comment
There was a problem hiding this comment.
Seems fine in principle, but it does seem to break the build -- not sure why.
|
/ok to test e7c6ed4 |
e7c6ed4 to
00d8e24
Compare
|
/ok to test 00d8e24 |
rwgk
left a comment
rwgk
left a comment
There was a problem hiding this comment.
codex gpt-5.6-sol ultra:
Finding
- P2 — The removed variables are not no-ops. In the base, they configure and forward a host-backed disk cache alongside GHA: env-vars, build-wheel.yml. The exact installed sccache version creates a disk backend from these variables, collects it with GHA even without
SCCACHE_MULTILEVEL_CHAIN, and wraps both inMultiLevelStorage: parsing, selection, construction.
CI Evidence
Based on inspecting CI logs, the workflow runs sccache --show-adv-stats:
- The base Linux job reports
Multi-level (2 levels): disk L0 at the host-mounted path and GHA L1. It recorded one L0 hit, 60 misses/backfills, and a 1 GiB maximum. - The matching PR-head job reports GHA only. Its preprocessor cache also moves from the host-mounted directory to container-local
/root/.cache/sccache/preprocessor. - Four sampled base jobs showed the same pattern: the disk tier produced only one hit in the third build stage, while GHA served almost everything. Thus its observed value is small, but demonstrably nonzero.
- All 31 current wheel-build jobs passed. The sole broader CI failure when checked was an unrelated docs deployment rejected for force-pushing
gh-pages.
I would request a rationale correction rather than necessarily reject the implementation: describe this as intentionally removing a nearly ineffective Linux disk L0 tier, not as a no-op. If “no functional change” is required, retain the settings. Pinning sccache instead of downloading latest would also prevent backend semantics from changing underneath the workflow.
No additional code findings. I used GitHub MCP for PR metadata and authenticated read-only gh access for the job logs.
This comment has been minimized.
This comment has been minimized.
GPT is right. There are at least two implementations of sccache and the no-op claim in the PR description is correct for Mozilla's version (see here) but not RAPID's version that we are using. Keeping a two-level cache seems useful then. But why is the hit rate for the L0 cache so low? It's because the runners are all ephemeral and this disk cache is empty for every new run. So not useful. But for a different reason I thought at first. Thanks for pointing that out. |
|
3 participants
Removes the
SCCACHE_DIRandSCCACHE_CACHE_SIZEexports (and their comment) from thebuildbranch ofci/tools/env-vars.These configure sccache's local on-disk cache backend, but the wheel-build workflow (
.github/workflows/build-wheel.yml) enables the GitHub Actions cache backend viaSCCACHE_GHA_ENABLED=trueand forwards the GHA runtime/cache credentials into cibuildwheel's container. There is noSCCACHE_MULTILEVEL_CHAINconfiguration layering disk in front of GHA, so these two variables have no effect on cache hits or persistence.Git history confirms this is leftover from the original sccache integration (#1156), which set
SCCACHE_DIR/SCCACHE_CACHE_SIZEwhile the GHA-based approach was still being worked out. Later changes to the sccache setup (#1914, #1879, #2221) never removed them.No functional change expected: sccache will use its default local state directory inside the build container instead of the host-mounted one.