Skip to content

Commit eacbfeb

Browse files
committed
Remove build test suffix requirement
This removes the requirement to suffix `*_build_test` targets at all. They are considered top-level targets now and used to collect the platform information similarly to other rules like `ios_application`
1 parent 47aa1f2 commit eacbfeb

18 files changed

+90
-88
lines changed

.bazelignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
.build
2+
.git
3+
Example

Example/HelloWorld/BUILD

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,4 @@ setup_sourcekit_bsp(
229229
"//HelloWorld:HelloWorldMacTests",
230230
"//HelloWorld:HelloWorldMacCLIApp",
231231
],
232-
build_test_suffix = "_(PLAT)_skbsp",
233-
build_test_platform_placeholder = "(PLAT)",
234-
index_build_batch_size = 10,
235232
)

README.md

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,8 @@
3131

3232
- Make sure your Bazel project is using compatible versions of all iOS-related Bazel rulesets (available on each release's description) and is configured to generate Swift/Obj-C indexing data and debug symbols, either by default or under a specific config.
3333
- Detailed information around configuring Bazel flags is currently WIP, but you can currently check out the [example project](./Example) for an example.
34-
- Make sure all libraries that you'd like to use the BSP for have accompanying `(platform)_build_test` rules that directly targets them and have a predictable suffix that includes the platform name. Example naming scheme: `(lib_name)_{ios,watchos,tvos,macos,visionos}_skbsp`
35-
- This is because Bazel is currently missing a couple of important features we need in order to make this work in a clean way. This requirement is thus only temporary and you can expect it to be removed in the future as we evolve the tool and those missing features are introduced.
36-
- Keep in mind that our current focus are iOS targets, so as of writing your mileage may vary when it comes to other Apple platforms.
34+
- Make sure all libraries that you'd like to use the BSP for have accompanying top-level targets (e.g. `ios_application`, `ios_build_test`, etc.) that directly depends on them.
35+
- Keep in mind that our current focus are iOS targets, so as of writing your mileage may vary when it comes to other Apple platforms, file an issue if you run into any problems.
3736
- Download and install [the official Swift extension](https://marketplace.visualstudio.com/items?itemName=swiftlang.swift-vscode) for Cursor / VSCode.
3837
- On Cursor / VSCode, open a workspace containing the repository in question.
3938
- On the settings page for the Swift extension, enable `SourceKit-LSP: Background Indexing` at the **workspace level**. It **has** to be workspace settings; this specific setting is not supported at the folder level.
@@ -72,7 +71,7 @@ This will result in a `.bsp/skbsp.json` file being added to your workspace. User
7271

7372
#### After Integrating
7473

75-
- Reload your workspace (`Cmd+Shift+P -> Reload Window`)
74+
- Reload your workspace (`Cmd+Shift+P -> Reload Window`) or restart the language server (`Cmd+Shift+P -> Swift: Restart LSP Server`)
7675

7776
After following these steps, the `SourceKit Language Server` output tab (_Cmd+Shift+U_) should show up when opening Swift or Obj-C files, and indexing-related actions will start popping up at the bottom of the IDE after a while alongside a new `SourceKit-LSP: Indexing` output tab when working with those files.
7877

Sources/SourceKitBazelBSP/RequestHandlers/BuildTargets/BazelTargetStore.swift

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ protocol BazelTargetStore: AnyObject {
4343
// including the top-level parent that the target is built against.
4444
struct BazelTargetPlatformInfo {
4545
let label: String
46-
let buildTestLabel: String
4746
let topLevelParentLabel: String
4847
let topLevelParentRuleType: TopLevelRuleType
4948

@@ -96,6 +95,15 @@ final class BazelTargetStoreImpl: BazelTargetStore {
9695
// These are collected from the top-level targets that depend on them.
9796
static let supportedKinds: Set<String> = ["source file", "swift_library", "objc_library"]
9897

98+
// The mnemonics representing compilation acitons
99+
static let compileMnemonics: Set<String> = ["SwiftCompile", "ObjcCompile", "CppCompile"]
100+
101+
// The mnemonics representing top-level rule actions
102+
// - `BundleTreeApp` for finding bundling rules like `ios_unit_test`, `ios_application`
103+
// - `SignBinary` for finding macOS CLI app rules like `macos_command_line_application`
104+
// - `TestRunner` for finding build test rules like `ios_build_test`
105+
static let topLevelMnemonics: Set<String> = ["BundleTreeApp", "SignBinary", "TestRunner"]
106+
99107
// Users of BazelTargetStore are expected to acquire this lock before reading or writing any of the internal state.
100108
// This is to prevent race conditions between concurrent requests. It's easier to have each request handle critical sections
101109
// on their own instead of trying to solve it entirely within this class.
@@ -172,16 +180,16 @@ final class BazelTargetStoreImpl: BazelTargetStore {
172180
// FIXME: When a target can compile to multiple platforms, the way Xcode handles it is by selecting
173181
// the one matching your selected simulator in the IDE. We don't have any sort of special IDE integration
174182
// at the moment, so for now we just select the first parent.
175-
let parentToUse = parents[0]
176-
let rule = try topLevelRuleType(forBazelLabel: parentToUse)
177-
let baseSuffix = initializedConfig.baseConfig.buildTestSuffix
178-
let platformPlaceholder = initializedConfig.baseConfig.buildTestPlatformPlaceholder
179-
let platformBuildSuffix = baseSuffix.replacingOccurrences(of: platformPlaceholder, with: rule.platform)
183+
let topLevelParent = parents[0]
184+
if parents.count > 1 {
185+
logger.warning("Target \(uri.description, privacy: .public) has multiple top-level parents will use first one: \(topLevelParent, privacy: .public)")
186+
}
187+
188+
let topLevelRuleType = try topLevelRuleType(forBazelLabel: topLevelParent)
180189
return BazelTargetPlatformInfo(
181190
label: bazelLabel,
182-
buildTestLabel: "\(bazelLabel)\(platformBuildSuffix)",
183-
topLevelParentLabel: parentToUse,
184-
topLevelParentRuleType: rule,
191+
topLevelParentLabel: topLevelParent,
192+
topLevelParentRuleType: topLevelRuleType
185193
)
186194
}
187195

@@ -267,13 +275,13 @@ final class BazelTargetStoreImpl: BazelTargetStore {
267275

268276
// Now, run a broad aquery against all top-level targets
269277
// to get the compiler arguments for all targets we're interested in.
270-
// We pass BundleTreeApp and SignBinary as a trick to gain access to the parent's configuration id.
278+
// We pass top-level mnemonics in addition to compile ones as a method to gain access to the parent's configuration id.
271279
// We can then use this to locate the exact variant of the target we are looking for.
272280
// BundleTreeApp is used by most rule types, while SignBinary is for macOS CLI apps specifically.
273281
targetsAqueryResult = try bazelTargetAquerier.aquery(
274282
targets: topLevelLabelToRuleMap.keys.map { $0 },
275283
config: initializedConfig,
276-
mnemonics: ["SwiftCompile", "ObjcCompile", "CppCompile", "BundleTreeApp", "SignBinary"],
284+
mnemonics: Self.compileMnemonics.union(Self.topLevelMnemonics),
277285
additionalFlags: [
278286
"--noinclude_artifacts",
279287
"--noinclude_aspects",

Sources/SourceKitBazelBSP/RequestHandlers/BuildTargets/TopLevelRuleType.swift

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,38 +24,48 @@ public enum TopLevelRuleType: String, CaseIterable, ExpressibleByArgument {
2424
case iosApplication = "ios_application"
2525
case iosUnitTest = "ios_unit_test"
2626
case iosUiTest = "ios_ui_test"
27+
case iosBuildTest = "ios_build_test"
2728
case watchosApplication = "watchos_application"
2829
case watchosUnitTest = "watchos_unit_test"
2930
case watchosUiTest = "watchos_ui_test"
31+
case watchosBuildTest = "watchos_build_test"
3032
case macosApplication = "macos_application"
3133
case macosCommandLineApplication = "macos_command_line_application"
3234
case macosUnitTest = "macos_unit_test"
3335
case macosUiTest = "macos_ui_test"
36+
case macosBuildTest = "macos_build_test"
3437
case tvosApplication = "tvos_application"
3538
case tvosUnitTest = "tvos_unit_test"
3639
case tvosUiTest = "tvos_ui_test"
40+
case tvosBuildTest = "tvos_build_test"
3741
case visionosApplication = "visionos_application"
3842
case visionosUnitTest = "visionos_unit_test"
3943
case visionosUiTest = "visionos_ui_test"
44+
case visionosBuildTest = "visionos_build_test"
4045

4146
var platform: String {
4247
switch self {
4348
case .iosApplication: return "ios"
4449
case .iosUnitTest: return "ios"
4550
case .iosUiTest: return "ios"
51+
case .iosBuildTest: return "ios"
4652
case .watchosApplication: return "watchos"
4753
case .watchosUnitTest: return "watchos"
4854
case .watchosUiTest: return "watchos"
55+
case .watchosBuildTest: return "watchos"
4956
case .macosApplication: return "macos"
5057
case .macosCommandLineApplication: return "macos"
5158
case .macosUnitTest: return "macos"
5259
case .macosUiTest: return "macos"
60+
case .macosBuildTest: return "macos"
5361
case .tvosApplication: return "tvos"
5462
case .tvosUnitTest: return "tvos"
5563
case .tvosUiTest: return "tvos"
64+
case .tvosBuildTest: return "tvos"
5665
case .visionosApplication: return "visionos"
5766
case .visionosUnitTest: return "visionos"
5867
case .visionosUiTest: return "visionos"
68+
case .visionosBuildTest: return "visionos"
5969
}
6070
}
6171

@@ -75,19 +85,24 @@ public enum TopLevelRuleType: String, CaseIterable, ExpressibleByArgument {
7585
case .iosApplication: return "iphonesimulator"
7686
case .iosUnitTest: return "iphonesimulator"
7787
case .iosUiTest: return "iphonesimulator"
88+
case .iosBuildTest: return "iphonesimulator"
7889
case .watchosApplication: return "watchsimulator"
7990
case .watchosUnitTest: return "watchsimulator"
8091
case .watchosUiTest: return "watchsimulator"
92+
case .watchosBuildTest: return "watchsimulator"
8193
case .macosApplication: return "macosx"
8294
case .macosCommandLineApplication: return "macosx"
8395
case .macosUnitTest: return "macosx"
8496
case .macosUiTest: return "macosx"
97+
case .macosBuildTest: return "macosx"
8598
case .tvosApplication: return "appletvsimulator"
8699
case .tvosUnitTest: return "appletvsimulator"
87100
case .tvosUiTest: return "appletvsimulator"
101+
case .tvosBuildTest: return "appletvsimulator"
88102
case .visionosApplication: return "xrsimulator"
89103
case .visionosUnitTest: return "xrsimulator"
90104
case .visionosUiTest: return "xrsimulator"
105+
case .visionosBuildTest: return "xrsimulator"
91106
}
92107
}
93108
}

Sources/SourceKitBazelBSP/RequestHandlers/PrepareHandler.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ final class PrepareHandler {
8686
)
8787
didStartTask = true
8888
nonisolated(unsafe) let reply = reply
89-
try build(bazelLabels: platformInfo.map { $0.buildTestLabel }, id: id) { [connection] error in
89+
try build(bazelLabels: platformInfo.map { $0.topLevelParentLabel }, id: id) { [connection] error in
9090
if let error = error {
9191
connection?.finishTask(id: taskId, status: .error)
9292
reply(.failure(error))

Sources/SourceKitBazelBSP/RequestHandlers/SKOptions/BazelTargetCompilerArgsExtractor.swift

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ enum BazelTargetCompilerArgsExtractorError: Error, LocalizedError {
3232
case sdkRootNotFound(String)
3333
case parentTargetNotFound(String, String)
3434
case parentActionNotFound(String, UInt32)
35-
case multipleParentActions(String, String)
35+
case multipleParentActions(String, String, [String])
3636
case targetNotFound(String)
3737
case actionNotFound(String, UInt32)
3838
case multipleTargetActions(String, UInt32)
@@ -49,8 +49,8 @@ enum BazelTargetCompilerArgsExtractorError: Error, LocalizedError {
4949
return "Parent target \(parent) of \(target) was not found in the aquery output."
5050
case .parentActionNotFound(let parent, let id):
5151
return "Parent action \(id) for parent \(parent) not found in the aquery output."
52-
case .multipleParentActions(let parent, let target):
53-
return "Multiple parent actions found for parent \(parent) of \(target). This is unexpected."
52+
case .multipleParentActions(let parent, let target, let mnemonics):
53+
return "Multiple parent actions found for parent \(parent) of \(target) with mnemonics: \(mnemonics)"
5454
case .targetNotFound(let target): return "Target \(target) not found in the aquery output."
5555
case .actionNotFound(let target, let id):
5656
return "Action \(id) for target \(target) not found in the aquery output."
@@ -131,6 +131,7 @@ final class BazelTargetCompilerArgsExtractor {
131131
fromAquery aquery: AqueryResult,
132132
forTarget platformInfo: BazelTargetPlatformInfo,
133133
withStrategy strategy: ParsingStrategy,
134+
compileMnemonics: Set<String>
134135
) throws -> [String] {
135136
// Ignore Obj-C header requests as these don't compile.
136137
if case .cHeader = strategy {
@@ -169,7 +170,8 @@ final class BazelTargetCompilerArgsExtractor {
169170
forTarget: platformInfo,
170171
fromAquery: aquery,
171172
basedOn: parentAction,
172-
strategy: strategy
173+
strategy: strategy,
174+
compileMnemonics: compileMnemonics
173175
)
174176

175177
// Then, extract the compiler arguments for the target file from the resulting aquery.
@@ -229,7 +231,11 @@ final class BazelTargetCompilerArgsExtractor {
229231
throw BazelTargetCompilerArgsExtractorError.parentActionNotFound(effectiveParentLabel, parentTarget.id)
230232
}
231233
guard parentActions.count == 1 else {
232-
throw BazelTargetCompilerArgsExtractorError.multipleParentActions(effectiveParentLabel, bazelTarget)
234+
throw BazelTargetCompilerArgsExtractorError.multipleParentActions(
235+
effectiveParentLabel,
236+
bazelTarget,
237+
parentActions.map(\.mnemonic)
238+
)
233239
}
234240
let parentAction = parentActions[0]
235241
let parentConfigurationId = parentAction.configurationID
@@ -240,7 +246,8 @@ final class BazelTargetCompilerArgsExtractor {
240246
forTarget platformInfo: BazelTargetPlatformInfo,
241247
fromAquery aquery: AqueryResult,
242248
basedOn parentAction: ParentData,
243-
strategy: ParsingStrategy
249+
strategy: ParsingStrategy,
250+
compileMnemonics: Set<String>,
244251
) throws -> Analysis_Action {
245252
let bazelTarget = platformInfo.label
246253
guard let target = aquery.targets[bazelTarget] else {
@@ -249,12 +256,15 @@ final class BazelTargetCompilerArgsExtractor {
249256
guard let actions = aquery.actions[target.id] else {
250257
throw BazelTargetCompilerArgsExtractorError.actionNotFound(bazelTarget, target.id)
251258
}
259+
252260
// `actions` will contain all different configurations for the target we're processing.
253261
// We need to now locate the one that matches the configuration from the parent action
254262
// we're using as a reference.
255263
var candidateActions = actions.filter {
256264
$0.configurationID == parentAction.configurationID
257265
}
266+
let compileActions = actions.filter { compileMnemonics.contains($0.mnemonic) }
267+
258268
let contentBeingQueried: String
259269
switch strategy {
260270
case .swiftModule, .cHeader:
@@ -272,12 +282,30 @@ final class BazelTargetCompilerArgsExtractor {
272282
return false
273283
}
274284
}
285+
275286
guard candidateActions.count > 0 else {
276-
throw BazelTargetCompilerArgsExtractorError.relevantTargetActionsNotFound(contentBeingQueried)
287+
guard compileActions.count > 0 else {
288+
logger.fault(
289+
"No actions for: \(contentBeingQueried) with parent id: \(parentAction.configurationID, privacy: .public) or compile mnemonic found: \(actions.map(\.mnemonic).joined(separator: " "), privacy: .public)"
290+
)
291+
throw BazelTargetCompilerArgsExtractorError.relevantTargetActionsNotFound(contentBeingQueried)
292+
}
293+
guard compileActions.count == 1 else {
294+
logger.fault(
295+
"Ambigious compile actions found for: \(contentBeingQueried): \(compileActions.map(\.mnemonic).joined(separator: " "), privacy: .public)"
296+
)
297+
throw BazelTargetCompilerArgsExtractorError.multipleTargetActions(contentBeingQueried, target.id)
298+
}
299+
logger.debug(
300+
"Did not determine matching parent action for: \(contentBeingQueried, privacy: .public), falling back to compile action"
301+
)
302+
return compileActions[0]
277303
}
304+
278305
guard candidateActions.count == 1 else {
279306
throw BazelTargetCompilerArgsExtractorError.multipleTargetActions(contentBeingQueried, target.id)
280307
}
308+
281309
return candidateActions[0]
282310
}
283311

Sources/SourceKitBazelBSP/RequestHandlers/SKOptions/SKOptionsHandler.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ final class SKOptionsHandler {
8989
fromAquery: aqueryResult,
9090
forTarget: platformInfo,
9191
withStrategy: strategy,
92+
compileMnemonics: BazelTargetStoreImpl.compileMnemonics
9293
)
9394

9495
// If no compiler arguments are found, return nil to avoid sourcekit indexing with no input files

Sources/SourceKitBazelBSP/Server/BaseServerConfig.swift

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,6 @@ package struct BaseServerConfig: Equatable {
2828
let bazelWrapper: String
2929
let targets: [String]
3030
let indexFlags: [String]
31-
let buildTestSuffix: String
32-
let buildTestPlatformPlaceholder: String
3331
let filesToWatch: String?
3432
let useSeparateOutputBaseForAquery: Bool
3533
let indexBuildBatchSize: Int?
@@ -38,17 +36,13 @@ package struct BaseServerConfig: Equatable {
3836
bazelWrapper: String,
3937
targets: [String],
4038
indexFlags: [String],
41-
buildTestSuffix: String,
42-
buildTestPlatformPlaceholder: String,
4339
filesToWatch: String?,
4440
useSeparateOutputBaseForAquery: Bool = false,
4541
indexBuildBatchSize: Int? = nil
4642
) {
4743
self.bazelWrapper = bazelWrapper
4844
self.targets = targets
4945
self.indexFlags = indexFlags
50-
self.buildTestSuffix = buildTestSuffix
51-
self.buildTestPlatformPlaceholder = buildTestPlatformPlaceholder
5246
self.filesToWatch = filesToWatch
5347
self.useSeparateOutputBaseForAquery = useSeparateOutputBaseForAquery
5448
self.indexBuildBatchSize = indexBuildBatchSize

Sources/SourceKitBazelBSP/Server/LSPConnection.swift

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,10 @@ package protocol LSPTaskLogger: AnyObject {
3030
/// Extends the original sourcekit-lsp `Connection` type to include JSONRPCConnection's start method
3131
/// and task logging utilities.
3232
package protocol LSPConnection: Connection, LSPTaskLogger, AnyObject {
33-
func start(receiveHandler: MessageHandler, closeHandler: @escaping @Sendable () async -> Void)
33+
func start(
34+
receiveHandler: MessageHandler,
35+
closeHandler: @escaping @Sendable () async -> Void
36+
)
3437
}
3538

3639
extension JSONRPCConnection: LSPConnection {

0 commit comments

Comments
 (0)