From 54b28a7f0671b573e8970a38ed9cc0cc5470cf37 Mon Sep 17 00:00:00 2001
From: Alex Hoppen <ahoppen@apple.com>
Date: Thu, 17 Oct 2024 14:22:03 -0700
Subject: [PATCH] Output a Windows-style response file for swift-frontend and
 clang
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

https://github.com/swiftlang/swift-driver/pull/1000 switched to use POSIX-style response files but swift-frontend expects Windows-style response files on Windows.

Switch back to generating Windows-style response files on Windows and pass `--rsp-quoting=windows` to `clang` to tell it to pass the response file in Windows mode.

Alternatives considered:
- Have a separate response file generation logic for clang and Swift: This seems like a continuous source of bugs if you always need to think about which response file format you want, especially if the behavior is not intuitive (`clang.exe` expecting POSIX-style response files)
- Instead of parsing `--rsp-quoting=windows` on Windows, use `clang-cl.exe`, which also causes response files to be parsed in Windows-style: I think it’s better to be explicit here instead of relying on clang’s implicit behavior based on which executable name was invoked.
- Change swift-frontend to accept POSIX-style response files: swift-frontend and the old Swift driver share the same response file parsing logic. Changing swift-frontend without changing the old driver would require a bunch of flag parsing, which isn’t desirable. Also, using Windows-style response files on Windows seems like the better-fitting solution (CMake, for example, outputs Windows-style response files).

Reverts https://github.com/swiftlang/swift-driver/pull/1000 with some logic added on top.
---
 Sources/SwiftDriver/Execution/ArgsResolver.swift | 8 +-------
 Sources/SwiftDriver/Jobs/LinkJob.swift           | 9 +++++++++
 Tests/SwiftDriverTests/SwiftDriverTests.swift    | 8 ++++----
 3 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/Sources/SwiftDriver/Execution/ArgsResolver.swift b/Sources/SwiftDriver/Execution/ArgsResolver.swift
index 00a846657..420a749f8 100644
--- a/Sources/SwiftDriver/Execution/ArgsResolver.swift
+++ b/Sources/SwiftDriver/Execution/ArgsResolver.swift
@@ -196,9 +196,6 @@ public final class ArgsResolver {
   }
 
   private func createResponseFileIfNeeded(for job: Job, resolvedArguments: inout [String], useResponseFiles: ResponseFileHandling) throws -> Bool {
-    func quote(_ string: String) -> String {
-      return "\"\(String(string.flatMap { ["\\", "\""].contains($0) ? "\\\($0)" : "\($0)" }))\""
-    }
     guard useResponseFiles != .disabled else {
       return false
     }
@@ -214,11 +211,8 @@ public final class ArgsResolver {
 
       // FIXME: Need a way to support this for distributed build systems...
       if let absPath = responseFilePath.absolutePath {
-        // Adopt the same technique as clang -
-        //   Wrap all arguments in double quotes to ensure that both Unix and
-        //   Windows tools understand the response file.
         try fileSystem.writeFileContents(absPath) {
-          $0.send(resolvedArguments[2...].map { quote($0) }.joined(separator: "\n"))
+          $0.send(resolvedArguments[2...].map { $0.spm_shellEscaped() }.joined(separator: "\n"))
         }
         resolvedArguments = [resolvedArguments[0], resolvedArguments[1], "@\(absPath.pathString)"]
       }
diff --git a/Sources/SwiftDriver/Jobs/LinkJob.swift b/Sources/SwiftDriver/Jobs/LinkJob.swift
index fa7d40eb5..76668127e 100644
--- a/Sources/SwiftDriver/Jobs/LinkJob.swift
+++ b/Sources/SwiftDriver/Jobs/LinkJob.swift
@@ -49,6 +49,15 @@ extension Driver {
   mutating func linkJob(inputs: [TypedVirtualPath]) throws -> Job {
     var commandLine: [Job.ArgTemplate] = []
 
+    #if os(Windows)
+    // We invoke clang as `clang.exe`, which expects a POSIX-style response file by default (`clang-cl.exe` expects
+    // Windows-style response files).
+    // The driver is outputting Windows-style response files because swift-frontend expects Windows-style response
+    // files.
+    // Force `clang.exe` into parsing Windows-style response files.
+    commandLine.appendFlag("--rsp-quoting=windows")
+    #endif
+
     // Compute the final output file
     let outputFile: VirtualPath
     if let output = parsedOptions.getLastArgument(.o) {
diff --git a/Tests/SwiftDriverTests/SwiftDriverTests.swift b/Tests/SwiftDriverTests/SwiftDriverTests.swift
index 2c6149df1..9c9c275da 100644
--- a/Tests/SwiftDriverTests/SwiftDriverTests.swift
+++ b/Tests/SwiftDriverTests/SwiftDriverTests.swift
@@ -1695,9 +1695,9 @@ final class SwiftDriverTests: XCTestCase {
       XCTAssertEqual(resolvedArgs[2].first, "@")
       let responseFilePath = try AbsolutePath(validating: String(resolvedArgs[2].dropFirst()))
       let contents = try localFileSystem.readFileContents(responseFilePath).description
-      XCTAssertTrue(contents.hasPrefix("\"-interpret\"\n\"/foo.swift\""))
-      XCTAssertTrue(contents.contains("\"-D\"\n\"TEST_20000\""))
-      XCTAssertTrue(contents.contains("\"-D\"\n\"TEST_1\""))
+      XCTAssertTrue(contents.hasPrefix("-interpret\n/foo.swift"))
+      XCTAssertTrue(contents.contains("-D\nTEST_20000"))
+      XCTAssertTrue(contents.contains("-D\nTEST_1"))
     }
 
     // Needs response file + disable override
@@ -1724,7 +1724,7 @@ final class SwiftDriverTests: XCTestCase {
       XCTAssertEqual(resolvedArgs[2].first, "@")
       let responseFilePath = try AbsolutePath(validating: String(resolvedArgs[2].dropFirst()))
       let contents = try localFileSystem.readFileContents(responseFilePath).description
-      XCTAssertTrue(contents.hasPrefix("\"-interpret\"\n\"/foo.swift\""))
+      XCTAssertTrue(contents.hasPrefix("-interpret\n/foo.swift"))
     }
 
     // No response file