From ca781f67c3818c06d9e5c282ce4ea1fe748ea28f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Boris=20B=C3=BCgling?= Date: Thu, 25 May 2023 16:11:19 -0700 Subject: [PATCH] Write link file list as a build command This moves the generation of link file lists into the build system instead of doing it ad-hoc outside of the build. For this, we have a new `WriteAuxiliaryFile` tool and associated command that should be usable for any kind of writing of auxiliary files during the build. Note that this change opted to not touch the existing infrastructure for in-process tools, so any inputs that are needed for the file generation will need to be flattened into a generic array of input nodes. The different types of file generation are keyed off a virtual node at the start of that array. (cherry picked from commit ee7f064d7796533deb5ce4fe2094b72cfd510faf) --- .../ProductBuildDescription.swift | 12 -- Sources/Build/BuildOperation.swift | 2 + ...dOperationBuildSystemDelegateHandler.swift | 77 +++++++++ Sources/Build/BuildPlan.swift | 6 - Sources/Build/LLBuildManifestBuilder.swift | 6 +- Sources/LLBuildManifest/BuildManifest.swift | 50 ++++++ Sources/LLBuildManifest/Tools.swift | 20 +++ Tests/BuildTests/BuildPlanTests.swift | 151 +++++++++--------- 8 files changed, 230 insertions(+), 94 deletions(-) diff --git a/Sources/Build/BuildDescription/ProductBuildDescription.swift b/Sources/Build/BuildDescription/ProductBuildDescription.swift index 337bb9bd618..4b6998b5bc2 100644 --- a/Sources/Build/BuildDescription/ProductBuildDescription.swift +++ b/Sources/Build/BuildDescription/ProductBuildDescription.swift @@ -339,18 +339,6 @@ public final class ProductBuildDescription: SPMBuildCore.ProductBuildDescription return self.stripInvalidArguments(args) } - /// Writes link filelist to the filesystem. - func writeLinkFilelist(_ fs: FileSystem) throws { - let stream = BufferedOutputByteStream() - - for object in self.objects { - stream <<< object.pathString.spm_shellEscaped() <<< "\n" - } - - try fs.createDirectory(self.linkFileListPath.parentDirectory, recursive: true) - try fs.writeFileContents(self.linkFileListPath, bytes: stream.bytes) - } - /// Returns the build flags from the declared build settings. private func buildSettingsFlags() -> [String] { var flags: [String] = [] diff --git a/Sources/Build/BuildOperation.swift b/Sources/Build/BuildOperation.swift index 3570a436513..47097a4ec27 100644 --- a/Sources/Build/BuildOperation.swift +++ b/Sources/Build/BuildOperation.swift @@ -694,6 +694,7 @@ extension BuildDescription { let testDiscoveryCommands = llbuild.manifest.getCmdToolMap(kind: TestDiscoveryTool.self) let testEntryPointCommands = llbuild.manifest.getCmdToolMap(kind: TestEntryPointTool.self) let copyCommands = llbuild.manifest.getCmdToolMap(kind: CopyTool.self) + let writeCommands = llbuild.manifest.getCmdToolMap(kind: WriteAuxiliaryFile.self) // Create the build description. let buildDescription = try BuildDescription( @@ -703,6 +704,7 @@ extension BuildDescription { testDiscoveryCommands: testDiscoveryCommands, testEntryPointCommands: testEntryPointCommands, copyCommands: copyCommands, + writeCommands: writeCommands, pluginDescriptions: plan.pluginDescriptions ) try fileSystem.createDirectory( diff --git a/Sources/Build/BuildOperationBuildSystemDelegateHandler.swift b/Sources/Build/BuildOperationBuildSystemDelegateHandler.swift index 03f037c8fc3..c23a74f3a98 100644 --- a/Sources/Build/BuildOperationBuildSystemDelegateHandler.swift +++ b/Sources/Build/BuildOperationBuildSystemDelegateHandler.swift @@ -297,6 +297,9 @@ public struct BuildDescription: Codable { /// The map of copy commands. let copyCommands: [BuildManifest.CmdName: LLBuildManifest.CopyTool] + /// The map of write commands. + let writeCommands: [BuildManifest.CmdName: LLBuildManifest.WriteAuxiliaryFile] + /// A flag that inidcates this build should perform a check for whether targets only import /// their explicitly-declared dependencies let explicitTargetDependencyImportCheckingMode: BuildParameters.TargetDependencyImportCheckingMode @@ -323,6 +326,7 @@ public struct BuildDescription: Codable { testDiscoveryCommands: [BuildManifest.CmdName: LLBuildManifest.TestDiscoveryTool], testEntryPointCommands: [BuildManifest.CmdName: LLBuildManifest.TestEntryPointTool], copyCommands: [BuildManifest.CmdName: LLBuildManifest.CopyTool], + writeCommands: [BuildManifest.CmdName: LLBuildManifest.WriteAuxiliaryFile], pluginDescriptions: [PluginDescription] ) throws { self.swiftCommands = swiftCommands @@ -330,6 +334,7 @@ public struct BuildDescription: Codable { self.testDiscoveryCommands = testDiscoveryCommands self.testEntryPointCommands = testEntryPointCommands self.copyCommands = copyCommands + self.writeCommands = writeCommands self.explicitTargetDependencyImportCheckingMode = plan.buildParameters .explicitTargetDependencyImportCheckingMode self.targetDependencyMap = try plan.targets.reduce(into: [TargetName: [TargetName]]()) { @@ -458,6 +463,76 @@ public final class BuildExecutionContext { } } +final class WriteAuxiliaryFileCommand: CustomLLBuildCommand { + override func getSignature(_ command: SPMLLBuild.Command) -> [UInt8] { + guard let buildDescription = self.context.buildDescription else { + return [] + } + guard let tool = buildDescription.copyCommands[command.name] else { + return [] + } + + do { + let encoder = JSONEncoder.makeWithDefaults() + var hash = Data() + hash += try encoder.encode(tool.inputs) + hash += try encoder.encode(tool.outputs) + return [UInt8](hash) + } catch { + self.context.observabilityScope.emit(error: "getSignature() failed: \(error.interpolationDescription)") + return [] + } + } + + override func execute( + _ command: SPMLLBuild.Command, + _: SPMLLBuild.BuildSystemCommandInterface + ) -> Bool { + let outputFilePath: AbsolutePath + let tool: WriteAuxiliaryFile! + + do { + guard let buildDescription = self.context.buildDescription else { + throw InternalError("unknown build description") + } + guard let _tool = buildDescription.writeCommands[command.name] else { + throw StringError("command \(command.name) not registered") + } + tool = _tool + + guard let output = tool.outputs.first, output.kind == .file else { + throw StringError("invalid output path") + } + outputFilePath = try AbsolutePath(validating: output.name) + } catch { + self.context.observabilityScope.emit(error: "failed to write auxiliary file: \(error.interpolationDescription)") + return false + } + + do { + try self.context.fileSystem.writeFileContents(outputFilePath, string: getFileContents(tool: tool)) + return true + } catch { + self.context.observabilityScope.emit(error: "failed to write auxiliary file '\(outputFilePath.pathString)': \(error.interpolationDescription)") + return false + } + } + + func getFileContents(tool: WriteAuxiliaryFile) throws -> String { + guard tool.inputs.first?.kind == .virtual, let generatedFileType = tool.inputs.first?.name.dropFirst().dropLast() else { + throw StringError("invalid inputs") + } + + for fileType in WriteAuxiliary.fileTypes { + if generatedFileType == fileType.name { + return try fileType.getFileContents(inputs: Array(tool.inputs.dropFirst())) + } + } + + throw InternalError("unhandled generated file type '\(generatedFileType)'") + } +} + public protocol PackageStructureDelegate { func packageStructureChanged() -> Bool } @@ -577,6 +652,8 @@ final class BuildOperationBuildSystemDelegateHandler: LLBuildBuildSystemDelegate return InProcessTool(buildExecutionContext, type: PackageStructureCommand.self) case CopyTool.name: return InProcessTool(buildExecutionContext, type: CopyCommand.self) + case WriteAuxiliaryFile.name: + return InProcessTool(buildExecutionContext, type: WriteAuxiliaryFileCommand.self) default: return nil } diff --git a/Sources/Build/BuildPlan.swift b/Sources/Build/BuildPlan.swift index 66fadfb6cae..ed864ac5410 100644 --- a/Sources/Build/BuildPlan.swift +++ b/Sources/Build/BuildPlan.swift @@ -694,12 +694,6 @@ public class BuildPlan: SPMBuildCore.BuildPlan { } buildProduct.libraryBinaryPaths = dependencies.libraryBinaryPaths - // Write the link filelist file. - // - // FIXME: We should write this as a custom llbuild task once we adopt it - // as a library. - try buildProduct.writeLinkFilelist(fileSystem) - buildProduct.availableTools = dependencies.availableTools } diff --git a/Sources/Build/LLBuildManifestBuilder.swift b/Sources/Build/LLBuildManifestBuilder.swift index cc3e140ea61..a5a420c761d 100644 --- a/Sources/Build/LLBuildManifestBuilder.swift +++ b/Sources/Build/LLBuildManifestBuilder.swift @@ -986,13 +986,13 @@ extension LLBuildManifestBuilder { try self.manifest.addShellCmd( name: cmdName, description: "Archiving \(buildProduct.binaryPath.prettyPath())", - inputs: buildProduct.objects.map(Node.file), + inputs: (buildProduct.objects + [buildProduct.linkFileListPath]).map(Node.file), outputs: [.file(buildProduct.binaryPath)], arguments: try buildProduct.archiveArguments() ) default: - let inputs = try buildProduct.objects + buildProduct.dylibs.map{ try $0.binaryPath } + let inputs = try buildProduct.objects + buildProduct.dylibs.map{ try $0.binaryPath } + [buildProduct.linkFileListPath] try self.manifest.addShellCmd( name: cmdName, @@ -1020,6 +1020,8 @@ extension LLBuildManifestBuilder { } self.addNode(output, toTarget: .test) } + + self.manifest.addWriteLinkFileListCommand(objects: Array(buildProduct.objects), linkFileListPath: buildProduct.linkFileListPath) } } diff --git a/Sources/LLBuildManifest/BuildManifest.swift b/Sources/LLBuildManifest/BuildManifest.swift index 635c3371887..7c9680473e4 100644 --- a/Sources/LLBuildManifest/BuildManifest.swift +++ b/Sources/LLBuildManifest/BuildManifest.swift @@ -13,6 +13,46 @@ import Basics import struct TSCBasic.AbsolutePath +public protocol AuxiliaryFileType { + static var name: String { get } + + static func getFileContents(inputs: [Node]) throws -> String +} + +public enum WriteAuxiliary { + public static let fileTypes: [AuxiliaryFileType.Type] = [LinkFileList.self] + + public struct LinkFileList: AuxiliaryFileType { + public static let name = "link-file-list" + + // FIXME: We should extend the `InProcessTool` support to allow us to specify these in a typed way, but today we have to flatten all the inputs into a generic `Node` array (rdar://109844243). + public static func computeInputs(objects: [AbsolutePath]) -> [Node] { + return [.virtual(Self.name)] + objects.map { Node.file($0) } + } + + public static func getFileContents(inputs: [Node]) throws -> String { + let objects = inputs.compactMap { + if $0.kind == .file { + return $0.name + } else { + return nil + } + } + + var content = objects + .map { $0.spm_shellEscaped() } + .joined(separator: "\n") + + // not sure this is needed, added here for backward compatibility + if !content.isEmpty { + content.append("\n") + } + + return content + } + } +} + public struct BuildManifest { public typealias TargetName = String public typealias CmdName = String @@ -88,6 +128,16 @@ public struct BuildManifest { commands[name] = Command(name: name, tool: tool) } + public mutating func addWriteLinkFileListCommand( + objects: [AbsolutePath], + linkFileListPath: AbsolutePath + ) { + let inputs = WriteAuxiliary.LinkFileList.computeInputs(objects: objects) + let tool = WriteAuxiliaryFile(inputs: inputs, outputFilePath: linkFileListPath) + let name = linkFileListPath.pathString + commands[name] = Command(name: name, tool: tool) + } + public mutating func addPkgStructureCmd( name: String, inputs: [Node], diff --git a/Sources/LLBuildManifest/Tools.swift b/Sources/LLBuildManifest/Tools.swift index 7291681e8ea..844eec45b14 100644 --- a/Sources/LLBuildManifest/Tools.swift +++ b/Sources/LLBuildManifest/Tools.swift @@ -151,6 +151,26 @@ public struct ShellTool: ToolProtocol { } } +public struct WriteAuxiliaryFile: ToolProtocol { + public static let name: String = "write-auxiliary-file" + + public let inputs: [Node] + private let outputFilePath: AbsolutePath + + public init(inputs: [Node], outputFilePath: AbsolutePath) { + self.inputs = inputs + self.outputFilePath = outputFilePath + } + + public var outputs: [Node] { + return [.file(outputFilePath)] + } + + public func write(to stream: ManifestToolStream) { + stream["description"] = "Write auxiliary file \(outputFilePath.pathString)" + } +} + public struct ClangTool: ToolProtocol { public static let name: String = "clang" diff --git a/Tests/BuildTests/BuildPlanTests.swift b/Tests/BuildTests/BuildPlanTests.swift index f998a35958d..cf7469f9d3a 100644 --- a/Tests/BuildTests/BuildPlanTests.swift +++ b/Tests/BuildTests/BuildPlanTests.swift @@ -868,9 +868,11 @@ final class BuildPlanTests: XCTestCase { let buildPath: AbsolutePath = plan.buildParameters.dataPath.appending(components: "release") - let linkedFileList: String = try fs.readFileContents("/path/to/build/release/exe.product/Objects.LinkFileList") - XCTAssertMatch(linkedFileList, .contains("PkgLib")) - XCTAssertNoMatch(linkedFileList, .contains("ExtLib")) + let result = try BuildPlanResult(plan: plan) + let buildProduct = try result.buildProduct(for: "exe") + let objectDirectoryNames = buildProduct.objects.map { $0.parentDirectory.basename } + XCTAssertTrue(objectDirectoryNames.contains("PkgLib.build")) + XCTAssertFalse(objectDirectoryNames.contains("ExtLib.build")) let yaml = try fs.tempDirectory.appending(components: UUID().uuidString, "release.yaml") try fs.createDirectory(yaml.parentDirectory, recursive: true) @@ -894,9 +896,11 @@ final class BuildPlanTests: XCTestCase { observabilityScope: observability.topScope ) - let linkedFileList: String = try fs.readFileContents("/path/to/build/debug/exe.product/Objects.LinkFileList") - XCTAssertNoMatch(linkedFileList, .contains("PkgLib")) - XCTAssertNoMatch(linkedFileList, .contains("ExtLib")) + let result = try BuildPlanResult(plan: plan) + let buildProduct = try result.buildProduct(for: "exe") + let objectDirectoryNames = buildProduct.objects.map { $0.parentDirectory.basename } + XCTAssertFalse(objectDirectoryNames.contains("PkgLib.build")) + XCTAssertFalse(objectDirectoryNames.contains("ExtLib.build")) let yaml = try fs.tempDirectory.appending(components: UUID().uuidString, "debug.yaml") try fs.createDirectory(yaml.parentDirectory, recursive: true) @@ -1263,13 +1267,12 @@ final class BuildPlanTests: XCTestCase { ]) #endif - let linkedFileList: String = try fs.readFileContents(buildPath.appending(components: "exe.product", "Objects.LinkFileList")) - XCTAssertEqual(linkedFileList, """ - \(buildPath.appending(components: "exe.build", "main.c.o")) - \(buildPath.appending(components: "extlib.build", "extlib.c.o")) - \(buildPath.appending(components: "lib.build", "lib.c.o")) - - """) + let buildProduct = try XCTUnwrap(result.productMap["exe"]) + XCTAssertEqual(Array(buildProduct.objects), [ + buildPath.appending(components: "exe.build", "main.c.o"), + buildPath.appending(components: "extlib.build", "extlib.c.o"), + buildPath.appending(components: "lib.build", "lib.c.o") + ]) } func testClangConditionalDependency() throws { @@ -4065,75 +4068,75 @@ final class BuildPlanTests: XCTestCase { } func testArchiving() throws { - let fs = InMemoryFileSystem(emptyFiles: - "/Package/Sources/rary/rary.swift" - ) + let fs: FileSystem = InMemoryFileSystem(emptyFiles: + "/Package/Sources/rary/rary.swift" + ) - let observability = ObservabilitySystem.makeForTesting() - let graph = try loadPackageGraph( - fileSystem: fs, - manifests: [ - Manifest.createRootManifest( - displayName: "Package", - path: .init(path: "/Package"), - products: [ - ProductDescription(name: "rary", type: .library(.static), targets: ["rary"]), - ], - targets: [ - TargetDescription(name: "rary", dependencies: []), - ] - ), - ], - observabilityScope: observability.topScope - ) - XCTAssertNoDiagnostics(observability.diagnostics) + let observability = ObservabilitySystem.makeForTesting() + let graph = try loadPackageGraph( + fileSystem: fs, + manifests: [ + Manifest.createRootManifest( + displayName: "Package", + path: "/Package", + products: [ + ProductDescription(name: "rary", type: .library(.static), targets: ["rary"]), + ], + targets: [ + TargetDescription(name: "rary", dependencies: []), + ] + ), + ], + observabilityScope: observability.topScope + ) + XCTAssertNoDiagnostics(observability.diagnostics) - let result = try BuildPlanResult(plan: BuildPlan( - buildParameters: mockBuildParameters(), - graph: graph, - fileSystem: fs, - observabilityScope: observability.topScope - )) + let result = try BuildPlanResult(plan: BuildPlan( + buildParameters: mockBuildParameters(), + graph: graph, + fileSystem: fs, + observabilityScope: observability.topScope + )) - let buildPath: AbsolutePath = result.plan.buildParameters.dataPath.appending(components: "debug") + let buildPath = result.plan.buildParameters.dataPath.appending(components: "debug") - let yaml = try fs.tempDirectory.appending(components: UUID().uuidString, "debug.yaml") - try fs.createDirectory(yaml.parentDirectory, recursive: true) + let yaml = try fs.tempDirectory.appending(components: UUID().uuidString, "debug.yaml") + try fs.createDirectory(yaml.parentDirectory, recursive: true) - let llbuild = LLBuildManifestBuilder(result.plan, fileSystem: fs, observabilityScope: observability.topScope) - try llbuild.generateManifest(at: yaml) + let llbuild = LLBuildManifestBuilder(result.plan, fileSystem: fs, observabilityScope: observability.topScope) + try llbuild.generateManifest(at: yaml) - let contents: String = try fs.readFileContents(yaml) + let contents: String = try fs.readFileContents(yaml) - if result.plan.buildParameters.triple.isWindows() { - XCTAssertMatch(contents, .contains(""" - "C.rary-debug.a": - tool: shell - inputs: ["\(buildPath.appending(components: "rary.build", "rary.swift.o").escapedPathString())","\(buildPath.appending(components: "rary.build", "rary.swiftmodule.o").escapedPathString())"] - outputs: ["\(buildPath.appending(components: "library.a").escapedPathString())"] - description: "Archiving \(buildPath.appending(components: "library.a").escapedPathString())" - args: ["\(result.plan.buildParameters.toolchain.librarianPath.escapedPathString())","/LIB","/OUT:\(buildPath.appending(components: "library.a").escapedPathString())","@\(buildPath.appending(components: "rary.product", "Objects.LinkFileList").escapedPathString())"] - """)) - } else if result.plan.buildParameters.triple.isDarwin() { - XCTAssertMatch(contents, .contains(""" - "C.rary-debug.a": - tool: shell - inputs: ["\(buildPath.appending(components: "rary.build", "rary.swift.o").escapedPathString())"] - outputs: ["\(buildPath.appending(components: "library.a").escapedPathString())"] - description: "Archiving \(buildPath.appending(components: "library.a").escapedPathString())" - args: ["\(result.plan.buildParameters.toolchain.librarianPath.escapedPathString())","-static","-o","\(buildPath.appending(components: "library.a").escapedPathString())","@\(buildPath.appending(components: "rary.product", "Objects.LinkFileList").escapedPathString())"] - """)) - } else { // assume Unix `ar` is the librarian - XCTAssertMatch(contents, .contains(""" - "C.rary-debug.a": - tool: shell - inputs: ["\(buildPath.appending(components: "rary.build", "rary.swift.o").escapedPathString())","\(buildPath.appending(components: "rary.build", "rary.swiftmodule.o").escapedPathString())"] - outputs: ["\(buildPath.appending(components: "library.a").escapedPathString())"] - description: "Archiving \(buildPath.appending(components: "library.a").escapedPathString())" - args: ["\(result.plan.buildParameters.toolchain.librarianPath.escapedPathString())","crs","\(buildPath.appending(components: "library.a").escapedPathString())","@\(buildPath.appending(components: "rary.product", "Objects.LinkFileList").escapedPathString())"] - """)) + if result.plan.buildParameters.triple.isWindows() { + XCTAssertMatch(contents, .contains(""" + "C.rary-debug.a": + tool: shell + inputs: ["\(buildPath.appending(components: "rary.build", "rary.swift.o").escapedPathString())","\(buildPath.appending(components: "rary.build", "rary.swiftmodule.o").escapedPathString())","\(buildPath.appending(components: "rary.product", "Objects.LinkFileList").escapedPathString())"] + outputs: ["\(buildPath.appending(components: "library.a").escapedPathString())"] + description: "Archiving \(buildPath.appending(components: "library.a").escapedPathString())" + args: ["\(result.plan.buildParameters.toolchain.librarianPath.escapedPathString())","/LIB","/OUT:\(buildPath.appending(components: "library.a").escapedPathString())","@\(buildPath.appending(components: "rary.product", "Objects.LinkFileList").escapedPathString())"] + """)) + } else if result.plan.buildParameters.triple.isDarwin() { + XCTAssertMatch(contents, .contains(""" + "C.rary-debug.a": + tool: shell + inputs: ["\(buildPath.appending(components: "rary.build", "rary.swift.o").escapedPathString())","\(buildPath.appending(components: "rary.product", "Objects.LinkFileList").escapedPathString())"] + outputs: ["\(buildPath.appending(components: "library.a").escapedPathString())"] + description: "Archiving \(buildPath.appending(components: "library.a").escapedPathString())" + args: ["\(result.plan.buildParameters.toolchain.librarianPath.escapedPathString())","-static","-o","\(buildPath.appending(components: "library.a").escapedPathString())","@\(buildPath.appending(components: "rary.product", "Objects.LinkFileList").escapedPathString())"] + """)) + } else { // assume Unix `ar` is the librarian + XCTAssertMatch(contents, .contains(""" + "C.rary-debug.a": + tool: shell + inputs: ["\(buildPath.appending(components: "rary.build", "rary.swift.o").escapedPathString())","\(buildPath.appending(components: "rary.build", "rary.swiftmodule.o").escapedPathString())","\(buildPath.appending(components: "rary.product", "Objects.LinkFileList").escapedPathString())"] + outputs: ["\(buildPath.appending(components: "library.a").escapedPathString())"] + description: "Archiving \(buildPath.appending(components: "library.a").escapedPathString())" + args: ["\(result.plan.buildParameters.toolchain.librarianPath.escapedPathString())","crs","\(buildPath.appending(components: "library.a").escapedPathString())","@\(buildPath.appending(components: "rary.product", "Objects.LinkFileList").escapedPathString())"] + """)) + } } - } func testSwiftBundleAccessor() throws { // This has a Swift and ObjC target in the same package.