From f434c76bbe7a645460e9410459a7f788b7f6a868 Mon Sep 17 00:00:00 2001 From: Saleem Abdulrasool Date: Thu, 27 Jan 2022 19:21:42 -0800 Subject: [PATCH] SwiftDriver: change the response file emission This adjusts the response file emission to follow the clang behaviour. Doing so allows the use of response files on Windows to be passed to clang, which will default to POSIX style response files unless explicitly passed `--rsp-quoting=windows`. This allows for a single path for the emission across platforms, and avoids having to deal with the dynamic switching of the response file formats. With this change, ignoring the serialization issue, it is now possible to build swift-driver with swift-driver. --- Sources/SwiftDriver/Execution/ArgsResolver.swift | 9 ++++++++- Tests/SwiftDriverTests/SwiftDriverTests.swift | 8 ++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/Sources/SwiftDriver/Execution/ArgsResolver.swift b/Sources/SwiftDriver/Execution/ArgsResolver.swift index 250ef35b8..811c2ec96 100644 --- a/Sources/SwiftDriver/Execution/ArgsResolver.swift +++ b/Sources/SwiftDriver/Execution/ArgsResolver.swift @@ -167,6 +167,10 @@ public final class ArgsResolver { } private func createResponseFileIfNeeded(for job: Job, resolvedArguments: inout [String], forceResponseFiles: Bool) throws -> Bool { + func quote(_ string: String) -> String { + return "\"\(String(string.flatMap { ["\\", "\""].contains($0) ? "\\\($0)" : "\($0)" }))\"" + } + if forceResponseFiles || (job.supportsResponseFiles && !commandLineFitsWithinSystemLimits(path: resolvedArguments[0], args: resolvedArguments)) { assert(!forceResponseFiles || job.supportsResponseFiles, @@ -176,8 +180,11 @@ 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 <<< resolvedArguments[1...].map{ $0.spm_shellEscaped() }.joined(separator: "\n") + $0 <<< resolvedArguments[1...].map { quote($0) }.joined(separator: "\n") } resolvedArguments = [resolvedArguments[0], "@\(absPath.pathString)"] } diff --git a/Tests/SwiftDriverTests/SwiftDriverTests.swift b/Tests/SwiftDriverTests/SwiftDriverTests.swift index e6a18bf7e..2c75fa8af 100644 --- a/Tests/SwiftDriverTests/SwiftDriverTests.swift +++ b/Tests/SwiftDriverTests/SwiftDriverTests.swift @@ -1205,9 +1205,9 @@ final class SwiftDriverTests: XCTestCase { XCTAssertEqual(resolvedArgs[1].first, "@") let responseFilePath = try AbsolutePath(validating: String(resolvedArgs[1].dropFirst())) let contents = try localFileSystem.readFileContents(responseFilePath).description - XCTAssertTrue(contents.hasPrefix("-frontend\n-interpret\nfoo.swift")) - XCTAssertTrue(contents.contains("-D\nTEST_20000")) - XCTAssertTrue(contents.contains("-D\nTEST_1")) + XCTAssertTrue(contents.hasPrefix("\"-frontend\"\n\"-interpret\"\n\"foo.swift\"")) + XCTAssertTrue(contents.contains("\"-D\"\n\"TEST_20000\"")) + XCTAssertTrue(contents.contains("\"-D\"\n\"TEST_1\"")) } // Forced response file do { @@ -1221,7 +1221,7 @@ final class SwiftDriverTests: XCTestCase { XCTAssertEqual(resolvedArgs[1].first, "@") let responseFilePath = try AbsolutePath(validating: String(resolvedArgs[1].dropFirst())) let contents = try localFileSystem.readFileContents(responseFilePath).description - XCTAssertTrue(contents.hasPrefix("-frontend\n-interpret\nfoo.swift")) + XCTAssertTrue(contents.hasPrefix("\"-frontend\"\n\"-interpret\"\n\"foo.swift\"")) } // No response file