From fb654e2124c6a8c470e3e394e49eeb7ac2563df3 Mon Sep 17 00:00:00 2001 From: Boris Buegling Date: Thu, 18 May 2023 13:42:16 -0700 Subject: [PATCH] Stop using llbuild's Swift compiler tool This allows us to take control of these compiler invocations and is a first step towards unifying all the various ways of generating compiler arguments that are currently in `SwiftTargetBuildDescription`. This PR aims to keep the behavior of the existing tool as far as possible, the only known differences are single quotes instead of double for arguments with spaces and the fact that the output file map will be created eagerly. resolves https://github.com/apple/swift-package-manager/issues/6575 --- .../Tests/IntegrationTests/BasicTests.swift | 2 +- .../SwiftTargetBuildDescription.swift | 2 +- Sources/Build/LLBuildManifestBuilder.swift | 3 +- Sources/LLBuildManifest/BuildManifest.swift | 6 +- Sources/LLBuildManifest/Tools.swift | 59 +++++++++++++------ Tests/CommandsTests/BuildToolTests.swift | 8 ++- 6 files changed, 55 insertions(+), 25 deletions(-) diff --git a/IntegrationTests/Tests/IntegrationTests/BasicTests.swift b/IntegrationTests/Tests/IntegrationTests/BasicTests.swift index 5160877a731..f12abd15123 100644 --- a/IntegrationTests/Tests/IntegrationTests/BasicTests.swift +++ b/IntegrationTests/Tests/IntegrationTests/BasicTests.swift @@ -215,7 +215,7 @@ final class BasicTests: XCTestCase { // Check the build. let buildOutput = try sh(swiftBuild, "--package-path", packagePath, "-v").stdout - XCTAssertMatch(buildOutput, .regex(#"swiftc.* -module-name special_tool .* ".*/more spaces/special tool/some file.swift""#)) + XCTAssertMatch(buildOutput, .regex(#"swiftc.* -module-name special_tool .* '.*/more spaces/special tool/some file.swift'"#)) XCTAssertMatch(buildOutput, .contains("Build complete")) // Verify that the tool exists and works. diff --git a/Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift b/Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift index ee4b28c13c5..480d44a0c91 100644 --- a/Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift +++ b/Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift @@ -677,7 +677,7 @@ public final class SwiftTargetBuildDescription { self.buildParameters.triple.isDarwin() && self.target.type == .library } - private func writeOutputFileMap() throws -> AbsolutePath { + func writeOutputFileMap() throws -> AbsolutePath { let path = self.tempsPath.appending("output-file-map.json") let masterDepsPath = self.tempsPath.appending("master.swiftdeps") diff --git a/Sources/Build/LLBuildManifestBuilder.swift b/Sources/Build/LLBuildManifestBuilder.swift index 77520c6174a..c78b9ef2d34 100644 --- a/Sources/Build/LLBuildManifestBuilder.swift +++ b/Sources/Build/LLBuildManifestBuilder.swift @@ -629,7 +629,8 @@ extension LLBuildManifestBuilder { otherArguments: try target.compileArguments(), sources: target.sources, isLibrary: isLibrary, - wholeModuleOptimization: self.buildParameters.configuration == .release + wholeModuleOptimization: self.buildParameters.configuration == .release, + outputFileMapPath: try target.writeOutputFileMap() // FIXME: Eliminate side effect. ) } diff --git a/Sources/LLBuildManifest/BuildManifest.swift b/Sources/LLBuildManifest/BuildManifest.swift index c2a2826db08..dd3a8eeb3de 100644 --- a/Sources/LLBuildManifest/BuildManifest.swift +++ b/Sources/LLBuildManifest/BuildManifest.swift @@ -174,7 +174,8 @@ public struct BuildManifest { otherArguments: [String], sources: [AbsolutePath], isLibrary: Bool, - wholeModuleOptimization: Bool + wholeModuleOptimization: Bool, + outputFileMapPath: AbsolutePath ) { assert(commands[name] == nil, "already had a command named '\(name)'") let tool = SwiftCompilerTool( @@ -190,7 +191,8 @@ public struct BuildManifest { otherArguments: otherArguments, sources: sources, isLibrary: isLibrary, - wholeModuleOptimization: wholeModuleOptimization + wholeModuleOptimization: wholeModuleOptimization, + outputFileMapPath: outputFileMapPath ) commands[name] = Command(name: name, tool: tool) } diff --git a/Sources/LLBuildManifest/Tools.swift b/Sources/LLBuildManifest/Tools.swift index fa73b001c7d..2b964a00189 100644 --- a/Sources/LLBuildManifest/Tools.swift +++ b/Sources/LLBuildManifest/Tools.swift @@ -226,7 +226,7 @@ public struct SwiftFrontendTool: ToolProtocol { /// Swift compiler llbuild tool. public struct SwiftCompilerTool: ToolProtocol { - public static let name: String = "swift-compiler" + public static let name: String = "shell" public static let numThreads: Int = ProcessInfo.processInfo.activeProcessorCount @@ -244,6 +244,7 @@ public struct SwiftCompilerTool: ToolProtocol { public var sources: [AbsolutePath] public var isLibrary: Bool public var wholeModuleOptimization: Bool + public var outputFileMapPath: AbsolutePath init( inputs: [Node], @@ -258,7 +259,8 @@ public struct SwiftCompilerTool: ToolProtocol { otherArguments: [String], sources: [AbsolutePath], isLibrary: Bool, - wholeModuleOptimization: Bool + wholeModuleOptimization: Bool, + outputFileMapPath: AbsolutePath ) { self.inputs = inputs self.outputs = outputs @@ -273,24 +275,43 @@ public struct SwiftCompilerTool: ToolProtocol { self.sources = sources self.isLibrary = isLibrary self.wholeModuleOptimization = wholeModuleOptimization + self.outputFileMapPath = outputFileMapPath } - public func write(to stream: ManifestToolStream) { - stream["executable"] = executable - stream["module-name"] = moduleName - if let moduleAliases { - // Format the key and value to pass to -module-alias flag - let formatted = moduleAliases.map {$0.key + "=" + $0.value} - stream["module-aliases"] = formatted + var description: String { + return "Compiling Swift Module '\(moduleName)' (\(sources.count) sources)" + } + + var arguments: [String] { + var arguments = [ + executable.pathString, + "-module-name", moduleName, + ] + if let moduleAliases = moduleAliases { + for (original, alias) in moduleAliases { + arguments += ["-module-alias", "\(original)=\(alias)"] + } + } + arguments += [ + "-incremental", + "-emit-dependencies", + "-emit-module", + "-emit-module-path", moduleOutputPath.pathString, + "-output-file-map", outputFileMapPath.pathString, + ] + if isLibrary { + arguments += ["-parse-as-library"] } - stream["module-output-path"] = moduleOutputPath - stream["import-paths"] = [importPath] - stream["temps-path"] = tempsPath - stream["objects"] = objects - stream["other-args"] = otherArguments - stream["sources"] = sources - stream["is-library"] = isLibrary - stream["enable-whole-module-optimization"] = wholeModuleOptimization - stream["num-threads"] = Self.numThreads - } + if wholeModuleOptimization { + arguments += ["-whole-module-optimization", "-num-threads", "\(Self.numThreads)"] + } + arguments += ["-c"] + sources.map { $0.pathString } + arguments += ["-I", importPath.pathString] + arguments += otherArguments + return arguments + } + + public func write(to stream: ManifestToolStream) { + ShellTool(description: description, inputs: inputs, outputs: outputs, arguments: arguments).write(to: stream) + } } diff --git a/Tests/CommandsTests/BuildToolTests.swift b/Tests/CommandsTests/BuildToolTests.swift index 3fd5922a081..7f9f79fe4ff 100644 --- a/Tests/CommandsTests/BuildToolTests.swift +++ b/Tests/CommandsTests/BuildToolTests.swift @@ -41,7 +41,13 @@ final class BuildToolTests: CommandsTestCase { defer { try! SwiftPM.Package.execute(["clean"], packagePath: packagePath) } let (binPathOutput, _) = try execute(["--show-bin-path"], packagePath: packagePath) let binPath = try AbsolutePath(validating: binPathOutput.trimmingCharacters(in: .whitespacesAndNewlines)) - let binContents = try localFileSystem.getDirectoryContents(binPath) + let binContents = try localFileSystem.getDirectoryContents(binPath).filter { + guard let contents = try? localFileSystem.getDirectoryContents(binPath.appending(component: $0)) else { + return true + } + // Filter directories which only contain an output file map since we didn't build anything for those which is what `binContents` is meant to represent. + return contents != ["output-file-map.json"] + } return BuildResult(binPath: binPath, output: output, binContents: binContents) }