From f525da52529bcc8c3e51985d99f72ae44725de70 Mon Sep 17 00:00:00 2001 From: Ben Barham Date: Fri, 7 Feb 2025 19:56:18 -0800 Subject: [PATCH] Use the new SwiftPM API to load the build plan 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. --- .../SwiftPMBuildSystem.swift | 75 +++++-- Sources/SKTestSupport/SkipUnless.swift | 79 +++++++ .../SemanticIndex/SemanticIndexManager.swift | 33 ++- .../SwiftPMIntegrationTests.swift | 201 ++++++++++++++++++ 4 files changed, 359 insertions(+), 29 deletions(-) diff --git a/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift b/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift index 599dbbdb4..1979a4288 100644 --- a/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift +++ b/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift @@ -33,7 +33,7 @@ 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 @@ -41,7 +41,7 @@ import BuildServerProtocol import Foundation import LanguageServerProtocol import SKOptions -import SourceKitLSPAPI +@preconcurrency import SourceKitLSPAPI import ToolchainRegistry import class ToolchainRegistry.Toolchain #endif @@ -131,6 +131,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 @@ -170,13 +172,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, ]) @@ -291,6 +290,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 @@ -334,24 +346,47 @@ package actor SwiftPMBuildSystem: BuiltInBuildSystem { signposter.emitEvent("Finished loading modules graph", id: signpostID) - 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") - ) + // 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 plan", id: signpostID) + signposter.emitEvent("Finished generating build description", id: signpostID) - let buildDescription = BuildDescription(buildPlan: plan) - self.buildDescription = buildDescription + 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 index bb5b06036..99677fa1d 100644 --- a/Sources/SKTestSupport/SkipUnless.swift +++ b/Sources/SKTestSupport/SkipUnless.swift @@ -270,6 +270,85 @@ package actor SkipUnless { } } } + + 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 index 4236c001f..100494743 100644 --- 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() + 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 index 1db7e0e49..acc39e2ab 100644 --- a/Tests/SourceKitLSPTests/SwiftPMIntegrationTests.swift +++ b/Tests/SourceKitLSPTests/SwiftPMIntegrationTests.swift @@ -15,6 +15,7 @@ import Foundation import LanguageServerProtocol import SKTestSupport import SourceKitLSP +import SwiftExtensions import XCTest final class SwiftPMIntegrationTests: XCTestCase { @@ -359,4 +360,204 @@ final class SwiftPMIntegrationTests: XCTestCase { ["Cannot convert value of type 'Int' to specified type 'String'"] ) } + + func testToolPluginWithBuild() async throws { + let project = try await SwiftPMTestProject( + files: [ + "Plugins/plugin.swift": #""" + 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 generated = 1".write(to: URL(fileURLWithPath: CommandLine.arguments[1]), atomically: true, encoding: String.Encoding.utf8) + """#, + + "Sources/TestLib/TestLib.swift": #""" + func useGenerated() { + _ = 1️⃣generated + } + """#, + ], + 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: false + ) + + let (uri, positions) = try project.openDocument("TestLib.swift") + let result = try await project.testClient.send( + DefinitionRequest(textDocument: TextDocumentIdentifier(uri), position: positions["1️⃣"]) + ) + // We cannot run plugins when not using background indexing, so we expect no result here. + XCTAssertNil(result) + } + + func testToolPluginWithBackgroundIndexing() async throws { + try await SkipUnless.canLoadPluginsBuiltByToolchain() + + 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: [ + URL(fileURLWithPath: "$TEST_DIR_BACKSLASH_ESCAPED/topDep.txt"), + URL(fileURLWithPath: "$TEST_DIR_BACKSLASH_ESCAPED/Sources/TestLib/targetDep.txt") + ], + outputFiles: [generatedFile] + )] + } + } + """#, + + "Sources/CodeGenerator/CodeGenerator.swift": #""" + import Foundation + let topGenerated = try String(contentsOf: URL(fileURLWithPath: "$TEST_DIR_BACKSLASH_ESCAPED/topDep.txt")) + let targetGenerated = try String(contentsOf: URL(fileURLWithPath: "$TEST_DIR_BACKSLASH_ESCAPED/Sources/TestLib/targetDep.txt")) + try "\(topGenerated)\n\(targetGenerated)".write( + to: URL(fileURLWithPath: CommandLine.arguments[1]), + atomically: true, + encoding: String.Encoding.utf8 + ) + """#, + + "Sources/TestLib/TestLib.swift": #""" + func useGenerated() { + _ = 1️⃣topGenerated + _ = 2️⃣targetGenerated + } + """#, + + "/topDep.txt": "let topGenerated = 1", + "Sources/TestLib/targetDep.txt": "let targetGenerated = 1", + ], + 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") + + // We should have run plugins and thus created generated.swift + do { + let topResult = try await project.testClient.send( + DefinitionRequest(textDocument: TextDocumentIdentifier(uri), position: positions["1️⃣"]) + ) + let topLocation = try XCTUnwrap(topResult?.locations?.only) + XCTAssertTrue(topLocation.uri.pseudoPath.hasSuffix("generated.swift")) + XCTAssertEqual(topLocation.range.lowerBound, Position(line: 0, utf16index: 4)) + XCTAssertEqual(topLocation.range.upperBound, Position(line: 0, utf16index: 4)) + + let targetResult = try await project.testClient.send( + DefinitionRequest(textDocument: TextDocumentIdentifier(uri), position: positions["2️⃣"]) + ) + let targetLocation = try XCTUnwrap(targetResult?.locations?.only) + XCTAssertTrue(targetLocation.uri.pseudoPath.hasSuffix("generated.swift")) + XCTAssertEqual(targetLocation.range.lowerBound, Position(line: 1, utf16index: 4)) + XCTAssertEqual(targetLocation.range.upperBound, Position(line: 1, utf16index: 4)) + } + + // Make a change to the top level input file of the plugin command + let topDepUri = try project.uri(for: "topDep.txt") + let topDepUrl = try XCTUnwrap(topDepUri.fileURL) + try "// some change\nlet topGenerated = 2".write( + to: topDepUrl, + atomically: true, + encoding: .utf8 + ) + project.testClient.send(DidChangeWatchedFilesNotification(changes: [FileEvent(uri: topDepUri, type: .changed)])) + try await project.testClient.send(PollIndexRequest()) + + // Expect that the position has been updated in the dependency + try await repeatUntilExpectedResult { + let result = try await project.testClient.send( + DefinitionRequest(textDocument: TextDocumentIdentifier(uri), position: positions["1️⃣"]) + ) + let location = try XCTUnwrap(result?.locations?.only) + return location.range.lowerBound == Position(line: 1, utf16index: 4) + } + + // Make a change to the target level input file of the plugin command + let targetDepUri = try project.uri(for: "targetDep.txt") + let targetDepUrl = try XCTUnwrap(targetDepUri.fileURL) + try "// some change\nlet targetGenerated = 2".write( + to: targetDepUrl, + atomically: true, + encoding: .utf8 + ) + project.testClient.send(DidChangeWatchedFilesNotification(changes: [FileEvent(uri: targetDepUri, type: .changed)])) + try await project.testClient.send(PollIndexRequest()) + + // Expect that the position has been updated in the dependency + try await repeatUntilExpectedResult { + let result = try await project.testClient.send( + DefinitionRequest(textDocument: TextDocumentIdentifier(uri), position: positions["2️⃣"]) + ) + let location = try XCTUnwrap(result?.locations?.only) + return location.range.lowerBound == Position(line: 3, utf16index: 4) + } + } }