When SourceKit-LSP is shut down, we should make sure that we don’t leave behind child processes, which will become orphans after SourceKit-LSP has terminated. What’s worse, when SourceKit-LSP has exited, these processes might not have any process to read their stdout/stderr, which can lead to them running indefinitely.
This change does not cover the termination of subprocess trees. For example, if we launch `swift build` and need to kill it because it doesn’t honor SIGINT, its child processes will still live on. Similarly, if we kill a BSP server, its child processes might live on. Fixing this is a drastically bigger endeavor, likely requiring changes to Foundation and/or TSC. I filed https://github.com/swiftlang/sourcekit-lsp/issues/2080 for it.
This allows us to more easily test behavior for build servers that have different behavior than SwiftPM and compile commands without having to implement the build server in Python.
We were blocking the initialization response on `self.buildSystemManager.testFiles`, which requires the list of test files to be determined. Make that operation asynchronous so that a slow build server can’t take down all of SourceKit-LSP.
This can be useful to IDEs that want to perform some additional semantic processing of source files, which requires knowledge of a file’s build settings.
The timers were racy and on Windows we would get the updated build targets / build settings before we expect them. Change the tests to explicitly tell the build server when to change the targets / settings it reports by sending it a `workspace/didChangeWatchedFiles` notification.
This should also speed up the tests because we don’t have a 1s timer anymore.
`URL.path` returns forward slashes in the path on Windows (https://github.com/swiftlang/swift-foundation/issues/973) where we expect backslashes. Work around that by defining our own `filePath` property that is backed by `withUnsafeFileSystemRepresentation`, which produces backslashes.
rdar://137963660
I saw a nondeterministic test failure of `AsyncUtilsTests.testWithTimeoutEscalatesPriority` and I believe this was the cause.
Also refactor a few test helper functions on the way.
The original idea behind this test was that only opening `Crash.swift` would retrieve build settings for `Crash.swift`. But that wasn’t actually true: Scanning for tests also retrieved build settings (to get the test’s module name). This caused test failures if we set `SOURCEKIT_LSP_TEST_TIMEOUT: 10`, like we do in `settings.json`.
Improve the test to actually check that the BSP server has crashed and increase timeout to be bigger than the 10s restart delay.
If a BSP server crashes, we should try to relaunch it. Use a similar relaunch logic as the one we use for `clangd`: Try restarting immediately unless the BSP server has crashed within the last 30 seconds, in which case we delay the restart by 10s.
rdar://136311009
Fixes#1686
The interaction to an out-of-process BSP server still went through the `BuildServerBuildSystem`, which doesn’t forward all messages to the build system and uses the old push-based model for build settings.
If we discover that the BSP server supports the new pull-based build settings model, we now forward all methods to it directly, without going through `BuiltInBuildSystemAdapter`, which has been renamed to `LegacyBuildServerBuildSystem`.
rdar://136106323
rdar://127606323
rdar://126493405
Fixes#1226Fixes#1173
`buildTarget/inverseSources` is not required to be implemented by BSP servers and we have almost all information needed for it in `BuildSystemManager`.
This also makes sure that `buildTarget/sources` and `buildTarget/inverseSources` actually match each other. Before this change, we had source files like `Package.swift` for which we returned a target from `buildTarget/inverseSources` but that weren’t part of that target’s sources retrieved using `buildTarget/sources`.
The implementation of which file’s dependencies have been updated is common across all build systems and thus build systems shouldn’t need to implement this logic. This also allows us to remove `BuildSystemDelegate`.
This finalizes the move of `BuiltInBuildSystem` creation into `BuiltInBuildSystemAdapter` and means that we can set the message handler of the `BuiltInBuildSystem` during initialization instead of using a setter method.
Do one of the following for every `FIXME` or `TODO` comment
- Add an issue that tracks the task
- Remove the comment if we are not planning to address it
