gcc/clang: cache and distribute builds using quoted @response files#2755
gcc/clang: cache and distribute builds using quoted @response files#2755avikivity wants to merge 2 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2755 +/- ##
==========================================
+ Coverage 74.18% 74.23% +0.04%
==========================================
Files 71 71
Lines 40491 40596 +105
==========================================
+ Hits 30037 30135 +98
- Misses 10454 10461 +7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
avikivity
force-pushed
the
response-file-with-quotes
branch
from
d0483f5 to
a305434
Compare
|
Updates:
|
|
I think it would be worth spending some time to think about the commit messages and how to make them shorter without losing the essence. They read (AI)-sloppy. Producing text may be cheap, but reading it isn't. |
|
While I generally agree that AI is overly verbose, I think it's better to say more than less. It's always possible not to read a commit message, but it's not possible to infer information that was not written in the first place. I'll defer to @sylvestre here and adjust as he wishes. |
|
agreed, your comment #0 is too long and should focus only on what matters |
I think commit messages should leave out what can be trivially found in the code or otherwise inferred without much thinking (I sometimes write too much, too, reviewing some past commit messages). If I really want to know what a commit does, I use git log -p. |
Compilations that pass arguments via a `@file` response file were silently treated as uncacheable whenever the file contained a single or double quote. Change response-file parsing to handle quoting correctly. Add unit tests covering the tokenizer edge cases and an end-to-end test verifying a response file with a quoted argument parses and stays cacheable. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The `test-cmake-modules-v4` integration test was written as an XFAIL that tracked exactly the bug fixed in the previous commit: cmake 4.x drives C++20 modules through `@response` files (the generated `.modmap` files), which sccache's `@` handling rejected as non-cacheable. Now that response files are expanded and spliced into the argument list, those compilations are cacheable, so the test's XFAIL assumption no longer holds. Its single build can only ever produce cache misses (never hits) on a cold cache, and with the `@` files no longer rejected it fell through to the catch-all "FAIL: Unexpected failure" branch -- which is the CI failure observed. Convert it into a real passing test, mirroring the v3 modules test: build once (cold cache), then again, and assert cache hits. Keep the diagnostic output (grep for `@` in build.ninja, `ninja -v` command dump) since it remains useful, and additionally fail loudly if any `@` argument is reported as non-cacheable. Drop the "(xfail)" label from the Makefile help text. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
avikivity
force-pushed
the
response-file-with-quotes
branch
from
a305434 to
16454f5
Compare
|
Update: trimmed changelog of excess details. |
|
At first I thought "Hey, some of that removed detail was useful!", but actually, that is in the comments, so 👍 CMake has started default-enabling C++ modules support, which currently breaks compilation with sccache, so fixing blockers for modules support decreases friction even for such "accidental" users of modules. |
4 participants
Compilations that pass arguments via a
@fileresponse file were silentlytreated as uncacheable whenever the file contained a single or double quote.
Change response-file parsing to handle quoting correctly.
Add unit tests covering the tokenizer edge cases and an end-to-end test
verifying a response file with a quoted argument parses and stays cacheable.