Add requests that can be used to implement a BSP server based on swift-builds build description #761
Conversation
ahoppen
requested review from
aciidgh,
jakepetroules,
mhrawdon,
mirza-garibovic,
neonichu and
owenv
as code owners
Right now we will never evict the most recent index workspace description from the cache, but it would be nice to have something better. Being able to treat the build description ID as a handle clients can retain makes sense to me |
I think e can put a test for the new API in the SwiftBuildTests target. IndexingInfoTests.indexingInfoDetailed is a good example of an existing test that stands up a test session with a mock project. I think it should be possible to copy that and adapt it to call into the new API you've added here |
…t-builds build description The design of these requests closely resemble the BSP requests required by SourceKit-LSP and should be useful to implement a BSP server based on swift-build.
ahoppen
force-pushed
the
bsp-requests
branch
from
2883621 to
b9e4b1e
Compare
|
@swift-ci Please test |
|
I added a few tests now. Before being able to use this in production, I think we need a way to create build descriptions and keep them in memory without evicting them from the cache but I’d like to make that a follow-up change. |
| if let toolchainID = toolchainIDs(in: configuredTarget, of: buildDescription)?.first { | ||
| toolchain = session.core.toolchainRegistry.lookup(toolchainID)?.path | ||
| if toolchain == nil { | ||
| log("Unable to find path for toolchain with identifier \(toolchainID)", isError: true) |
There was a problem hiding this comment.
@ahoppen We don't conventionally have this level of logging in the product; it seems like debug printing that should've been removed.
| let configuredTargetsByID = Dictionary( | ||
| buildDescription.allConfiguredTargets.map { ($0.guid, $0) } | ||
| ) { lhs, rhs in | ||
| log("Found conflicting targets for the same ID: \(lhs.guid)", isError: true) |
There was a problem hiding this comment.
@ahoppen These type of scenarios should throw an error, not "pick one and log"; it tends to lead to unpredictable and nondeterministic behavior
|
Why is it the right approach to add more low-level requests at the Swift Build layer for a higher level layer to implement the BSP, versus Swift Build itself hosting the BSP server? |
4 participants
The design of these requests closely resemble the BSP requests required by SourceKit-LSP and should be useful to implement a BSP server based on swift-build.
Follow-up task:
BuildDescriptionManagerbut there’s no way to ensure that. I was thinking to add a request to retain or release a build description. As long as the build description has a retain count greater than 0, it won’t be purged from theBuildDescriptionManager. But I’d like to hear feedback about this idea before implementing it.