Skip to content

Add ability to override/disable response file use in ArgsResolver #1165

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 30, 2022
Merged

Add ability to override/disable response file use in ArgsResolver #1165

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Sources/SwiftDriver/Driver/Driver.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1380,7 +1380,7 @@ extension Driver {
// In verbose mode, print out the job
if parsedOptions.contains(.v) {
let arguments: [String] = try executor.resolver.resolveArgumentList(for: inPlaceJob,
forceResponseFiles: forceResponseFiles)
useResponseFiles: forceResponseFiles ? .forced : .heuristic)
stdoutStream <<< arguments.map { $0.spm_shellEscaped() }.joined(separator: " ") <<< "\n"
stdoutStream.flush()
}
Expand Down
37 changes: 31 additions & 6 deletions Sources/SwiftDriver/Execution/ArgsResolver.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,13 @@ import class Foundation.NSLock
import TSCBasic
@_implementationOnly import Yams

/// How the resolver is to handle usage of response files
public enum ResponseFileHandling {
case forced
case disabled
case heuristic
}

/// Resolver for a job's argument template.
public final class ArgsResolver {
/// The map of virtual path to the actual path.
Expand Down Expand Up @@ -45,22 +52,36 @@ public final class ArgsResolver {
}
}

public func resolveArgumentList(for job: Job, forceResponseFiles: Bool,
public func resolveArgumentList(for job: Job, useResponseFiles: ResponseFileHandling = .heuristic,
quotePaths: Bool = false) throws -> [String] {
let (arguments, _) = try resolveArgumentList(for: job, forceResponseFiles: forceResponseFiles,
let (arguments, _) = try resolveArgumentList(for: job, useResponseFiles: useResponseFiles,
quotePaths: quotePaths)
return arguments
}

public func resolveArgumentList(for job: Job, forceResponseFiles: Bool,
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) })
let usingResponseFile = try createResponseFileIfNeeded(for: job, resolvedArguments: &arguments,
forceResponseFiles: forceResponseFiles)
useResponseFiles: useResponseFiles)
return (arguments, usingResponseFile)
}

@available(*, deprecated, message: "use resolveArgumentList(for:,useResponseFiles:,quotePaths:)")
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)
}

@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)
}

/// Resolve the given argument.
public func resolve(_ arg: Job.ArgTemplate,
quotePaths: Bool = false) throws -> String {
Expand Down Expand Up @@ -167,11 +188,15 @@ public final class ArgsResolver {
return string.trimmingCharacters(in: .whitespacesAndNewlines)
}

private func createResponseFileIfNeeded(for job: Job, resolvedArguments: inout [String], forceResponseFiles: Bool) throws -> Bool {
private func createResponseFileIfNeeded(for job: Job, resolvedArguments: inout [String], useResponseFiles: ResponseFileHandling) throws -> Bool {
func quote(_ string: String) -> String {
return "\"\(String(string.flatMap { ["\\", "\""].contains($0) ? "\\\($0)" : "\($0)" }))\""
}

guard useResponseFiles != .disabled else {
return false
}

let forceResponseFiles = useResponseFiles == .forced
if forceResponseFiles ||
(job.supportsResponseFiles && !commandLineFitsWithinSystemLimits(path: resolvedArguments[0], args: resolvedArguments)) {
assert(!forceResponseFiles || job.supportsResponseFiles,
Expand Down
10 changes: 5 additions & 5 deletions Sources/SwiftDriver/ExplicitModuleBuilds/ModuleDependencyScanning.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ public extension Driver {
if isSwiftScanLibAvailable {
let cwd = workingDirectory ?? fileSystem.currentWorkingDirectory!
var command = try itemizedJobCommand(of: preScanJob,
forceResponseFiles: forceResponseFiles,
useResponseFiles: .disabled,
using: executor.resolver)
sanitizeCommandForLibScanInvocation(&command)
imports =
Expand Down Expand Up @@ -154,7 +154,7 @@ public extension Driver {
if isSwiftScanLibAvailable {
let cwd = workingDirectory ?? fileSystem.currentWorkingDirectory!
var command = try itemizedJobCommand(of: scannerJob,
forceResponseFiles: forceResponseFiles,
useResponseFiles: .disabled,
using: executor.resolver)
sanitizeCommandForLibScanInvocation(&command)
dependencyGraph =
Expand Down Expand Up @@ -183,7 +183,7 @@ public extension Driver {
if isSwiftScanLibAvailable {
let cwd = workingDirectory ?? fileSystem.currentWorkingDirectory!
var command = try itemizedJobCommand(of: batchScanningJob,
forceResponseFiles: forceResponseFiles,
useResponseFiles: .disabled,
using: executor.resolver)
sanitizeCommandForLibScanInvocation(&command)
moduleVersionedGraphMap =
Expand Down Expand Up @@ -328,10 +328,10 @@ public extension Driver {
contents)
}

fileprivate func itemizedJobCommand(of job: Job, forceResponseFiles: Bool,
fileprivate func itemizedJobCommand(of job: Job, useResponseFiles: ResponseFileHandling,
using resolver: ArgsResolver) throws -> [String] {
let (args, _) = try resolver.resolveArgumentList(for: job,
forceResponseFiles: forceResponseFiles)
useResponseFiles: useResponseFiles)
return args
}
}
Expand Down
2 changes: 1 addition & 1 deletion Sources/SwiftDriverExecution/MultiJobExecutor.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,7 @@ class ExecuteJobRule: LLBuildRule {
var pid = 0
do {
let arguments: [String] = try resolver.resolveArgumentList(for: job,
forceResponseFiles: context.forceResponseFiles)
useResponseFiles: context.forceResponseFiles ? .forced : .heuristic)


let process : ProcessProtocol
Expand Down
6 changes: 4 additions & 2 deletions Sources/SwiftDriverExecution/SwiftDriverExecutor.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,9 @@ public final class SwiftDriverExecutor: DriverExecutor {
public func execute(job: Job,
forceResponseFiles: Bool = false,
recordedInputModificationDates: [TypedVirtualPath: Date] = [:]) throws -> ProcessResult {
let useResponseFiles : ResponseFileHandling = forceResponseFiles ? .forced : .heuristic
let arguments: [String] = try resolver.resolveArgumentList(for: job,
forceResponseFiles: forceResponseFiles)
useResponseFiles: useResponseFiles)

try job.verifyInputsNotModified(since: recordedInputModificationDates,
fileSystem: fileSystem)
Expand Down Expand Up @@ -86,7 +87,8 @@ 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)
let useResponseFiles : ResponseFileHandling = forceResponseFiles ? .forced : .heuristic
let (args, usedResponseFile) = try resolver.resolveArgumentList(for: job, useResponseFiles: useResponseFiles)
var result = args.map { $0.spm_shellEscaped() }.joined(separator: " ")

if usedResponseFile {
Expand Down
3 changes: 1 addition & 2 deletions Tests/SwiftDriverTests/ExplicitModuleBuildTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1054,8 +1054,7 @@ final class ExplicitModuleBuildTests: XCTestCase {

let resolver = try ArgsResolver(fileSystem: localFileSystem)
let (args, _) = try resolver.resolveArgumentList(for: scannerJob,
forceResponseFiles: false,
quotePaths: true)
useResponseFiles: .disabled)
XCTAssertTrue(args.count > 1)
XCTAssertFalse(args[0].hasSuffix(".resp"))
}
Expand Down
7 changes: 3 additions & 4 deletions Tests/SwiftDriverTests/ParsableMessageTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ final class ParsableMessageTests: XCTestCase {
"-working-directory", workdir.pathString])
let jobs = try driver.planBuild()
let compileJob = jobs[0]
let args : [String] = try resolver.resolveArgumentList(for: compileJob, forceResponseFiles: false)
let args : [String] = try resolver.resolveArgumentList(for: compileJob, useResponseFiles: .disabled)
let toolDelegate = ToolExecutionDelegate(mode: .parsableOutput,
buildRecordInfo: nil,
showJobLifecycle: false,
Expand Down Expand Up @@ -237,7 +237,7 @@ final class ParsableMessageTests: XCTestCase {
"-working-directory", "/WorkDir"])
let jobs = try driver.planBuild()
compileJob = jobs[0]
args = try resolver.resolveArgumentList(for: compileJob!, forceResponseFiles: false)
args = try resolver.resolveArgumentList(for: compileJob!)
toolDelegate = ToolExecutionDelegate(mode: .parsableOutput,
buildRecordInfo: nil,
showJobLifecycle: false,
Expand Down Expand Up @@ -313,8 +313,7 @@ final class ParsableMessageTests: XCTestCase {
"-working-directory", "/WorkDir"])
let jobs = try driver.planBuild()
compileJob = jobs[0]
args = try resolver.resolveArgumentList(for: compileJob!,
forceResponseFiles: false)
args = try resolver.resolveArgumentList(for: compileJob!)
toolDelegate = ToolExecutionDelegate(mode: .parsableOutput,
buildRecordInfo: nil,
showJobLifecycle: false,
Expand Down
30 changes: 21 additions & 9 deletions Tests/SwiftDriverTests/SwiftDriverTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1485,7 +1485,7 @@ final class SwiftDriverTests: XCTestCase {
XCTAssertTrue(jobs.count == 1 && jobs[0].kind == .interpret)
let interpretJob = jobs[0]
let resolver = try ArgsResolver(fileSystem: localFileSystem)
let resolvedArgs: [String] = try resolver.resolveArgumentList(for: interpretJob, forceResponseFiles: false)
let resolvedArgs: [String] = try resolver.resolveArgumentList(for: interpretJob)
XCTAssertTrue(resolvedArgs.count == 2)
XCTAssertEqual(resolvedArgs[1].first, "@")
let responseFilePath = try AbsolutePath(validating: String(resolvedArgs[1].dropFirst()))
Expand All @@ -1494,14 +1494,26 @@ final class SwiftDriverTests: XCTestCase {
XCTAssertTrue(contents.contains("\"-D\"\n\"TEST_20000\""))
XCTAssertTrue(contents.contains("\"-D\"\n\"TEST_1\""))
}

// Needs response file + disable override
do {
var driver = try Driver(args: ["swift"] + manyArgs + ["foo.swift"])
let jobs = try driver.planBuild()
XCTAssertTrue(jobs.count == 1 && jobs[0].kind == .interpret)
let interpretJob = jobs[0]
let resolver = try ArgsResolver(fileSystem: localFileSystem)
let resolvedArgs: [String] = try resolver.resolveArgumentList(for: interpretJob, useResponseFiles: .disabled)
XCTAssertFalse(resolvedArgs.contains { $0.hasPrefix("@") })
}

// Forced response file
do {
var driver = try Driver(args: ["swift"] + ["foo.swift"])
let jobs = try driver.planBuild()
XCTAssertTrue(jobs.count == 1 && jobs[0].kind == .interpret)
let interpretJob = jobs[0]
let resolver = try ArgsResolver(fileSystem: localFileSystem)
let resolvedArgs: [String] = try resolver.resolveArgumentList(for: interpretJob, forceResponseFiles: true)
let resolvedArgs: [String] = try resolver.resolveArgumentList(for: interpretJob, useResponseFiles: .forced)
XCTAssertTrue(resolvedArgs.count == 2)
XCTAssertEqual(resolvedArgs[1].first, "@")
let responseFilePath = try AbsolutePath(validating: String(resolvedArgs[1].dropFirst()))
Expand All @@ -1516,8 +1528,8 @@ final class SwiftDriverTests: XCTestCase {
XCTAssertTrue(jobs.count == 1 && jobs[0].kind == .interpret)
let interpretJob = jobs[0]
let resolver = try ArgsResolver(fileSystem: localFileSystem)
let resolvedArgs: [String] = try resolver.resolveArgumentList(for: interpretJob, forceResponseFiles: false)
XCTAssertFalse(resolvedArgs.map { $0.hasPrefix("@") }.reduce(false){ $0 || $1 })
let resolvedArgs: [String] = try resolver.resolveArgumentList(for: interpretJob)
XCTAssertFalse(resolvedArgs.contains { $0.hasPrefix("@") })
}
}

Expand All @@ -1539,15 +1551,15 @@ final class SwiftDriverTests: XCTestCase {

let emitModuleJob = jobs.first(where: { $0.kind == .emitModule })!
let emitModuleResolvedArgs: [String] =
try resolver.resolveArgumentList(for: emitModuleJob, forceResponseFiles: false)
try resolver.resolveArgumentList(for: emitModuleJob)
XCTAssertEqual(emitModuleResolvedArgs.count, 2)
XCTAssertEqual(emitModuleResolvedArgs[1].first, "@")

let compileJobs = jobs.filter { $0.kind == .compile }
for compileJob in compileJobs {
XCTAssertEqual(compileJobs.count, 2)
let compileResolvedArgs: [String] =
try resolver.resolveArgumentList(for: compileJob, forceResponseFiles: false)
try resolver.resolveArgumentList(for: compileJob)
XCTAssertEqual(compileResolvedArgs.count, 2)
XCTAssertEqual(compileResolvedArgs[1].first, "@")
}
Expand All @@ -1565,15 +1577,15 @@ final class SwiftDriverTests: XCTestCase {

let mergeModuleJob = jobs.first(where: { $0.kind == .mergeModule })!
let mergeModuleResolvedArgs: [String] =
try resolver.resolveArgumentList(for: mergeModuleJob, forceResponseFiles: false)
try resolver.resolveArgumentList(for: mergeModuleJob)
XCTAssertEqual(mergeModuleResolvedArgs.count, 2)
XCTAssertEqual(mergeModuleResolvedArgs[1].first, "@")

let compileJobs = jobs.filter { $0.kind == .compile }
for compileJob in compileJobs {
XCTAssertEqual(compileJobs.count, 2)
let compileResolvedArgs: [String] =
try resolver.resolveArgumentList(for: compileJob, forceResponseFiles: false)
try resolver.resolveArgumentList(for: compileJob)
XCTAssertEqual(compileResolvedArgs.count, 2)
XCTAssertEqual(compileResolvedArgs[1].first, "@")
}
Expand All @@ -1590,7 +1602,7 @@ final class SwiftDriverTests: XCTestCase {

let generatePCMJob = jobs[0]
let generatePCMResolvedArgs: [String] =
try resolver.resolveArgumentList(for: generatePCMJob, forceResponseFiles: false)
try resolver.resolveArgumentList(for: generatePCMJob)
XCTAssertEqual(generatePCMResolvedArgs.count, 2)
XCTAssertEqual(generatePCMResolvedArgs[1].first, "@")
}
Expand Down