Skip to content

Commit f2d236a

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 23be92b commit f2d236a

14 files changed

+57
-179
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
@@ -18,7 +18,6 @@ import LanguageServerProtocol
1818

1919
fileprivate let requestTypes: [_RequestType.Type] = [
2020
BuildShutdownRequest.self,
21-
BuildTargetOutputPathsRequest.self,
2221
BuildTargetPrepareRequest.self,
2322
BuildTargetSourcesRequest.self,
2423
CreateWorkDoneProgressRequest.self,

Sources/BuildServerProtocol/Messages/BuildTargetOutputPathsRequest.swift

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

Sources/BuildServerProtocol/Messages/BuildTargetSourcesRequest.swift

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

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

138149
public init?(fromLSPDictionary dictionary: [String: LanguageServerProtocol.LSPAny]) {
@@ -142,6 +153,9 @@ public struct SourceKitSourceItemData: LSPAnyCodable, Codable {
142153
if case .bool(let isHeader) = dictionary[CodingKeys.isHeader.stringValue] {
143154
self.isHeader = isHeader
144155
}
156+
if case .string(let outputFilePath) = dictionary[CodingKeys.outputPath.stringValue] {
157+
self.outputPath = outputFilePath
158+
}
145159
}
146160

147161
public func encodeToLSPAny() -> LanguageServerProtocol.LSPAny {
@@ -152,6 +166,9 @@ public struct SourceKitSourceItemData: LSPAnyCodable, Codable {
152166
if let isHeader {
153167
result[CodingKeys.isHeader.stringValue] = .bool(isHeader)
154168
}
169+
if let outputPath {
170+
result[CodingKeys.outputPath.stringValue] = .string(outputPath)
171+
}
155172
return .dictionary(result)
156173
}
157174
}

Sources/BuildServerProtocol/Messages/InitializeBuildRequest.swift

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

288288
@available(*, deprecated, message: "Use initializer with alphabetical order of parameters")
289+
@_disfavoredOverload
289290
public init(
290291
indexDatabasePath: String? = nil,
291292
indexStorePath: String? = nil,

Sources/BuildSystemIntegration/BuildSystemManager.swift

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

380380
private var cachedTargetSources = RequestCache<BuildTargetSourcesRequest>()
381381

382-
private var cachedTargetOutputPaths = RequestCache<BuildTargetOutputPathsRequest>()
383-
384382
/// `SourceFilesAndDirectories` is a global property that only gets reset when the build targets change and thus
385383
/// has no real key.
386384
private struct SourceFilesAndDirectoriesKey: Hashable {}
@@ -625,13 +623,6 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
625623
}
626624
return !updatedTargets.intersection(cacheKey.targets).isEmpty
627625
}
628-
self.cachedTargetOutputPaths.clear(isolation: self) { cacheKey in
629-
guard let updatedTargets else {
630-
// All targets might have changed
631-
return true
632-
}
633-
return !updatedTargets.isDisjoint(with: cacheKey.targets)
634-
}
635626
self.cachedSourceFilesAndDirectories.clearAll(isolation: self)
636627

637628
await delegate?.buildTargetsChanged(notification.changes)
@@ -1168,7 +1159,7 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
11681159

11691160
/// Return the output paths for all source files known to the build server.
11701161
///
1171-
/// See `BuildTargetOutputPathsRequest` for details.
1162+
/// See `SourceKitSourceItemData.outputFilePath` for details.
11721163
package func outputPathInAllTargets() async throws -> [String] {
11731164
return try await outputPaths(in: Set(buildTargets().map(\.key)))
11741165
}
@@ -1177,29 +1168,7 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
11771168
///
11781169
/// See `BuildTargetOutputPathsRequest` for details.
11791170
package func outputPaths(in targets: Set<BuildTargetIdentifier>) async throws -> [String] {
1180-
guard let buildSystemAdapter = await buildSystemAdapterAfterInitialized, !targets.isEmpty else {
1181-
return []
1182-
}
1183-
1184-
let request = BuildTargetOutputPathsRequest(targets: targets.sorted { $0.uri.stringValue < $1.uri.stringValue })
1185-
1186-
// If we have a cached request for a superset of the targets, serve the result from that cache entry.
1187-
let fromSuperset = await orLog("Getting output paths from superset request") {
1188-
try await cachedTargetOutputPaths.getDerived(
1189-
isolation: self,
1190-
request,
1191-
canReuseKey: { targets.isSubset(of: $0.targets) },
1192-
transform: { BuildTargetOutputPathsResponse(items: $0.items.filter { targets.contains($0.target) }) }
1193-
)
1194-
}
1195-
if let fromSuperset {
1196-
return fromSuperset.items.flatMap(\.outputPaths)
1197-
}
1198-
1199-
let response = try await cachedTargetOutputPaths.get(request, isolation: self) { request in
1200-
try await buildSystemAdapter.send(request)
1201-
}
1202-
return response.items.flatMap(\.outputPaths)
1171+
return try await sourceFiles(in: targets).flatMap(\.sources).compactMap(\.sourceKitData?.outputPath)
12031172
}
12041173

12051174
/// 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
@@ -90,8 +90,6 @@ package enum BuildSystemMessageDependencyTracker: QueueBasedMessageHandlerDepend
9090
switch request {
9191
case is BuildShutdownRequest:
9292
self = .stateChange
93-
case is BuildTargetOutputPathsRequest:
94-
self = .stateRead
9593
case is BuildTargetPrepareRequest:
9694
self = .stateRead
9795
case is BuildTargetSourcesRequest:

Sources/BuildSystemIntegration/BuiltInBuildSystem.swift

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

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

5152
/// Returns all targets in the build system
@@ -54,9 +55,6 @@ package protocol BuiltInBuildSystem: AnyObject, Sendable {
5455
/// Returns all the source files in the given targets
5556
func buildTargetSources(request: BuildTargetSourcesRequest) async throws -> BuildTargetSourcesResponse
5657

57-
/// Returns all the output paths for the source files in the given targets.
58-
func buildTargetOutputPaths(request: BuildTargetOutputPathsRequest) async throws -> BuildTargetOutputPathsResponse
59-
6058
/// Called when files in the project change.
6159
func didChangeWatchedFiles(notification: OnWatchedFilesDidChangeNotification) async
6260

Sources/BuildSystemIntegration/BuiltInBuildSystemAdapter.swift

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,6 @@ actor BuiltInBuildSystemAdapter: QueueBasedMessageHandler {
131131
switch request {
132132
case let request as RequestAndReply<BuildShutdownRequest>:
133133
await request.reply { VoidResponse() }
134-
case let request as RequestAndReply<BuildTargetOutputPathsRequest>:
135-
await request.reply { try await underlyingBuildSystem.buildTargetOutputPaths(request: request.params) }
136134
case let request as RequestAndReply<BuildTargetPrepareRequest>:
137135
await request.reply { try await underlyingBuildSystem.prepare(request: request.params) }
138136
case let request as RequestAndReply<BuildTargetSourcesRequest>:

Sources/BuildSystemIntegration/FixedCompilationDatabaseBuildSystem.swift

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

125-
package func buildTargetOutputPaths(
126-
request: BuildTargetOutputPathsRequest
127-
) async throws -> BuildTargetOutputPathsResponse {
128-
throw ResponseError.methodNotFound(BuildTargetOutputPathsRequest.method)
129-
}
130-
131125
package func sourceKitOptions(
132126
request: TextDocumentSourceKitOptionsRequest
133127
) async throws -> TextDocumentSourceKitOptionsResponse? {

Sources/BuildSystemIntegration/JSONCompilationDatabaseBuildSystem.swift

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

152-
package func buildTargetOutputPaths(
153-
request: BuildTargetOutputPathsRequest
154-
) async throws -> BuildTargetOutputPathsResponse {
155-
throw ResponseError.methodNotFound(BuildTargetOutputPathsRequest.method)
156-
}
157-
158152
package func sourceKitOptions(
159153
request: TextDocumentSourceKitOptionsRequest
160154
) 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)