From a7387cae2ca9d5e114246e6fada98bfe7808e1d0 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Fri, 12 Feb 2021 15:58:29 +0100 Subject: Fix slow tests sometimes failing In some situations we reloaded the workspace in the tests after having reported to be ready. There's two fixes here: 1. Add a version to the VFS config and include that version in progress reports, so that we don't think we're done prematurely; 2. Delay status transitions until after changes are applied. Otherwise the last change during loading can potentially trigger a workspace reload, if it contains interesting changes. --- crates/rust-analyzer/src/cli/load_cargo.rs | 8 ++++++-- crates/rust-analyzer/src/global_state.rs | 2 ++ crates/rust-analyzer/src/main_loop.rs | 20 +++++++++++++------- crates/rust-analyzer/src/reload.rs | 17 ++++++++++++++++- crates/vfs-notify/src/lib.rs | 10 ++++++++-- crates/vfs/src/loader.rs | 8 ++++++-- 6 files changed, 51 insertions(+), 14 deletions(-) (limited to 'crates') diff --git a/crates/rust-analyzer/src/cli/load_cargo.rs b/crates/rust-analyzer/src/cli/load_cargo.rs index cc63c6cc2..cf0cd81de 100644 --- a/crates/rust-analyzer/src/cli/load_cargo.rs +++ b/crates/rust-analyzer/src/cli/load_cargo.rs @@ -59,7 +59,11 @@ pub fn load_cargo(root: &Path, config: &LoadCargoConfig) -> Result<(AnalysisHost ); let project_folders = ProjectFolders::new(&[ws], &[], build_data.as_ref()); - loader.set_config(vfs::loader::Config { load: project_folders.load, watch: vec![] }); + loader.set_config(vfs::loader::Config { + load: project_folders.load, + watch: vec![], + version: 0, + }); log::debug!("crate graph: {:?}", crate_graph); let host = load(crate_graph, project_folders.source_root_config, &mut vfs, &receiver); @@ -79,7 +83,7 @@ fn load( // wait until Vfs has loaded all roots for task in receiver { match task { - vfs::loader::Message::Progress { n_done, n_total } => { + vfs::loader::Message::Progress { n_done, n_total, config_version: _ } => { if n_done == n_total { break; } diff --git a/crates/rust-analyzer/src/global_state.rs b/crates/rust-analyzer/src/global_state.rs index 6374a9f3c..c3bc8791d 100644 --- a/crates/rust-analyzer/src/global_state.rs +++ b/crates/rust-analyzer/src/global_state.rs @@ -67,6 +67,7 @@ pub(crate) struct GlobalState { req_queue: ReqQueue, pub(crate) task_pool: Handle, Receiver>, pub(crate) loader: Handle, Receiver>, + pub(crate) vfs_config_version: u32, pub(crate) flycheck: Vec, pub(crate) flycheck_sender: Sender, pub(crate) flycheck_receiver: Receiver, @@ -120,6 +121,7 @@ impl GlobalState { GlobalState { sender, req_queue: ReqQueue::default(), + vfs_config_version: 0, task_pool, loader, flycheck: Vec::new(), diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs index bdffff5ae..2829d5970 100644 --- a/crates/rust-analyzer/src/main_loop.rs +++ b/crates/rust-analyzer/src/main_loop.rs @@ -5,6 +5,7 @@ use std::{ time::{Duration, Instant}, }; +use always_assert::always; use crossbeam_channel::{select, Receiver}; use ide::PrimeCachesProgress; use ide::{Canceled, FileId}; @@ -186,7 +187,7 @@ impl GlobalState { log::info!("task queue len: {}", task_queue_len); } - let prev_status = self.status; + let mut new_status = self.status; match event { Event::Lsp(msg) => match msg { lsp_server::Message::Request(req) => self.on_request(loop_start, req)?, @@ -298,22 +299,23 @@ impl GlobalState { } } } - vfs::loader::Message::Progress { n_total, n_done } => { + vfs::loader::Message::Progress { n_total, n_done, config_version } => { + always!(config_version <= self.vfs_config_version); if n_total == 0 { - self.transition(Status::Invalid); + new_status = Status::Invalid; } else { let state = if n_done == 0 { - self.transition(Status::Loading); + new_status = Status::Loading; Progress::Begin } else if n_done < n_total { Progress::Report } else { assert_eq!(n_done, n_total); - let status = Status::Ready { + new_status = Status::Ready { partial: self.config.load_out_dirs_from_check() - && self.workspace_build_data.is_none(), + && self.workspace_build_data.is_none() + || config_version < self.vfs_config_version, }; - self.transition(status); Progress::End }; self.report_progress( @@ -398,6 +400,10 @@ impl GlobalState { } let state_changed = self.process_changes(); + let prev_status = self.status; + if prev_status != new_status { + self.transition(new_status); + } let is_ready = matches!(self.status, Status::Ready { .. }); if prev_status == Status::Loading && is_ready { for flycheck in &self.flycheck { diff --git a/crates/rust-analyzer/src/reload.rs b/crates/rust-analyzer/src/reload.rs index c4f1d098b..c07efa330 100644 --- a/crates/rust-analyzer/src/reload.rs +++ b/crates/rust-analyzer/src/reload.rs @@ -50,6 +50,16 @@ impl GlobalState { Status::Loading | Status::NeedsReload => return, Status::Ready { .. } | Status::Invalid => (), } + log::info!( + "Reloading workspace because of the following changes: {}", + itertools::join( + changes + .iter() + .filter(|(path, kind)| is_interesting(path, *kind)) + .map(|(path, kind)| format!("{}/{:?}", path.display(), kind)), + ", " + ) + ); if self.config.cargo_autoreload() { self.fetch_workspaces_request(); } else { @@ -290,7 +300,12 @@ impl GlobalState { FilesWatcher::Client => vec![], FilesWatcher::Notify => project_folders.watch, }; - self.loader.handle.set_config(vfs::loader::Config { load: project_folders.load, watch }); + self.vfs_config_version += 1; + self.loader.handle.set_config(vfs::loader::Config { + load: project_folders.load, + watch, + version: self.vfs_config_version, + }); // Create crate graph from all the workspaces let crate_graph = { diff --git a/crates/vfs-notify/src/lib.rs b/crates/vfs-notify/src/lib.rs index c605bcf3c..f7ae0577d 100644 --- a/crates/vfs-notify/src/lib.rs +++ b/crates/vfs-notify/src/lib.rs @@ -86,8 +86,10 @@ impl NotifyActor { self.watcher = watcher.map(|it| (it, watcher_receiver)); } + let config_version = config.version; + let n_total = config.load.len(); - self.send(loader::Message::Progress { n_total, n_done: 0 }); + self.send(loader::Message::Progress { n_total, n_done: 0, config_version }); self.watched_entries.clear(); @@ -98,7 +100,11 @@ impl NotifyActor { } let files = self.load_entry(entry, watch); self.send(loader::Message::Loaded { files }); - self.send(loader::Message::Progress { n_total, n_done: i + 1 }); + self.send(loader::Message::Progress { + n_total, + n_done: i + 1, + config_version, + }); } } Message::Invalidate(path) => { diff --git a/crates/vfs/src/loader.rs b/crates/vfs/src/loader.rs index d3bdae562..473b29fcb 100644 --- a/crates/vfs/src/loader.rs +++ b/crates/vfs/src/loader.rs @@ -32,6 +32,9 @@ pub struct Directories { /// [`Handle`]'s configuration. #[derive(Debug)] pub struct Config { + /// Version number to associate progress updates to the right config + /// version. + pub version: u32, /// Set of initially loaded files. pub load: Vec, /// Index of watched entries in `load`. @@ -45,7 +48,7 @@ pub enum Message { /// Indicate a gradual progress. /// /// This is supposed to be the number of loaded files. - Progress { n_total: usize, n_done: usize }, + Progress { n_total: usize, n_done: usize, config_version: u32 }, /// The handle loaded the following files' content. Loaded { files: Vec<(AbsPathBuf, Option>)> }, } @@ -196,10 +199,11 @@ impl fmt::Debug for Message { Message::Loaded { files } => { f.debug_struct("Loaded").field("n_files", &files.len()).finish() } - Message::Progress { n_total, n_done } => f + Message::Progress { n_total, n_done, config_version } => f .debug_struct("Progress") .field("n_total", n_total) .field("n_done", n_done) + .field("config_version", config_version) .finish(), } } -- cgit v1.2.3