rust-analyzer
rust-analyzer copied to clipboard
rust-lang
Include files in vfs on demand
fix #12181 fix #11025 fix #10178
I'm not sure if this is in line with vfs abstraction or not, but I think we can start with it.
Ye CI fails because the proc-macro server doesnt support 1.64 which released yesterday
Sorry that I didn't get back to this. I was pondering about this change a bit(in part because I am still not too familiar with our VFS). I think there is a problem here now though, whenever a path won't be resolvable from a given source root (so not because the file ending isn't tracked but because it might lie outside the source root, think .. segments), this will now subscribe that path over and over again as the source root will keep on never resolving the path.
So the problem is that we may push a file multiple times in watched_entries and it would be solved if we check existence before pushing, right? Would it be only specific to paths outside the source root or would it happen with non existent files inside of source root as well?
Not quite, what I mean is something like the following scenario, we have a module like foo/src/lib.rs which has an include for include!("../../../some_other/path.rs"). This include's path may lie outside of the sourceroot containing foo. Now with the logic here, we'll try to resolve the path from the include, fail and therefor subscribe it. This will trigger a recalculation of the FileSets and SourceRoots, and afterwards as this new path lies outside the sourceroot of foo, it will end up in another sourceroot so we will try to resubscribe that path again, as the foo sourceroot still cant resolve it.
This is basically https://github.com/rust-lang/rust-analyzer/issues/9173. Now the issue here specifically is about fixing non rs-files but the way this is implemented it will try to register files from different sourceroots and fail. So either we try to fix the sourceroot problem as well (which is gonna be very tough because we do not want to break the concept of source roots so this would require a lot of though), or we try to limit subscribing to only unknown files that are still within our current sourceroot, though I don't know how simple that would be either (although I assume that to be simpler).
I found a more fundamental problem. This PR in current state actually doesn't work, since watched_entries and watcher fields in NotifyActor don't do anything. They are either dead code or are for a specific config or usages without LSP. Normally, watching is on the client, and LSP doesn't have a request (or I didn't find) for adding a file to watch list. This makes me wonder, how bad it would be to include all files (with some exceptions, for example target directory) without explicit demand?
Right .. the reason for that I think is that we load the data of all files in the VFS into memory, so if we add all files, we'll have a lot of memory wasted on unnecessary files. So I think this kind of lazy loading would probably be the best choice, though as you said i dont think we can even make that work with client watching ...
In the last changes, the client will watch all the files, but we will read from disk and store in memory only if the file is in the watched_entries. I also fixed the source root problem and now we won't subscribe if the target path belongs to another FileSet.
