Skip to content

Commit 5a29b26

Browse files
authored
Allow the build test suffix to be further customized (#57)
* Allow the build test suffix to be further customized * Allow the placeholder to be modified too * Fix typo, use same Bazel version * Make it optional to not break some environments * Fix warnings * Fix broken bzl file
1 parent 9ab916d commit 5a29b26

File tree

13 files changed

+175
-212
lines changed

13 files changed

+175
-212
lines changed

.bazelversion

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
7.4.1
1+
8.3.1

Example/.bsp/config.json

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,10 @@
2929
"--index-flag",
3030
"config=index_build",
3131
"--files-to-watch",
32-
"HelloWorld/**/*.swift,HelloWorld/**/*.m,HelloWorld/**/*.h"
32+
"HelloWorld/**/*.swift,HelloWorld/**/*.m,HelloWorld/**/*.h",
33+
"--build-test-suffix",
34+
"_(PLAT)_skbsp",
35+
"--build-test-platform-placeholder",
36+
"(PLAT)"
3337
]
3438
}

MODULE.bazel.lock

Lines changed: 104 additions & 174 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
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 the `(lib_name)_{ios,watchos,tvos,macos,visionos}_skbsp` naming scheme.
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`
3535
- 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.
3636
- 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.
3737
- Download and install [the official Swift extension](https://marketplace.visualstudio.com/items?itemName=swiftlang.swift-vscode) for Cursor / VSCode.

Sources/SourceKitBazelBSP/RequestHandlers/BuildTargets/BazelTargetStore.swift

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,8 +129,11 @@ final class BazelTargetStoreImpl: BazelTargetStore {
129129
// at the moment, so for now we just select the first parent.
130130
let parentToUse = parents[0]
131131
let rule = try topLevelRuleType(forBazelLabel: parentToUse)
132+
let baseSuffix = initializedConfig.baseConfig.buildTestSuffix
133+
let platformPlaceholder = initializedConfig.baseConfig.buildTestPlatformPlaceholder
134+
let platformBuildSuffix = baseSuffix.replacingOccurrences(of: platformPlaceholder, with: rule.platform)
132135
return (
133-
"\(bazelLabel)_\(rule.platform)\(initializedConfig.baseConfig.buildTestSuffix)",
136+
"\(bazelLabel)\(platformBuildSuffix)",
134137
rule
135138
)
136139
}

Sources/SourceKitBazelBSP/Server/BaseServerConfig.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ package struct BaseServerConfig: Equatable {
2929
let targets: [String]
3030
let indexFlags: [String]
3131
let buildTestSuffix: String
32+
let buildTestPlatformPlaceholder: String
3233
let filesToWatch: String?
3334
let useSeparateOutputBaseForAquery: Bool
3435

@@ -37,13 +38,15 @@ package struct BaseServerConfig: Equatable {
3738
targets: [String],
3839
indexFlags: [String],
3940
buildTestSuffix: String,
41+
buildTestPlatformPlaceholder: String,
4042
filesToWatch: String?,
4143
useSeparateOutputBaseForAquery: Bool = false
4244
) {
4345
self.bazelWrapper = bazelWrapper
4446
self.targets = targets
4547
self.indexFlags = indexFlags
4648
self.buildTestSuffix = buildTestSuffix
49+
self.buildTestPlatformPlaceholder = buildTestPlatformPlaceholder
4750
self.filesToWatch = filesToWatch
4851
self.useSeparateOutputBaseForAquery = useSeparateOutputBaseForAquery
4952
}

Sources/sourcekit-bazel-bsp/Commands/Serve.swift

Lines changed: 27 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ import SourceKitBazelBSP
2525
private let logger = makeFileLevelBSPLogger()
2626

2727
struct Serve: ParsableCommand {
28+
private static let defaultBuildTestPlatformPlaceholder = "(PLAT)"
29+
2830
@Option(help: "The name of the Bazel CLI to invoke (e.g. 'bazelisk')")
2931
var bazelWrapper: String = "bazel"
3032

@@ -44,9 +46,15 @@ struct Serve: ParsableCommand {
4446

4547
@Option(
4648
help:
47-
"The expected suffix for build_test targets."
49+
"The expected suffix format for build_test targets. Use the value of `--build-test-platform-placeholder` as a platform placeholder.",
50+
)
51+
var buildTestSuffix: String = "_\(Self.defaultBuildTestPlatformPlaceholder)_skbsp"
52+
53+
@Option(
54+
help:
55+
"The expected platform placeholder for build_test targets.",
4856
)
49-
var buildTestSuffix: String = "_skbsp"
57+
var buildTestPlatformPlaceholder: String = Self.defaultBuildTestPlatformPlaceholder
5058

5159
// FIXME: This should be enabled by default, but I ran into some weird race condition issues with rules_swift I'm not sure about.
5260
@Flag(
@@ -61,33 +69,33 @@ struct Serve: ParsableCommand {
6169
func run() throws {
6270
logger.info("`serve` invoked, initializing BSP server...")
6371

64-
// If the user provided no specific targets, try to discover them
65-
// in the workspace.
6672
let targets: [String]
67-
do {
68-
targets = try {
69-
if !target.isEmpty {
70-
return target
71-
}
72-
logger.warning(
73-
"No targets specified (--target)! Will now try to discover them. This can cause the BSP to perform poorly if we find too many targets. Prefer using --target explicitly if possible."
74-
)
75-
return try BazelTargetDiscoverer.discoverTargets(
73+
if !target.isEmpty {
74+
targets = target
75+
} else {
76+
// If the user provided no specific targets, try to discover them
77+
// in the workspace.
78+
logger.warning(
79+
"No targets specified (--target)! Will now try to discover them. This can cause the BSP to perform poorly if we find too many targets. Prefer using --target explicitly if possible."
80+
)
81+
do {
82+
targets = try BazelTargetDiscoverer.discoverTargets(
7683
bazelWrapper: bazelWrapper
7784
)
78-
}()
79-
} catch {
80-
logger.error(
81-
"Server startup failed due to target discovery error. Please check your Bazel configuration and try specifying targets explicitly with --target. \(error)"
82-
)
83-
throw error
85+
} catch {
86+
logger.error(
87+
"Failed to initialize server: Could not discover targets. Please check your Bazel configuration or try specifying targets explicitly with `--target` instead. Failure: \(error)"
88+
)
89+
throw error
90+
}
8491
}
8592

8693
let config = BaseServerConfig(
8794
bazelWrapper: bazelWrapper,
8895
targets: targets,
8996
indexFlags: indexFlag.map { "--" + $0 },
9097
buildTestSuffix: buildTestSuffix,
98+
buildTestPlatformPlaceholder: buildTestPlatformPlaceholder,
9199
filesToWatch: filesToWatch,
92100
useSeparateOutputBaseForAquery: separateAqueryOutput
93101
)

Tests/SourceKitBazelBSPTests/BazelTargetCompilerArgsExtractorTests.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,8 @@ struct BazelTargetCompilerArgsExtractorTests {
4040
bazelWrapper: "bazel",
4141
targets: ["//HelloWorld"],
4242
indexFlags: [],
43-
buildTestSuffix: "_skbsp",
43+
buildTestSuffix: "_(PLAT)_skbsp",
44+
buildTestPlatformPlaceholder: "(PLAT)",
4445
filesToWatch: nil
4546
),
4647
rootUri: mockRootUri,

Tests/SourceKitBazelBSPTests/BazelTargetParserTests.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,8 @@ struct BazelTargetParserTests {
3232
bazelWrapper: "bazel",
3333
targets: ["//HelloWorld:HelloWorld"],
3434
indexFlags: [],
35-
buildTestSuffix: "_skbsp",
35+
buildTestSuffix: "_(PLAT)_skbsp",
36+
buildTestPlatformPlaceholder: "(PLAT)",
3637
filesToWatch: nil
3738
)
3839

Tests/SourceKitBazelBSPTests/BazelTargetQuerierTests.swift

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@ struct BazelTargetQuerierTests {
3434
bazelWrapper: "bazelisk",
3535
targets: ["//HelloWorld"],
3636
indexFlags: ["--config=test"],
37-
buildTestSuffix: "_skbsp",
37+
buildTestSuffix: "_(PLAT)_skbsp",
38+
buildTestPlatformPlaceholder: "(PLAT)",
3839
filesToWatch: nil
3940
)
4041

@@ -67,7 +68,8 @@ struct BazelTargetQuerierTests {
6768
bazelWrapper: "bazelisk",
6869
targets: ["//HelloWorld", "//Tests"],
6970
indexFlags: ["--config=test"],
70-
buildTestSuffix: "_skbsp",
71+
buildTestSuffix: "_(PLAT)_skbsp",
72+
buildTestPlatformPlaceholder: "(PLAT)",
7173
filesToWatch: nil
7274
)
7375

@@ -100,7 +102,8 @@ struct BazelTargetQuerierTests {
100102
bazelWrapper: "bazel",
101103
targets: ["//HelloWorld"],
102104
indexFlags: [],
103-
buildTestSuffix: "_skbsp",
105+
buildTestSuffix: "_(PLAT)_skbsp",
106+
buildTestPlatformPlaceholder: "(PLAT)",
104107
filesToWatch: nil
105108
)
106109

@@ -154,7 +157,8 @@ struct BazelTargetQuerierTests {
154157
bazelWrapper: "bazel",
155158
targets: ["//HelloWorld:HelloWorld"],
156159
indexFlags: [],
157-
buildTestSuffix: "_skbsp",
160+
buildTestSuffix: "_(PLAT)_skbsp",
161+
buildTestPlatformPlaceholder: "(PLAT)",
158162
filesToWatch: nil
159163
)
160164

0 commit comments

Comments
 (0)