Thanks a lot @richardh (and @DavidGoldman) for continuing to push on the build system design! I wanted to respond to the design you proposed (PR and spec) holistically, as well as talk about some other build system integration points outside the scope of what you proposed. We don't need to implement all of this right away
Certain parts of your proposed design feel like they fit closely into the Build Server Protocol (BSP) design. Specifically, the discovery and initialization of the server, as well as the list of sources are very close, and I think a small extension to BSP would allow us to handle the output paths as well.
I think it would be premature for us to adopt the BSP wholesale, since it's unclear if we need most of the features. However, I think for these particular API, we should align with the BSP to take advantage of the experience of that community. It also leaves the door open for implementing the whole spec in the future. Concretely, I propose we adopt the following BSP API for our own build system integration:
As a small extension, we could add a request that piggy-backs on the target (and would be updated using buildTarget/didChange notifications).
While the existing BSP extension for Scala uses a target-level query for compiler arguments, I prefer to use a file-level API similar to your proposal. That gives the build system the ability to minimize work for us, and also is a nicer API to work with for the language server.
With respect to the file settings API, I see that you added scheme/destination information to the queries. Argyrios and I discussed this today, and we’d like to propose that the build system be responsible for deciding how to compute file settings based on its own knowledge of the configuration (including scheme and destination) rather than exposing it in every request. We might need to expose scheme/destiantion information to the indexer for other reasons, but if we can keep it out of the file settings API, I think that would be an improvement. What do you think?
Other than removing the scheme/destination from the API, I propose the following names
- One-off request for settings for a given URL.
- The name is based on buildTarget/scalacOptions, but at the file level.
- textDocument/registerForSourceKitOptions (and unregister)
- Registers a document to watch for settings changes.
- Notification of new settings
When Dependencies Change
One of the things we want the build system to be able to provide to the LSP server when possible, is the ability to efficiently generate the dependencies when they are not already built, or when they need updating (note: this may not be feasible for all build systems). To that end, I think we want to provide hooks for the language server to tell the build server about changes to files that are part of the project, as well as let the build server tell the language server when build intermediates like modules have been regenerated and we may want to refresh the document.
I propose the following API:
- Parameters: URL
- Notify the build server that a source file from the workspace has changed, and allow it to compute what impact it has on the build graph of registered files.
- Parameters: Array
- Notification from the build system that the dependencies of the given URLs are out of date, for example because of source file changes to modules that are imported.
- Parameters: URL
- Notification from the language server to the build server to update the dependencies of the given document. By making this an explicit request we let the language server decide whether it is necessary, as you may have several documents registered for file settings changes, but not want to actively build all of their dependencies.
- Parameters: Array
- Notification from the build server that the dependencies of the given URLs have changed and that we may want to refresh them. This is a notification rather than a reply to the previous message because it allows the build server to tell us about dependency updates that came from a build rather than through a request from the language server.
Another point of integration with the build system would be if we can offload some of the work of background indexing. I am not proposing a specific API here, but I wanted to write down some thoughts on this related topic. Argyrios already mentioned that we would like to add indexing based on the live AST, which should give us an efficient way to update the index for files the user is actively editing. However, there is still a niche for doing background indexing for files that are not built (so no data from indexing-while-building), and not actively edited. A common example is unit tests, which may not be built by default, or files that are part of inactive schemes in Xcode.
We have some experience doing background indexing where the language server is responsible for the whole process, but this is suboptimal in a number of ways. For one, we are doing the indexing one file at a time, but the build system for swift can use batch mode. For another, the build system already has a high quality dependency graph and scheduler that we don’t want to reimplement.
We think that an API where the language server requests indexing based on a list of sources may give us a good balance, as it allow the language server to avoid indexing files it already has data from or which it is uninterested in, while giving the build system a bulk operation that it can efficiently schedule and execute. In normal operation, we would expect incremental index updates to be done using the live AST, combined with data from indexing-while-building for the majority of code. Background indexing would kick in for files outside the build when opening a new project, or after e.g. a git-pull.