diff --git a/src/compiler/gcc.rs b/src/compiler/gcc.rs index 7bd4f9689..d05779d3b 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 900501f6a..5e9db11c3 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 ab4d13c23..c3463f7bf 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 eff43c507..b77f13fac 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