diff --git a/Sources/SwiftDriver/Execution/ArgsResolver.swift b/Sources/SwiftDriver/Execution/ArgsResolver.swift index 57b2b7eb0..c2c6baea3 100644 --- a/Sources/SwiftDriver/Execution/ArgsResolver.swift +++ b/Sources/SwiftDriver/Execution/ArgsResolver.swift @@ -57,17 +57,16 @@ public final class ArgsResolver { } } - public func resolveArgumentList(for job: Job, useResponseFiles: ResponseFileHandling = .heuristic, - quotePaths: Bool = false) throws -> [String] { - let (arguments, _) = try resolveArgumentList(for: job, useResponseFiles: useResponseFiles, - quotePaths: quotePaths) + public func resolveArgumentList(for job: Job, useResponseFiles: ResponseFileHandling = .heuristic) + throws -> [String] { + let (arguments, _) = try resolveArgumentList(for: job, useResponseFiles: useResponseFiles) return arguments } - public func resolveArgumentList(for job: Job, useResponseFiles: ResponseFileHandling = .heuristic, - quotePaths: Bool = false) throws -> ([String], usingResponseFile: Bool) { - let tool = try resolve(.path(job.tool), quotePaths: quotePaths) - var arguments = [tool] + (try job.commandLine.map { try resolve($0, quotePaths: quotePaths) }) + public func resolveArgumentList(for job: Job, useResponseFiles: ResponseFileHandling = .heuristic) + throws -> ([String], usingResponseFile: Bool) { + let tool = try resolve(.path(job.tool)) + var arguments = [tool] + (try job.commandLine.map { try resolve($0) }) let usingResponseFile = try createResponseFileIfNeeded(for: job, resolvedArguments: &arguments, useResponseFiles: useResponseFiles) return (arguments, usingResponseFile) @@ -77,46 +76,45 @@ public final class ArgsResolver { public func resolveArgumentList(for job: Job, forceResponseFiles: Bool, quotePaths: Bool = false) throws -> [String] { let useResponseFiles: ResponseFileHandling = forceResponseFiles ? .forced : .heuristic - return try resolveArgumentList(for: job, useResponseFiles: useResponseFiles, quotePaths: quotePaths) + return try resolveArgumentList(for: job, useResponseFiles: useResponseFiles) } @available(*, deprecated, message: "use resolveArgumentList(for:,useResponseFiles:,quotePaths:)") public func resolveArgumentList(for job: Job, forceResponseFiles: Bool, quotePaths: Bool = false) throws -> ([String], usingResponseFile: Bool) { let useResponseFiles: ResponseFileHandling = forceResponseFiles ? .forced : .heuristic - return try resolveArgumentList(for: job, useResponseFiles: useResponseFiles, quotePaths: quotePaths) + return try resolveArgumentList(for: job, useResponseFiles: useResponseFiles) } /// Resolve the given argument. - public func resolve(_ arg: Job.ArgTemplate, - quotePaths: Bool = false) throws -> String { + public func resolve(_ arg: Job.ArgTemplate) throws -> String { switch arg { case .flag(let flag): return flag case .path(let path): return try lock.withLock { - return try unsafeResolve(path: path, quotePaths: quotePaths) + return try unsafeResolve(path: path) } case .responseFilePath(let path): - return "@\(try resolve(.path(path), quotePaths: quotePaths))" + return "@\(try resolve(.path(path)))" case let .joinedOptionAndPath(option, path): - return option + (try resolve(.path(path), quotePaths: quotePaths)) + return option + (try resolve(.path(path))) case let .squashedArgumentList(option: option, args: args): return try option + args.map { - try resolve($0, quotePaths: quotePaths) + try resolve($0) }.joined(separator: " ") } } /// Needs to be done inside of `lock`. Marked unsafe to make that more obvious. - private func unsafeResolve(path: VirtualPath, quotePaths: Bool) throws -> String { + private func unsafeResolve(path: VirtualPath) throws -> String { // If there was a path mapping, use it. if let actualPath = pathMapping[path] { - return quotePaths ? quoteArgument(actualPath) : actualPath + return actualPath } // Return the path from the temporary directory if this is a temporary file. @@ -140,13 +138,13 @@ public final class ArgsResolver { let result = actualPath.name pathMapping[path] = result - return quotePaths ? "'\(result)'" : result + return result } // Otherwise, return the path. let result = path.name pathMapping[path] = result - return quotePaths ? quoteArgument(result) : result + return result } private func createFileList(path: VirtualPath, contents: [VirtualPath]) throws { @@ -154,7 +152,7 @@ public final class ArgsResolver { if let absPath = path.absolutePath { try fileSystem.writeFileContents(absPath) { out in for path in contents { - try! out <<< unsafeResolve(path: path, quotePaths: false) <<< "\n" + try! out <<< unsafeResolve(path: path) <<< "\n" } } } @@ -184,7 +182,7 @@ public final class ArgsResolver { } private func quoteAndEscape(path: VirtualPath) -> String { - let inputNode = Node.scalar(Node.Scalar(try! unsafeResolve(path: path, quotePaths: false), + let inputNode = Node.scalar(Node.Scalar(try! unsafeResolve(path: path), Tag(.str), .doubleQuoted)) // Width parameter of -1 sets preferred line-width to unlimited so that no extraneous // line-breaks will be inserted during serialization. diff --git a/Sources/SwiftDriver/ExplicitModuleBuilds/ModuleDependencyScanning.swift b/Sources/SwiftDriver/ExplicitModuleBuilds/ModuleDependencyScanning.swift index 7edbd2909..066205f3a 100644 --- a/Sources/SwiftDriver/ExplicitModuleBuilds/ModuleDependencyScanning.swift +++ b/Sources/SwiftDriver/ExplicitModuleBuilds/ModuleDependencyScanning.swift @@ -415,12 +415,11 @@ public extension Driver { static func itemizedJobCommand(of job: Job, useResponseFiles: ResponseFileHandling, using resolver: ArgsResolver) throws -> [String] { - // FIXME: this is to walkaround rdar://108769167 - let quotePaths = job.kind != .scanDependencies - let (args, _) = try resolver.resolveArgumentList(for: job, - useResponseFiles: useResponseFiles, - quotePaths: quotePaths) - return args + // Because the command-line passed to libSwiftScan does not go through the shell + // we must ensure that we generate a shell-escaped string for all arguments/flags that may + // potentially need it. + return try resolver.resolveArgumentList(for: job, + useResponseFiles: useResponseFiles).0.map { $0.spm_shellEscaped() } } static func getRootPath(of toolchain: Toolchain, env: [String: String]) diff --git a/Tests/SwiftDriverTests/ExplicitModuleBuildTests.swift b/Tests/SwiftDriverTests/ExplicitModuleBuildTests.swift index 05d9858bc..f506e360b 100644 --- a/Tests/SwiftDriverTests/ExplicitModuleBuildTests.swift +++ b/Tests/SwiftDriverTests/ExplicitModuleBuildTests.swift @@ -1558,6 +1558,28 @@ final class ExplicitModuleBuildTests: XCTestCase { } } + func testDependencyScanCommandLineEscape() throws { +#if os(Windows) + let quoteCharacter: Character = "\"" +#else + let quoteCharacter: Character = "'" +#endif + let swiftInputWithSpace = "/tmp/input example/test.swift" + let swiftInputWithoutSpace = "/tmp/baz.swift" + let clangInputWithSpace = "/tmp/input example/bar.o" + var driver = try Driver(args: ["swiftc", "-explicit-module-build", + "-module-name", "testDependencyScanning", + swiftInputWithSpace, swiftInputWithoutSpace, + "-Xcc", clangInputWithSpace]) + let scanJob = try driver.dependencyScanningJob() + let scanJobCommand = try Driver.itemizedJobCommand(of: scanJob, + useResponseFiles: .disabled, + using: ArgsResolver(fileSystem: InMemoryFileSystem())) + XCTAssertTrue(scanJobCommand.contains(String(quoteCharacter) + swiftInputWithSpace + String(quoteCharacter))) + XCTAssertTrue(scanJobCommand.contains(String(quoteCharacter) + clangInputWithSpace + String(quoteCharacter))) + XCTAssertTrue(scanJobCommand.contains(swiftInputWithoutSpace)) + } + func testExplicitSwiftModuleMap() throws { let jsonExample : String = """ [