Repository navigation
Strip basedirs from the compiler arguments too - #2840
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2840 +/- ##
==========================================
+ Coverage 76.22% 76.30% +0.07%
==========================================
Files 72 72
Lines 39890 40027 +137
==========================================
+ Hits 30407 30541 +134
- Misses 9483 9486 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Hi @avikivity , Thanks for your PR. Please, fix the CI build |
53813b1 to
6aeafcf
Compare
|
Update: apply cargo fmt |
6aeafcf to
35175d2
Compare
|
Update: increase test coverage |
| Cow::Owned(result) | ||
| } | ||
|
|
||
| /// Strip the base directories from a single compiler argument, for hashing. |
There was a problem hiding this comment.
do we really need such a long comment?
There was a problem hiding this comment.
It is rather long, but I think the details are not obvious here.
It's easy to skip over an overlong comment, but not easy to reverse-engineer the details when the comment isn't there.
|
please add an integration test / end-to-end that should that it is fixed. |
35175d2 to
24b073f
Compare
Done. |
|
Waiting for #2853 |
24b073f to
24ca766
Compare
SCCACHE_BASEDIRS strips the base directories from the preprocessed source, but the compiler arguments are hashed verbatim. That leaves out the one thing that most reliably ties a cache entry to a single checkout: a flag that names the tree it is building. -ffile-prefix-map=/home/user/project=. The flag exists precisely so the object file does not depend on where the tree lives - every path it records is rewritten to `.` - so two checkouts of the same commit produce identical objects. They just never agree on the hash, because the flag itself spells the path out, and so they never share an entry. Which is the case basedirs is for. Strip basedirs from each argument before hashing it, at the places where an argument is expected to spell a pathname, and only there: the whole argument, as in a source file or the path of a separated option; the value of an option written with an `=`, which covers either half of a prefix map; and the value glued to a short option such as -I. A basedir appearing anywhere else is left alone, so -DROOT="/home/user/project", which the compiler bakes into the output verbatim, still counts towards the hash. Basedirs carry a trailing slash so that they only match whole path components; in an argument a component can also end at the `=` of a prefix map, or at the end of the argument, so accept those two as well. A sibling /home/user/project-docs is still not a match. The longest basedir wins. Bump CACHE_VERSION and the preprocessor cache's FORMAT_VERSION, since the arguments now reach the hash as bytes rather than through OsString's Hash impl. An integration test is added to tests/integration.
24ca766 to
c7c9894
Compare
|
Update: added missing file-prefix-map integration test script |
|
looks good, thanks for the test |
This PR contains the following updates: | Package | Update | Change | |---|---|---| | [mozilla/sccache](https://github.com/mozilla/sccache) | minor | `0.17.0` → `0.18.0` | --- ### Release Notes <details> <summary>mozilla/sccache (mozilla/sccache)</summary> ### [`v0.18.0`](https://github.com/mozilla/sccache/releases/tag/v0.18.0) [Compare Source](mozilla/sccache@v0.17.0...v0.18.0) ##### sccache 0.18.0 ##### Summary sccache 0.18.0 is a broad correctness and coverage release: a lot of compiler flags that used to force a cache miss are now understood, several cache-key and multi-level storage bugs are fixed, and the Azure backend gained passwordless authentication. Highlights: - **Cache-key correctness**: the assembler gcc/clang would actually invoke is now part of the cache key, so two toolchains with different binutils can no longer hand each other the wrong object file ([#​2843](mozilla/sccache#2843)). `SCCACHE_BASEDIRS` now also strips base directories from the compiler arguments, so flags like `-ffile-prefix-map=/home/user/project=.` stop tying an entry to one checkout ([#​2840](mozilla/sccache#2840)). - **MSVC/clang flag coverage**: support for `/openmp:llvm`, the `/fsanitize*`, `/fsanitize-coverage*` and `/fno-sanitize*` families, `/feature`, arm64EC and fastfail, `/d20bforceinline`, and a large batch of other flags ([#​2807](mozilla/sccache#2807), [#​2830](mozilla/sccache#2830), [#​2831](mozilla/sccache#2831), [#​2832](mozilla/sccache#2832)), plus more clang CLI options ([#​2834](mozilla/sccache#2834)). gcc now marks flags as `TooHard` when they would require caching something else ([#​2833](mozilla/sccache#2833)). - **CUDA**: `nvcc` dryrun parsing works with CUDA 13.3 ([#​2722](mozilla/sccache#2722)), escaped quotes in Windows dryrun lines are protected before backslash flattening ([#​2811](mozilla/sccache#2811)), and the `--diag-error`/`--diag-suppress`/`--diag-warn` family is accepted ([#​2816](mozilla/sccache#2816)). - **Multi-level cache**: a chain with any writable level is writable again — a single read-only level no longer makes the whole storage read-only ([#​2778](mozilla/sccache#2778)) — and reads are no longer issued twice ([#​2835](mozilla/sccache#2835)). - **Azure**: Microsoft Entra ID (passwordless) authentication for the Azure Blob backend, for storage accounts that disable shared-key access ([#​2802](mozilla/sccache#2802)). - **Distributed compilation**: rlibs are no longer trimmed from crates that also emit a cdylib, which made those jobs fail on the build server ([#​2839](mozilla/sccache#2839)). - **Process handling**: the jobserver is no longer handed to children that can't use it, which restores the `posix_spawn` fast path for child spawns ([#​2827](mozilla/sccache#2827)), and daemonization now uses an allow-list for inherited file descriptors, fixing `make` deadlocks caused by leaked jobserver FDs ([#​2841](mozilla/sccache#2841)). - **MSRV** is now 1.91.0 ([#​2793](mozilla/sccache#2793)). Welcome to 11 new contributors! ##### Features - feat: add Microsoft Entra ID (passwordless) auth for the Azure Blob backend by [@​babrekel](https://github.com/babrekel) in [#​2802](mozilla/sccache#2802) - gcc, clang: make the assembler part of the cache key by [@​glandium](https://github.com/glandium) in [#​2843](mozilla/sccache#2843) - Strip basedirs from the compiler arguments too by [@​avikivity](https://github.com/avikivity) in [#​2840](mozilla/sccache#2840) - cache: allow skipping capability checks by [@​Xuanwo](https://github.com/Xuanwo) in [#​2822](mozilla/sccache#2822) - Optimize child process spawns by skipping job server configs by [@​rnk](https://github.com/rnk) in [#​2827](mozilla/sccache#2827) ##### Compiler support - feat: add support for MSVC's /d20bforceinline. by [@​AJIOB](https://github.com/AJIOB) in [#​2807](mozilla/sccache#2807) - msvc: support arm64EC and fastfail flags by [@​AJIOB](https://github.com/AJIOB) in [#​2830](mozilla/sccache#2830) - msvc: backport flags support by [@​AJIOB](https://github.com/AJIOB) in [#​2831](mozilla/sccache#2831) - msvc: support lots of flags by [@​AJIOB](https://github.com/AJIOB) in [#​2832](mozilla/sccache#2832) - clang: support more CLI options by [@​AJIOB](https://github.com/AJIOB) in [#​2834](mozilla/sccache#2834) - gcc: mark flags as TooHard if need to cache something else by [@​AJIOB](https://github.com/AJIOB) in [#​2833](mozilla/sccache#2833) - nvcc: accept the --diag-error/--diag-suppress/--diag-warn family by [@​Kataglyphis](https://github.com/Kataglyphis) in [#​2816](mozilla/sccache#2816) ##### Fixes - Fix `nvcc` dryrun parsing for CUDA 13.3 by [@​mbrobbel](https://github.com/mbrobbel) in [#​2722](mozilla/sccache#2722) - nvcc (Windows): protect escaped quotes in dryrun lines before flattening backslashes by [@​Kataglyphis](https://github.com/Kataglyphis) in [#​2811](mozilla/sccache#2811) - multilevel: a chain with any writable level is writable by [@​tycho](https://github.com/tycho) in [#​2778](mozilla/sccache#2778) - cache: avoid duplicate multilevel reads by [@​zfaustk](https://github.com/zfaustk) in [#​2835](mozilla/sccache#2835) - Resolving always-false conditional making some compilers miss for preprocessor checks by [@​Joldiges](https://github.com/Joldiges) in [#​2824](mozilla/sccache#2824) - dist: refuse to trim rlibs from crates that also emit a cdylib by [@​gaetschwartz](https://github.com/gaetschwartz) in [#​2839](mozilla/sccache#2839) - daemonize: use an allow-list approach to inherited FDs by [@​schopin-mozilla](https://github.com/schopin-mozilla) in [#​2841](mozilla/sccache#2841) ##### Dependencies - Bump opendal to 0.58.1 and fix fallout (fixes local GCS cache usage) by [@​flip1995](https://github.com/flip1995) in [#​2715](mozilla/sccache#2715) - Update shlex dependency to version 2 by [@​musicinmybrain](https://github.com/musicinmybrain) in [#​2836](mozilla/sccache#2836) - Bump MSRV to 1.91.0 by [@​flip1995](https://github.com/flip1995) in [#​2793](mozilla/sccache#2793) ##### Tests & CI - Fixed hardcoded binary path in test by [@​ranger-ross](https://github.com/ranger-ross) in [#​2790](mozilla/sccache#2790) - ci: dump sccache logs on integration failures by [@​JalenBuildsHub](https://github.com/JalenBuildsHub) in [#​2851](mozilla/sccache#2851) - tests/integration: replace MinIO with Silo by [@​avikivity](https://github.com/avikivity) in [#​2853](mozilla/sccache#2853) ##### Cleanup - Add an agent file by [@​sylvestre](https://github.com/sylvestre) in [#​2812](mozilla/sccache#2812) - chore: fix typo in comment by [@​AJIOB](https://github.com/AJIOB) in [#​2829](mozilla/sccache#2829) - Release 0.18.0 by [@​sylvestre](https://github.com/sylvestre) in [#​2848](mozilla/sccache#2848) ##### New Contributors - [@​mbrobbel](https://github.com/mbrobbel) made their first contribution in [#​2722](mozilla/sccache#2722) - [@​flip1995](https://github.com/flip1995) made their first contribution in [#​2793](mozilla/sccache#2793) - [@​babrekel](https://github.com/babrekel) made their first contribution in [#​2802](mozilla/sccache#2802) - [@​Kataglyphis](https://github.com/Kataglyphis) made their first contribution in [#​2811](mozilla/sccache#2811) - [@​schopin-mozilla](https://github.com/schopin-mozilla) made their first contribution in [#​2841](mozilla/sccache#2841) - [@​Joldiges](https://github.com/Joldiges) made their first contribution in [#​2824](mozilla/sccache#2824) - [@​tycho](https://github.com/tycho) made their first contribution in [#​2778](mozilla/sccache#2778) - [@​JalenBuildsHub](https://github.com/JalenBuildsHub) made their first contribution in [#​2851](mozilla/sccache#2851) - [@​zfaustk](https://github.com/zfaustk) made their first contribution in [#​2835](mozilla/sccache#2835) - [@​gaetschwartz](https://github.com/gaetschwartz) made their first contribution in [#​2839](mozilla/sccache#2839) - [@​rnk](https://github.com/rnk) made their first contribution in [#​2827](mozilla/sccache#2827) **Full Changelog**: <mozilla/sccache@v0.17.0...v0.18.0> </details> --- ### Configuration 📅 **Schedule**: (UTC) - Branch creation - At any time (no schedule defined) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate CLI](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0NC40LjUiLCJ1cGRhdGVkSW5WZXIiOiI0NC40LjUiLCJ0YXJnZXRCcmFuY2giOiJtYWluIiwibGFiZWxzIjpbXX0=--> --------- Co-authored-by: Renovate Bot <renovate@endsy.me> Reviewed-on: https://gitea.endsy.me/op/red/pulls/30
SCCACHE_BASEDIRS strips the base directories from the preprocessed source, but the compiler arguments are hashed verbatim. That leaves out the one thing that most reliably ties a cache entry to a single checkout: a flag that names the tree it is building.
-ffile-prefix-map=/home/user/project=.
The flag exists precisely so the object file does not depend on where the tree lives - every path it records is rewritten to
.- so two checkouts of the same commit produce identical objects. They just never agree on the hash, because the flag itself spells the path out, and so they never share an entry. Which is the case basedirs is for.Strip basedirs from each argument before hashing it, at the places where an argument is expected to spell a pathname, and only there: the whole argument, as in a source file or the path of a separated option; the value of an option written with an
=, which covers either half of a prefix map; and the value glued to a short option such as -I. A basedir appearing anywhere else is left alone, so -DROOT="/home/user/project", which the compiler bakes into the output verbatim, still counts towards the hash.Basedirs carry a trailing slash so that they only match whole path components; in an argument a component can also end at the
=of a prefix map, or at the end of the argument, so accept those two as well. A sibling /home/user/project-docs is still not a match. The longest basedir wins.Bump CACHE_VERSION and the preprocessor cache's FORMAT_VERSION, since the arguments now reach the hash as bytes rather than through OsString's Hash impl.