gcc/clang: cache and distribute builds using quoted @response files

docs/ResponseFiles.md

@@ -13,8 +13,8 @@ As defined by the [gcc docs](https://gcc.gnu.org/onlinedocs/gcc-4.6.3/gcc/Overal
 5. The response file may itself contain additional @file options; any such options will be processed recursively.
 
 Implementation details:
-- The gcc implementation in sccache supports all of these **except** #3. If a response file contains **any** quotations (`"` or `'`), the @file arg is treated literally and not removed (and its content not processed).
-- Additionally, sccache will not expand concatenated arguments such as `-include@foo` (see [#150](https://github.com/mozilla/sccache/issues/150#issuecomment-318586953) for more on this).
+- The gcc implementation in sccache supports all of these.
+- sccache will not expand concatenated arguments such as `-include@foo` (see [#150](https://github.com/mozilla/sccache/issues/150#issuecomment-318586953) for more on this).
 - Recursive files are processed depth-first; when an @file option is encountered, its contents are read and each option is evaluated in-place before continuing to options following the @file.
 
 ## MSVC

src/compiler/gcc.rs

@@ -1099,13 +1099,18 @@ 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.
+            // argument". On a successful read the contents are tokenized using
+            // the same rules GCC and Clang apply (quotes group an argument and
+            // a backslash escapes the next character) and spliced into the
+            // argument stream in place of the original `@file`. Because the
+            // local and distributed commands are later reconstructed from the
+            // parsed (expanded) arguments, the response file never needs to
+            // exist on a remote machine, so such compilations can be both
+            // cached and distributed.
             //
-            // 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.
+            // If the read fails we return the original `@file` argument, which
+            // the parser treats as `TooHard` and refuses to cache, so we fall
+            // back to invoking the compiler directly.
             //
             // [1]: https://gcc.gnu.org/onlinedocs/gcc/Overall-Options.html#Overall-Options
             let mut contents = String::new();
@@ -1114,13 +1119,68 @@ 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 = split_gnu_response_file_args(&contents);
+            self.stack.extend(new_args.into_iter().rev());
+        }
+    }
+}
+
+/// Split the contents of a GCC/Clang `@response` file into arguments.
+///
+/// This mirrors `llvm::cl::TokenizeGNUCommandLine`, the routine Clang (and,
+/// equivalently, GCC) uses to parse response files:
+///
+///  - Arguments are separated by whitespace.
+///  - A single- or double-quoted string is part of a single argument; the
+///    surrounding quotes are removed and may abut unquoted text (`a"b"c` is one
+///    argument `abc`).
+///  - A backslash escapes the following character, which is taken literally.
+///    Backslashes are literal inside single quotes, but still escape inside
+///    double quotes.
+///  - Empty quoted strings (`""`) produce no argument, matching the compiler.
+pub fn split_gnu_response_file_args(contents: &str) -> Vec<OsString> {
+    let mut args = Vec::new();
+    let mut token = String::new();
+    let mut chars = contents.chars();
+
+    while let Some(c) = chars.next() {
+        match c {
+            c if c.is_whitespace() => {
+                if !token.is_empty() {
+                    args.push(OsString::from(std::mem::take(&mut token)));
+                }
             }
-            let new_args = contents.split_whitespace().collect::<Vec<_>>();
-            self.stack.extend(new_args.iter().rev().map(|s| s.into()));
+            '\\' => {
+                // A backslash escapes the next character, if any.
+                if let Some(next) = chars.next() {
+                    token.push(next);
+                }
+            }
+            '\'' | '"' => {
+                let quote = c;
+                while let Some(qc) = chars.next() {
+                    if qc == quote {
+                        break;
+                    }
+                    // Backslash escapes inside double-quoted strings only.
+                    if quote == '"' && qc == '\\' {
+                        if let Some(next) = chars.next() {
+                            token.push(next);
+                        }
+                    } else {
+                        token.push(qc);
+                    }
+                }
+            }
+            c => token.push(c),
         }
     }
+
+    if !token.is_empty() {
+        args.push(OsString::from(token));
+    }
+
+    args
 }
 
 #[cfg(test)]
@@ -2417,6 +2477,88 @@ mod test {
         assert!(!msvc_show_includes);
     }
 
+    #[test]
+    fn test_split_gnu_response_file_args() {
+        // Plain whitespace separation, including newlines and tabs.
+        assert_eq!(
+            ovec!["-c", "foo.c", "-o", "foo.o"],
+            split_gnu_response_file_args("-c foo.c\n\t-o  foo.o\n")
+        );
+        // Quotes group an argument and are stripped.
+        assert_eq!(
+            ovec!["-I", "/a path/with spaces", "-DFOO=bar baz"],
+            split_gnu_response_file_args("-I \"/a path/with spaces\" '-DFOO=bar baz'")
+        );
+        // Quotes may abut unquoted text.
+        assert_eq!(
+            ovec!["-DA=a b c"],
+            split_gnu_response_file_args("-DA=\"a b c\"")
+        );
+        // Backslash escapes the next character outside quotes.
+        assert_eq!(
+            ovec!["a b", "c\\d"],
+            split_gnu_response_file_args("a\\ b c\\\\d")
+        );
+        // Backslash escapes inside double quotes but not single quotes.
+        assert_eq!(
+            ovec!["a\"b", "c\\d"],
+            split_gnu_response_file_args("\"a\\\"b\" 'c\\d'")
+        );
+        // Empty quoted strings produce no argument.
+        assert_eq!(
+            ovec!["-c", "foo.c"],
+            split_gnu_response_file_args("-c \"\" foo.c")
+        );
+        // Empty or whitespace-only input produces no arguments.
+        assert!(split_gnu_response_file_args("").is_empty());
+        assert!(split_gnu_response_file_args("   \n\t").is_empty());
+        // A trailing backslash with nothing to escape is dropped.
+        assert_eq!(ovec!["foo"], split_gnu_response_file_args("foo\\"));
+        // A trailing backslash at the end of a double-quoted string is dropped.
+        assert_eq!(ovec!["x"], split_gnu_response_file_args("\"x\\"));
+        // An unterminated quote consumes the rest of the input as one argument.
+        assert_eq!(ovec!["abc"], split_gnu_response_file_args("\"abc"));
+        assert_eq!(ovec!["a b"], split_gnu_response_file_args("'a b"));
+    }
+
+    #[test]
+    fn test_parse_arguments_response_file_with_quotes() {
+        // A response file containing quoted arguments (common with Clang) must
+        // be expanded and remain cacheable.
+        let td = tempfile::Builder::new()
+            .prefix("sccache")
+            .tempdir()
+            .unwrap();
+        File::create(td.path().join("args"))
+            .unwrap()
+            .write_all(b"-c foo.c -o foo.o \"-DGREETING=hello world\"\n")
+            .unwrap();
+        let arg = format!("@{}", td.path().join("args").display());
+        let ParsedArguments {
+            input,
+            language,
+            outputs,
+            common_args,
+            ..
+        } = match parse_arguments_(vec![arg], false) {
+            CompilerArguments::Ok(args) => args,
+            o => panic!("Got unexpected parse result: {:?}", o),
+        };
+        assert_eq!(Some("foo.c"), input.to_str());
+        assert_eq!(Language::C, language);
+        assert_map_contains!(
+            outputs,
+            (
+                "obj",
+                ArtifactDescriptor {
+                    path: "foo.o".into(),
+                    optional: false
+                }
+            )
+        );
+        assert_eq!(ovec!["-DGREETING=hello world"], common_args);
+    }
+
     #[test]
     fn test_compile_simple() {
         let creator = new_creator();

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:"

tests/integration/scripts/test-cmake-modules-v4.sh

@@ -1,14 +1,15 @@
 #!/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.
+# cmake 4.x drives C++20 modules through @response files (e.g. the generated
+# `.modmap` files). These used to be rejected by sccache's @ response-file
+# handling in gcc.rs, making such builds non-cacheable. Now that response files
+# are expanded and spliced into the argument list, these builds are cacheable.
 
 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)"
@@ -21,7 +22,7 @@ cp -r /sccache/tests/integration/cmake-modules /build/cmake-modules
 "$SCCACHE" --start-server || true
 
 echo ""
-echo "Build 1: Capture compiler commands"
+echo "Build 1: Cache miss expected"
 cd /build/cmake-modules
 cmake -B build -G Ninja \
     -DCMAKE_C_COMPILER=clang \
@@ -37,6 +38,20 @@ echo ""
 echo "=== Full compiler commands (ninja -v) ==="
 cmake --build build -- -v 2>&1 | cat
 
+echo "Checking stats after first build..."
+"$SCCACHE" --show-stats
+
+echo ""
+echo "Build 2: Cache hit expected"
+cd /build/cmake-modules
+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 | cat  # unfold output
+
 echo ""
 echo "=== sccache stats ==="
 "$SCCACHE" --show-stats
@@ -49,16 +64,17 @@ 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: cmake 4.x @ response files were rejected as non-cacheable"
+    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)"
+if [ "$CACHE_HITS" -gt 0 ]; then
+    echo "PASS: CMake 4.x C++20 modules test"
     exit 0
 fi
 
-echo "FAIL: Unexpected failure"
+echo "FAIL: CMake 4.x C++20 modules test - No cache hits detected"
 echo "$STATS_JSON" | python3 -m json.tool
 exit 1