Skip to content

Use response files when running subcommands if needed #57

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.

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 5 commits into from
Jan 25, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
88 changes: 60 additions & 28 deletions Sources/SwiftDriver/Driver/Driver.swift
Original file line number Diff line number Diff line change
Expand Up @@ -423,48 +423,61 @@ extension Driver {
/// This method supports response files with:
/// 1. Double slash comments at the beginning of a line.
/// 2. Backslash escaping.
/// 3. Space character (U+0020 SPACE).
/// 3. Shell Quoting
///
/// - Returns: One line String ready to be used in the shell, if any.
/// - Returns: An array of 0 or more command line arguments
///
/// - Complexity: O(*n*), where *n* is the length of the line.
private static func tokenizeResponseFileLine<S: StringProtocol>(_ line: S) -> String? {
if line.isEmpty { return nil }

private static func tokenizeResponseFileLine<S: StringProtocol>(_ line: S) -> [String] {
// Support double dash comments only if they start at the beginning of a line.
if line.hasPrefix("//") { return nil }

var result: String = ""
/// Indicates if we just parsed an escaping backslash.
if line.hasPrefix("//") { return [] }

var tokens: [String] = []
var token: String = ""
// Conservatively assume ~1 token per line.
token.reserveCapacity(line.count)
// Indicates if we just parsed an escaping backslash.
var isEscaping = false
// Indicates if we are currently parsing quoted text.
var quoted = false

for char in line {
if char.isNewline { return result }

// Backslash escapes to the next character.
if char == #"\"#, !isEscaping {
isEscaping = true
continue
} else if isEscaping {
// Disable escaping and keep parsing.
isEscaping = false
} else if char.isShellQuote {
// If an unescaped shell quote appears, begin or end quoting.
quoted.toggle()
continue
} else if char.isWhitespace && !quoted {
// This is unquoted, unescaped whitespace, start a new token.
tokens.append(token)
token = ""
continue
}

// Ignore spacing characters, except by the space character.
if char.isWhitespace && char != " " { continue }

result.append(char)

token.append(char)
}
return result.isEmpty ? nil : result
// Add the final token
tokens.append(token)

// Filter any empty tokens that might be parsed if there's excessive whitespace.
return tokens.filter { !$0.isEmpty }
}

/// Tokenize each line of the response file, omitting empty lines.
///
/// - Parameter content: response file's content to be tokenized.
private static func tokenizeResponseFile(_ content: String) -> [String] {
return content
.split(separator: "\n")
.compactMap { tokenizeResponseFileLine($0) }
#if !os(macOS) && !os(Linux)
#warning("Response file tokenization unimplemented for platform; behavior may be incorrect")
#endif
return content.split { $0 == "\n" || $0 == "\r\n" }
.flatMap { tokenizeResponseFileLine($0) }
}

/// Recursively expands the response files.
Expand Down Expand Up @@ -580,10 +593,12 @@ extension Driver {
let jobs = try planBuild()
if jobs.isEmpty { return }

let forceResponseFiles = parsedOptions.contains(.driverForceResponseFiles)

// If we're only supposed to print the jobs, do so now.
if parsedOptions.contains(.driverPrintJobs) {
for job in jobs {
print(job)
try Self.printJob(job, resolver: resolver, forceResponseFiles: forceResponseFiles)
}
return
}
Expand All @@ -597,7 +612,7 @@ extension Driver {

if jobs.contains(where: { $0.requiresInPlaceExecution }) {
assert(jobs.count == 1, "Cannot execute in place for multi-job build plans")
return try executeJobInPlace(jobs[0], resolver: resolver)
return try executeJobInPlace(jobs[0], resolver: resolver, forceResponseFiles: forceResponseFiles)
}

// Create and use the tool execution delegate if one is not provided explicitly.
Expand All @@ -608,7 +623,8 @@ extension Driver {
jobs: jobs, resolver: resolver,
executorDelegate: executorDelegate,
numParallelJobs: numParallelJobs,
processSet: processSet
processSet: processSet,
forceResponseFiles: forceResponseFiles
)
try jobExecutor.execute(env: env)
}
Expand All @@ -627,16 +643,32 @@ extension Driver {
}

/// Execute a single job in-place.
private func executeJobInPlace(_ job: Job, resolver: ArgsResolver) throws {
let tool = try resolver.resolve(.path(job.tool))
let commandLine = try job.commandLine.map{ try resolver.resolve($0) }
let arguments = [tool] + commandLine
private func executeJobInPlace(_ job: Job, resolver: ArgsResolver, forceResponseFiles: Bool) throws {
let arguments: [String] = try resolver.resolveArgumentList(for: job, forceResponseFiles: forceResponseFiles)

for (envVar, value) in job.extraEnvironment {
try ProcessEnv.setVar(envVar, value: value)
}

return try exec(path: tool, args: arguments)
return try exec(path: arguments[0], args: arguments)
}

public static func printJob(_ job: Job, resolver: ArgsResolver, forceResponseFiles: Bool) throws {
let (args, usedResponseFile) = try resolver.resolveArgumentList(for: job, forceResponseFiles: forceResponseFiles)
var result = args.joined(separator: " ")

if usedResponseFile {
// Print the response file arguments as a comment.
result += " # \(job.commandLine.joinedArguments)"
}

if !job.extraEnvironment.isEmpty {
result += " #"
for (envVar, val) in job.extraEnvironment {
result += " \(envVar)=\(val)"
}
}
print(result)
}

private func printVersion<S: OutputByteStream>(outputStream: inout S) throws {
Expand Down
51 changes: 45 additions & 6 deletions Sources/SwiftDriver/Execution/JobExecutor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,19 @@ public struct ArgsResolver {
}
}

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

public func resolveArgumentList(for job: Job, forceResponseFiles: Bool) 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,
forceResponseFiles: forceResponseFiles)
return (arguments, usingResponseFile)
}

/// Resolve the given argument.
public func resolve(_ arg: Job.ArgTemplate) throws -> String {
switch arg {
Expand All @@ -55,6 +68,22 @@ public struct ArgsResolver {
}
}

private func createResponseFileIfNeeded(for job: Job, resolvedArguments: inout [String], forceResponseFiles: Bool) throws -> Bool {
if forceResponseFiles ||
(job.supportsResponseFiles && !commandLineFitsWithinSystemLimits(path: resolvedArguments[0], args: resolvedArguments)) {
assert(!forceResponseFiles || job.supportsResponseFiles,
"Platform does not support response files for job: \(job)")
// Match the integrated driver's behavior, which uses response file names of the form "arguments-[0-9a-zA-Z].resp".
let responseFilePath = temporaryDirectory.appending(component: "arguments-\(abs(job.hashValue)).resp")
try localFileSystem.writeFileContents(responseFilePath) {
$0 <<< resolvedArguments[1...].map{ $0.spm_shellEscaped() }.joined(separator: "\n")
}
resolvedArguments = [resolvedArguments[0], "@\(responseFilePath.pathString)"]
return true
}
return false
}

/// Remove the temporary directory from disk.
public func removeTemporaryDirectory() throws {
_ = try FileManager.default.removeItem(atPath: temporaryDirectory.pathString)
Expand Down Expand Up @@ -106,20 +135,25 @@ public final class JobExecutor {
/// The process set to use when launching new processes.
let processSet: ProcessSet?

/// If true, always use response files to pass command line arguments.
let forceResponseFiles: Bool

init(
argsResolver: ArgsResolver,
env: [String: String],
producerMap: [VirtualPath: Job],
executorDelegate: JobExecutorDelegate,
jobQueue: OperationQueue,
processSet: ProcessSet?
processSet: ProcessSet?,
forceResponseFiles: Bool
) {
self.producerMap = producerMap
self.argsResolver = argsResolver
self.env = env
self.executorDelegate = executorDelegate
self.jobQueue = jobQueue
self.processSet = processSet
self.forceResponseFiles = forceResponseFiles
}
}

Expand All @@ -138,18 +172,23 @@ public final class JobExecutor {
/// The process set to use when launching new processes.
let processSet: ProcessSet?

/// If true, always use response files to pass command line arguments.
let forceResponseFiles: Bool

public init(
jobs: [Job],
resolver: ArgsResolver,
executorDelegate: JobExecutorDelegate,
numParallelJobs: Int? = nil,
processSet: ProcessSet? = nil
processSet: ProcessSet? = nil,
forceResponseFiles: Bool = false
) {
self.jobs = jobs
self.argsResolver = resolver
self.executorDelegate = executorDelegate
self.numParallelJobs = numParallelJobs ?? 1
self.processSet = processSet
self.forceResponseFiles = forceResponseFiles
}

/// Execute all jobs.
Expand Down Expand Up @@ -187,7 +226,8 @@ public final class JobExecutor {
producerMap: producerMap,
executorDelegate: executorDelegate,
jobQueue: jobQueue,
processSet: processSet
processSet: processSet,
forceResponseFiles: forceResponseFiles
)
}
}
Expand Down Expand Up @@ -339,9 +379,8 @@ class ExecuteJobRule: LLBuildRule {
let value: DriverBuildValue
var pid = 0
do {
let tool = try resolver.resolve(.path(job.tool))
let commandLine = try job.commandLine.map{ try resolver.resolve($0) }
let arguments = [tool] + commandLine
let arguments: [String] = try resolver.resolveArgumentList(for: job,
forceResponseFiles: context.forceResponseFiles)

let process = try context.executorDelegate.launchProcess(
for: job, arguments: arguments, env: env
Expand Down
3 changes: 2 additions & 1 deletion Sources/SwiftDriver/Jobs/AutolinkExtractJob.swift
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ extension Driver {
tool: .absolute(try toolchain.getToolPath(.swiftAutolinkExtract)),
commandLine: commandLine,
inputs: inputs,
outputs: [.init(file: output, type: .autolink)]
outputs: [.init(file: output, type: .autolink)],
supportsResponseFiles: true
)
}
}
3 changes: 2 additions & 1 deletion Sources/SwiftDriver/Jobs/CompileJob.swift
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,8 @@ extension Driver {
commandLine: commandLine,
displayInputs: primaryInputs,
inputs: inputs,
outputs: outputs
outputs: outputs,
supportsResponseFiles: true
)
}
}
Expand Down
3 changes: 2 additions & 1 deletion Sources/SwiftDriver/Jobs/GeneratePCHJob.swift
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ extension Driver {
commandLine: commandLine,
displayInputs: [],
inputs: inputs,
outputs: outputs
outputs: outputs,
supportsResponseFiles: true
)
}
}
3 changes: 2 additions & 1 deletion Sources/SwiftDriver/Jobs/InterpretJob.swift
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ extension Driver {
inputs:inputs,
outputs: [],
extraEnvironment: extraEnvironment,
requiresInPlaceExecution: true
requiresInPlaceExecution: true,
supportsResponseFiles: true
)
}
}
26 changes: 8 additions & 18 deletions Sources/SwiftDriver/Jobs/Job.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import TSCBasic

/// A job represents an individual subprocess that should be invoked during compilation.
public struct Job: Codable, Equatable {
public struct Job: Codable, Equatable, Hashable {
public enum Kind: String, Codable {
case compile
case mergeModule = "merge-module"
Expand All @@ -29,7 +29,7 @@ public struct Job: Codable, Equatable {
case verifyDebugInfo = "verify-debug-info"
}

public enum ArgTemplate: Equatable {
public enum ArgTemplate: Equatable, Hashable {
/// Represents a command-line flag that is substitued as-is.
case flag(String)

Expand All @@ -43,6 +43,9 @@ public struct Job: Codable, Equatable {
/// The command-line arguments of the job.
public var commandLine: [ArgTemplate]

/// Whether or not the job supports using response files to pass command line arguments.
public var supportsResponseFiles: Bool

/// The list of inputs to use for displaying purposes.
public var displayInputs: [TypedVirtualPath]

Expand All @@ -69,7 +72,8 @@ public struct Job: Codable, Equatable {
inputs: [TypedVirtualPath],
outputs: [TypedVirtualPath],
extraEnvironment: [String: String] = [:],
requiresInPlaceExecution: Bool = false
requiresInPlaceExecution: Bool = false,
supportsResponseFiles: Bool = false
) {
self.kind = kind
self.tool = tool
Expand All @@ -79,21 +83,7 @@ public struct Job: Codable, Equatable {
self.outputs = outputs
self.extraEnvironment = extraEnvironment
self.requiresInPlaceExecution = requiresInPlaceExecution
}
}

extension Job: CustomStringConvertible {
public var description: String {
var result: String = "\(tool.name) \(commandLine.joinedArguments)"

if !self.extraEnvironment.isEmpty {
result += " #"
for (envVar, val) in extraEnvironment {
result += " \(envVar)=\(val)"
}
}

return result
self.supportsResponseFiles = supportsResponseFiles
}
}

Expand Down
1 change: 1 addition & 0 deletions Sources/SwiftDriver/Jobs/LinkJob.swift
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ extension Driver {
targetTriple: targetTriple
)

// TODO: some, but not all, linkers support response files.
return Job(
kind: .link,
tool: .absolute(toolPath),
Expand Down
3 changes: 2 additions & 1 deletion Sources/SwiftDriver/Jobs/MergeModuleJob.swift
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ extension Driver {
tool: .absolute(try toolchain.getToolPath(.swiftCompiler)),
commandLine: commandLine,
inputs: inputs,
outputs: outputs
outputs: outputs,
supportsResponseFiles: true
)
}
}
6 changes: 6 additions & 0 deletions Sources/SwiftDriver/Utilities/StringAdditions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -133,3 +133,9 @@ extension Unicode.Scalar {
}
}
}

extension Character {
var isShellQuote: Bool {
return self == "\"" || self == "'"
}
}
Loading