Do not re-index deleted files #2078
Conversation
This wasn’t an issue when we were always waiting for an up-to-date build graph before indexing files but now we could still have an old build graph that contains the deleted files and thus start an indexing process for a file that doesn’t exist anymore on disk. And it’s cleaner anyway.
|
@swift-ci Please test |
| @@ -379,7 +379,7 @@ package final actor SemanticIndexManager { | |||
| package func filesDidChange(_ events: [FileEvent]) async { | |||
| // We only re-index the files that were changed and don't re-index any of their dependencies. See the | |||
| // `Documentation/Files_To_Reindex.md` file. | |||
| let changedFiles = events.map(\.uri) | |||
| let changedFiles = events.filter { $0.type != .deleted }.map(\.uri) | |||
There was a problem hiding this comment.
Should we not be marking preparation for dependent targets out-of-date for deleted files?
There was a problem hiding this comment.
In general, we can’t rely on being able to get the target that the file used to be part of. The build system might have already processed the removal of the file, which means that it won’t give us any targets for the file.
This should happen through the build system. It also receives the workspace/didChangeWatchedFiles notification and should send us a buildTargets/didChange notification if the deletion of the file changed that target. And we mark the file’s target as out-of-date based on this from SemanticIndexManager.buildTargetsChanged.
Does this make sense?
There was a problem hiding this comment.
I see, yeah makes sense
This wasn’t an issue when we were always waiting for an up-to-date build graph before indexing files but now we could still have an old build graph that contains the deleted files and thus start an indexing process for a file that doesn’t exist anymore on disk. And it’s cleaner anyway.