Use the new SwiftPM API to load the build plan

bnbarham · bnbarham · commit 165b4bd74bca · 2025-02-14T14:21:38.000-08:00
We previously skipped building/running tool plugins here, which meant that the compiler arguments for a target also missed any generated sources. Use the new `BuildDescription.load` API from SwiftPM to address this. Resolves rdar://102242345. (cherry picked from commit f525da5)
diff --git a/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift b/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift
@@ -33,15 +33,15 @@ package import BuildServerProtocol
 package import Foundation
 package import LanguageServerProtocol
 package import SKOptions
-package import SourceKitLSPAPI
+@preconcurrency package import SourceKitLSPAPI
 package import ToolchainRegistry
 package import class ToolchainRegistry.Toolchain
 #else
 import BuildServerProtocol
 import Foundation
 import LanguageServerProtocol
 import SKOptions
-import SourceKitLSPAPI
+@preconcurrency import SourceKitLSPAPI
 import ToolchainRegistry
 import class ToolchainRegistry.Toolchain
 #endif
@@ -153,6 +153,8 @@ package actor SwiftPMBuildSystem: BuiltInBuildSystem {
   private let toolchain: Toolchain
   private let swiftPMWorkspace: Workspace
 
+  private let pluginConfiguration: PluginConfiguration
+
   /// A `ObservabilitySystem` from `SwiftPM` that logs.
   private let observabilitySystem: ObservabilitySystem
 
@@ -192,13 +194,10 @@ package actor SwiftPMBuildSystem: BuiltInBuildSystem {
   ) async throws {
     self.projectRoot = projectRoot
     self.options = options
-    self.fileWatchers =
-      try ["Package.swift", "Package@swift*.swift", "Package.resolved"].map {
-        FileSystemWatcher(globPattern: try projectRoot.appendingPathComponent($0).filePath, kind: [.change])
-      }
-      + FileRuleDescription.builtinRules.flatMap({ $0.fileTypes }).map { fileExtension in
-        FileSystemWatcher(globPattern: "**/*.\(fileExtension)", kind: [.create, .change, .delete])
-      }
+    // We could theoretically dynamically register all known files when we get back the build graph, but that seems
+    // more errorprone than just watching everything and then filtering when we need to (eg. in
+    // `SemanticIndexManager.filesDidChange`).
+    self.fileWatchers = [FileSystemWatcher(globPattern: "**/*", kind: [.create, .change, .delete])]
     let toolchain = await toolchainRegistry.preferredToolchain(containing: [
       \.clang, \.clangd, \.sourcekitd, \.swift, \.swiftc,
     ])
@@ -313,6 +312,19 @@ package actor SwiftPMBuildSystem: BuiltInBuildSystem {
       prepareForIndexing: options.backgroundPreparationModeOrDefault.toSwiftPMPreparation
     )
 
+    let pluginScriptRunner = DefaultPluginScriptRunner(
+      fileSystem: localFileSystem,
+      cacheDir: location.pluginWorkingDirectory.appending("cache"),
+      toolchain: hostSwiftPMToolchain,
+      extraPluginSwiftCFlags: [],
+      enableSandbox: !(options.swiftPMOrDefault.disableSandbox ?? false)
+    )
+    self.pluginConfiguration = PluginConfiguration(
+      scriptRunner: pluginScriptRunner,
+      workDirectory: location.pluginWorkingDirectory,
+      disableSandbox: options.swiftPMOrDefault.disableSandbox ?? false
+    )
+
     packageLoadingQueue.async {
       await orLog("Initial package loading") {
         // Schedule an initial generation of the build graph. Once the build graph is loaded, the build system will send
@@ -350,21 +362,49 @@ package actor SwiftPMBuildSystem: BuiltInBuildSystem {
       observabilityScope: observabilitySystem.topScope.makeChildScope(description: "Load package graph")
     )
 
-    let plan = try await BuildPlan(
-      destinationBuildParameters: destinationBuildParameters,
-      toolsBuildParameters: toolsBuildParameters,
-      graph: modulesGraph,
-      disableSandbox: options.swiftPMOrDefault.disableSandbox ?? false,
-      fileSystem: localFileSystem,
-      observabilityScope: observabilitySystem.topScope.makeChildScope(description: "Create SwiftPM build plan")
-    )
-    let buildDescription = BuildDescription(buildPlan: plan)
-    self.buildDescription = buildDescription
+    signposter.emitEvent("Finished loading modules graph", id: signpostID)
+
+    // We have a whole separate arena if we're performing background indexing. This allows us to also build and run
+    // plugins, without having to worry about messing up any regular build state.
+    let buildDescription: SourceKitLSPAPI.BuildDescription
+    if isForIndexBuild {
+      let loaded = try await BuildDescription.load(
+        destinationBuildParameters: destinationBuildParameters,
+        toolsBuildParameters: toolsBuildParameters,
+        packageGraph: modulesGraph,
+        pluginConfiguration: pluginConfiguration,
+        disableSandbox: options.swiftPMOrDefault.disableSandbox ?? false,
+        scratchDirectory: swiftPMWorkspace.location.scratchDirectory.asURL,
+        fileSystem: localFileSystem,
+        observabilityScope: observabilitySystem.topScope.makeChildScope(description: "Create SwiftPM build description")
+      )
+      if !loaded.errors.isEmpty {
+        logger.error("Loading SwiftPM description had errors: \(loaded.errors)")
+      }
+
+      signposter.emitEvent("Finished generating build description", id: signpostID)
+
+      buildDescription = loaded.description
+    } else {
+      let plan = try await BuildPlan(
+        destinationBuildParameters: destinationBuildParameters,
+        toolsBuildParameters: toolsBuildParameters,
+        graph: modulesGraph,
+        disableSandbox: options.swiftPMOrDefault.disableSandbox ?? false,
+        fileSystem: localFileSystem,
+        observabilityScope: observabilitySystem.topScope.makeChildScope(description: "Create SwiftPM build plan")
+      )
+
+      signposter.emitEvent("Finished generating build plan", id: signpostID)
+
+      buildDescription = BuildDescription(buildPlan: plan)
+    }
 
     /// Make sure to execute any throwing statements before setting any
     /// properties because otherwise we might end up in an inconsistent state
     /// with only some properties modified.
 
+    self.buildDescription = buildDescription
     self.swiftPMTargets = [:]
     self.targetDependencies = [:]
 
diff --git a/Sources/SKTestSupport/SkipUnless.swift b/Sources/SKTestSupport/SkipUnless.swift
@@ -565,6 +565,85 @@ package actor SkipUnless {
       return locations.count > 0
     }
   }
+
+  package static func canLoadPluginsBuiltByToolchain(
+    file: StaticString = #filePath,
+    line: UInt = #line
+  ) async throws {
+    return try await shared.skipUnlessSupported(file: file, line: line) {
+      let project = try await SwiftPMTestProject(
+        files: [
+          "Plugins/plugin.swift": #"""
+          import Foundation
+          import PackagePlugin
+          @main struct CodeGeneratorPlugin: BuildToolPlugin {
+            func createBuildCommands(context: PluginContext, target: Target) throws -> [Command] {
+              let genSourcesDir = context.pluginWorkDirectoryURL.appending(path: "GeneratedSources")
+              guard let target = target as? SourceModuleTarget else { return [] }
+              let codeGenerator = try context.tool(named: "CodeGenerator").url
+              let generatedFile = genSourcesDir.appending(path: "\(target.name)-generated.swift")
+              return [.buildCommand(
+                displayName: "Generating code for \(target.name)",
+                executable: codeGenerator,
+                arguments: [
+                  generatedFile.path
+                ],
+                inputFiles: [],
+                outputFiles: [generatedFile]
+              )]
+            }
+          }
+          """#,
+
+          "Sources/CodeGenerator/CodeGenerator.swift": #"""
+          import Foundation
+          try "let foo = 1".write(
+            to: URL(fileURLWithPath: CommandLine.arguments[1]),
+            atomically: true,
+            encoding: String.Encoding.utf8
+          )
+          """#,
+
+          "Sources/TestLib/TestLib.swift": #"""
+          func useGenerated() {
+            _ = 1️⃣foo
+          }
+          """#,
+        ],
+        manifest: """
+          // swift-tools-version: 6.0
+          import PackageDescription
+          let package = Package(
+            name: "PluginTest",
+            targets: [
+              .executableTarget(name: "CodeGenerator"),
+              .target(
+                name: "TestLib",
+                plugins: [.plugin(name: "CodeGeneratorPlugin")]
+              ),
+              .plugin(
+                name: "CodeGeneratorPlugin",
+                capability: .buildTool(),
+                dependencies: ["CodeGenerator"]
+              ),
+            ]
+          )
+          """,
+        enableBackgroundIndexing: true
+      )
+
+      let (uri, positions) = try project.openDocument("TestLib.swift")
+
+      let result = try await project.testClient.send(
+        DefinitionRequest(textDocument: TextDocumentIdentifier(uri), position: positions["1️⃣"])
+      )
+
+      if result?.locations?.only == nil {
+        return .featureUnsupported(skipMessage: "Skipping because plugin protocols do not match.")
+      }
+      return .featureSupported
+    }
+  }
 }
 
 // MARK: - Parsing Swift compiler version
diff --git a/Sources/SemanticIndex/SemanticIndexManager.swift b/Sources/SemanticIndex/SemanticIndexManager.swift
@@ -10,6 +10,8 @@
 //
 //===----------------------------------------------------------------------===//
 
+import LanguageServerProtocolExtensions
+
 #if compiler(>=6)
 package import BuildServerProtocol
 package import BuildSystemIntegration
@@ -385,15 +387,28 @@ package final actor SemanticIndexManager {
     let changedFiles = events.map(\.uri)
     await indexStoreUpToDateTracker.markOutOfDate(changedFiles)
 
-    let targets = await changedFiles.asyncMap { await buildSystemManager.targets(for: $0) }.flatMap { $0 }
-    let dependentTargets = await buildSystemManager.targets(dependingOn: Set(targets))
-    logger.info(
-      """
-      Marking targets as out-of-date: \
-      \(String(dependentTargets.map(\.uri.stringValue).joined(separator: ", ")))
-      """
-    )
-    await preparationUpToDateTracker.markOutOfDate(dependentTargets)
+    // Preparation tracking should be per file. For now consider any non-known-language change as having to re-prepare
+    // the target itself so that we re-prepare potential input files to plugins.
+    // https://github.com/swiftlang/sourcekit-lsp/issues/1975
+    var outOfDateTargets = Set<BuildTargetIdentifier>()
+    for file in changedFiles {
+      let changedTargets = await buildSystemManager.targets(for: file)
+      if Language(inferredFromFileExtension: file) == nil {
+        outOfDateTargets.formUnion(changedTargets)
+      }
+
+      let dependentTargets = await buildSystemManager.targets(dependingOn: changedTargets)
+      outOfDateTargets.formUnion(dependentTargets)
+    }
+    if !outOfDateTargets.isEmpty {
+      logger.info(
+        """
+        Marking dependent targets as out-of-date: \
+        \(String(outOfDateTargets.map(\.uri.stringValue).joined(separator: ", ")))
+        """
+      )
+      await preparationUpToDateTracker.markOutOfDate(outOfDateTargets)
+    }
 
     await scheduleBuildGraphGenerationAndBackgroundIndexAllFiles(
       filesToIndex: changedFiles,
diff --git a/Tests/SourceKitLSPTests/SwiftPMIntegrationTests.swift b/Tests/SourceKitLSPTests/SwiftPMIntegrationTests.swift
