From 7cf36a71e0afcccfefe80eaa8fd394005413bd14 Mon Sep 17 00:00:00 2001 From: Tony Allevato Date: Mon, 21 Mar 2022 12:53:46 -0700 Subject: [PATCH] Refactor job response file handling to (hopefully) prevent regressions. The new "emit module job" in Swift 5.6 was not marked as supporting response files (even though it's a `swift-frontend` invocation, all of which should support response files). This requires certain long `swiftc` invocations to pass `-no-emit-module-separately` in order to succeed. There are some other inconsistencies as well; for example, "emit PCM job" as implemented today does not support response files, when it should. This change refactors the interaction between `Tool`s and `Job`s. Whether or not response files are accepted is a property of the **tool** (and the toolchain), not of individual jobs. So, this adds a `supportsResponseFiles` method to `Tool` that returns that value (with the toolchain as an argument so that it can be conditional based on that, as it is for linking). Then, instead of asking the toolchain for the path to a tool when creating a job, you ask it for a `ResolvedTool`, which encapsulates the path and the knowledge about response files. The path can still be overridden if needed (as it is for linking in non-Darwin toolchains). This change aims to prevent future regressions for response file handling in a couple ways: * When creating a job, the caller no longer has to remember to pass a parameter indiciating whether it supports response files. (Worse, that parameter was optional, so auto-complete could easily leave it out!) * When introducing a new **tool,** the exhaustive switch in `Tool.supportsResponseFiles(in:)` requires the author of the change to acknowledge how it handles response files. * I've added regression tests to handle the common cases that involve large command lines (compiling Swift and generating PCMs). --- .../ExplicitDependencyBuildPlanner.swift | 4 +- .../ModuleDependencyScanning.swift | 15 ++-- .../SwiftDriver/Jobs/APIDigesterJobs.swift | 15 ++-- .../SwiftDriver/Jobs/AutolinkExtractJob.swift | 5 +- Sources/SwiftDriver/Jobs/BackendJob.swift | 5 +- Sources/SwiftDriver/Jobs/CompileJob.swift | 5 +- .../Jobs/DarwinToolchain+LinkerSupport.swift | 4 +- Sources/SwiftDriver/Jobs/EmitModuleJob.swift | 2 +- .../Jobs/EmitSupportedFeaturesJob.swift | 5 +- .../SwiftDriver/Jobs/GenerateDSYMJob.swift | 2 +- Sources/SwiftDriver/Jobs/GeneratePCHJob.swift | 5 +- Sources/SwiftDriver/Jobs/GeneratePCMJob.swift | 4 +- .../GenericUnixToolchain+LinkerSupport.swift | 14 ++-- Sources/SwiftDriver/Jobs/InterpretJob.swift | 5 +- Sources/SwiftDriver/Jobs/Job.swift | 9 +-- Sources/SwiftDriver/Jobs/LinkJob.swift | 9 +-- Sources/SwiftDriver/Jobs/MergeModuleJob.swift | 5 +- Sources/SwiftDriver/Jobs/ModuleWrapJob.swift | 5 +- Sources/SwiftDriver/Jobs/Planning.swift | 4 +- .../SwiftDriver/Jobs/PrebuiltModulesJob.swift | 4 +- .../SwiftDriver/Jobs/PrintTargetInfoJob.swift | 5 +- Sources/SwiftDriver/Jobs/ReplJob.swift | 2 +- .../SwiftDriver/Jobs/SwiftHelpIntroJob.swift | 2 +- .../SwiftDriver/Jobs/VerifyDebugInfoJob.swift | 2 +- .../Jobs/VerifyModuleInterfaceJob.swift | 2 +- .../WebAssemblyToolchain+LinkerSupport.swift | 6 +- .../Jobs/WindowsToolchain+LinkerSupport.swift | 12 +-- .../SwiftDriver/Toolchains/Toolchain.swift | 59 ++++++++++++++- Tests/SwiftDriverTests/JobExecutorTests.swift | 16 ++-- .../ModuleDependencyGraphTests.swift | 2 +- Tests/SwiftDriverTests/SwiftDriverTests.swift | 75 +++++++++++++++++++ 31 files changed, 212 insertions(+), 97 deletions(-) diff --git a/Sources/SwiftDriver/ExplicitModuleBuilds/ExplicitDependencyBuildPlanner.swift b/Sources/SwiftDriver/ExplicitModuleBuilds/ExplicitDependencyBuildPlanner.swift index 5ab4f31b1..7c684cd6a 100644 --- a/Sources/SwiftDriver/ExplicitModuleBuilds/ExplicitDependencyBuildPlanner.swift +++ b/Sources/SwiftDriver/ExplicitModuleBuilds/ExplicitDependencyBuildPlanner.swift @@ -172,7 +172,7 @@ public typealias ExternalTargetModuleDetailsMap = [ModuleDependencyId: ExternalT jobs.append(Job( moduleName: moduleId.moduleName, kind: .emitModule, - tool: .absolute(try toolchain.getToolPath(.swiftCompiler)), + tool: try toolchain.resolvedTool(.swiftCompiler), commandLine: commandLine, inputs: inputs, primaryInputs: [], @@ -237,7 +237,7 @@ public typealias ExternalTargetModuleDetailsMap = [ModuleDependencyId: ExternalT jobs.append(Job( moduleName: moduleId.moduleName, kind: .generatePCM, - tool: .absolute(try toolchain.getToolPath(.swiftCompiler)), + tool: try toolchain.resolvedTool(.swiftCompiler), commandLine: commandLine, inputs: inputs, primaryInputs: [], diff --git a/Sources/SwiftDriver/ExplicitModuleBuilds/ModuleDependencyScanning.swift b/Sources/SwiftDriver/ExplicitModuleBuilds/ModuleDependencyScanning.swift index 6952f4792..1fbf865e1 100644 --- a/Sources/SwiftDriver/ExplicitModuleBuilds/ModuleDependencyScanning.swift +++ b/Sources/SwiftDriver/ExplicitModuleBuilds/ModuleDependencyScanning.swift @@ -30,13 +30,12 @@ public extension Driver { // Construct the scanning job. return Job(moduleName: moduleOutputInfo.name, kind: .scanDependencies, - tool: VirtualPath.absolute(try toolchain.getToolPath(.swiftCompiler)), + tool: try toolchain.resolvedTool(.swiftCompiler), commandLine: commandLine, displayInputs: inputs, inputs: inputs, primaryInputs: [], - outputs: [TypedVirtualPath(file: .standardOutput, type: .jsonDependencies)], - supportsResponseFiles: false) + outputs: [TypedVirtualPath(file: .standardOutput, type: .jsonDependencies)]) } /// Generate a full command-line invocation to be used for the dependency scanning action @@ -263,13 +262,12 @@ public extension Driver { // Construct the scanning job. return Job(moduleName: moduleOutputInfo.name, kind: .scanDependencies, - tool: VirtualPath.absolute(try toolchain.getToolPath(.swiftCompiler)), + tool: try toolchain.resolvedTool(.swiftCompiler), commandLine: commandLine, displayInputs: inputs, inputs: inputs, primaryInputs: [], - outputs: [TypedVirtualPath(file: .standardOutput, type: .jsonDependencies)], - supportsResponseFiles: false) + outputs: [TypedVirtualPath(file: .standardOutput, type: .jsonDependencies)]) } /// Precompute the dependencies for a given collection of modules using swift frontend's batch scanning mode @@ -312,13 +310,12 @@ public extension Driver { // Construct the scanning job. return Job(moduleName: moduleOutputInfo.name, kind: .scanDependencies, - tool: VirtualPath.absolute(try toolchain.getToolPath(.swiftCompiler)), + tool: try toolchain.resolvedTool(.swiftCompiler), commandLine: commandLine, displayInputs: inputs, inputs: inputs, primaryInputs: [], - outputs: outputs, - supportsResponseFiles: false) + outputs: outputs) } /// Serialize a collection of modules into an input format expected by the batch module dependency scanner. diff --git a/Sources/SwiftDriver/Jobs/APIDigesterJobs.swift b/Sources/SwiftDriver/Jobs/APIDigesterJobs.swift index dacd0ad5f..84d8299e2 100644 --- a/Sources/SwiftDriver/Jobs/APIDigesterJobs.swift +++ b/Sources/SwiftDriver/Jobs/APIDigesterJobs.swift @@ -56,12 +56,11 @@ extension Driver { return Job( moduleName: moduleOutputInfo.name, kind: mode.baselineGenerationJobKind, - tool: .absolute(try toolchain.getToolPath(.swiftAPIDigester)), + tool: try toolchain.resolvedTool(.swiftAPIDigester), commandLine: commandLine, inputs: [.init(file: modulePath, type: .swiftModule)], primaryInputs: [], - outputs: [.init(file: outputPath, type: mode.baselineFileType)], - supportsResponseFiles: true + outputs: [.init(file: outputPath, type: mode.baselineFileType)] ) } @@ -100,12 +99,11 @@ extension Driver { return Job( moduleName: moduleOutputInfo.name, kind: .compareABIBaseline, - tool: .absolute(try toolchain.getToolPath(.swiftAPIDigester)), + tool: try toolchain.resolvedTool(.swiftAPIDigester), commandLine: commandLine, inputs: inputs, primaryInputs: [], - outputs: [diag], - supportsResponseFiles: true + outputs: [diag] ) } @@ -141,12 +139,11 @@ extension Driver { return Job( moduleName: moduleOutputInfo.name, kind: mode.baselineComparisonJobKind, - tool: .absolute(try toolchain.getToolPath(.swiftAPIDigester)), + tool: try toolchain.resolvedTool(.swiftAPIDigester), commandLine: commandLine, inputs: inputs, primaryInputs: [], - outputs: [.init(file: serializedDiagnosticsPath ?? VirtualPath.Handle.standardOutput, type: .diagnostics)], - supportsResponseFiles: true + outputs: [.init(file: serializedDiagnosticsPath ?? VirtualPath.Handle.standardOutput, type: .diagnostics)] ) } diff --git a/Sources/SwiftDriver/Jobs/AutolinkExtractJob.swift b/Sources/SwiftDriver/Jobs/AutolinkExtractJob.swift index 6bfce40e8..f1553eb28 100644 --- a/Sources/SwiftDriver/Jobs/AutolinkExtractJob.swift +++ b/Sources/SwiftDriver/Jobs/AutolinkExtractJob.swift @@ -43,12 +43,11 @@ extension Driver { return Job( moduleName: moduleOutputInfo.name, kind: .autolinkExtract, - tool: .absolute(try toolchain.getToolPath(.swiftAutolinkExtract)), + tool: try toolchain.resolvedTool(.swiftAutolinkExtract), commandLine: commandLine, inputs: inputs, primaryInputs: [], - outputs: [.init(file: output.intern(), type: .autolink)], - supportsResponseFiles: true + outputs: [.init(file: output.intern(), type: .autolink)] ) } } diff --git a/Sources/SwiftDriver/Jobs/BackendJob.swift b/Sources/SwiftDriver/Jobs/BackendJob.swift index b5c5bd7c2..dc6fa0718 100644 --- a/Sources/SwiftDriver/Jobs/BackendJob.swift +++ b/Sources/SwiftDriver/Jobs/BackendJob.swift @@ -70,13 +70,12 @@ extension Driver { return Job( moduleName: moduleOutputInfo.name, kind: .backend, - tool: .absolute(try toolchain.getToolPath(.swiftCompiler)), + tool: try toolchain.resolvedTool(.swiftCompiler), commandLine: commandLine, displayInputs: inputs, inputs: inputs, primaryInputs: [], - outputs: outputs, - supportsResponseFiles: true + outputs: outputs ) } } diff --git a/Sources/SwiftDriver/Jobs/CompileJob.swift b/Sources/SwiftDriver/Jobs/CompileJob.swift index 8f5461c29..9aeb94e9f 100644 --- a/Sources/SwiftDriver/Jobs/CompileJob.swift +++ b/Sources/SwiftDriver/Jobs/CompileJob.swift @@ -383,14 +383,13 @@ extension Driver { return Job( moduleName: moduleOutputInfo.name, kind: .compile, - tool: .absolute(try toolchain.getToolPath(.swiftCompiler)), + tool: try toolchain.resolvedTool(.swiftCompiler), commandLine: commandLine, displayInputs: displayInputs, inputs: inputs, primaryInputs: primaryInputs, outputs: outputs, - inputOutputMap: inputOutputMap, - supportsResponseFiles: true + inputOutputMap: inputOutputMap ) } } diff --git a/Sources/SwiftDriver/Jobs/DarwinToolchain+LinkerSupport.swift b/Sources/SwiftDriver/Jobs/DarwinToolchain+LinkerSupport.swift index a70e91afd..0ed583ffa 100644 --- a/Sources/SwiftDriver/Jobs/DarwinToolchain+LinkerSupport.swift +++ b/Sources/SwiftDriver/Jobs/DarwinToolchain+LinkerSupport.swift @@ -206,7 +206,7 @@ extension DarwinToolchain { lto: LTOKind?, sanitizers: Set, targetInfo: FrontendTargetInfo - ) throws -> AbsolutePath { + ) throws -> ResolvedTool { // Set up for linking. let linkerTool: Tool switch linkerOutputType { @@ -251,7 +251,7 @@ extension DarwinToolchain { commandLine.appendFlag("-o") commandLine.appendPath(outputFile) - return try getToolPath(linkerTool) + return try resolvedTool(linkerTool) } private func addLinkInputs(shouldUseInputFileList: Bool, diff --git a/Sources/SwiftDriver/Jobs/EmitModuleJob.swift b/Sources/SwiftDriver/Jobs/EmitModuleJob.swift index f501ea178..bdfb14610 100644 --- a/Sources/SwiftDriver/Jobs/EmitModuleJob.swift +++ b/Sources/SwiftDriver/Jobs/EmitModuleJob.swift @@ -120,7 +120,7 @@ extension Driver { return Job( moduleName: moduleOutputInfo.name, kind: .emitModule, - tool: .absolute(try toolchain.getToolPath(.swiftCompiler)), + tool: try toolchain.resolvedTool(.swiftCompiler), commandLine: commandLine, inputs: inputs, primaryInputs: [], diff --git a/Sources/SwiftDriver/Jobs/EmitSupportedFeaturesJob.swift b/Sources/SwiftDriver/Jobs/EmitSupportedFeaturesJob.swift index 37e0b555f..460695273 100644 --- a/Sources/SwiftDriver/Jobs/EmitSupportedFeaturesJob.swift +++ b/Sources/SwiftDriver/Jobs/EmitSupportedFeaturesJob.swift @@ -40,14 +40,13 @@ extension Toolchain { return Job( moduleName: "", kind: .emitSupportedFeatures, - tool: .absolute(try getToolPath(.swiftCompiler)), + tool: try resolvedTool(.swiftCompiler), commandLine: commandLine, displayInputs: [], inputs: inputs, primaryInputs: [], outputs: [.init(file: .standardOutput, type: .jsonCompilerFeatures)], - requiresInPlaceExecution: requiresInPlaceExecution, - supportsResponseFiles: false + requiresInPlaceExecution: requiresInPlaceExecution ) } } diff --git a/Sources/SwiftDriver/Jobs/GenerateDSYMJob.swift b/Sources/SwiftDriver/Jobs/GenerateDSYMJob.swift index 525625ea6..c03348ee9 100644 --- a/Sources/SwiftDriver/Jobs/GenerateDSYMJob.swift +++ b/Sources/SwiftDriver/Jobs/GenerateDSYMJob.swift @@ -33,7 +33,7 @@ extension Driver { return Job( moduleName: moduleOutputInfo.name, kind: .generateDSYM, - tool: .absolute(try toolchain.getToolPath(.dsymutil)), + tool: try toolchain.resolvedTool(.dsymutil), commandLine: commandLine, displayInputs: [], inputs: inputs, diff --git a/Sources/SwiftDriver/Jobs/GeneratePCHJob.swift b/Sources/SwiftDriver/Jobs/GeneratePCHJob.swift index 299655652..5637c3760 100644 --- a/Sources/SwiftDriver/Jobs/GeneratePCHJob.swift +++ b/Sources/SwiftDriver/Jobs/GeneratePCHJob.swift @@ -64,13 +64,12 @@ extension Driver { return Job( moduleName: moduleOutputInfo.name, kind: .generatePCH, - tool: .absolute(try toolchain.getToolPath(.swiftCompiler)), + tool: try toolchain.resolvedTool(.swiftCompiler), commandLine: commandLine, displayInputs: [], inputs: inputs, primaryInputs: [], - outputs: outputs, - supportsResponseFiles: true + outputs: outputs ) } } diff --git a/Sources/SwiftDriver/Jobs/GeneratePCMJob.swift b/Sources/SwiftDriver/Jobs/GeneratePCMJob.swift index 305c107e2..7a75a22af 100644 --- a/Sources/SwiftDriver/Jobs/GeneratePCMJob.swift +++ b/Sources/SwiftDriver/Jobs/GeneratePCMJob.swift @@ -55,7 +55,7 @@ extension Driver { return Job( moduleName: moduleOutputInfo.name, kind: .generatePCM, - tool: .absolute(try toolchain.getToolPath(.swiftCompiler)), + tool: try toolchain.resolvedTool(.swiftCompiler), commandLine: commandLine, displayInputs: [], inputs: inputs, @@ -84,7 +84,7 @@ extension Driver { return Job( moduleName: moduleOutputInfo.name, kind: .dumpPCM, - tool: .absolute(try toolchain.getToolPath(.swiftCompiler)), + tool: try toolchain.resolvedTool(.swiftCompiler), commandLine: commandLine, displayInputs: [], inputs: inputs, diff --git a/Sources/SwiftDriver/Jobs/GenericUnixToolchain+LinkerSupport.swift b/Sources/SwiftDriver/Jobs/GenericUnixToolchain+LinkerSupport.swift index 2da0f108c..ff2799a82 100644 --- a/Sources/SwiftDriver/Jobs/GenericUnixToolchain+LinkerSupport.swift +++ b/Sources/SwiftDriver/Jobs/GenericUnixToolchain+LinkerSupport.swift @@ -58,7 +58,7 @@ extension GenericUnixToolchain { lto: LTOKind?, sanitizers: Set, targetInfo: FrontendTargetInfo - ) throws -> AbsolutePath { + ) throws -> ResolvedTool { let targetTriple = targetInfo.target.triple switch linkerOutputType { case .dynamicLibrary: @@ -103,9 +103,9 @@ extension GenericUnixToolchain { // Windows rather than msvcprt). When C++ interop is enabled, we will need to // surface this via a driver flag. For now, opt for the simpler approach of // just using `clang` and avoid a dependency on the C++ runtime. - var clangPath = try parsedOptions.hasArgument(.enableExperimentalCxxInterop) - ? getToolPath(.clangxx) - : getToolPath(.clang) + let clangTool: Tool = + parsedOptions.hasArgument(.enableExperimentalCxxInterop) ? .clangxx : .clang + var clangPath = try getToolPath(clangTool) if let toolsDirPath = parsedOptions.getLastArgument(.toolsDirectory) { // FIXME: What if this isn't an absolute path? let toolsDir = try AbsolutePath(validating: toolsDirPath.asSingle) @@ -296,7 +296,7 @@ extension GenericUnixToolchain { // This should be the last option, for convenience in checking output. commandLine.appendFlag(.o) commandLine.appendPath(outputFile) - return clangPath + return try resolvedTool(clangTool, pathOverride: clangPath) case .staticLibrary: // We're using 'ar' as a linker commandLine.appendFlag("crs") @@ -308,9 +308,9 @@ extension GenericUnixToolchain { }.map { .path($0.file) }) if targetTriple.environment == .android { // Always use the LTO archiver llvm-ar for Android - return try getToolPath(.staticLinker(.llvmFull)) + return try resolvedTool(.staticLinker(.llvmFull)) } else { - return try getToolPath(.staticLinker(lto)) + return try resolvedTool(.staticLinker(lto)) } } diff --git a/Sources/SwiftDriver/Jobs/InterpretJob.swift b/Sources/SwiftDriver/Jobs/InterpretJob.swift index aae66aca4..ba91f91cf 100644 --- a/Sources/SwiftDriver/Jobs/InterpretJob.swift +++ b/Sources/SwiftDriver/Jobs/InterpretJob.swift @@ -44,14 +44,13 @@ extension Driver { return Job( moduleName: moduleOutputInfo.name, kind: .interpret, - tool: .absolute(try toolchain.getToolPath(.swiftCompiler)), + tool: try toolchain.resolvedTool(.swiftCompiler), commandLine: commandLine, inputs: inputs, primaryInputs: [], outputs: [], extraEnvironment: extraEnvironment, - requiresInPlaceExecution: true, - supportsResponseFiles: true + requiresInPlaceExecution: true ) } } diff --git a/Sources/SwiftDriver/Jobs/Job.swift b/Sources/SwiftDriver/Jobs/Job.swift index 8263849ce..6eee77987 100644 --- a/Sources/SwiftDriver/Jobs/Job.swift +++ b/Sources/SwiftDriver/Jobs/Job.swift @@ -106,7 +106,7 @@ public struct Job: Codable, Equatable, Hashable { public init( moduleName: String, kind: Kind, - tool: VirtualPath, + tool: ResolvedTool, commandLine: [ArgTemplate], displayInputs: [TypedVirtualPath]? = nil, inputs: [TypedVirtualPath], @@ -114,12 +114,11 @@ public struct Job: Codable, Equatable, Hashable { outputs: [TypedVirtualPath], inputOutputMap: [TypedVirtualPath : [TypedVirtualPath]] = [:], extraEnvironment: [String: String] = [:], - requiresInPlaceExecution: Bool = false, - supportsResponseFiles: Bool = false + requiresInPlaceExecution: Bool = false ) { self.moduleName = moduleName self.kind = kind - self.tool = tool + self.tool = .absolute(tool.path) self.commandLine = commandLine self.displayInputs = displayInputs ?? [] self.inputs = inputs @@ -128,7 +127,7 @@ public struct Job: Codable, Equatable, Hashable { self.compileInputOutputMap = inputOutputMap self.extraEnvironment = extraEnvironment self.requiresInPlaceExecution = requiresInPlaceExecution - self.supportsResponseFiles = supportsResponseFiles + self.supportsResponseFiles = tool.supportsResponseFiles } public var primarySwiftSourceFiles: [SwiftSourceFile] { primaryInputs.swiftSourceFiles } diff --git a/Sources/SwiftDriver/Jobs/LinkJob.swift b/Sources/SwiftDriver/Jobs/LinkJob.swift index 3ed9353ea..257c642c7 100644 --- a/Sources/SwiftDriver/Jobs/LinkJob.swift +++ b/Sources/SwiftDriver/Jobs/LinkJob.swift @@ -52,7 +52,7 @@ extension Driver { } // Defer to the toolchain for platform-specific linking - let toolPath = try toolchain.addPlatformSpecificLinkerArgs( + let linkTool = try toolchain.addPlatformSpecificLinkerArgs( to: &commandLine, parsedOptions: &parsedOptions, linkerOutputType: linkerOutputType!, @@ -67,15 +67,12 @@ extension Driver { return Job( moduleName: moduleOutputInfo.name, kind: .link, - tool: .absolute(toolPath), + tool: linkTool, commandLine: commandLine, displayInputs: inputs, inputs: inputs, primaryInputs: [], - outputs: [.init(file: outputFile.intern(), type: .image)], - // FIXME: newer ld64 supports response files as well, though really, - // Darwin should use clang as the linker driver like the other targets - supportsResponseFiles: !frontendTargetInfo.target.triple.isDarwin + outputs: [.init(file: outputFile.intern(), type: .image)] ) } } diff --git a/Sources/SwiftDriver/Jobs/MergeModuleJob.swift b/Sources/SwiftDriver/Jobs/MergeModuleJob.swift index 34788ccee..4f1cfc0ae 100644 --- a/Sources/SwiftDriver/Jobs/MergeModuleJob.swift +++ b/Sources/SwiftDriver/Jobs/MergeModuleJob.swift @@ -86,12 +86,11 @@ extension Driver { return Job( moduleName: moduleOutputInfo.name, kind: .mergeModule, - tool: .absolute(try toolchain.getToolPath(.swiftCompiler)), + tool: try toolchain.resolvedTool(.swiftCompiler), commandLine: commandLine, inputs: inputs, primaryInputs: [], - outputs: outputs, - supportsResponseFiles: true + outputs: outputs ) } } diff --git a/Sources/SwiftDriver/Jobs/ModuleWrapJob.swift b/Sources/SwiftDriver/Jobs/ModuleWrapJob.swift index 9f498b66d..4d3b6d342 100644 --- a/Sources/SwiftDriver/Jobs/ModuleWrapJob.swift +++ b/Sources/SwiftDriver/Jobs/ModuleWrapJob.swift @@ -29,12 +29,11 @@ extension Driver { return Job( moduleName: moduleOutputInfo.name, kind: .moduleWrap, - tool: .absolute(try toolchain.getToolPath(.swiftCompiler)), + tool: try toolchain.resolvedTool(.swiftCompiler), commandLine: commandLine, inputs: [moduleInput], primaryInputs: [], - outputs: [.init(file: outputPath.intern(), type: .object)], - supportsResponseFiles: true + outputs: [.init(file: outputPath.intern(), type: .object)] ) } } diff --git a/Sources/SwiftDriver/Jobs/Planning.swift b/Sources/SwiftDriver/Jobs/Planning.swift index 05d254842..d51bc20bf 100644 --- a/Sources/SwiftDriver/Jobs/Planning.swift +++ b/Sources/SwiftDriver/Jobs/Planning.swift @@ -672,7 +672,7 @@ extension Driver { return Job( moduleName: moduleOutputInfo.name, kind: .versionRequest, - tool: .absolute(try toolchain.getToolPath(.swiftCompiler)), + tool: try toolchain.resolvedTool(.swiftCompiler), commandLine: [.flag("--version")], inputs: [], primaryInputs: [], @@ -688,7 +688,7 @@ extension Driver { return Job( moduleName: moduleOutputInfo.name, kind: .help, - tool: .absolute(try toolchain.getToolPath(.swiftHelp)), + tool: try toolchain.resolvedTool(.swiftHelp), commandLine: commandLine, inputs: [], primaryInputs: [], diff --git a/Sources/SwiftDriver/Jobs/PrebuiltModulesJob.swift b/Sources/SwiftDriver/Jobs/PrebuiltModulesJob.swift index b0393c741..a4b181c34 100644 --- a/Sources/SwiftDriver/Jobs/PrebuiltModulesJob.swift +++ b/Sources/SwiftDriver/Jobs/PrebuiltModulesJob.swift @@ -435,7 +435,7 @@ extension Driver { return Job( moduleName: moduleName, kind: .compareABIBaseline, - tool: .absolute(try toolchain.getToolPath(.swiftAPIDigester)), + tool: try toolchain.resolvedTool(.swiftAPIDigester), commandLine: commandLine, inputs: [currentABI, baselineABI], primaryInputs: [], @@ -500,7 +500,7 @@ extension Driver { allJobs.append(Job( moduleName: moduleName, kind: .compile, - tool: .absolute(try toolchain.getToolPath(.swiftCompiler)), + tool: try toolchain.resolvedTool(.swiftCompiler), commandLine: commandLine, inputs: allInputs, primaryInputs: [], diff --git a/Sources/SwiftDriver/Jobs/PrintTargetInfoJob.swift b/Sources/SwiftDriver/Jobs/PrintTargetInfoJob.swift index 8226ab9eb..a5ff3723b 100644 --- a/Sources/SwiftDriver/Jobs/PrintTargetInfoJob.swift +++ b/Sources/SwiftDriver/Jobs/PrintTargetInfoJob.swift @@ -163,14 +163,13 @@ extension Toolchain { return Job( moduleName: "", kind: .printTargetInfo, - tool: .absolute(try getToolPath(.swiftCompiler)), + tool: try resolvedTool(.swiftCompiler), commandLine: commandLine, displayInputs: [], inputs: [], primaryInputs: [], outputs: [.init(file: .standardOutput, type: .jsonTargetInfo)], - requiresInPlaceExecution: requiresInPlaceExecution, - supportsResponseFiles: false + requiresInPlaceExecution: requiresInPlaceExecution ) } } diff --git a/Sources/SwiftDriver/Jobs/ReplJob.swift b/Sources/SwiftDriver/Jobs/ReplJob.swift index 74f4bbe00..94ef06700 100644 --- a/Sources/SwiftDriver/Jobs/ReplJob.swift +++ b/Sources/SwiftDriver/Jobs/ReplJob.swift @@ -30,7 +30,7 @@ extension Driver { return Job( moduleName: moduleOutputInfo.name, kind: .repl, - tool: .absolute(try toolchain.getToolPath(.lldb)), + tool: try toolchain.resolvedTool(.lldb), commandLine: lldbCommandLine, inputs: inputs, primaryInputs: [], diff --git a/Sources/SwiftDriver/Jobs/SwiftHelpIntroJob.swift b/Sources/SwiftDriver/Jobs/SwiftHelpIntroJob.swift index e04c15523..51c6f766c 100644 --- a/Sources/SwiftDriver/Jobs/SwiftHelpIntroJob.swift +++ b/Sources/SwiftDriver/Jobs/SwiftHelpIntroJob.swift @@ -17,7 +17,7 @@ extension Driver { Job( moduleName: moduleOutputInfo.name, kind: .help, - tool: .absolute(try toolchain.getToolPath(.swiftHelp)), + tool: try toolchain.resolvedTool(.swiftHelp), commandLine: [.flag("intro")], inputs: [], primaryInputs: [], diff --git a/Sources/SwiftDriver/Jobs/VerifyDebugInfoJob.swift b/Sources/SwiftDriver/Jobs/VerifyDebugInfoJob.swift index 86268cbb4..3b1c42a25 100644 --- a/Sources/SwiftDriver/Jobs/VerifyDebugInfoJob.swift +++ b/Sources/SwiftDriver/Jobs/VerifyDebugInfoJob.swift @@ -23,7 +23,7 @@ extension Driver { return Job( moduleName: moduleOutputInfo.name, kind: .verifyDebugInfo, - tool: .absolute(try toolchain.getToolPath(.dwarfdump)), + tool: try toolchain.resolvedTool(.dwarfdump), commandLine: commandLine, displayInputs: [], inputs: inputs, diff --git a/Sources/SwiftDriver/Jobs/VerifyModuleInterfaceJob.swift b/Sources/SwiftDriver/Jobs/VerifyModuleInterfaceJob.swift index fd7d8e951..11c88be09 100644 --- a/Sources/SwiftDriver/Jobs/VerifyModuleInterfaceJob.swift +++ b/Sources/SwiftDriver/Jobs/VerifyModuleInterfaceJob.swift @@ -33,7 +33,7 @@ extension Driver { return Job( moduleName: moduleOutputInfo.name, kind: .verifyModuleInterface, - tool: .absolute(try toolchain.getToolPath(.swiftCompiler)), + tool: try toolchain.resolvedTool(.swiftCompiler), commandLine: commandLine, displayInputs: [interfaceInput], inputs: inputs, diff --git a/Sources/SwiftDriver/Jobs/WebAssemblyToolchain+LinkerSupport.swift b/Sources/SwiftDriver/Jobs/WebAssemblyToolchain+LinkerSupport.swift index 72f00e6be..012cb7745 100644 --- a/Sources/SwiftDriver/Jobs/WebAssemblyToolchain+LinkerSupport.swift +++ b/Sources/SwiftDriver/Jobs/WebAssemblyToolchain+LinkerSupport.swift @@ -23,7 +23,7 @@ extension WebAssemblyToolchain { lto: LTOKind?, sanitizers: Set, targetInfo: FrontendTargetInfo - ) throws -> AbsolutePath { + ) throws -> ResolvedTool { let targetTriple = targetInfo.target.triple switch linkerOutputType { case .dynamicLibrary: @@ -151,14 +151,14 @@ extension WebAssemblyToolchain { // This should be the last option, for convenience in checking output. commandLine.appendFlag(.o) commandLine.appendPath(outputFile) - return clangPath + return try resolvedTool(.clang, pathOverride: clangPath) case .staticLibrary: // We're using 'ar' as a linker commandLine.appendFlag("crs") commandLine.appendPath(outputFile) commandLine.append(contentsOf: inputs.map { .path($0.file) }) - return try getToolPath(.staticLinker(lto)) + return try resolvedTool(.staticLinker(lto)) } } } diff --git a/Sources/SwiftDriver/Jobs/WindowsToolchain+LinkerSupport.swift b/Sources/SwiftDriver/Jobs/WindowsToolchain+LinkerSupport.swift index 3851ae95b..2c7de9b84 100644 --- a/Sources/SwiftDriver/Jobs/WindowsToolchain+LinkerSupport.swift +++ b/Sources/SwiftDriver/Jobs/WindowsToolchain+LinkerSupport.swift @@ -36,7 +36,7 @@ extension WindowsToolchain { lto: LTOKind?, sanitizers: Set, targetInfo: FrontendTargetInfo) - throws -> AbsolutePath { + throws -> ResolvedTool { // Special case static linking as clang cannot drive the operation. if linkerOutputType == .staticLibrary { let librarian: String @@ -57,12 +57,12 @@ extension WindowsToolchain { commandLine.append(contentsOf: inputs.lazy.filter { types.contains($0.type) } .map { .path($0.file) }) - return try lookup(executable: librarian) + return try resolvedTool(.staticLinker(lto), pathOverride: lookup(executable: librarian)) } - var clang = try parsedOptions.hasArgument(.enableExperimentalCxxInterop) - ? getToolPath(.clangxx) - : getToolPath(.clang) + let clangTool: Tool = + parsedOptions.hasArgument(.enableExperimentalCxxInterop) ? .clangxx : .clang + var clang = try getToolPath(clangTool) let targetTriple = targetInfo.target.triple if !targetTriple.triple.isEmpty { @@ -194,6 +194,6 @@ extension WindowsToolchain { addLinkedLibArgs(to: &commandLine, parsedOptions: &parsedOptions) // TODO(compnerd) handle static libraries - return clang + return try resolvedTool(clangTool, pathOverride: clang) } } diff --git a/Sources/SwiftDriver/Toolchains/Toolchain.swift b/Sources/SwiftDriver/Toolchains/Toolchain.swift index fe6205ad7..e01af7c51 100644 --- a/Sources/SwiftDriver/Toolchains/Toolchain.swift +++ b/Sources/SwiftDriver/Toolchains/Toolchain.swift @@ -26,6 +26,46 @@ public enum Tool: Hashable { case dwarfdump case swiftHelp case swiftAPIDigester + + /// Returns a value indicating whether or not the tool supports passing arguments via response + /// files. + public func supportsResponseFiles(in toolchain: Toolchain) -> Bool { + switch self { + case .swiftCompiler, .clang, .clangxx, .swiftAutolinkExtract, .swiftAPIDigester: + return true + + case .dsymutil, .lldb, .dwarfdump, .swiftHelp: + // NOTE: Consider *very carefully* whether a tool actually belongs here when adding a new + // entry. Incorrectly marking a tool as not supporting response files when it does may cause + // large builds to fail that would have otherwise succeeded. + return false + + case .staticLinker, .dynamicLinker: + // FIXME: newer ld64 supports response files as well, though really, + // Darwin should use clang as the linker driver like the other targets + return !(toolchain is DarwinToolchain) + } + } +} + +/// Encapsulates the path to a tool and the knowledge of whether or not it supports taking long +/// command lines as response files. +public struct ResolvedTool { + /// The absolute path to the tool's executable. + public var path: AbsolutePath + + /// Indicates whether the tool can accept long command lines in a response file. + public var supportsResponseFiles: Bool + + /// Creates a new resolved tool with the given path and response file nature. + /// + /// - Note: In most cases, you should **not** call this initializer directly. Instead, use the + /// `Toolchain.resolvedTool(_:pathOverride:)` method, which computes these values based on the + /// requested tool and toolchain. + @_spi(Testing) public init(path: AbsolutePath, supportsResponseFiles: Bool) { + self.path = path + self.supportsResponseFiles = supportsResponseFiles + } } /// Describes a toolchain, which includes information about compilers, linkers @@ -88,7 +128,7 @@ public protocol Toolchain { lto: LTOKind?, sanitizers: Set, targetInfo: FrontendTargetInfo - ) throws -> AbsolutePath + ) throws -> ResolvedTool func runtimeLibraryName( for sanitizer: Sanitizer, @@ -233,6 +273,23 @@ extension Toolchain { frontendTargetInfo: FrontendTargetInfo, driver: Driver ) throws {} + + /// Resolves the path to the given tool and whether or not it supports response files so that it + /// can be passed to a job. + /// + /// - Parameters: + /// - tool: The `Tool` to resolve. Whether or not the invocation supports response files is + /// determined based on how this value responds to the `supportsResponseFiles(in:)` method. + /// - pathOverride: If provided, this path will be used as the path to the tool's executable + /// instead of the default path determined by the toolchain. + /// - Returns: A `ResolvedTool` value that provides the path and response file information about + /// the tool when creating a `Job`. + public func resolvedTool(_ tool: Tool, pathOverride: AbsolutePath? = nil) throws -> ResolvedTool { + return ResolvedTool( + path: try pathOverride ?? getToolPath(tool), + supportsResponseFiles: tool.supportsResponseFiles(in: self) + ) + } } @_spi(Testing) public enum ToolchainError: Swift.Error { diff --git a/Tests/SwiftDriverTests/JobExecutorTests.swift b/Tests/SwiftDriverTests/JobExecutorTests.swift index 6b53c316a..51568f186 100644 --- a/Tests/SwiftDriverTests/JobExecutorTests.swift +++ b/Tests/SwiftDriverTests/JobExecutorTests.swift @@ -130,7 +130,7 @@ final class JobExecutorTests: XCTestCase { let compileFoo = Job( moduleName: "main", kind: .compile, - tool: .absolute(try toolchain.getToolPath(.swiftCompiler)), + tool: try toolchain.resolvedTool(.swiftCompiler), commandLine: [ "-frontend", "-c", @@ -152,7 +152,7 @@ final class JobExecutorTests: XCTestCase { let compileMain = Job( moduleName: "main", kind: .compile, - tool: .absolute(try toolchain.getToolPath(.swiftCompiler)), + tool: try toolchain.resolvedTool(.swiftCompiler), commandLine: [ "-frontend", "-c", @@ -174,7 +174,7 @@ final class JobExecutorTests: XCTestCase { let link = Job( moduleName: "main", kind: .link, - tool: .absolute(try toolchain.getToolPath(.dynamicLinker)), + tool: try toolchain.resolvedTool(.dynamicLinker), commandLine: [ .path(.temporary(RelativePath("foo.o"))), .path(.temporary(RelativePath("main.o"))), @@ -224,7 +224,7 @@ final class JobExecutorTests: XCTestCase { let compile = Job( moduleName: "main", kind: .compile, - tool: .absolute(try toolchain.getToolPath(.swiftCompiler)), + tool: try toolchain.resolvedTool(.swiftCompiler), commandLine: [ "-frontend", "-c", @@ -246,7 +246,7 @@ final class JobExecutorTests: XCTestCase { let link = Job( moduleName: "main", kind: .link, - tool: .absolute(try toolchain.getToolPath(.dynamicLinker)), + tool: try toolchain.resolvedTool(.dynamicLinker), commandLine: [ .path(.temporary(RelativePath("main.o"))), .path(.absolute(try toolchain.clangRT.get())), @@ -294,7 +294,7 @@ final class JobExecutorTests: XCTestCase { let job = Job( moduleName: "main", kind: .compile, - tool: .absolute(AbsolutePath("/usr/bin/swift")), + tool: ResolvedTool(path: AbsolutePath("/usr/bin/swift"), supportsResponseFiles: false), commandLine: [.flag("something")], inputs: [], primaryInputs: [], @@ -380,7 +380,9 @@ final class JobExecutorTests: XCTestCase { env: [:]) let job = Job(moduleName: "Module", kind: .compile, - tool: .absolute(.init("/path/to/the tool")), + tool: ResolvedTool( + path: AbsolutePath("/path/to/the tool"), + supportsResponseFiles: false), commandLine: [.path(.absolute(.init("/with space"))), .path(.absolute(.init("/withoutspace")))], inputs: [], primaryInputs: [], outputs: []) diff --git a/Tests/SwiftDriverTests/ModuleDependencyGraphTests.swift b/Tests/SwiftDriverTests/ModuleDependencyGraphTests.swift index 9e618bdda..998899c27 100644 --- a/Tests/SwiftDriverTests/ModuleDependencyGraphTests.swift +++ b/Tests/SwiftDriverTests/ModuleDependencyGraphTests.swift @@ -1451,7 +1451,7 @@ extension Job { type: .swift) try! self.init(moduleName: "nothing", kind: .compile, - tool: VirtualPath(path: ""), + tool: ResolvedTool(path: AbsolutePath("/dummy"), supportsResponseFiles: false), commandLine: [], inputs: [input], primaryInputs: [input], diff --git a/Tests/SwiftDriverTests/SwiftDriverTests.swift b/Tests/SwiftDriverTests/SwiftDriverTests.swift index fcb4343e4..f97fb0e46 100644 --- a/Tests/SwiftDriverTests/SwiftDriverTests.swift +++ b/Tests/SwiftDriverTests/SwiftDriverTests.swift @@ -1246,6 +1246,81 @@ final class SwiftDriverTests: XCTestCase { } } + func testSpecificJobsResponseFiles() throws { + // The jobs below often take large command lines (e.g., when passing a large number of Clang + // modules to Swift). Ensure that they don't regress in their ability to pass response files + // from the driver to the frontend. + let manyArgs = (1...20000).map { "-DTEST_\($0)" } + + // Compile + separate emit module job + do { + let resolver = try ArgsResolver(fileSystem: localFileSystem) + var driver = try Driver( + args: ["swiftc", "-emit-module"] + manyArgs + + ["-module-name", "foo", "foo.swift", "bar.swift"]) + let jobs = try driver.planBuild().removingAutolinkExtractJobs() + XCTAssertEqual(jobs.count, 3) + XCTAssertEqual(Set(jobs.map { $0.kind }), Set([.emitModule, .compile])) + + let emitModuleJob = jobs.first(where: { $0.kind == .emitModule })! + let emitModuleResolvedArgs: [String] = + try resolver.resolveArgumentList(for: emitModuleJob, forceResponseFiles: false) + 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) + XCTAssertEqual(compileResolvedArgs.count, 2) + XCTAssertEqual(compileResolvedArgs[1].first, "@") + } + } + + // Compile + no separate emit module job + do { + let resolver = try ArgsResolver(fileSystem: localFileSystem) + var driver = try Driver( + args: ["swiftc", "-emit-module", "-no-emit-module-separately"] + manyArgs + + ["-module-name", "foo", "foo.swift", "bar.swift"]) + let jobs = try driver.planBuild().removingAutolinkExtractJobs() + XCTAssertEqual(jobs.count, 3) + XCTAssertEqual(Set(jobs.map { $0.kind }), Set([.compile, .mergeModule])) + + let mergeModuleJob = jobs.first(where: { $0.kind == .mergeModule })! + let mergeModuleResolvedArgs: [String] = + try resolver.resolveArgumentList(for: mergeModuleJob, forceResponseFiles: false) + 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) + XCTAssertEqual(compileResolvedArgs.count, 2) + XCTAssertEqual(compileResolvedArgs[1].first, "@") + } + } + + // Generate PCM (precompiled Clang module) job + do { + let resolver = try ArgsResolver(fileSystem: localFileSystem) + var driver = try Driver( + args: ["swiftc", "-emit-pcm"] + manyArgs + ["-module-name", "foo", "foo.modulemap"]) + let jobs = try driver.planBuild().removingAutolinkExtractJobs() + XCTAssertEqual(jobs.count, 1) + XCTAssertEqual(jobs[0].kind, .generatePCM) + + let generatePCMJob = jobs[0] + let generatePCMResolvedArgs: [String] = + try resolver.resolveArgumentList(for: generatePCMJob, forceResponseFiles: false) + XCTAssertEqual(generatePCMResolvedArgs.count, 2) + XCTAssertEqual(generatePCMResolvedArgs[1].first, "@") + } + } + func testLinking() throws { var env = ProcessEnv.vars env["SWIFT_DRIVER_TESTS_ENABLE_EXEC_PATH_FALLBACK"] = "1"