Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Documentation/Configuration File.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ The structure of the file is currently not guaranteed to be stable. Options may
- `linkerFlags: string[]`: Extra arguments passed to the linker. Equivalent to SwiftPM's `-Xlinker` option.
- `buildToolsSwiftCompilerFlags: string[]`: Extra arguments passed to the compiler for Swift files or plugins. Equivalent to SwiftPM's `-Xbuild-tools-swiftc` option.
- `disableSandbox: boolean`: Disables running subprocesses from SwiftPM in a sandbox. Equivalent to SwiftPM's `--disable-sandbox` option. Useful when running `sourcekit-lsp` in a sandbox because nested sandboxes are not supported.
- `buildSystem: "native"|"swiftbuild"`: Which SwiftPM build system should be used when opening a package.
- `compilationDatabase`: Dictionary with the following keys, defining options for workspaces with a compilation database.
- `searchPaths: string[]`: Additional paths to search for a compilation database, relative to a workspace root.
- `fallbackBuildSystem`: Dictionary with the following keys, defining options for files that aren't managed by any build server.
Expand Down
51 changes: 36 additions & 15 deletions Sources/BuildServerIntegration/BuildServerManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -280,22 +280,43 @@ private extension BuildServerSpec {
)
}
case .swiftPM:
#if !NO_SWIFTPM_DEPENDENCY
return await createBuiltInBuildServerAdapter(
messagesToSourceKitLSPHandler: messagesToSourceKitLSPHandler,
buildServerHooks: buildServerHooks
) { connectionToSourceKitLSP in
try await SwiftPMBuildServer(
projectRoot: projectRoot,
toolchainRegistry: toolchainRegistry,
options: options,
connectionToSourceKitLSP: connectionToSourceKitLSP,
testHooks: buildServerHooks.swiftPMTestHooks
)
switch options.swiftPMOrDefault.buildSystem {
case .swiftbuild:
let buildServer = await orLog("Creating external SwiftPM build server") {
try await ExternalBuildServerAdapter(
projectRoot: projectRoot,
config: BuildServerConfig.forSwiftPMBuildServer(
projectRoot: projectRoot,
swiftPMOptions: options.swiftPMOrDefault,
toolchainRegistry: toolchainRegistry
),
messagesToSourceKitLSPHandler: messagesToSourceKitLSPHandler
)
}
guard let buildServer else {
logger.log("Failed to create external SwiftPM build server at \(projectRoot)")
return nil
}
logger.log("Created external SwiftPM build server at \(projectRoot)")
return .external(buildServer)
case .native, nil:
#if !NO_SWIFTPM_DEPENDENCY
return await createBuiltInBuildServerAdapter(
messagesToSourceKitLSPHandler: messagesToSourceKitLSPHandler,
buildServerHooks: buildServerHooks
) { connectionToSourceKitLSP in
try await SwiftPMBuildServer(
projectRoot: projectRoot,
toolchainRegistry: toolchainRegistry,
options: options,
connectionToSourceKitLSP: connectionToSourceKitLSP,
testHooks: buildServerHooks.swiftPMTestHooks
)
}
#else
return nil
#endif
}
#else
return nil
#endif
case .injected(let injector):
let connectionToSourceKitLSP = LocalConnection(
receiverName: "BuildServerManager for \(projectRoot.lastPathComponent)",
Expand Down
102 changes: 96 additions & 6 deletions Sources/BuildServerIntegration/ExternalBuildServerAdapter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import Foundation
import SKOptions
import SwiftExtensions
import TSCExtensions
import ToolchainRegistry

import func TSCBasic.getEnvSearchPaths
import var TSCBasic.localFileSystem
Expand Down Expand Up @@ -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

Expand All @@ -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: "",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use Swift version tag?

Copy link
Contributor

Choose a reason for hiding this comment

The 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
Expand Down Expand Up @@ -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`.
Expand Down Expand Up @@ -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...])

Expand Down
1 change: 1 addition & 0 deletions Sources/SKOptions/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ add_library(SKOptions STATIC
BuildConfiguration.swift
ExperimentalFeatures.swift
SourceKitLSPOptions.swift
SwiftPMBuildSystem.swift
WorkspaceType.swift)
set_target_properties(SKOptions PROPERTIES
INTERFACE_INCLUDE_DIRECTORIES ${CMAKE_Swift_MODULE_DIRECTORY})
Expand Down
10 changes: 8 additions & 2 deletions Sources/SKOptions/SourceKitLSPOptions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,9 @@ public struct SourceKitLSPOptions: Sendable, Codable, Equatable {
/// background indexing.
public var skipPlugins: Bool?

/// Which SwiftPM build system should be used when opening a package.
public var buildSystem: SwiftPMBuildSystem?

public init(
configuration: BuildConfiguration? = nil,
scratchPath: String? = nil,
Expand All @@ -90,7 +93,8 @@ public struct SourceKitLSPOptions: Sendable, Codable, Equatable {
linkerFlags: [String]? = nil,
buildToolsSwiftCompilerFlags: [String]? = nil,
disableSandbox: Bool? = nil,
skipPlugins: Bool? = nil
skipPlugins: Bool? = nil,
buildSystem: SwiftPMBuildSystem? = nil
) {
self.configuration = configuration
self.scratchPath = scratchPath
Expand All @@ -105,6 +109,7 @@ public struct SourceKitLSPOptions: Sendable, Codable, Equatable {
self.linkerFlags = linkerFlags
self.buildToolsSwiftCompilerFlags = buildToolsSwiftCompilerFlags
self.disableSandbox = disableSandbox
self.buildSystem = buildSystem
}

static func merging(base: SwiftPMOptions, override: SwiftPMOptions?) -> SwiftPMOptions {
Expand All @@ -122,7 +127,8 @@ public struct SourceKitLSPOptions: Sendable, Codable, Equatable {
linkerFlags: override?.linkerFlags ?? base.linkerFlags,
buildToolsSwiftCompilerFlags: override?.buildToolsSwiftCompilerFlags ?? base.buildToolsSwiftCompilerFlags,
disableSandbox: override?.disableSandbox ?? base.disableSandbox,
skipPlugins: override?.skipPlugins ?? base.skipPlugins
skipPlugins: override?.skipPlugins ?? base.skipPlugins,
buildSystem: override?.buildSystem ?? base.buildSystem
)
}
}
Expand Down
16 changes: 16 additions & 0 deletions Sources/SKOptions/SwiftPMBuildSystem.swift
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
}
38 changes: 30 additions & 8 deletions Tests/BuildServerIntegrationTests/SwiftPMBuildServerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -132,8 +138,8 @@ struct SwiftPMBuildServerTests {
}
}

@Test
func testBasicSwiftArgs() async throws {
@Test(arguments: [SourceKitLSPOptions(), .forTestingExperimentalSwiftPMBuildServer])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you try running all tests that use SwiftPMTestProject with the new build server?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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

Copy link
Member

Choose a reason for hiding this comment

The 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 swift test twice 🤔) but that’s something for the future.

func testBasicSwiftArgs(options: SourceKitLSPOptions) async throws {
try await withTestScratchDir { tempDir in
try FileManager.default.createFiles(
root: tempDir,
Expand All @@ -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 }
Expand Down Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The 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 -target/-sdk/-I/-F (and not their values) is probably enough. And to be clear, no need to block the PR on this/moving tests in general.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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)
}
Expand Down
9 changes: 9 additions & 0 deletions config.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,15 @@
"description" : "Options for SwiftPM workspaces.",
"markdownDescription" : "Options for SwiftPM workspaces.",
"properties" : {
"buildSystem" : {
"description" : "Which SwiftPM build system should be used when opening a package.",
"enum" : [
"native",
"swiftbuild"
],
"markdownDescription" : "Which SwiftPM build system should be used when opening a package.",
"type" : "string"
},
"buildToolsSwiftCompilerFlags" : {
"description" : "Extra arguments passed to the compiler for Swift files or plugins. Equivalent to SwiftPM's `-Xbuild-tools-swiftc` option.",
"items" : {
Expand Down