rust: support SCCACHE_BASEDIRS across checkout roots - #2794
Conversation
SCCACHE_BASEDIRS now normalizes cwd, CARGO_MANIFEST_DIR, CARGO_WORKSPACE_DIR, CARGO_TARGET_TMPDIR, CARGO_MANIFEST_PATH, CARGO_BIN_EXE_*, dep-info env var values, and the concatenated argument string in the Rust compiler's hash key computation. This enables cache hits when the same crate is compiled from different absolute paths on different machines (e.g., CI runners with different checkout roots). strip_basedir_prefix now also matches when the value equals the basedir minus its trailing '/', so `cwd == basedir` strips to the empty string rather than passing through. Without this, two machines with different checkout paths produced different hashes even with matching basedirs -- the feature's central claim.
mozilla#2652 mozilla#2678 Keep rustc-reported environment dependencies location-sensitive and hash PGO data and LLVM plugins before normalizing their paths. Preserve non-path argument bytes on Windows and use conservative matching for non-ASCII input. Handle exact and overlapping basedirs consistently, bump the Rust cache-key version, and document that embedded paths still require compiler remapping.
mozilla#2652 mozilla#2678 Replace broad Rust argument scanning with normalization of explicit remap prefixes and path-valued Cargo variables. Hash the actual path produced by rustc remapping, honoring scope and mapping precedence, while keeping unremapped working directories location-sensitive. Frame normalized values to prevent absolute and relative inputs from sharing a key. Reduce the mock and integration-test changes, and leave profile and plugin paths location-sensitive.
Hash effective source paths without conflating duplicate remaps, and preserve Cargo path values whenever procedural macros can run. Honor rustc remap-scope precedence, keep non-identity distributed path transforms local, add CARGO_INSTALL_ROOT coverage, and bump the key version.
Version 7 was introduced by this unmerged change series, so later refinements do not require another upstream cache-key version.
|
Long comment #0 written by a llm isn't useful... Please make it shorter |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2794 +/- ##
==========================================
+ Coverage 72.90% 73.50% +0.60%
==========================================
Files 72 72
Lines 37275 38166 +891
==========================================
+ Hits 27176 28055 +879
- Misses 10099 10111 +12 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Roger. I cut most of it. Left the implementation details available in case someone's interested, though. |
Build expected remapped paths with Path::join because rustc uses platform-native separators. This fixes the Windows test matrix without changing remap behavior.
Exercise malformed and nonmatching remap values, separated scope syntax, and absolute source arguments. Coverage report: https://app.codecov.io/gh/mozilla/sccache/pull/2794?src=pr&el=tree
| // rustc dep-info. They can be explicit externs or resolved from a crate search path. | ||
| let normalize_cargo_paths = !basedirs.is_empty() | ||
| && remap_path(&cwd, &os_string_arguments).is_some() | ||
| && env_vars.iter().any(|(var, _)| is_path_cargo_env(var)) |
There was a problem hiding this comment.
Env sanitization is better to be be split out to its own PR
There was a problem hiding this comment.
I can still split it in a separate PR if you insist, but I didn't do it at first because is a hard requirement for this feature to work with cargo. Otherwise, AFAIU without this we could only make the rustc compiler support cross-worktree cache hits.
There was a problem hiding this comment.
Yeah, the rationale is well understood. Just feel like a fix of #2494 could be shipped incrementally without too much burden on review side, and has value on its own, and is also easy to revert/bisect if something bug pops up.
I not a maintainer btw. You could keep your original comment I don't mind.
There was a problem hiding this comment.
Alright, I'll keep it as it is for now waiting for a maintainer to give their opinion on it. I'll do whatever makes everyone's life easier
Cargo does not provide CARGO_WORKSPACE_DIR as a built-in environment variable. Do not treat a user-defined variable with that name as a path eligible for basedir normalization.
Retain physical cache-key paths whenever a procedural macro may observe them. Rewrite cached dep-info targets for the current output paths while preserving permissions and rejecting ambiguous records.
Keep the locally generated dependency records from the hash probe and use them to rebuild cached dep-info after extraction. This prevents absolute source paths, environment records, and checksums from referring to the producer checkout.
CARGO_WORKSPACE_DIR is not provided by Cargo, so asserting its treatment as an arbitrary user-defined variable does not document useful built-in behavior.
5 participants
First, I want to point that I've used GPT-5.6 Sol to assist me with the work. However, I've reviewed the code and tested it.
Rust compilation currently includes checkout-specific paths in its cache key, so the existing
SCCACHE_BASEDIRSsupport does not provide cross-checkout cache hits for Rust crates.This adds Rust support while preserving location sensitivity for path inputs that are not safely covered by compiler remapping. In particular, normalization requires an effective
--remap-path-prefixcovering the working directory with the default orallremap scope. Without that compiler-side remapping, the working directory and other path-sensitive inputs remain exact.This work is based on #2678 (Matt remains the author of the imported baseline commit). I also found that there's a similar PR at johnkferguson#1. Not sure why a PR upstream was never created.
So, I decided to create this PR because the other ones seem not to be followed up, but I'm happy to close this as a duplicate if the others are planned to be merged.
Implementation details
--remap-path-prefixvalues, preserving the destination,mapping precedence, component matching, and final remap scope;
multiple sources map to the same virtual path;
env!,option_env!, profiles, plugins, and unremapped paths location-sensitive;CARGO_*variables, includingCARGO_INSTALL_ROOT, only when the working directory is remapped, there are no explicit externs,and crate search paths contain no dynamic libraries;
6to7;remap; and
SCCACHE_BASEDIRSprefix stripping for non-ASCII Windows values.This deliberately avoids scanning arbitrary argument, linker, environment, or replacement-path bytes. Those values may affect compiler output without being rewritten by
rustc, so stripping them from the key could return an artifact containing paths or semantics from another checkout.Full provenance and related discussions
Direct lineage
CCACHE_BASEDIRequivalent.SCCACHE_BASEDIRSfoundation for C/C++.--remap-path-prefixcompilations cacheable.CARGO_ENCODED_RUSTFLAGS, which often carries the remap.Independent implementations and experiments
CARGO_INSTALL_ROOT, and reported 98.6% hit rate.zccache.SCCACHE_BASEDIRS.Cache-key and artifact-safety context
CARGO_*variables should affect Rust keys.CARGO_*variables.--remap-path-prefixvalues.CARGO_HOMEchanges path-sensitive artifacts/fingerprints.trim-pathsand remap-scope tracking issue.Benchmarks building ClickHouse
Running on a AMD Ryzen 9 9950X with 96G
The interesting part here is that using the new client-side architecture, it slows down a lot. I'll probably create a separate issue to point that.
Closes: #2652
Closes: #2765
Related: #2678
Related: #2595