Skip to content

Commit 4656789

Browse files
committed
Make output paths part of the buildTarget/sources request instead of a separate request
We need a mapping from source file to its output path in order to support source files that are part of multiple targets (because we need the output path to check if we have an up-to-date unit for a file in a given target). To achieve this mapping, it’s easier to tag the output path for each source file onto the `buildTarget/sources` request.
1 parent 76c8f75 commit 4656789

14 files changed

+56
-175
lines changed

Contributor Documentation/BSP Extensions.md

Lines changed: 14 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,14 @@ export interface SourceKitInitializeBuildResponseData {
2222
* for `swiftc` or `clang` invocations **/
2323
indexStorePath?: string;
2424

25-
/** Whether the server implements the `buildTarget/outputPaths` request. */
25+
/** Whether the server set the `outputPath` property in the `buildTarget/sources` request */
2626
outputPathsProvider?: bool;
2727

2828
/** Whether the build server supports the `buildTarget/prepare` request */
2929
prepareProvider?: bool;
3030

3131
/** Whether the server implements the `textDocument/sourceKitOptions` request. */
32-
sourceKitOptionsProvider?: bool;
32+
sourceKitOp tionsProvider?: bool;
3333

3434
/** The files to watch for changes.
3535
* Changes to these files are sent to the BSP server using `workspace/didChangeWatchedFiles`.
@@ -46,37 +46,6 @@ If `data` contains a string value for the `workDoneProgressTitle` key, then the
4646

4747
`changes` can be `null` to indicate that all targets have changed.
4848

49-
## `buildTarget/outputPaths`
50-
51-
For all the source files in this target, the output paths that are used during indexing, ie. the `-index-unit-output-path` for the file, if it is specified in the compiler arguments or the file that is passed as `-o`, if `-index-unit-output-path` is not specified.
52-
53-
This allows SourceKit-LSP to remove index entries for source files that are removed from a target but remain present on disk.
54-
55-
The server communicates during the initialize handshake whether this method is supported or not by setting `outputPathsProvider: true` in `SourceKitInitializeBuildResponseData`.
56-
57-
- method: `buildTarget/outputPaths`
58-
- params: `OutputPathsParams`
59-
- result: `OutputPathsResult`
60-
61-
```ts
62-
export interface BuildTargetOutputPathsParams {
63-
/** A list of build targets to get the output paths for. */
64-
targets: BuildTargetIdentifier[];
65-
}
66-
67-
export interface BuildTargetOutputPathsItem {
68-
/** The target these output file paths are for. */
69-
target: BuildTargetIdentifier;
70-
71-
/** The output paths for all source files in this target. */
72-
outputPaths: string[];
73-
}
74-
75-
export interface BuildTargetOutputPathsResult {
76-
items: BuildTargetOutputPathsItem[];
77-
}
78-
```
79-
8049
## `buildTarget/prepare`
8150

8251
The prepare build target request is sent from the client to the server to prepare the given list of build targets for editor functionality.
@@ -119,6 +88,18 @@ export interface SourceKitSourceItemData {
11988
* inferring build settings from it. Listing header files in `buildTarget/sources` allows SourceKit-LSP to provide
12089
* semantic functionality for header files if they haven't been included by any main file. **/
12190
isHeader?: bool;
91+
92+
/**
93+
* The output paths that are used during indexing for this file, ie. the `-index-unit-output-path`, if it is specified
94+
* in the compiler arguments or the file that is passed as `-o`, if `-index-unit-output-path` is not specified.
95+
*
96+
* This allows SourceKit-LSP to remove index entries for source files that are removed from a target but remain
97+
* present on disk.
98+
*
99+
* The server communicates during the initialize handshake whether it populates this property by setting
100+
* `outputPathsProvider: true` in `SourceKitInitializeBuildResponseData`.
101+
*/
102+
outputPath?: string;
122103
}
123104
```
124105

Sources/BuildServerProtocol/CMakeLists.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ add_library(BuildServerProtocol STATIC
22
Messages.swift
33

44
Messages/BuildShutdownRequest.swift
5-
Messages/BuildTargetOutputPathsRequest.swift
65
Messages/BuildTargetPrepareRequest.swift
76
Messages/BuildTargetSourcesRequest.swift
87
Messages/InitializeBuildRequest.swift

Sources/BuildServerProtocol/Messages.swift

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ public import LanguageServerProtocol
1414

1515
fileprivate let requestTypes: [_RequestType.Type] = [
1616
BuildShutdownRequest.self,
17-
BuildTargetOutputPathsRequest.self,
1817
BuildTargetPrepareRequest.self,
1918
BuildTargetSourcesRequest.self,
2019
CreateWorkDoneProgressRequest.self,

Sources/BuildServerProtocol/Messages/BuildTargetOutputPathsRequest.swift

Lines changed: 0 additions & 50 deletions
This file was deleted.

Sources/BuildServerProtocol/Messages/BuildTargetSourcesRequest.swift

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,9 +126,20 @@ public struct SourceKitSourceItemData: LSPAnyCodable, Codable {
126126
/// semantic functionality for header files if they haven't been included by any main file.
127127
public var isHeader: Bool?
128128

129-
public init(language: Language? = nil, isHeader: Bool? = nil) {
129+
/// The output paths that are used during indexing for this file, ie. the `-index-unit-output-path`, if it is specified
130+
/// in the compiler arguments or the file that is passed as `-o`, if `-index-unit-output-path` is not specified.
131+
///
132+
/// This allows SourceKit-LSP to remove index entries for source files that are removed from a target but remain
133+
/// present on disk.
134+
///
135+
/// The server communicates during the initialize handshake whether it populates this property by setting
136+
/// `outputPathsProvider: true` in `SourceKitInitializeBuildResponseData`.
137+
public var outputPath: String?
138+
139+
public init(language: Language? = nil, isHeader: Bool? = nil, outputPath: String? = nil) {
130140
self.language = language
131141
self.isHeader = isHeader
142+
self.outputPath = outputPath
132143
}
133144

134145
public init?(fromLSPDictionary dictionary: [String: LanguageServerProtocol.LSPAny]) {
@@ -138,6 +149,9 @@ public struct SourceKitSourceItemData: LSPAnyCodable, Codable {
138149
if case .bool(let isHeader) = dictionary[CodingKeys.isHeader.stringValue] {
139150
self.isHeader = isHeader
140151
}
152+
if case .string(let outputFilePath) = dictionary[CodingKeys.outputPath.stringValue] {
153+
self.outputPath = outputFilePath
154+
}
141155
}
142156

143157
public func encodeToLSPAny() -> LanguageServerProtocol.LSPAny {
@@ -148,6 +162,9 @@ public struct SourceKitSourceItemData: LSPAnyCodable, Codable {
148162
if let isHeader {
149163
result[CodingKeys.isHeader.stringValue] = .bool(isHeader)
150164
}
165+
if let outputPath {
166+
result[CodingKeys.outputPath.stringValue] = .string(outputPath)
167+
}
151168
return .dictionary(result)
152169
}
153170
}

Sources/BuildServerProtocol/Messages/InitializeBuildRequest.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,7 @@ public struct SourceKitInitializeBuildResponseData: LSPAnyCodable, Codable, Send
282282
public var watchers: [FileSystemWatcher]?
283283

284284
@available(*, deprecated, message: "Use initializer with alphabetical order of parameters")
285+
@_disfavoredOverload
285286
public init(
286287
indexDatabasePath: String? = nil,
287288
indexStorePath: String? = nil,

Sources/BuildSystemIntegration/BuildSystemManager.swift

Lines changed: 2 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -363,8 +363,6 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
363363

364364
private var cachedTargetSources = RequestCache<BuildTargetSourcesRequest>()
365365

366-
private var cachedTargetOutputPaths = RequestCache<BuildTargetOutputPathsRequest>()
367-
368366
/// `SourceFilesAndDirectories` is a global property that only gets reset when the build targets change and thus
369367
/// has no real key.
370368
private struct SourceFilesAndDirectoriesKey: Hashable {}
@@ -609,13 +607,6 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
609607
}
610608
return !updatedTargets.intersection(cacheKey.targets).isEmpty
611609
}
612-
self.cachedTargetOutputPaths.clear(isolation: self) { cacheKey in
613-
guard let updatedTargets else {
614-
// All targets might have changed
615-
return true
616-
}
617-
return !updatedTargets.isDisjoint(with: cacheKey.targets)
618-
}
619610
self.cachedSourceFilesAndDirectories.clearAll(isolation: self)
620611

621612
await delegate?.buildTargetsChanged(notification.changes)
@@ -1152,7 +1143,7 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
11521143

11531144
/// Return the output paths for all source files known to the build server.
11541145
///
1155-
/// See `BuildTargetOutputPathsRequest` for details.
1146+
/// See `SourceKitSourceItemData.outputFilePath` for details.
11561147
package func outputPathInAllTargets() async throws -> [String] {
11571148
return try await outputPaths(in: Set(buildTargets().map(\.key)))
11581149
}
@@ -1161,29 +1152,7 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
11611152
///
11621153
/// See `BuildTargetOutputPathsRequest` for details.
11631154
package func outputPaths(in targets: Set<BuildTargetIdentifier>) async throws -> [String] {
1164-
guard let buildSystemAdapter = await buildSystemAdapterAfterInitialized, !targets.isEmpty else {
1165-
return []
1166-
}
1167-
1168-
let request = BuildTargetOutputPathsRequest(targets: targets.sorted { $0.uri.stringValue < $1.uri.stringValue })
1169-
1170-
// If we have a cached request for a superset of the targets, serve the result from that cache entry.
1171-
let fromSuperset = await orLog("Getting output paths from superset request") {
1172-
try await cachedTargetOutputPaths.getDerived(
1173-
isolation: self,
1174-
request,
1175-
canReuseKey: { targets.isSubset(of: $0.targets) },
1176-
transform: { BuildTargetOutputPathsResponse(items: $0.items.filter { targets.contains($0.target) }) }
1177-
)
1178-
}
1179-
if let fromSuperset {
1180-
return fromSuperset.items.flatMap(\.outputPaths)
1181-
}
1182-
1183-
let response = try await cachedTargetOutputPaths.get(request, isolation: self) { request in
1184-
try await buildSystemAdapter.send(request)
1185-
}
1186-
return response.items.flatMap(\.outputPaths)
1155+
return try await sourceFiles(in: targets).flatMap(\.sources).compactMap(\.sourceKitData?.outputPath)
11871156
}
11881157

11891158
/// Returns all source files in the project.

Sources/BuildSystemIntegration/BuildSystemMessageDependencyTracker.swift

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,6 @@ package enum BuildSystemMessageDependencyTracker: QueueBasedMessageHandlerDepend
8282
switch request {
8383
case is BuildShutdownRequest:
8484
self = .stateChange
85-
case is BuildTargetOutputPathsRequest:
86-
self = .stateRead
8785
case is BuildTargetPrepareRequest:
8886
self = .stateRead
8987
case is BuildTargetSourcesRequest:

Sources/BuildSystemIntegration/BuiltInBuildSystem.swift

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,8 @@ package protocol BuiltInBuildSystem: AnyObject, Sendable {
3636
var indexDatabasePath: URL? { get async }
3737

3838
/// Whether the build system is capable of preparing a target for indexing and determining the output paths for the
39-
/// target, ie. whether the `prepare` and `buildTargetOutputPaths` methods have been implemented.
39+
/// target, ie. whether the `prepare` method has been implemented and this build server populates the `outputPath`
40+
/// property in the `buildTarget/sources` request.
4041
var supportsPreparationAndOutputPaths: Bool { get }
4142

4243
/// Returns all targets in the build system
@@ -45,9 +46,6 @@ package protocol BuiltInBuildSystem: AnyObject, Sendable {
4546
/// Returns all the source files in the given targets
4647
func buildTargetSources(request: BuildTargetSourcesRequest) async throws -> BuildTargetSourcesResponse
4748

48-
/// Returns all the output paths for the source files in the given targets.
49-
func buildTargetOutputPaths(request: BuildTargetOutputPathsRequest) async throws -> BuildTargetOutputPathsResponse
50-
5149
/// Called when files in the project change.
5250
func didChangeWatchedFiles(notification: OnWatchedFilesDidChangeNotification) async
5351

Sources/BuildSystemIntegration/BuiltInBuildSystemAdapter.swift

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,8 +125,6 @@ actor BuiltInBuildSystemAdapter: QueueBasedMessageHandler {
125125
switch request {
126126
case let request as RequestAndReply<BuildShutdownRequest>:
127127
await request.reply { VoidResponse() }
128-
case let request as RequestAndReply<BuildTargetOutputPathsRequest>:
129-
await request.reply { try await underlyingBuildSystem.buildTargetOutputPaths(request: request.params) }
130128
case let request as RequestAndReply<BuildTargetPrepareRequest>:
131129
await request.reply { try await underlyingBuildSystem.prepare(request: request.params) }
132130
case let request as RequestAndReply<BuildTargetSourcesRequest>:

Sources/BuildSystemIntegration/FixedCompilationDatabaseBuildSystem.swift

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -115,12 +115,6 @@ package actor FixedCompilationDatabaseBuildSystem: BuiltInBuildSystem {
115115
throw ResponseError.methodNotFound(BuildTargetPrepareRequest.method)
116116
}
117117

118-
package func buildTargetOutputPaths(
119-
request: BuildTargetOutputPathsRequest
120-
) async throws -> BuildTargetOutputPathsResponse {
121-
throw ResponseError.methodNotFound(BuildTargetOutputPathsRequest.method)
122-
}
123-
124118
package func sourceKitOptions(
125119
request: TextDocumentSourceKitOptionsRequest
126120
) async throws -> TextDocumentSourceKitOptionsResponse? {

Sources/BuildSystemIntegration/JSONCompilationDatabaseBuildSystem.swift

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -141,12 +141,6 @@ package actor JSONCompilationDatabaseBuildSystem: BuiltInBuildSystem {
141141
throw ResponseError.methodNotFound(BuildTargetPrepareRequest.method)
142142
}
143143

144-
package func buildTargetOutputPaths(
145-
request: BuildTargetOutputPathsRequest
146-
) async throws -> BuildTargetOutputPathsResponse {
147-
throw ResponseError.methodNotFound(BuildTargetOutputPathsRequest.method)
148-
}
149-
150144
package func sourceKitOptions(
151145
request: TextDocumentSourceKitOptionsRequest
152146
) async throws -> TextDocumentSourceKitOptionsResponse? {

Sources/BuildSystemIntegration/LegacyBuildServerBuildSystem.swift

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -172,12 +172,6 @@ actor LegacyBuildServerBuildSystem: MessageHandler, BuiltInBuildSystem {
172172
throw ResponseError.methodNotFound(BuildTargetPrepareRequest.method)
173173
}
174174

175-
package func buildTargetOutputPaths(
176-
request: BuildTargetOutputPathsRequest
177-
) async throws -> BuildTargetOutputPathsResponse {
178-
throw ResponseError.methodNotFound(BuildTargetOutputPathsRequest.method)
179-
}
180-
181175
package func sourceKitOptions(
182176
request: TextDocumentSourceKitOptionsRequest
183177
) async throws -> TextDocumentSourceKitOptionsResponse? {

0 commit comments

Comments
 (0)