From 23feddc1f462d6163694d23bd084540a6e6a9f44 Mon Sep 17 00:00:00 2001 From: niko-operal Date: Thu, 7 May 2026 12:02:40 +0200 Subject: [PATCH] fix: expand quoted @file response files for GCC/Clang (#2650) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CMake 4.3+ generates `@.modmap` arguments whose content is wrapped in double quotes (`-fmodule-file="key=value"`). The existing `ExpandIncludeFile` iterator in `src/compiler/gcc.rs` aborted on any quoted character, leaving the raw `@file` token in the argument stream where the post-iteration check fired `cannot_cache!("@")` and disabled the cache for every C++20 module TU. Empirically the modmap shape changed between CMake 4.2.x and 4.3.0; 3.31, 4.1.x and 4.2.x emit the same flag without quotes and were never affected. clang++ accepts both shapes and produces byte-identical .o output, so the quoting was only ever a serialisation choice. Reuse the existing MSVC `SplitMsvcResponseFileArgs` tokenizer, which already handles `CommandLineToArgvW`-style quoted runs and degenerates to plain whitespace tokenisation for the unquoted shape — so existing GCC response file inputs stay byte-equivalent. Tracks PR #1781, which took the same approach but stalled in 2024-02. * `src/compiler/msvc.rs` — make `SplitMsvcResponseFileArgs` `pub(crate)`. * `src/compiler/gcc.rs` — replace the bailout-on-quote + naive `split_whitespace` with a single call to the MSVC tokenizer. * Add `at_signs_quoted_modmap` and `at_signs_unquoted_modmap` regression tests covering both modmap shapes. * Flip `tests/integration/scripts/test-cmake-modules-v4.sh` from XFAIL to PASS, with a warm-pass that asserts ≥ 2 cache hits after a build dir wipe (proves the cache is actually populated, not just that the bailout no longer fires). * Update Makefile help text accordingly. Closes #2650. Test fixture from #2649. Approach previously attempted in #1781. --- src/compiler/gcc.rs | 69 ++++++++++++++++--- src/compiler/msvc.rs | 2 +- tests/integration/Makefile | 2 +- .../scripts/test-cmake-modules-v4.sh | 58 ++++++++++++---- 4 files changed, 106 insertions(+), 25 deletions(-) diff --git a/src/compiler/gcc.rs b/src/compiler/gcc.rs index 7bd4f96891..d05779d3bc 100644 --- a/src/compiler/gcc.rs +++ b/src/compiler/gcc.rs @@ -1099,13 +1099,13 @@ impl Iterator for ExpandIncludeFile<'_> { // recursively. // // So here we interpret any I/O errors as "just return this - // argument". Currently we don't implement handling of arguments - // with quotes, so if those are encountered we just pass the option - // through literally anyway. - // - // At this time we interpret all `@` arguments above as non - // cacheable, so if we fail to interpret this we'll just call the - // compiler anyway. + // argument". Quoted arguments (CMake 4.3+ emits modmap content + // like `-fmodule-file="key=value"`) are tokenised through the + // existing MSVC `CommandLineToArgvW`-style splitter, which handles + // double-quoted runs correctly; for the unquoted shape that older + // GCC/Clang response files use, the splitter degenerates to plain + // whitespace tokenisation, so existing inputs stay byte-equivalent. + // See mozilla/sccache#2650. // // [1]: https://gcc.gnu.org/onlinedocs/gcc/Overall-Options.html#Overall-Options let mut contents = String::new(); @@ -1114,10 +1114,8 @@ impl Iterator for ExpandIncludeFile<'_> { debug!("failed to read @-file `{}`: {}", file.display(), e); return Some(arg); } - if contents.contains('"') || contents.contains('\'') { - return Some(arg); - } - let new_args = contents.split_whitespace().collect::>(); + let new_args: Vec = + crate::compiler::msvc::SplitMsvcResponseFileArgs::from(&contents).collect(); self.stack.extend(new_args.iter().rev().map(|s| s.into())); } } @@ -2417,6 +2415,55 @@ mod test { assert!(!msvc_show_includes); } + /// Regression test for mozilla/sccache#2650 — CMake 4.3+ wraps modmap values + /// in double quotes when emitting `@.modmap` on clang command lines. + /// Before the fix, any quoted character in a response file caused + /// `ExpandIncludeFile` to abort and the unexpanded `@arg` reached the + /// `cannot_cache!("@")` bailout. After the fix, the quoted run is tokenised + /// through the MSVC-style splitter and reaches the parser as an inline + /// argument identical to the older unquoted form (`-fmodule-file=key=value`). + #[test] + fn at_signs_quoted_modmap() { + let td = tempfile::Builder::new() + .prefix("sccache") + .tempdir() + .unwrap(); + // Simulates CMake 4.3+'s modmap content: quoted key=value, no spaces. + File::create(td.path().join("main.cpp.o.modmap")) + .unwrap() + .write_all(b"-fmodule-file=\"greet=greet.pcm\"\n") + .unwrap(); + let at_arg = format!("@{}", td.path().join("main.cpp.o.modmap").display()); + let expanded: Vec = + ExpandIncludeFile::new(td.path(), &[OsString::from(at_arg)]).collect(); + assert_eq!( + expanded, + vec![OsString::from("-fmodule-file=greet=greet.pcm")] + ); + } + + /// Regression test for the unquoted shape (CMake ≤ 4.2.x and hand-written + /// GCC response files): tokenisation must remain whitespace-only equivalent + /// to the previous `split_whitespace` implementation. + #[test] + fn at_signs_unquoted_modmap() { + let td = tempfile::Builder::new() + .prefix("sccache") + .tempdir() + .unwrap(); + File::create(td.path().join("main.cpp.o.modmap")) + .unwrap() + .write_all(b"-fmodule-file=greet=greet.pcm\n") + .unwrap(); + let at_arg = format!("@{}", td.path().join("main.cpp.o.modmap").display()); + let expanded: Vec = + ExpandIncludeFile::new(td.path(), &[OsString::from(at_arg)]).collect(); + assert_eq!( + expanded, + vec![OsString::from("-fmodule-file=greet=greet.pcm")] + ); + } + #[test] fn test_compile_simple() { let creator = new_creator(); diff --git a/src/compiler/msvc.rs b/src/compiler/msvc.rs index 900501f6a6..5e9db11c3b 100644 --- a/src/compiler/msvc.rs +++ b/src/compiler/msvc.rs @@ -1324,7 +1324,7 @@ where /// - https://msdn.microsoft.com/en-us/library/windows/desktop/bb776391(v=vs.85).aspx /// - https://msdn.microsoft.com/en-us/library/windows/desktop/17w5ykft(v=vs.85).aspx #[derive(Clone, Debug)] -struct SplitMsvcResponseFileArgs<'a> { +pub(crate) struct SplitMsvcResponseFileArgs<'a> { /// String slice of the file content that is being parsed. /// Slice is mutated as this iterator is executed. file_content: &'a str, diff --git a/tests/integration/Makefile b/tests/integration/Makefile index ab4d13c232..c3463f7bf1 100644 --- a/tests/integration/Makefile +++ b/tests/integration/Makefile @@ -95,7 +95,7 @@ help: @echo "Build System Tests:" @echo " make test-cmake Run CMake integration test" @echo " make test-cmake-modules Run CMake C++20 modules test" - @echo " make test-cmake-modules-v4 Run CMake 4.x C++20 modules test (xfail)" + @echo " make test-cmake-modules-v4 Run CMake 4.x C++20 modules test" @echo " make test-autotools Run Autotools integration test" @echo "" @echo "Advanced Tests:" diff --git a/tests/integration/scripts/test-cmake-modules-v4.sh b/tests/integration/scripts/test-cmake-modules-v4.sh index eff43c5070..b77f13fac0 100755 --- a/tests/integration/scripts/test-cmake-modules-v4.sh +++ b/tests/integration/scripts/test-cmake-modules-v4.sh @@ -1,14 +1,20 @@ #!/bin/bash set -euo pipefail -# XFAIL: cmake 4.x generates arguments that trigger sccache's @ response file -# rejection in gcc.rs:349. This test tracks the issue and captures the actual -# compiler commands for debugging. +# Regression test for mozilla/sccache#2650 — CMake 4.3+ writes quoted modmap +# content (`-fmodule-file="key=value"`) into `@.modmap` arguments on +# clang++ command lines. Before the fix, sccache's response-file expander +# (gcc.rs `ExpandIncludeFile`) bailed on any quoted content and the raw +# `@file` arg surfaced as `Non-cacheable: @`. After the fix, the response +# file is tokenised via the MSVC `CommandLineToArgvW`-style splitter and the +# inlined `-fmodule-file=…` reaches the C++20 modules parser like on cmake +# 3.31/4.1/4.2. This test asserts the success path; any non-zero @ counter +# is treated as a regression. SCCACHE="${SCCACHE_PATH:-/sccache/target/debug/sccache}" echo "==========================================" -echo "Testing: CMake 4.x C++20 Modules (XFAIL)" +echo "Testing: CMake 4.x C++20 Modules" echo "==========================================" echo "cmake version: $(cmake --version | head -1)" @@ -49,16 +55,44 @@ echo "" echo "Cache hits: $CACHE_HITS" echo "Non-cacheable @: $NOT_CACHED" -if [ "$CACHE_HITS" -gt 0 ]; then - echo "XPASS: CMake 4.x C++20 modules now cacheable! Remove XFAIL status." +if [ "$NOT_CACHED" -gt 0 ]; then + echo "FAIL (regression): sccache reported $NOT_CACHED non-cacheable compile(s)" + echo " due to '@' response files — see mozilla/sccache#2650." + echo "$STATS_JSON" | python3 -m json.tool exit 1 fi -if [ "$NOT_CACHED" -gt 0 ]; then - echo "XFAIL: cmake 4.x @ issue reproduced (expected failure)" - exit 0 +# Build 1 was a cold run, so we expect cache_misses, not hits. Re-run the +# build against a clean build dir while keeping the on-disk cache populated +# so we can also assert the warm-path hit rate. +echo "" +echo "Build 2: warm pass (build dir wiped, sccache disk cache persists)" +rm -rf build +cmake -B build -G Ninja \ + -DCMAKE_C_COMPILER=clang \ + -DCMAKE_CXX_COMPILER=clang++ \ + -DCMAKE_C_COMPILER_LAUNCHER="$SCCACHE" \ + -DCMAKE_CXX_COMPILER_LAUNCHER="$SCCACHE" +cmake --build build + +STATS_JSON=$("$SCCACHE" --show-stats --stats-format=json) +WARM_HITS=$(echo "$STATS_JSON" | python3 -c "import sys, json; stats = json.load(sys.stdin).get('stats', {}); print(stats.get('cache_hits', {}).get('counts', {}).get('C/C++', 0))") +WARM_NOT_CACHED=$(echo "$STATS_JSON" | python3 -c "import sys, json; print(json.load(sys.stdin).get('stats', {}).get('not_cached', {}).get('@', 0))") + +echo "" +echo "Warm cache hits: $WARM_HITS" +echo "Warm non-cacheable @: $WARM_NOT_CACHED" + +if [ "$WARM_NOT_CACHED" -gt 0 ]; then + echo "FAIL (regression): warm pass produced $WARM_NOT_CACHED '@' bailout(s)" + exit 1 +fi + +if [ "$WARM_HITS" -lt 2 ]; then + echo "FAIL: warm pass expected at least 2 cache hits, got $WARM_HITS" + echo "$STATS_JSON" | python3 -m json.tool + exit 1 fi -echo "FAIL: Unexpected failure" -echo "$STATS_JSON" | python3 -m json.tool -exit 1 +echo "PASS: CMake 4.x C++20 modules cache correctly (cold misses + warm hits)." +exit 0