From 6be1d401b925eeecb9f5ed2871992851e56854e9 Mon Sep 17 00:00:00 2001 From: Yuta Saito Date: Thu, 18 Apr 2024 15:47:47 +0000 Subject: [PATCH 1/3] Place build.db under the scratch directory The build database should be shared between different triple builds to re-generate the build manifest correctly. Also this change splits llbuild target names for each triple to avoid cache invalidation when switching triple. --- .../LLBuildManifestBuilder.swift | 2 +- Sources/Build/BuildOperation.swift | 8 +- Sources/CoreCommands/BuildSystemSupport.swift | 1 + Sources/swift-bootstrap/main.swift | 1 + Tests/BuildTests/BuildOperationTests.swift | 135 ++++++++++++++++-- .../LLBuildManifestBuilderTests.swift | 2 +- 6 files changed, 132 insertions(+), 17 deletions(-) diff --git a/Sources/Build/BuildManifest/LLBuildManifestBuilder.swift b/Sources/Build/BuildManifest/LLBuildManifestBuilder.swift index 251d87c0e34..2ecd760327a 100644 --- a/Sources/Build/BuildManifest/LLBuildManifestBuilder.swift +++ b/Sources/Build/BuildManifest/LLBuildManifestBuilder.swift @@ -322,7 +322,7 @@ extension ResolvedModule { } public func getLLBuildTargetName(buildParameters: BuildParameters) -> String { - "\(self.name)-\(buildParameters.buildConfig)\(buildParameters.suffix(triple: self.buildTriple)).module" + "\(self.name)-\(buildParameters.triple.tripleString)-\(buildParameters.buildConfig)\(buildParameters.suffix(triple: self.buildTriple)).module" } public func getLLBuildResourcesCmdName(buildParameters: BuildParameters) -> String { diff --git a/Sources/Build/BuildOperation.swift b/Sources/Build/BuildOperation.swift index 9ce2f202210..0d4eab3a17b 100644 --- a/Sources/Build/BuildOperation.swift +++ b/Sources/Build/BuildOperation.swift @@ -52,6 +52,9 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS /// the plugin configuration for build plugins let pluginConfiguration: PluginConfiguration? + /// The path to scratch space (.build) directory. + let scratchDirectory: AbsolutePath + /// The llbuild build delegate reference. private var buildSystemDelegate: BuildOperationBuildSystemDelegateHandler? @@ -114,6 +117,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS cacheBuildManifest: Bool, packageGraphLoader: @escaping () throws -> ModulesGraph, pluginConfiguration: PluginConfiguration? = .none, + scratchDirectory: AbsolutePath, additionalFileRules: [FileRuleDescription], pkgConfigDirectories: [AbsolutePath], dependenciesByRootPackageIdentity: [PackageIdentity: [PackageIdentity]], @@ -136,6 +140,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS self.packageGraphLoader = packageGraphLoader self.additionalFileRules = additionalFileRules self.pluginConfiguration = pluginConfiguration + self.scratchDirectory = scratchDirectory self.pkgConfigDirectories = pkgConfigDirectories self.dependenciesByRootPackageIdentity = dependenciesByRootPackageIdentity self.rootPackageIdentityByTargetName = (try? Dictionary(throwingUniqueKeysWithValues: targetsByRootPackageIdentity.lazy.flatMap { e in e.value.map { ($0, e.key) } })) ?? [:] @@ -554,6 +559,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS toolsBuildParameters: pluginsBuildParameters, cacheBuildManifest: false, packageGraphLoader: { buildToolsGraph }, + scratchDirectory: pluginsBuildParameters.dataPath, additionalFileRules: self.additionalFileRules, pkgConfigDirectories: self.pkgConfigDirectories, dependenciesByRootPackageIdentity: [:], @@ -724,7 +730,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS ) self.buildSystemDelegate = buildSystemDelegate - let databasePath = self.productsBuildParameters.dataPath.appending("build.db").pathString + let databasePath = self.scratchDirectory.appending("build.db").pathString let buildSystem = SPMLLBuild.BuildSystem( buildFile: self.productsBuildParameters.llbuildManifest.pathString, databaseFile: databasePath, diff --git a/Sources/CoreCommands/BuildSystemSupport.swift b/Sources/CoreCommands/BuildSystemSupport.swift index 6106759a4e7..5d7d82e262f 100644 --- a/Sources/CoreCommands/BuildSystemSupport.swift +++ b/Sources/CoreCommands/BuildSystemSupport.swift @@ -49,6 +49,7 @@ private struct NativeBuildSystemFactory: BuildSystemFactory { workDirectory: try self.swiftCommandState.getActiveWorkspace().location.pluginWorkingDirectory, disableSandbox: self.swiftCommandState.shouldDisableSandbox ), + scratchDirectory: self.swiftCommandState.scratchDirectory, additionalFileRules: FileRuleDescription.swiftpmFileTypes, pkgConfigDirectories: self.swiftCommandState.options.locations.pkgConfigDirectories, dependenciesByRootPackageIdentity: rootPackageInfo.dependencies, diff --git a/Sources/swift-bootstrap/main.swift b/Sources/swift-bootstrap/main.swift index 2c40dd048c5..267dbc4463b 100644 --- a/Sources/swift-bootstrap/main.swift +++ b/Sources/swift-bootstrap/main.swift @@ -319,6 +319,7 @@ struct SwiftBootstrapBuildTool: ParsableCommand { toolsBuildParameters: buildParameters, cacheBuildManifest: false, packageGraphLoader: packageGraphLoader, + scratchDirectory: scratchDirectory, additionalFileRules: [], pkgConfigDirectories: [], dependenciesByRootPackageIdentity: [:], diff --git a/Tests/BuildTests/BuildOperationTests.swift b/Tests/BuildTests/BuildOperationTests.swift index 82f2519552e..62f2a1a14e7 100644 --- a/Tests/BuildTests/BuildOperationTests.swift +++ b/Tests/BuildTests/BuildOperationTests.swift @@ -14,34 +14,60 @@ @testable import PackageModel import Basics +import LLBuildManifest +@_spi(DontAdoptOutsideOfSwiftPMExposedForBenchmarksAndTestsOnly) +import PackageGraph +import SPMBuildCore import SPMTestSupport import XCTest import class TSCBasic.BufferedOutputByteStream import class TSCBasic.InMemoryFileSystem +private func mockBuildOperation( + buildParameters: BuildParameters, + cacheBuildManifest: Bool = false, + packageGraphLoader: @escaping () -> ModulesGraph = { fatalError() }, + scratchDirectory: AbsolutePath, + fs: any Basics.FileSystem, + observabilityScope: ObservabilityScope +) -> BuildOperation { + return BuildOperation( + productsBuildParameters: buildParameters, + toolsBuildParameters: buildParameters, + cacheBuildManifest: cacheBuildManifest, + packageGraphLoader: packageGraphLoader, + scratchDirectory: scratchDirectory, + additionalFileRules: [], + pkgConfigDirectories: [], + dependenciesByRootPackageIdentity: [:], + targetsByRootPackageIdentity: [:], + outputStream: BufferedOutputByteStream(), + logLevel: .info, + fileSystem: fs, + observabilityScope: observabilityScope + ) +} + final class BuildOperationTests: XCTestCase { func testDetectUnexpressedDependencies() throws { - let buildParameters = mockBuildParameters(shouldDisableLocalRpath: false) + let scratchDirectory = AbsolutePath("/path/to/build") + let triple = hostTriple + let buildParameters = mockBuildParameters( + buildPath: scratchDirectory.appending(triple.tripleString), + shouldDisableLocalRpath: false, + triple: triple + ) let fs = InMemoryFileSystem(files: [ "\(buildParameters.dataPath)/debug/Lunch.build/Lunch.d" : "/Best.framework" ]) let observability = ObservabilitySystem.makeForTesting() - let buildOp = BuildOperation( - productsBuildParameters: buildParameters, - toolsBuildParameters: buildParameters, - cacheBuildManifest: false, - packageGraphLoader: { fatalError() }, - additionalFileRules: [], - pkgConfigDirectories: [], - dependenciesByRootPackageIdentity: [:], - targetsByRootPackageIdentity: [:], - outputStream: BufferedOutputByteStream(), - logLevel: .info, - fileSystem: fs, - observabilityScope: observability.topScope + let buildOp = mockBuildOperation( + buildParameters: buildParameters, + scratchDirectory: scratchDirectory, + fs: fs, observabilityScope: observability.topScope ) buildOp.detectUnexpressedDependencies( availableLibraries: [ @@ -65,4 +91,85 @@ final class BuildOperationTests: XCTestCase { ["target 'Lunch' has an unexpressed depedency on 'foo'"] ) } + + func testDetectProductTripleChange() throws { + let observability = ObservabilitySystem.makeForTesting() + let fs = InMemoryFileSystem( + emptyFiles: "/Pkg/Sources/ATarget/foo.swift" + ) + let packageGraph = try loadModulesGraph( + fileSystem: fs, + manifests: [ + .createRootManifest( + displayName: "SwitchTriple", + path: "/Pkg", + targets: [ + TargetDescription(name: "ATarget"), + ] + ), + ], + observabilityScope: observability.topScope + ) + try withTemporaryDirectory { tmpDir in + let scratchDirectory = tmpDir.appending(".build") + let fs = localFileSystem + let triples = try [Triple("x86_64-unknown-linux-gnu"), Triple("wasm32-unknown-wasi")] + var llbuildManifestByTriple: [String: String] = [:] + + // Perform initial builds for each triple + for triple in triples { + let buildParameters = mockBuildParameters( + buildPath: scratchDirectory.appending(triple.tripleString), + config: .debug, + triple: triple + ) + let buildOp = mockBuildOperation( + buildParameters: buildParameters, + cacheBuildManifest: false, + packageGraphLoader: { packageGraph }, + scratchDirectory: scratchDirectory, + fs: fs, observabilityScope: observability.topScope + ) + // Generate initial llbuild manifest + let _ = try buildOp.getBuildDescription() + // Record the initial llbuild manifest as expected one + llbuildManifestByTriple[triple.tripleString] = try fs.readFileContents(buildParameters.llbuildManifest) + } + + XCTAssertTrue(fs.exists(scratchDirectory.appending("debug.yaml"))) + // FIXME: There should be a build database with manifest cache after the initial build. + // The initial build usually triggered with `cacheBuildManifest=false` because llbuild + // manifest file and description.json are not found. However, with `cacheBuildManifest=false`, + // `BuildOperation` does not trigger "PackageStructure" build, thus the initial build does + // not record the manifest cache. So "getBuildDescription" doesn't create build.db for the + // initial planning and the second build always need full-planning. + // + // XCTAssertTrue(fs.exists(scratchDirectory.appending("build.db"))) + + // Perform incremental build several times and switch triple for each time + for _ in 0..<4 { + for triple in triples { + let buildParameters = mockBuildParameters( + buildPath: scratchDirectory.appending(triple.tripleString), + config: .debug, + triple: triple + ) + let buildOp = mockBuildOperation( + buildParameters: buildParameters, + cacheBuildManifest: true, + packageGraphLoader: { packageGraph }, + scratchDirectory: scratchDirectory, + fs: fs, observabilityScope: observability.topScope + ) + // Generate llbuild manifest + let _ = try buildOp.getBuildDescription() + + // Ensure that llbuild manifest is updated to the expected one + let actualManifest: String = try fs.readFileContents(buildParameters.llbuildManifest) + let expectedManifest = try XCTUnwrap(llbuildManifestByTriple[triple.tripleString]) + XCTAssertEqual(actualManifest, expectedManifest) + } + } + } + } } diff --git a/Tests/BuildTests/LLBuildManifestBuilderTests.swift b/Tests/BuildTests/LLBuildManifestBuilderTests.swift index f4a7d10d62a..e6c1eab2906 100644 --- a/Tests/BuildTests/LLBuildManifestBuilderTests.swift +++ b/Tests/BuildTests/LLBuildManifestBuilderTests.swift @@ -218,6 +218,6 @@ final class LLBuildManifestBuilderTests: XCTestCase { let builder = LLBuildManifestBuilder(plan, fileSystem: fs, observabilityScope: scope) let manifest = try builder.generateManifest(at: "/manifest") - XCTAssertNotNil(manifest.commands["C.SwiftSyntax-debug-tool.module"]) + XCTAssertNotNil(manifest.commands["C.SwiftSyntax-aarch64-unknown-linux-gnu-debug-tool.module"]) } } From 8e8ce26a9e739017bc084c9a4e753b0510dae448 Mon Sep 17 00:00:00 2001 From: Yuta Saito Date: Tue, 28 May 2024 03:24:14 +0000 Subject: [PATCH 2/3] Split the build parameters for products and tools in mockBuildOperation --- Tests/BuildTests/BuildOperationTests.swift | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/Tests/BuildTests/BuildOperationTests.swift b/Tests/BuildTests/BuildOperationTests.swift index 62f2a1a14e7..458f1b7ec8e 100644 --- a/Tests/BuildTests/BuildOperationTests.swift +++ b/Tests/BuildTests/BuildOperationTests.swift @@ -25,7 +25,8 @@ import class TSCBasic.BufferedOutputByteStream import class TSCBasic.InMemoryFileSystem private func mockBuildOperation( - buildParameters: BuildParameters, + productsBuildParameters: BuildParameters, + toolsBuildParameters: BuildParameters, cacheBuildManifest: Bool = false, packageGraphLoader: @escaping () -> ModulesGraph = { fatalError() }, scratchDirectory: AbsolutePath, @@ -33,8 +34,8 @@ private func mockBuildOperation( observabilityScope: ObservabilityScope ) -> BuildOperation { return BuildOperation( - productsBuildParameters: buildParameters, - toolsBuildParameters: buildParameters, + productsBuildParameters: productsBuildParameters, + toolsBuildParameters: toolsBuildParameters, cacheBuildManifest: cacheBuildManifest, packageGraphLoader: packageGraphLoader, scratchDirectory: scratchDirectory, @@ -65,7 +66,8 @@ final class BuildOperationTests: XCTestCase { let observability = ObservabilitySystem.makeForTesting() let buildOp = mockBuildOperation( - buildParameters: buildParameters, + productsBuildParameters: buildParameters, + toolsBuildParameters: buildParameters, scratchDirectory: scratchDirectory, fs: fs, observabilityScope: observability.topScope ) @@ -124,7 +126,8 @@ final class BuildOperationTests: XCTestCase { triple: triple ) let buildOp = mockBuildOperation( - buildParameters: buildParameters, + productsBuildParameters: buildParameters, + toolsBuildParameters: buildParameters, cacheBuildManifest: false, packageGraphLoader: { packageGraph }, scratchDirectory: scratchDirectory, @@ -155,7 +158,8 @@ final class BuildOperationTests: XCTestCase { triple: triple ) let buildOp = mockBuildOperation( - buildParameters: buildParameters, + productsBuildParameters: buildParameters, + toolsBuildParameters: buildParameters, cacheBuildManifest: true, packageGraphLoader: { packageGraph }, scratchDirectory: scratchDirectory, From af246353a86e0643a812b8db776465b0b6434a76 Mon Sep 17 00:00:00 2001 From: Yuta Saito Date: Tue, 28 May 2024 03:34:37 +0000 Subject: [PATCH 3/3] Split llbuild targets of resources and products by target triple This change is to make the target names of llbuild targets unique by target triple so that switching triples will not invalidate target artifact cache. --- .../LLBuildManifestBuilder.swift | 11 +++++----- Tests/BuildTests/BuildPlanTests.swift | 7 ++++--- .../LLBuildManifestBuilderTests.swift | 20 +++++++++---------- 3 files changed, 20 insertions(+), 18 deletions(-) diff --git a/Sources/Build/BuildManifest/LLBuildManifestBuilder.swift b/Sources/Build/BuildManifest/LLBuildManifestBuilder.swift index 2ecd760327a..d1bf1f5f7c3 100644 --- a/Sources/Build/BuildManifest/LLBuildManifestBuilder.swift +++ b/Sources/Build/BuildManifest/LLBuildManifestBuilder.swift @@ -326,24 +326,25 @@ extension ResolvedModule { } public func getLLBuildResourcesCmdName(buildParameters: BuildParameters) -> String { - "\(self.name)-\(buildParameters.buildConfig)\(buildParameters.suffix(triple: self.buildTriple)).module-resources" + "\(self.name)-\(buildParameters.triple.tripleString)-\(buildParameters.buildConfig)\(buildParameters.suffix(triple: self.buildTriple)).module-resources" } } extension ResolvedProduct { public func getLLBuildTargetName(buildParameters: BuildParameters) throws -> String { + let triple = buildParameters.triple.tripleString let config = buildParameters.buildConfig let suffix = buildParameters.suffix(triple: self.buildTriple) - let potentialExecutableTargetName = "\(name)-\(config)\(suffix).exe" - let potentialLibraryTargetName = "\(name)-\(config)\(suffix).dylib" + let potentialExecutableTargetName = "\(name)-\(triple)-\(config)\(suffix).exe" + let potentialLibraryTargetName = "\(name)-\(triple)-\(config)\(suffix).dylib" switch type { case .library(.dynamic): return potentialLibraryTargetName case .test: - return "\(name)-\(config)\(suffix).test" + return "\(name)-\(triple)-\(config)\(suffix).test" case .library(.static): - return "\(name)-\(config)\(suffix).a" + return "\(name)-\(triple)-\(config)\(suffix).a" case .library(.automatic): throw InternalError("automatic library not supported") case .executable, .snippet: diff --git a/Tests/BuildTests/BuildPlanTests.swift b/Tests/BuildTests/BuildPlanTests.swift index a7cd94e8f8f..7f5d2782166 100644 --- a/Tests/BuildTests/BuildPlanTests.swift +++ b/Tests/BuildTests/BuildPlanTests.swift @@ -5227,12 +5227,13 @@ final class BuildPlanTests: XCTestCase { try llbuild.generateManifest(at: yaml) let contents: String = try fs.readFileContents(yaml) + let triple = result.plan.destinationBuildParameters.triple.tripleString if result.plan.destinationBuildParameters.triple.isWindows() { XCTAssertMatch( contents, .contains(""" - "C.rary-debug.a": + "C.rary-\(triple)-debug.a": tool: shell inputs: ["\( buildPath.appending(components: "rary.build", "rary.swift.o") @@ -5263,7 +5264,7 @@ final class BuildPlanTests: XCTestCase { contents, .contains( """ - "C.rary-debug.a": + "C.rary-\(triple)-debug.a": tool: shell inputs: ["\( buildPath.appending(components: "rary.build", "rary.swift.o") @@ -5291,7 +5292,7 @@ final class BuildPlanTests: XCTestCase { contents, .contains( """ - "C.rary-debug.a": + "C.rary-\(triple)-debug.a": tool: shell inputs: ["\( buildPath.appending(components: "rary.build", "rary.swift.o") diff --git a/Tests/BuildTests/LLBuildManifestBuilderTests.swift b/Tests/BuildTests/LLBuildManifestBuilderTests.swift index e6c1eab2906..aa68775414c 100644 --- a/Tests/BuildTests/LLBuildManifestBuilderTests.swift +++ b/Tests/BuildTests/LLBuildManifestBuilderTests.swift @@ -75,8 +75,8 @@ final class LLBuildManifestBuilderTests: XCTestCase { var basicReleaseCommandNames = [ AbsolutePath("/path/to/build/\(buildParameters.triple)/release/exe.product/Objects.LinkFileList").pathString, - "", - "C.exe-release.exe", + "", + "C.exe-\(buildParameters.triple)-release.exe", ] XCTAssertEqual( @@ -104,11 +104,11 @@ final class LLBuildManifestBuilderTests: XCTestCase { llbuild = LLBuildManifestBuilder(plan, fileSystem: fs, observabilityScope: observability.topScope) try llbuild.createProductCommand(buildProduct) - let entitlementsCommandName = "C.exe-debug.exe-entitlements" + let entitlementsCommandName = "C.exe-\(buildParameters.triple)-debug.exe-entitlements" var basicDebugCommandNames = [ AbsolutePath("/path/to/build/\(buildParameters.triple)/debug/exe.product/Objects.LinkFileList").pathString, - "", - "C.exe-debug.exe", + "", + "C.exe-\(buildParameters.triple)-debug.exe", ] XCTAssertEqual( @@ -134,7 +134,7 @@ final class LLBuildManifestBuilderTests: XCTestCase { XCTAssertEqual( entitlementsCommand.outputs, [ - .virtual("exe-debug.exe-CodeSigning"), + .virtual("exe-\(buildParameters.triple)-debug.exe-CodeSigning"), ] ) @@ -160,8 +160,8 @@ final class LLBuildManifestBuilderTests: XCTestCase { basicReleaseCommandNames = [ AbsolutePath("/path/to/build/\(buildParameters.triple)/release/exe.product/Objects.LinkFileList").pathString, - "", - "C.exe-release.exe", + "", + "C.exe-\(buildParameters.triple)-release.exe", ] XCTAssertEqual( @@ -191,8 +191,8 @@ final class LLBuildManifestBuilderTests: XCTestCase { basicDebugCommandNames = [ AbsolutePath("/path/to/build/\(buildParameters.triple)/debug/exe.product/Objects.LinkFileList").pathString, - "", - "C.exe-debug.exe", + "", + "C.exe-\(buildParameters.triple)-debug.exe", ] XCTAssertEqual(