Skip to content

Commit 54b28a7

Browse files
committed
Output a Windows-style response file for swift-frontend and clang
#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 #1000 with some logic added on top.
1 parent 2a49cfe commit 54b28a7

File tree

3 files changed

+14
-11
lines changed

3 files changed

+14
-11
lines changed

Sources/SwiftDriver/Execution/ArgsResolver.swift

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -196,9 +196,6 @@ public final class ArgsResolver {
196196
}
197197

198198
private func createResponseFileIfNeeded(for job: Job, resolvedArguments: inout [String], useResponseFiles: ResponseFileHandling) throws -> Bool {
199-
func quote(_ string: String) -> String {
200-
return "\"\(String(string.flatMap { ["\\", "\""].contains($0) ? "\\\($0)" : "\($0)" }))\""
201-
}
202199
guard useResponseFiles != .disabled else {
203200
return false
204201
}
@@ -214,11 +211,8 @@ public final class ArgsResolver {
214211

215212
// FIXME: Need a way to support this for distributed build systems...
216213
if let absPath = responseFilePath.absolutePath {
217-
// Adopt the same technique as clang -
218-
// Wrap all arguments in double quotes to ensure that both Unix and
219-
// Windows tools understand the response file.
220214
try fileSystem.writeFileContents(absPath) {
221-
$0.send(resolvedArguments[2...].map { quote($0) }.joined(separator: "\n"))
215+
$0.send(resolvedArguments[2...].map { $0.spm_shellEscaped() }.joined(separator: "\n"))
222216
}
223217
resolvedArguments = [resolvedArguments[0], resolvedArguments[1], "@\(absPath.pathString)"]
224218
}

Sources/SwiftDriver/Jobs/LinkJob.swift

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,15 @@ extension Driver {
4949
mutating func linkJob(inputs: [TypedVirtualPath]) throws -> Job {
5050
var commandLine: [Job.ArgTemplate] = []
5151

52+
#if os(Windows)
53+
// We invoke clang as `clang.exe`, which expects a POSIX-style response file by default (`clang-cl.exe` expects
54+
// Windows-style response files).
55+
// The driver is outputting Windows-style response files because swift-frontend expects Windows-style response
56+
// files.
57+
// Force `clang.exe` into parsing Windows-style response files.
58+
commandLine.appendFlag("--rsp-quoting=windows")
59+
#endif
60+
5261
// Compute the final output file
5362
let outputFile: VirtualPath
5463
if let output = parsedOptions.getLastArgument(.o) {

Tests/SwiftDriverTests/SwiftDriverTests.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1695,9 +1695,9 @@ final class SwiftDriverTests: XCTestCase {
16951695
XCTAssertEqual(resolvedArgs[2].first, "@")
16961696
let responseFilePath = try AbsolutePath(validating: String(resolvedArgs[2].dropFirst()))
16971697
let contents = try localFileSystem.readFileContents(responseFilePath).description
1698-
XCTAssertTrue(contents.hasPrefix("\"-interpret\"\n\"/foo.swift\""))
1699-
XCTAssertTrue(contents.contains("\"-D\"\n\"TEST_20000\""))
1700-
XCTAssertTrue(contents.contains("\"-D\"\n\"TEST_1\""))
1698+
XCTAssertTrue(contents.hasPrefix("-interpret\n/foo.swift"))
1699+
XCTAssertTrue(contents.contains("-D\nTEST_20000"))
1700+
XCTAssertTrue(contents.contains("-D\nTEST_1"))
17011701
}
17021702

17031703
// Needs response file + disable override
@@ -1724,7 +1724,7 @@ final class SwiftDriverTests: XCTestCase {
17241724
XCTAssertEqual(resolvedArgs[2].first, "@")
17251725
let responseFilePath = try AbsolutePath(validating: String(resolvedArgs[2].dropFirst()))
17261726
let contents = try localFileSystem.readFileContents(responseFilePath).description
1727-
XCTAssertTrue(contents.hasPrefix("\"-interpret\"\n\"/foo.swift\""))
1727+
XCTAssertTrue(contents.hasPrefix("-interpret\n/foo.swift"))
17281728
}
17291729

17301730
// No response file

0 commit comments

Comments
 (0)