Place build.db under the scratch directory (#7471)

### Motivation: The build database should be shared between different triple builds to re-generate the build manifest correctly. With the current implementation, the following build command sequence fails: ``` # 1st iteration: They both do not trigger "PackageStructure" build $ swift build --experimental-swift-sdk wasm32-unknown-wasi $ swift build # 2nd iteration: They both trigger "PackageStructure" build because there is description.json and debug.yaml $ swift build --experimental-swift-sdk wasm32-unknown-wasi $ swift build # 3rd iteration: Manifest cache entry in .build/wasm32-unknown-wasi/build.db created by 2nd iteration hits, # so do not update debug.yaml. <--- Incorrect $ swift build --experimental-swift-sdk wasm32-unknown-wasi ``` ### Modifications: This changes the llbuild build database to be placed under `.build/build.db` instead of `.build/<product triple>/build.db`. Also this change splits llbuild target names for each triple to avoid cache invalidation when switching triple. ### Result: `build.db` will be shared across product target triples, and SwiftPM will keep consistent cache state when switching triples.
swiftlang · May 29, 2024 · e9399c2 · e9399c2
1 parent 4dc41b0
commit e9399c2
Show file tree

Hide file tree

Showing 7 changed files with 154 additions and 33 deletions.
diff --git a/Sources/Build/BuildManifest/LLBuildManifestBuilder.swift b/Sources/Build/BuildManifest/LLBuildManifestBuilder.swift
@@ -322,28 +322,29 @@ 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 {
-        "\(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/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<String, PackageIdentity>(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
@@ -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
@@ -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
@@ -14,34 +14,62 @@
 @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(
+    productsBuildParameters: BuildParameters,
+    toolsBuildParameters: BuildParameters,
+    cacheBuildManifest: Bool = false,
+    packageGraphLoader: @escaping () -> ModulesGraph = { fatalError() },
+    scratchDirectory: AbsolutePath,
+    fs: any Basics.FileSystem,
+    observabilityScope: ObservabilityScope
+) -> BuildOperation {
+    return BuildOperation(
+        productsBuildParameters: productsBuildParameters,
+        toolsBuildParameters: toolsBuildParameters,
+        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(
+        let buildOp = mockBuildOperation(
             productsBuildParameters: buildParameters,
             toolsBuildParameters: buildParameters,
-            cacheBuildManifest: false,
-            packageGraphLoader: { fatalError() },
-            additionalFileRules: [],
-            pkgConfigDirectories: [],
-            dependenciesByRootPackageIdentity: [:],
-            targetsByRootPackageIdentity: [:],
-            outputStream: BufferedOutputByteStream(),
-            logLevel: .info,
-            fileSystem: fs,
-            observabilityScope: observability.topScope
+            scratchDirectory: scratchDirectory,
+            fs: fs, observabilityScope: observability.topScope
         )
         buildOp.detectUnexpressedDependencies(
             availableLibraries: [
@@ -65,4 +93,87 @@ 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(
+                    productsBuildParameters: buildParameters,
+                    toolsBuildParameters: 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(
+                        productsBuildParameters: buildParameters,
+                        toolsBuildParameters: 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/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
@@ -75,8 +75,8 @@ final class LLBuildManifestBuilderTests: XCTestCase {
 
         var basicReleaseCommandNames = [
             AbsolutePath("/path/to/build/\(buildParameters.triple)/release/exe.product/Objects.LinkFileList").pathString,
-            "<exe-release.exe>",
-            "C.exe-release.exe",
+            "<exe-\(buildParameters.triple)-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,
-            "<exe-debug.exe>",
-            "C.exe-debug.exe",
+            "<exe-\(buildParameters.triple)-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,
-            "<exe-release.exe>",
-            "C.exe-release.exe",
+            "<exe-\(buildParameters.triple)-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,
-            "<exe-debug.exe>",
-            "C.exe-debug.exe",
+            "<exe-\(buildParameters.triple)-debug.exe>",
+            "C.exe-\(buildParameters.triple)-debug.exe",
         ]
 
         XCTAssertEqual(
@@ -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"])
     }
 }