Skip to content

Commit

Permalink
Allow overriding the workspace buildsystem per workspace
Browse files Browse the repository at this point in the history
Merge options prior to determining the buildsystem to use for a
workspace, rather than after.

Do not search up the directory tree when determining a build system for
the given workspaces. Implicit workspaces may search up to the workspace
roots, but no further.
  • Loading branch information
bnbarham committed Dec 18, 2024
1 parent 1cd010d commit a26f35d
Show file tree
Hide file tree
Showing 7 changed files with 166 additions and 37 deletions.
2 changes: 1 addition & 1 deletion Documentation/Configuration File.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ The structure of the file is currently not guaranteed to be stable. Options may
- `logLevel: "debug"|"info"|"default"|"error"|"fault"`: The level from which one onwards log messages should be written.
- `privacyLevel: "public"|"private"|"sensitive"`: Whether potentially sensitive information should be redacted. Default is `public`, which redacts potentially sensitive information.
- `inputMirrorDirectory: string`: Write all input received by SourceKit-LSP on stdin to a file in this directory. Useful to record and replay an entire SourceKit-LSP session.
- `defaultWorkspaceType: "buildserver"|"compdb"|"swiftpm"`: Overrides workspace type selection logic.
- `defaultWorkspaceType: "swiftPM"|"compilationDatabase"|"buildServer"`: Overrides workspace type selection logic.
- `generatedFilesPath: string`: Directory in which generated interfaces and macro expansions should be stored.
- `backgroundIndexing: bool`: Explicitly enable or disable background indexing.
- `backgroundPreparationMode: "build"|"noLazy"|"enabled"`: Determines how background indexing should prepare a target. Possible values are:
Expand Down
4 changes: 3 additions & 1 deletion Sources/BuildSystemIntegration/DetermineBuildSystem.swift
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,13 @@ import struct TSCBasic.AbsolutePath
#endif

/// Determine which build system should be started to handle the given workspace folder and at which folder that build
/// system's project root is (see `BuiltInBuildSystem.projectRoot(for:options:)`).
/// system's project root is (see `BuiltInBuildSystem.projectRoot(for:options:)`). `onlyConsiderRoot` controls whether
/// paths outside the root should be considered (eg. configuration files in the user's home directory).
///
/// Returns `nil` if no build system can handle this workspace folder.
package func determineBuildSystem(
forWorkspaceFolder workspaceFolder: DocumentURI,
onlyConsiderRoot: Bool,
options: SourceKitLSPOptions
) -> BuildSystemSpec? {
var buildSystemPreference: [WorkspaceType] = [
Expand Down
13 changes: 4 additions & 9 deletions Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift
Original file line number Diff line number Diff line change
Expand Up @@ -230,17 +230,12 @@ package actor SwiftPMBuildSystem: BuiltInBuildSystem {
guard var path = orLog("Getting realpath for project root", { try path.realpath }) else {
return nil
}
while true {
let packagePath = path.appendingPathComponent("Package.swift")
if (try? String(contentsOf: packagePath, encoding: .utf8))?.contains("PackageDescription") ?? false {
return path
}

if (try? AbsolutePath(validating: path.filePath))?.isRoot ?? true {
break
}
path.deleteLastPathComponent()
let packagePath = path.appendingPathComponent("Package.swift")
if (try? String(contentsOf: packagePath, encoding: .utf8))?.contains("PackageDescription") ?? false {
return path
}

return nil
}

Expand Down
13 changes: 12 additions & 1 deletion Sources/SKOptions/SourceKitLSPOptions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ public struct SourceKitLSPOptions: Sendable, Codable, Equatable {
set { logging = newValue }
}

/// Default workspace type (buildserver|compdb|swiftpm). Overrides workspace type selection logic.
/// Default workspace type (swiftPM|compilationDatabase|buildServer). Overrides workspace type selection logic.
public var defaultWorkspaceType: WorkspaceType?
public var generatedFilesPath: String?

Expand Down Expand Up @@ -444,6 +444,17 @@ public struct SourceKitLSPOptions: Sendable, Codable, Equatable {
)
}

public static func merging(base: SourceKitLSPOptions, workspaceFolder: DocumentURI) -> SourceKitLSPOptions {
return SourceKitLSPOptions.merging(
base: base,
override: SourceKitLSPOptions(
path: workspaceFolder.fileURL?
.appendingPathComponent(".sourcekit-lsp")
.appendingPathComponent("config.json")
)
)
}

public var generatedFilesAbsolutePath: URL {
if let generatedFilesPath {
return URL(fileURLWithPath: generatedFilesPath)
Expand Down
54 changes: 31 additions & 23 deletions Sources/SourceKitLSP/SourceKitLSPServer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -209,29 +209,40 @@ package actor SourceKitLSPServer {
guard var url = uri.fileURL?.deletingLastPathComponent() else {
return nil
}
let projectRoots = await self.workspacesAndIsImplicit.filter { !$0.isImplicit }.asyncCompactMap {

// Roots of opened workspaces - only consider explicit here (all implicit must necessarily be subdirectories of
// the explicit workspace roots)
let workspaceRoots = workspacesAndIsImplicit.filter { !$0.isImplicit }.compactMap { $0.workspace.rootUri?.fileURL }

// We want to skip creating another workspace if the project roots are the same. This could happen if an existing
// workspace hasn't reloaded after a new file was added to it (and thus that build system needs to be reloaded).
let projectRoots = await workspacesAndIsImplicit.asyncCompactMap {
await $0.workspace.buildSystemManager.projectRoot
}
let rootURLs = workspacesAndIsImplicit.filter { !$0.isImplicit }.compactMap { $0.workspace.rootUri?.fileURL }
while url.pathComponents.count > 1 && rootURLs.contains(where: { $0.isPrefix(of: url) }) {

while url.pathComponents.count > 1 && workspaceRoots.contains(where: { $0.isPrefix(of: url) }) {
defer {
url.deleteLastPathComponent()
}
// Ignore workspaces that have the same project root as an existing workspace.
// This might happen if there is an existing SwiftPM workspace that hasn't been reloaded after a new file
// was added to it and thus currently doesn't know that it can handle that file. In that case, we shouldn't open
// a new workspace for the same root. Instead, the existing workspace's build system needs to be reloaded.

let uri = DocumentURI(url)
guard let buildSystemSpec = determineBuildSystem(forWorkspaceFolder: uri, options: self.options) else {
let options = SourceKitLSPOptions.merging(base: self.options, workspaceFolder: uri)

// Some build systems consider paths outside of the folder (eg. BSP has settings in the home directory). If we
// allowed those paths, then the very first folder that the file is in would always be its own build system - so
// skip them in that case.
guard let buildSystemSpec = determineBuildSystem(forWorkspaceFolder: uri, onlyConsiderRoot: true, options: options) else {
continue
}
guard !projectRoots.contains(buildSystemSpec.projectRoot) else {
continue
}

// No existing workspace matches this root - create one.
guard
let workspace = await orLog(
"Creating workspace",
{ try await createWorkspace(workspaceFolder: uri, buildSystemSpec: buildSystemSpec) }
"Creating implicit workspace",
{ try await createWorkspace(workspaceFolder: uri, options: options, buildSystemSpec: buildSystemSpec) }
)
else {
continue
Expand Down Expand Up @@ -818,26 +829,20 @@ extension SourceKitLSPServer {

/// Creates a workspace at the given `uri`.
///
/// If the build system that was determined for the workspace does not satisfy `condition`, `nil` is returned.
/// A workspace does not necessarily have any buildsystem attached to it, in which case `buildSystemSpec` may be
/// `nil` - consider eg. a top level workspace folder with multiple SwiftPM projects inside it.
private func createWorkspace(
workspaceFolder: DocumentURI,
options: SourceKitLSPOptions,
buildSystemSpec: BuildSystemSpec?
) async throws -> Workspace {
guard let capabilityRegistry = capabilityRegistry else {
struct NoCapabilityRegistryError: Error {}
logger.log("Cannot open workspace before server is initialized")
throw NoCapabilityRegistryError()
}
let testHooks = self.testHooks
let options = SourceKitLSPOptions.merging(
base: self.options,
override: SourceKitLSPOptions(
path: workspaceFolder.fileURL?
.appendingPathComponent(".sourcekit-lsp")
.appendingPathComponent("config.json")
)
)
logger.log("Creating workspace at \(workspaceFolder.forLogging) with options: \(options.forLogging)")

logger.log("Creating workspace at \(workspaceFolder.forLogging)")
logger.logFullObjectInMultipleLogMessages(header: "Options for workspace", options.loggingProxy)

let workspace = await Workspace(
Expand All @@ -848,7 +853,7 @@ extension SourceKitLSPServer {
buildSystemSpec: buildSystemSpec,
toolchainRegistry: self.toolchainRegistry,
options: options,
testHooks: testHooks,
testHooks: self.testHooks,
indexTaskScheduler: indexTaskScheduler
)
return workspace
Expand All @@ -857,9 +862,12 @@ extension SourceKitLSPServer {
/// Determines the build system for the given workspace folder and creates a `Workspace` that uses this inferred build
/// system.
private func createWorkspaceWithInferredBuildSystem(workspaceFolder: DocumentURI) async throws -> Workspace {
let options = SourceKitLSPOptions.merging(base: self.options, workspaceFolder: workspaceFolder)
let buildSystemSpec = determineBuildSystem(forWorkspaceFolder: workspaceFolder, onlyConsiderRoot: false, options: options)
return try await self.createWorkspace(
workspaceFolder: workspaceFolder,
buildSystemSpec: determineBuildSystem(forWorkspaceFolder: workspaceFolder, options: self.options)
options: options,
buildSystemSpec: buildSystemSpec
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -976,9 +976,9 @@ final class SwiftPMBuildSystemTests: XCTestCase {
.appendingPathComponent("pkg")
.appendingPathComponent("Sources")
.appendingPathComponent("lib")
let projectRoot = SwiftPMBuildSystem.projectRoot(for: workspaceRoot, options: .testDefault())

assertEqual(projectRoot, tempDir.appendingPathComponent("pkg", isDirectory: true))
let projectRoot = SwiftPMBuildSystem.projectRoot(for: workspaceRoot, options: .testDefault())
assertNil(projectRoot)
}
}

Expand Down
113 changes: 113 additions & 0 deletions Tests/SourceKitLSPTests/WorkspaceTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
//
//===----------------------------------------------------------------------===//

import BuildSystemIntegration
import Foundation
import LanguageServerProtocol
import SKLogging
Expand Down Expand Up @@ -967,4 +968,116 @@ final class WorkspaceTests: XCTestCase {
["Cannot convert value of type 'Int' to specified type 'String'"]
)
}

func testWorkspaceOptionsOverrideBuildSystem() async throws {
let swiftc = try await unwrap(ToolchainRegistry.forTesting.default?.swiftc)
let sdkArgs =
if let defaultSDKPath {
"""
-sdk '\(defaultSDKPath)'
"""
} else {
""
}

let project = try await MultiFileTestProject(files: [
".sourcekit-lsp/config.json": """
{
"defaultWorkspaceType": "compilationDatabase"
}
""",
"src/Foo.swift": """
#if HAVE_SETTINGS
#error("Have settings")
#endif
""",
"Sources/MyLib/Bar.swift": "",
"build/compile_commands.json": """
[
{
"directory": "$TEST_DIR",
"command": "\(swiftc.filePath) $TEST_DIR/src/Foo.swift -DHAVE_SETTINGS \(sdkArgs)",
"file": "src/Foo.swift",
"output": "$TEST_DIR/build/Foo.swift.o"
},
]
""",
"Package.swift": """
// swift-tools-version: 5.7
import PackageDescription
let package = Package(
name: "MyLib",
targets: [
.target(name: "MyLib"),
]
)
""",
])
let (uri, _) = try project.openDocument("Foo.swift")
let diagnostics = try await project.testClient.send(
DocumentDiagnosticsRequest(textDocument: TextDocumentIdentifier(uri))
)
XCTAssertEqual(
diagnostics.fullReport?.items.map(\.message),
["Have settings"]
)
}

func testImplicitWorkspaceOptionsOverrideBuildSystem() async throws {
let swiftc = try await unwrap(ToolchainRegistry.forTesting.default?.swiftc)
let sdkArgs =
if let defaultSDKPath {
"""
-sdk '\(defaultSDKPath)'
"""
} else {
""
}

let project = try await MultiFileTestProject(files: [
"projA/.sourcekit-lsp/config.json": """
{
"defaultWorkspaceType": "compilationDatabase"
}
""",
"projA/src/Foo.swift": """
#if HAVE_SETTINGS
#error("Have settings")
#endif
""",
"projA/Sources/MyLib/Bar.swift": "",
"projA/build/compile_commands.json": """
[
{
"directory": "$TEST_DIR/projA",
"command": "\(swiftc.filePath) $TEST_DIR/projA/src/Foo.swift -DHAVE_SETTINGS \(sdkArgs)",
"file": "src/Foo.swift",
"output": "$TEST_DIR/projA/build/Foo.swift.o"
},
]
""",
"projA/Package.swift": """
// swift-tools-version: 5.7
import PackageDescription
let package = Package(
name: "MyLib",
targets: [
.target(name: "MyLib"),
]
)
""",
])
let (uri, _) = try project.openDocument("Foo.swift")
let diagnostics = try await project.testClient.send(
DocumentDiagnosticsRequest(textDocument: TextDocumentIdentifier(uri))
)
XCTAssertEqual(
diagnostics.fullReport?.items.map(\.message),
["Have settings"]
)
}
}

0 comments on commit a26f35d

Please sign in to comment.