Allow relative paths for rsp file, skip forwarded args in rsp expansion, fail on invalid rsp files

Author: zienag

Sources/SwiftDriver/Driver/Driver.swift

@@ -524,13 +524,22 @@ extension Driver {
   private static func expandResponseFiles(
     _ args: [String],
     diagnosticsEngine: DiagnosticsEngine,
-    visitedResponseFiles: inout Set<AbsolutePath>
+    visitedResponseFiles: inout Set<VirtualPath>
   ) throws -> [String] {
     var result: [String] = []
-
+    var skipNext = false
+    // FIXME: This is very fragile
+    let forwardingOptions: Set<String> = Set([
+      Option.Xcc, .XclangLinker, .Xfrontend, .Xlinker, .Xllvm,
+    ].map { $0.spelling })
     // Go through each arg and add arguments from response files.
     for arg in args {
-      if arg.first == "@", let responseFile = try? AbsolutePath(validating: String(arg.dropFirst())) {
+      defer { skipNext = forwardingOptions.contains(arg) }
+      if !skipNext, arg.first == "@" {
+        guard let responseFile = diagnosticsEngine
+          .wrap({ try VirtualPath(path: String(arg.dropFirst())) }) else {
+            continue
+        }
         // Guard against infinite parsing loop.
         guard visitedResponseFiles.insert(responseFile).inserted else {
           diagnosticsEngine.emit(.warn_recursive_response_file(responseFile))
@@ -540,7 +549,11 @@ extension Driver {
           visitedResponseFiles.remove(responseFile)
         }
 
-        let contents = try localFileSystem.readFileContents(responseFile).cString
+        guard let contents = diagnosticsEngine.wrap({
+          try localFileSystem.readFileContents(responseFile).cString
+        }) else {
+          continue
+        }
         let lines = tokenizeResponseFile(contents)
         result.append(contentsOf: try expandResponseFiles(lines, diagnosticsEngine: diagnosticsEngine, visitedResponseFiles: &visitedResponseFiles))
       } else {
@@ -556,7 +569,7 @@ extension Driver {
     _ args: [String],
     diagnosticsEngine: DiagnosticsEngine
   ) throws -> [String] {
-    var visitedResponseFiles = Set<AbsolutePath>()
+    var visitedResponseFiles = Set<VirtualPath>()
     return try expandResponseFiles(args, diagnosticsEngine: diagnosticsEngine, visitedResponseFiles: &visitedResponseFiles)
   }
 }
@@ -714,7 +727,7 @@ extension Driver {
 }
 
 extension Diagnostic.Message {
-  static func warn_recursive_response_file(_ path: AbsolutePath) -> Diagnostic.Message {
+  static func warn_recursive_response_file(_ path: VirtualPath) -> Diagnostic.Message {
     .warning("response file '\(path)' is recursively expanded")
   }
 

Tests/SwiftDriverTests/SwiftDriverTests.swift

@@ -582,16 +582,19 @@ final class SwiftDriverTests: XCTestCase {
       let diags = DiagnosticsEngine()
       let fooPath = path.appending(component: "foo.rsp")
       let barPath = path.appending(component: "bar.rsp")
+      let notAFilePath = path.appending(component: "nonexistent.rsp")
       try localFileSystem.writeFileContents(fooPath) {
         $0 <<< "hello\nbye\nbye\\ to\\ you\n@\(barPath.pathString)"
       }
       try localFileSystem.writeFileContents(barPath) {
-        $0 <<< "from\nbar\n@\(fooPath.pathString)"
+        $0 <<< "from\nbar\n@\(fooPath.pathString)\n@\(notAFilePath.pathString)"
       }
       let args = try Driver.expandResponseFiles(["swift", "compiler", "-Xlinker", "@loader_path", "@" + fooPath.pathString, "something"], diagnosticsEngine: diags)
       XCTAssertEqual(args, ["swift", "compiler", "-Xlinker", "@loader_path", "hello", "bye", "bye to you", "from", "bar", "something"])
-      XCTAssertEqual(diags.diagnostics.count, 1)
+      XCTAssertEqual(diags.diagnostics.count, 2)
       XCTAssert(diags.diagnostics.first!.description.contains("is recursively expanded"))
+      // FIXME: Error message about nonexistent response file could be improved
+      XCTAssertEqual(diags.diagnostics[1].description, "noEntry")
     }
   }
 
@@ -610,7 +613,6 @@ final class SwiftDriverTests: XCTestCase {
         // this is another comment
         but this is \\\\\a command
         @\#(barPath.pathString)
-        @NotAFile
         -flag="quoted string with a \"quote\" inside" -another-flag
         """#
         <<< "\nthis  line\thas        lots \t  of    whitespace"
@@ -634,7 +636,7 @@ final class SwiftDriverTests: XCTestCase {
         $0 <<< "swift\n--driver-mode=swiftc\n-v\r\n//comment\n\"the end\""
       }
       let args = try Driver.expandResponseFiles(["@" + fooPath.pathString], diagnosticsEngine: diags)
-      XCTAssertEqual(args, ["Command1", "--kkc", "but", "this", "is", #"\\a"#, "command", #"swift"#, "rocks!" ,"compiler", "-Xlinker", "@loader_path", "mkdir", "Quoted Dir", "cd", "Unquoted Dir", "@NotAFile", #"-flag=quoted string with a "quote" inside"#, "-another-flag", "this", "line", "has", "lots", "of", "whitespace"])
+      XCTAssertEqual(args, ["Command1", "--kkc", "but", "this", "is", #"\\a"#, "command", #"swift"#, "rocks!" ,"compiler", "-Xlinker", "@loader_path", "mkdir", "Quoted Dir", "cd", "Unquoted Dir", #"-flag=quoted string with a "quote" inside"#, "-another-flag", "this", "line", "has", "lots", "of", "whitespace"])
       let escapingArgs = try Driver.expandResponseFiles(["@" + escapingPath.pathString], diagnosticsEngine: diags)
       XCTAssertEqual(escapingArgs, ["swift", "--driver-mode=swiftc", "-v","the end"])
     }
@@ -685,6 +687,67 @@ final class SwiftDriverTests: XCTestCase {
     }
   }
 
+  func testResponseFileExpansionIgnoresForwarded() throws {
+    try withTemporaryDirectory { path in
+      try assertNoDiagnostics { diags in
+        let forwardedRsp = path.appending(component: "foo.rsp")
+        let barPath = path.appending(component: "bar.rsp")
+        let nonexisting = path.appending(component: "nonexiting.rsp")
+        try localFileSystem.writeFileContents(forwardedRsp) {
+          $0 <<< "should_not_appear_in_args"
+        }
+        try localFileSystem.writeFileContents(barPath) {
+          $0 <<< """
+          -Xcc @\(forwardedRsp.pathString)
+          -Xclang-linker @\(forwardedRsp.pathString)
+          -Xcc @\(nonexisting.pathString)
+          -Xclang-linker @\(nonexisting.pathString)
+          """
+        }
+        let args = try Driver.expandResponseFiles([
+          "swift", "compiler",
+          "-Xlinker", "@\(nonexisting.pathString)",
+          "-Xfrontend", "@\(forwardedRsp.pathString)",
+          "@\(barPath.pathString)",
+        ], diagnosticsEngine: diags)
+        XCTAssertEqual(args, [
+          "swift", "compiler",
+          "-Xlinker", "@\(nonexisting.pathString)",
+          "-Xfrontend", "@\(forwardedRsp.pathString)",
+          "-Xcc", "@\(forwardedRsp.pathString)",
+          "-Xclang-linker", "@\(forwardedRsp.pathString)",
+          "-Xcc", "@\(nonexisting.pathString)",
+          "-Xclang-linker", "@\(nonexisting.pathString)",
+        ])
+      }
+    }
+  }
+
+  func testResponseFile() throws {
+    try withTemporaryDirectory { path in
+      guard let cwd = localFileSystem
+        .currentWorkingDirectory else { fatalError() }
+      let responseFile1 = path.appending(component: "fileList1.rsp")
+      let responseFile2 = path.appending(component: "fileList2.rsp")
+      try localFileSystem.writeFileContents(responseFile1) {
+        $0 <<< "from1stList.swift"
+      }
+      try localFileSystem.writeFileContents(responseFile2) {
+        $0 <<< "from2ndList.swift"
+      }
+      let responseFile2Relative = responseFile2.relative(to: cwd)
+      let driver = try Driver(args: [
+        "swiftc",
+        "@\(responseFile1.pathString)",
+        "@\(responseFile2Relative)",
+      ])
+      XCTAssertEqual(driver.inputFiles, [
+        .init(file: .relative(.init("from1stList.swift")), type: .swift),
+        .init(file: .relative(.init("from2ndList.swift")), type: .swift),
+      ])
+    }
+  }
+
   func testLinking() throws {
     var env = ProcessEnv.vars
     env["SWIFT_DRIVER_TESTS_ENABLE_EXEC_PATH_FALLBACK"] = "1"