Merge pull request #451 from owenv/resolve-repl-args

Author: owenv

Sources/SwiftDriver/Execution/ArgsResolver.swift

@@ -67,10 +67,15 @@ public final class ArgsResolver {
       return try lock.withLock {
         return try unsafeResolve(path: path)
       }
+
     case .responseFilePath(let path):
       return "@\(try resolve(.path(path)))"
+
     case let .joinedOptionAndPath(option, path):
       return option + (try resolve(.path(path)))
+
+    case let .squashedArgumentList(option: option, args: args):
+      return try option + args.map{ try resolve($0) }.joined(separator: " ")
     }
   }
 

Sources/SwiftDriver/Jobs/CommandLineArguments.swift

@@ -162,7 +162,8 @@ extension Array where Element == Job.ArgTemplate {
   }
 
   /// A shell-escaped string representation of the arguments, as they would appear on the command line.
-  public var joinedArguments: String {
+  /// Note: does not resolve arguments.
+  public var joinedUnresolvedArguments: String {
     return self.map {
       switch $0 {
         case .flag(let string):
@@ -173,6 +174,8 @@ extension Array where Element == Job.ArgTemplate {
         return "@\(path.name.spm_shellEscaped())"
       case let .joinedOptionAndPath(option, path):
         return option.spm_shellEscaped() + path.name.spm_shellEscaped()
+      case let .squashedArgumentList(option, args):
+        return (option + args.joinedUnresolvedArguments).spm_shellEscaped()
       }
     }.joined(separator: " ")
   }

Sources/SwiftDriver/Jobs/Job.swift

@@ -49,6 +49,9 @@ public struct Job: Codable, Equatable, Hashable {
 
     /// Represents a joined option+path combo.
     case joinedOptionAndPath(String, VirtualPath)
+
+    /// Represents a list of arguments squashed together and passed as a single argument.
+    case squashedArgumentList(option: String, args: [ArgTemplate])
   }
 
   /// The Swift module this job involves.
@@ -234,11 +237,15 @@ extension Job.Kind {
 
 extension Job.ArgTemplate: Codable {
   private enum CodingKeys: String, CodingKey {
-    case flag, path, responseFilePath, joinedOptionAndPath
+    case flag, path, responseFilePath, joinedOptionAndPath, squashedArgumentList
 
     enum JoinedOptionAndPathCodingKeys: String, CodingKey {
       case option, path
     }
+
+    enum SquashedArgumentListCodingKeys: String, CodingKey {
+      case option, args
+    }
   }
 
   public func encode(to encoder: Encoder) throws {
@@ -259,6 +266,12 @@ extension Job.ArgTemplate: Codable {
         forKey: .joinedOptionAndPath)
       try keyedContainer.encode(option, forKey: .option)
       try keyedContainer.encode(path, forKey: .path)
+    case .squashedArgumentList(option: let option, args: let args):
+      var keyedContainer = container.nestedContainer(
+        keyedBy: CodingKeys.SquashedArgumentListCodingKeys.self,
+        forKey: .squashedArgumentList)
+      try keyedContainer.encode(option, forKey: .option)
+      try keyedContainer.encode(args, forKey: .args)
     }
   }
 
@@ -286,6 +299,12 @@ extension Job.ArgTemplate: Codable {
         forKey: .joinedOptionAndPath)
       self = .joinedOptionAndPath(try keyedValues.decode(String.self, forKey: .option),
                                   try keyedValues.decode(VirtualPath.self, forKey: .path))
+    case .squashedArgumentList:
+      let keyedValues = try values.nestedContainer(
+        keyedBy: CodingKeys.SquashedArgumentListCodingKeys.self,
+        forKey: .squashedArgumentList)
+      self = .squashedArgumentList(option: try keyedValues.decode(String.self, forKey: .option),
+                                   args: try keyedValues.decode([Job.ArgTemplate].self, forKey: .args))
     }
   }
 }

Sources/SwiftDriver/Jobs/ReplJob.swift

@@ -22,12 +22,12 @@ extension Driver {
     try commandLine.appendAll(.l, .framework, .L, from: &parsedOptions)
 
     // Squash important frontend options into a single argument for LLDB.
-    let lldbArg = "--repl=\(commandLine.joinedArguments)"
+    let lldbCommandLine: [Job.ArgTemplate] = [.squashedArgumentList(option: "--repl=", args: commandLine)]
     return Job(
       moduleName: moduleOutputInfo.name,
       kind: .repl,
       tool: .absolute(try toolchain.getToolPath(.lldb)),
-      commandLine: [Job.ArgTemplate.flag(lldbArg)],
+      commandLine: lldbCommandLine,
       inputs: inputs,
       primaryInputs: [],
       outputs: [],

Sources/SwiftDriverExecution/SwiftDriverExecutor.swift

@@ -82,11 +82,11 @@ public final class SwiftDriverExecutor: DriverExecutor {
 
   public func description(of job: Job, forceResponseFiles: Bool) throws -> String {
     let (args, usedResponseFile) = try resolver.resolveArgumentList(for: job, forceResponseFiles: forceResponseFiles)
-    var result = args.joined(separator: " ")
+    var result = args.map { $0.spm_shellEscaped() }.joined(separator: " ")
 
     if usedResponseFile {
       // Print the response file arguments as a comment.
-      result += " # \(job.commandLine.joinedArguments)"
+      result += " # \(job.commandLine.joinedUnresolvedArguments)"
     }
 
     if !job.extraEnvironment.isEmpty {

Tests/SwiftDriverTests/JobExecutorTests.swift

@@ -289,6 +289,21 @@ final class JobExecutorTests: XCTestCase {
     }
   }
 
+  func testShellEscapingArgsInJobDescription() throws {
+    let executor = try SwiftDriverExecutor(diagnosticsEngine: DiagnosticsEngine(),
+                                           processSet: ProcessSet(),
+                                           fileSystem: localFileSystem,
+                                           env: [:])
+    let job = Job(moduleName: "Module",
+                  kind: .compile,
+                  tool: .absolute(.init("/path/to/the tool")),
+                  commandLine: [.path(.absolute(.init("/with space"))),
+                                .path(.absolute(.init("/withoutspace")))],
+                  inputs: [], primaryInputs: [], outputs: [])
+    XCTAssertEqual(try executor.description(of: job, forceResponseFiles: false),
+                   "'/path/to/the tool' '/with space' /withoutspace")
+  }
+
   func testInputModifiedDuringMultiJobBuild() throws {
     try withTemporaryDirectory { path in
       let main = path.appending(component: "main.swift")
@@ -330,6 +345,18 @@ final class JobExecutorTests: XCTestCase {
     }
   }
 
+  func testResolveSquashedArgs() throws {
+    try withTemporaryDirectory { path in
+      let resolver = try ArgsResolver(fileSystem: localFileSystem, temporaryDirectory: .absolute(path))
+      let tmpPath = VirtualPath.temporaryWithKnownContents(.init("one.txt"), "hello, world!".data(using: .utf8)!)
+      let tmpPath2 = VirtualPath.temporaryWithKnownContents(.init("two.txt"), "goodbye!".data(using: .utf8)!)
+      let resolvedCommandLine = try resolver.resolve(
+        .squashedArgumentList(option: "--opt=", args: [.path(tmpPath), .path(tmpPath2)]))
+      XCTAssertEqual(resolvedCommandLine, "--opt=\(path.appending(component: "one.txt").pathString) \(path.appending(component: "two.txt").pathString)")
+      XCTAssertEqual(resolvedCommandLine.spm_shellEscaped(), "'--opt=\(path.appending(component: "one.txt").pathString) \(path.appending(component: "two.txt").pathString)'")
+    }
+  }
+
   func testSaveTemps() throws {
     do {
       try withTemporaryDirectory { path in

Tests/SwiftDriverTests/SwiftDriverTests.swift

@@ -1626,7 +1626,7 @@ final class SwiftDriverTests: XCTestCase {
     let plannedJobs = try driver1.planBuild().removingAutolinkExtractJobs()
     XCTAssertEqual(plannedJobs.count, 2)
     XCTAssertEqual(plannedJobs[0].kind, .compile)
-    print(plannedJobs[0].commandLine.joinedArguments)
+    print(plannedJobs[0].commandLine.joinedUnresolvedArguments)
     XCTAssert(plannedJobs[0].commandLine.contains(.flag("-supplementary-output-file-map")))
   }
 
@@ -1821,10 +1821,9 @@ final class SwiftDriverTests: XCTestCase {
       throw XCTSkip()
     }
 
-    func isLLDBREPLFlag(_ arg: Job.ArgTemplate) -> Bool {
-      if case .flag(let replString) = arg {
-        return replString.hasPrefix("--repl=") &&
-          !replString.contains("-module-name")
+    func isExpectedLLDBREPLFlag(_ arg: Job.ArgTemplate) -> Bool {
+      if case let .squashedArgumentList(option: opt, args: args) = arg {
+        return opt == "--repl=" && !args.contains("-module-name")
       }
       return false
     }
@@ -1836,7 +1835,7 @@ final class SwiftDriverTests: XCTestCase {
       let replJob = plannedJobs.first!
       XCTAssertTrue(replJob.tool.name.contains("lldb"))
       XCTAssertTrue(replJob.requiresInPlaceExecution)
-      XCTAssert(replJob.commandLine.contains(where: { isLLDBREPLFlag($0) }))
+      XCTAssert(replJob.commandLine.contains(where: { isExpectedLLDBREPLFlag($0) }))
     }
 
     do {
@@ -1846,7 +1845,7 @@ final class SwiftDriverTests: XCTestCase {
       let replJob = plannedJobs.first!
       XCTAssertTrue(replJob.tool.name.contains("lldb"))
       XCTAssertTrue(replJob.requiresInPlaceExecution)
-      XCTAssert(replJob.commandLine.contains(where: { isLLDBREPLFlag($0) }))
+      XCTAssert(replJob.commandLine.contains(where: { isExpectedLLDBREPLFlag($0) }))
     }
 
     do {
@@ -1858,7 +1857,7 @@ final class SwiftDriverTests: XCTestCase {
       let replJob = plannedJobs.first!
       XCTAssertTrue(replJob.tool.name.contains("lldb"))
       XCTAssertTrue(replJob.requiresInPlaceExecution)
-      XCTAssert(replJob.commandLine.contains(where: { isLLDBREPLFlag($0) }))
+      XCTAssert(replJob.commandLine.contains(where: { isExpectedLLDBREPLFlag($0) }))
     }
 
     do {
@@ -4312,7 +4311,7 @@ private extension Array where Element == Job.ArgTemplate {
       switch $0 {
       case let .path(path):
         return path.basename == basename
-      case .flag, .responseFilePath, .joinedOptionAndPath:
+      case .flag, .responseFilePath, .joinedOptionAndPath, .squashedArgumentList:
         return false
       }
     }