-
Notifications
You must be signed in to change notification settings - Fork 322
Allow opting in to using the new SwiftPM BSP server when opening a package #2378
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,7 @@ import Foundation | |
| import SKOptions | ||
| import SwiftExtensions | ||
| import TSCExtensions | ||
| import ToolchainRegistry | ||
|
|
||
| import func TSCBasic.getEnvSearchPaths | ||
| import var TSCBasic.localFileSystem | ||
|
|
@@ -61,7 +62,7 @@ enum BuildServerNotFoundError: Error { | |
| /// BSP configuration | ||
| /// | ||
| /// See https://build-server-protocol.github.io/docs/overview/server-discovery#the-bsp-connection-details | ||
| private struct BuildServerConfig: Codable { | ||
| struct BuildServerConfig: Codable { | ||
| /// The name of the build tool. | ||
| let name: String | ||
|
|
||
|
|
@@ -82,15 +83,92 @@ private struct BuildServerConfig: Codable { | |
| let fileData = try Data(contentsOf: path) | ||
| return try decoder.decode(BuildServerConfig.self, from: fileData) | ||
| } | ||
|
|
||
| static func forSwiftPMBuildServer( | ||
| projectRoot: URL, | ||
| swiftPMOptions: SourceKitLSPOptions.SwiftPMOptions, | ||
| toolchainRegistry: ToolchainRegistry | ||
| ) async throws -> BuildServerConfig { | ||
| let toolchain = await toolchainRegistry.preferredToolchain(containing: [\.swift]) | ||
| guard let swiftPath = try toolchain?.swift?.filePath else { | ||
| throw ExecutableNotFoundError(executableName: "swift") | ||
| } | ||
| var args: [String] = [swiftPath, "package", "experimental-build-server"] | ||
| // The build server requires use of the Swift Build backend. | ||
| args.append(contentsOf: ["--build-system", "swiftbuild"]) | ||
| // Explicitly specify the package path. | ||
| try args.append(contentsOf: ["--package-path", projectRoot.filePath]) | ||
| // Map LSP SwiftPM options to build server flags | ||
| if let configuration = swiftPMOptions.configuration { | ||
| args.append(contentsOf: ["--configuration", configuration.rawValue]) | ||
| } | ||
| if let scratchPath = swiftPMOptions.scratchPath { | ||
| args.append(contentsOf: ["--scratch-path", scratchPath]) | ||
| } | ||
| if let swiftSDKsDirectory = swiftPMOptions.swiftSDKsDirectory { | ||
| args.append(contentsOf: ["--swift-sdks-path", swiftSDKsDirectory]) | ||
| } | ||
| if let swiftSDK = swiftPMOptions.swiftSDK { | ||
| args.append(contentsOf: ["--swift-sdk", swiftSDK]) | ||
| } | ||
| if let triple = swiftPMOptions.triple { | ||
| args.append(contentsOf: ["--triple", triple]) | ||
| } | ||
| if let toolsets = swiftPMOptions.toolsets { | ||
| for toolset in toolsets { | ||
| args.append(contentsOf: ["--toolset", toolset]) | ||
| } | ||
| } | ||
| if let traits = swiftPMOptions.traits { | ||
| args.append(contentsOf: ["--traits", traits.joined(separator: ",")]) | ||
| } | ||
| if let cCompilerFlags = swiftPMOptions.cCompilerFlags { | ||
| for flag in cCompilerFlags { | ||
| args.append(contentsOf: ["-Xcc", flag]) | ||
| } | ||
| } | ||
| if let cxxCompilerFlags = swiftPMOptions.cxxCompilerFlags { | ||
| for flag in cxxCompilerFlags { | ||
| args.append(contentsOf: ["-Xcxx", flag]) | ||
| } | ||
| } | ||
| if let swiftCompilerFlags = swiftPMOptions.swiftCompilerFlags { | ||
| for flag in swiftCompilerFlags { | ||
| args.append(contentsOf: ["-Xswiftc", flag]) | ||
| } | ||
| } | ||
| if let linkerFlags = swiftPMOptions.linkerFlags { | ||
| for flag in linkerFlags { | ||
| args.append(contentsOf: ["-Xlinker", flag]) | ||
| } | ||
| } | ||
| if let buildToolsSwiftCompilerFlags = swiftPMOptions.buildToolsSwiftCompilerFlags { | ||
| for flag in buildToolsSwiftCompilerFlags { | ||
| args.append(contentsOf: ["-Xbuild-tools-swiftc", flag]) | ||
| } | ||
| } | ||
| if swiftPMOptions.disableSandbox == true { | ||
| args.append("--disable-sandbox") | ||
| } | ||
| // The skipPlugins option isn't currently respected because the underlying build server does not support it. | ||
| // We may want to reconsider this in the future, or remove the option entirely. | ||
| return BuildServerConfig( | ||
| name: "SwiftPM Build Server", | ||
| version: "", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. use Swift version tag?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't have it in sourcekit-lsp right now (we have no --version) and until we have a common version that all the projects can use, I'd prefer not to add it. |
||
| bspVersion: "2.2.0", | ||
| languages: [Language.c, .cpp, .objective_c, .objective_cpp, .swift].map(\.rawValue), | ||
| argv: args | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| /// Launches a subprocess that is a BSP server and manages the process's lifetime. | ||
| actor ExternalBuildServerAdapter { | ||
| /// The root folder of the project. Used to resolve relative server paths. | ||
| private let projectRoot: URL | ||
|
|
||
| /// The file that specifies the configuration for this build server. | ||
| private let configPath: URL | ||
| /// The configuration for this build server. | ||
| private let serverConfig: BuildServerConfig | ||
|
|
||
| /// The `BuildServerManager` that handles messages from the BSP server to SourceKit-LSP. | ||
| var messagesToSourceKitLSPHandler: any MessageHandler | ||
|
|
@@ -123,15 +201,28 @@ actor ExternalBuildServerAdapter { | |
|
|
||
| init( | ||
| projectRoot: URL, | ||
| configPath: URL, | ||
| config: BuildServerConfig, | ||
| messagesToSourceKitLSPHandler: any MessageHandler | ||
| ) async throws { | ||
| self.projectRoot = projectRoot | ||
| self.configPath = configPath | ||
| self.serverConfig = config | ||
| self.messagesToSourceKitLSPHandler = messagesToSourceKitLSPHandler | ||
| self.connectionToBuildServer = try await self.createConnectionToBspServer() | ||
| } | ||
|
|
||
| init( | ||
| projectRoot: URL, | ||
| configPath: URL, | ||
| messagesToSourceKitLSPHandler: any MessageHandler | ||
| ) async throws { | ||
| let serverConfig = try BuildServerConfig.load(from: configPath) | ||
| try await self.init( | ||
| projectRoot: projectRoot, | ||
| config: serverConfig, | ||
| messagesToSourceKitLSPHandler: messagesToSourceKitLSPHandler | ||
| ) | ||
| } | ||
|
|
||
| /// Change the handler that handles messages from the build server. | ||
| /// | ||
| /// The intended use of this is to intercept messages from the build server by `LegacyBuildServer`. | ||
|
|
@@ -165,7 +256,6 @@ actor ExternalBuildServerAdapter { | |
|
|
||
| /// Create a new JSONRPCConnection to the build server. | ||
| private func createConnectionToBspServer() async throws -> JSONRPCConnection { | ||
| let serverConfig = try BuildServerConfig.load(from: configPath) | ||
| var serverPath = URL(fileURLWithPath: serverConfig.argv[0], relativeTo: projectRoot.ensuringCorrectTrailingSlash) | ||
| var serverArgs = Array(serverConfig.argv[1...]) | ||
|
|
||
|
|
||
owenv marked this conversation as resolved.
Show resolved
Hide resolved
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| //===----------------------------------------------------------------------===// | ||
| // | ||
| // This source file is part of the Swift.org open source project | ||
| // | ||
| // Copyright (c) 2014 - 2021 Apple Inc. and the Swift project authors | ||
| // Licensed under Apache License v2.0 with Runtime Library Exception | ||
| // | ||
| // See https://swift.org/LICENSE.txt for license information | ||
| // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors | ||
| // | ||
| //===----------------------------------------------------------------------===// | ||
|
|
||
| public enum SwiftPMBuildSystem: String, Codable, Sendable { | ||
| case native | ||
| case swiftbuild | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,6 +47,12 @@ private var hostTriple: Triple { | |
| } | ||
| } | ||
|
|
||
| fileprivate extension SourceKitLSPOptions { | ||
| static var forTestingExperimentalSwiftPMBuildServer: Self { | ||
| SourceKitLSPOptions(swiftPM: SwiftPMOptions(buildSystem: .swiftbuild)) | ||
| } | ||
| } | ||
|
|
||
| @Suite(.serialized, .configureLogging) | ||
| struct SwiftPMBuildServerTests { | ||
| @Test | ||
|
|
@@ -132,8 +138,8 @@ struct SwiftPMBuildServerTests { | |
| } | ||
| } | ||
|
|
||
| @Test | ||
| func testBasicSwiftArgs() async throws { | ||
| @Test(arguments: [SourceKitLSPOptions(), .forTestingExperimentalSwiftPMBuildServer]) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you try running all tests that use
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I haven't audited everything yet but I've gone through a decent number. Some failures due to tests making assumptions that don't hold under the new build system, and a few which look like bugs or bits of missing functionality that'll need to be fixed up
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice. I’d like to find some way to run tests using both configurations at some point (maybe an environment variable and just run |
||
| func testBasicSwiftArgs(options: SourceKitLSPOptions) async throws { | ||
| try await withTestScratchDir { tempDir in | ||
| try FileManager.default.createFiles( | ||
| root: tempDir, | ||
|
|
@@ -153,7 +159,7 @@ struct SwiftPMBuildServerTests { | |
| let buildServerManager = await BuildServerManager( | ||
| buildServerSpec: .swiftpmSpec(for: packageRoot), | ||
| toolchainRegistry: .forTesting, | ||
| options: SourceKitLSPOptions(), | ||
| options: options, | ||
| connectionToClient: DummyBuildServerManagerConnectionToClient(), | ||
| buildServerHooks: BuildServerHooks(), | ||
| createMainFilesProvider: { _, _ in nil } | ||
|
|
@@ -181,18 +187,34 @@ struct SwiftPMBuildServerTests { | |
| expectArgumentsContain("-target", arguments: arguments) // Only one! | ||
| #if os(macOS) | ||
| let versionString = PackageModel.Platform.macOS.oldestSupportedVersion.versionString | ||
| if options.swiftPMOrDefault.buildSystem == .swiftbuild { | ||
| expectArgumentsContain( | ||
| "-target", | ||
| // Account for differences in macOS naming canonicalization | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I assume this is a built-in vs swift-build difference rather than a built-in SwiftPM support vs build server? Given this depends on PackageModel anyway, this will probably be one of the tests that will end up moving. For in sourcekit-lsp, checking we have
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yep, one uses macosx, the other macos. It's pretty annoying but hard to easily align them. |
||
| try await hostTriple.tripleString(forPlatformVersion: versionString).replacing("macosx", with: "macos"), | ||
| arguments: arguments | ||
| ) | ||
| } else { | ||
| expectArgumentsContain( | ||
| "-target", | ||
| try await hostTriple.tripleString(forPlatformVersion: versionString), | ||
| arguments: arguments | ||
| ) | ||
| } | ||
| expectArgumentsContain( | ||
| "-target", | ||
| try await hostTriple.tripleString(forPlatformVersion: versionString), | ||
| arguments: arguments | ||
| "-sdk", | ||
| arguments: arguments, | ||
| allowMultiple: options.swiftPMOrDefault.buildSystem == .swiftbuild | ||
| ) | ||
| expectArgumentsContain("-sdk", arguments: arguments) | ||
| expectArgumentsContain("-F", arguments: arguments, allowMultiple: true) | ||
| #else | ||
| expectArgumentsContain("-target", try await hostTriple.tripleString, arguments: arguments) | ||
| #endif | ||
|
|
||
| expectArgumentsContain("-I", try build.appending(component: "Modules").filePath, arguments: arguments) | ||
| if options.swiftPMOrDefault.buildSystem != .swiftbuild { | ||
| // Swift Build and the native build system setup search paths differently. We deliberately avoid testing implementation details of Swift Build here. | ||
| expectArgumentsContain("-I", try build.appending(component: "Modules").filePath, arguments: arguments) | ||
| } | ||
|
|
||
| expectArgumentsContain(try aswift.filePath, arguments: arguments) | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.