I noticed that the prepare/ requests mark targets as up-to-date even if the build fails, which can cause a scenario where a target can end up stuck in a broken state if the user happens to be editing a file at the same time a prepare/ request is triggered (until you do something that triggers the LSP to refresh that target).
Although the fix for this is easy, I wasn't sure if this scenario was intentional because I can see why this would be the case (I guess we don't want to keep retrying something that receives no changes, as that would most likely just fail again)
That leads me to the question: If this is intentional, how should BSPs handle this scenario? Is the expectation that we trigger a OnBuildTargetDidChangeNotification on every file change? (currently we only do so on additions/deletions, as it seemed to be the most optimal setup)
Marking the target as prepared even if preparation fails is intentional because we don’t want to constantly re-prepare a target if it fails. We should, however, not mark the target as up-to-date. That’s why we keep track of the preparation start here. So, I’m surprised how you hit the scenario that you describe or if there’s a bug in the date-handling logic.
So that's the thing, I think my issue is that I may be misunderstanding how to correctly trigger the “target marked out of date” condition mentioned here. From what I can see in the codebase a target would only be considered out of date when sending an explicit OnBuildTargetDidChangeNotification, because regular file changes seem to only invalidate dependencies and not the file's target itself. Meaning if I understood this correctly, the scheduleIndexing call here wouldn't know the target needs to be re-prepared.
There are three reasons why a target may get marked as out of date:
The build server sends a buildTarget/didChange notification to SourceKit-LSP (it does this eg. if a new file gets added to the target, if the build settings of one of the files in the target were changed, a dependency was added to the target, …). In that case, the changed target + all of its dependencies are marked as out-of-date and thus be re-prepared when it next needs to be prepared (eg. for semantic functionality or background indexing). See here.
A source file was changed on disk and the client (aka. editor) sends a workspace/didChangeWatchedFiles notification to SourceKit-LSP. In that case we mark all targets that depend on the file’s target as out-of-date. We don’t need to mark the file’s target itself as out-of-date because preparation prepares a target’s dependencies and the dependencies of the file’s target haven’t changed. The only exception is that we re-prepare the target itself if the changed file has an unknown file extension, see comment here. This check is implemented here.
If the client sends a workspace/triggerReindex request to SourceKit-LSP, here.
When a target is prepared, it is marked as up-to-date only if it hasn’t been marked as out-of-date since the preparation task was started. This is modeled with the updateOperationStartDatehere and here.
You should be able too see logs that explain why targets get marked as out-of-date if you enable extended logging and run log stream --predicate 'subsystem CONTAINS "sourcekit-lsp"' --signpost --debug.
Thanks Alex! With your description and the logs I could see that everything is working fine. My confusion was that I was expecting to see rebuilds as soon as the file was changed, but now with your explanation I was able to see that the LSP was just waiting for me to open a file from one of the dependencies.
Yeah, we try to be lazy here and only perform the re-preparation when it’s actually necessary so we don’t eg. re-prepare a target 20 times while you’re making changes in one of its dependencies before you actually navigate to the dependent target.