From 4c03231863904422bf7c661aa68ca74268f7671f Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 13 Jun 2023 16:01:53 -0700 Subject: [PATCH 01/21] Maintain on the background scanner a set of expanded directories --- crates/collab/src/rpc.rs | 2 + crates/project/src/project.rs | 113 ++++++++++++++++++++++ crates/project/src/worktree.rs | 101 ++++++++++++++----- crates/project_panel/src/project_panel.rs | 62 +++++++----- crates/rpc/proto/zed.proto | 12 +++ crates/rpc/src/proto.rs | 6 ++ crates/rpc/src/rpc.rs | 2 +- 7 files changed, 250 insertions(+), 48 deletions(-) diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index 8d210513c2..fd27557041 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -224,6 +224,8 @@ impl Server { .add_request_handler(forward_project_request::) .add_request_handler(forward_project_request::) .add_request_handler(forward_project_request::) + .add_request_handler(forward_project_request::) + .add_request_handler(forward_project_request::) .add_request_handler(forward_project_request::) .add_message_handler(create_buffer_for_peer) .add_request_handler(update_buffer) diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 27d424879f..9242168754 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -51,6 +51,7 @@ use lsp_command::*; use postage::watch; use project_settings::ProjectSettings; use rand::prelude::*; +use rpc::proto::PeerId; use search::SearchQuery; use serde::Serialize; use settings::SettingsStore; @@ -478,6 +479,8 @@ impl Project { client.add_model_request_handler(Self::handle_rename_project_entry); client.add_model_request_handler(Self::handle_copy_project_entry); client.add_model_request_handler(Self::handle_delete_project_entry); + client.add_model_request_handler(Self::handle_expand_project_entry); + client.add_model_request_handler(Self::handle_collapse_project_entry); client.add_model_request_handler(Self::handle_apply_additional_edits_for_completion); client.add_model_request_handler(Self::handle_apply_code_action); client.add_model_request_handler(Self::handle_on_type_formatting); @@ -5403,6 +5406,56 @@ impl Project { Some(ProjectPath { worktree_id, path }) } + pub fn mark_entry_expanded( + &mut self, + worktree_id: WorktreeId, + entry_id: ProjectEntryId, + cx: &mut ModelContext, + ) -> Option<()> { + if self.is_local() { + let worktree = self.worktree_for_id(worktree_id, cx)?; + worktree.update(cx, |worktree, cx| { + worktree + .as_local_mut() + .unwrap() + .mark_entry_expanded(entry_id, true, 0, cx); + }); + } else if let Some(project_id) = self.remote_id() { + cx.background() + .spawn(self.client.request(proto::ExpandProjectEntry { + project_id, + entry_id: entry_id.to_proto(), + })) + .log_err(); + } + Some(()) + } + + pub fn mark_entry_collapsed( + &mut self, + worktree_id: WorktreeId, + entry_id: ProjectEntryId, + cx: &mut ModelContext, + ) -> Option<()> { + if self.is_local() { + let worktree = self.worktree_for_id(worktree_id, cx)?; + worktree.update(cx, |worktree, cx| { + worktree + .as_local_mut() + .unwrap() + .mark_entry_expanded(entry_id, false, 0, cx); + }); + } else if let Some(project_id) = self.remote_id() { + cx.background() + .spawn(self.client.request(proto::CollapseProjectEntry { + project_id, + entry_id: entry_id.to_proto(), + })) + .log_err(); + } + Some(()) + } + pub fn absolute_path(&self, project_path: &ProjectPath, cx: &AppContext) -> Option { let workspace_root = self .worktree_for_id(project_path.worktree_id, cx)? @@ -5705,6 +5758,66 @@ impl Project { }) } + async fn handle_expand_project_entry( + this: ModelHandle, + envelope: TypedEnvelope, + _: Arc, + cx: AsyncAppContext, + ) -> Result { + Self::handle_expand_or_collapse_project_entry( + this, + envelope.payload.entry_id, + envelope.original_sender_id, + true, + cx, + ) + .await + } + + async fn handle_collapse_project_entry( + this: ModelHandle, + envelope: TypedEnvelope, + _: Arc, + cx: AsyncAppContext, + ) -> Result { + Self::handle_expand_or_collapse_project_entry( + this, + envelope.payload.entry_id, + envelope.original_sender_id, + false, + cx, + ) + .await + } + + async fn handle_expand_or_collapse_project_entry( + this: ModelHandle, + entry_id: u64, + original_sender_id: Option, + is_expanded: bool, + mut cx: AsyncAppContext, + ) -> Result { + let entry_id = ProjectEntryId::from_proto(entry_id); + let (worktree, replica_id) = this + .read_with(&cx, |this, cx| { + let replica_id = original_sender_id + .and_then(|peer_id| this.collaborators.get(&peer_id))? + .replica_id; + let worktree = this.worktree_for_entry(entry_id, cx)?; + Some((worktree, replica_id)) + }) + .ok_or_else(|| anyhow!("invalid request"))?; + worktree.update(&mut cx, |worktree, cx| { + worktree.as_local_mut().unwrap().mark_entry_expanded( + entry_id, + is_expanded, + replica_id, + cx, + ) + }); + Ok(proto::Ack {}) + } + async fn handle_update_diagnostic_summary( this: ModelHandle, envelope: TypedEnvelope, diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 2b0ba3d521..5ffbbe5b83 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -5,7 +5,7 @@ use ::ignore::gitignore::{Gitignore, GitignoreBuilder}; use anyhow::{anyhow, Context, Result}; use client::{proto, Client}; use clock::ReplicaId; -use collections::{HashMap, VecDeque}; +use collections::{HashMap, HashSet, VecDeque}; use fs::{ repository::{GitFileStatus, GitRepository, RepoPath}, Fs, LineEnding, @@ -67,7 +67,7 @@ pub enum Worktree { pub struct LocalWorktree { snapshot: LocalSnapshot, - path_changes_tx: channel::Sender<(Vec, barrier::Sender)>, + scan_requests_tx: channel::Sender, is_scanning: (watch::Sender, watch::Receiver), _background_scanner_task: Task<()>, share: Option, @@ -84,6 +84,18 @@ pub struct LocalWorktree { visible: bool, } +enum ScanRequest { + RescanPaths { + paths: Vec, + done: barrier::Sender, + }, + SetDirExpanded { + entry_id: ProjectEntryId, + replica_id: ReplicaId, + is_expanded: bool, + }, +} + pub struct RemoteWorktree { snapshot: Snapshot, background_snapshot: Arc>, @@ -214,6 +226,7 @@ pub struct LocalSnapshot { struct BackgroundScannerState { snapshot: LocalSnapshot, + expanded_dirs: HashSet<(ProjectEntryId, ReplicaId)>, /// The ids of all of the entries that were removed from the snapshot /// as part of the current update. These entry ids may be re-used /// if the same inode is discovered at a new path, or if the given @@ -330,7 +343,7 @@ impl Worktree { ); } - let (path_changes_tx, path_changes_rx) = channel::unbounded(); + let (scan_requests_tx, scan_requests_rx) = channel::unbounded(); let (scan_states_tx, mut scan_states_rx) = mpsc::unbounded(); cx.spawn_weak(|this, mut cx| async move { @@ -370,7 +383,7 @@ impl Worktree { fs, scan_states_tx, background, - path_changes_rx, + scan_requests_rx, ) .run(events) .await; @@ -381,7 +394,7 @@ impl Worktree { snapshot, is_scanning: watch::channel_with(true), share: None, - path_changes_tx, + scan_requests_tx, _background_scanner_task: background_scanner_task, diagnostics: Default::default(), diagnostic_summaries: Default::default(), @@ -1068,8 +1081,11 @@ impl LocalWorktree { this.update(&mut cx, |this, _| { this.as_local_mut() .unwrap() - .path_changes_tx - .try_send((vec![abs_path], tx)) + .scan_requests_tx + .try_send(ScanRequest::RescanPaths { + paths: vec![abs_path], + done: tx, + }) })?; rx.recv().await; Ok(()) @@ -1135,6 +1151,22 @@ impl LocalWorktree { })) } + pub fn mark_entry_expanded( + &mut self, + entry_id: ProjectEntryId, + is_expanded: bool, + replica_id: ReplicaId, + _cx: &mut ModelContext, + ) { + self.scan_requests_tx + .try_send(ScanRequest::SetDirExpanded { + entry_id, + replica_id, + is_expanded, + }) + .ok(); + } + fn refresh_entry( &self, path: Arc, @@ -1143,7 +1175,7 @@ impl LocalWorktree { ) -> Task> { let fs = self.fs.clone(); let abs_root_path = self.abs_path.clone(); - let path_changes_tx = self.path_changes_tx.clone(); + let path_changes_tx = self.scan_requests_tx.clone(); cx.spawn_weak(move |this, mut cx| async move { let abs_path = fs.canonicalize(&abs_root_path).await?; let mut paths = Vec::with_capacity(2); @@ -1161,7 +1193,7 @@ impl LocalWorktree { } let (tx, mut rx) = barrier::channel(); - path_changes_tx.try_send((paths, tx))?; + path_changes_tx.try_send(ScanRequest::RescanPaths { paths, done: tx })?; rx.recv().await; this.upgrade(&cx) .ok_or_else(|| anyhow!("worktree was dropped"))? @@ -2784,7 +2816,7 @@ struct BackgroundScanner { fs: Arc, status_updates_tx: UnboundedSender, executor: Arc, - refresh_requests_rx: channel::Receiver<(Vec, barrier::Sender)>, + scan_requests_rx: channel::Receiver, next_entry_id: Arc, phase: BackgroundScannerPhase, } @@ -2803,17 +2835,18 @@ impl BackgroundScanner { fs: Arc, status_updates_tx: UnboundedSender, executor: Arc, - refresh_requests_rx: channel::Receiver<(Vec, barrier::Sender)>, + scan_requests_rx: channel::Receiver, ) -> Self { Self { fs, status_updates_tx, executor, - refresh_requests_rx, + scan_requests_rx, next_entry_id, state: Mutex::new(BackgroundScannerState { prev_snapshot: snapshot.snapshot.clone(), snapshot, + expanded_dirs: Default::default(), removed_entry_ids: Default::default(), changed_paths: Default::default(), }), @@ -2898,9 +2931,9 @@ impl BackgroundScanner { select_biased! { // Process any path refresh requests from the worktree. Prioritize // these before handling changes reported by the filesystem. - request = self.refresh_requests_rx.recv().fuse() => { - let Ok((paths, barrier)) = request else { break }; - if !self.process_refresh_request(paths.clone(), barrier).await { + request = self.scan_requests_rx.recv().fuse() => { + let Ok(request) = request else { break }; + if !self.process_scan_request(request).await { return; } } @@ -2917,9 +2950,29 @@ impl BackgroundScanner { } } - async fn process_refresh_request(&self, paths: Vec, barrier: barrier::Sender) -> bool { - self.reload_entries_for_paths(paths, None).await; - self.send_status_update(false, Some(barrier)) + async fn process_scan_request(&self, request: ScanRequest) -> bool { + match request { + ScanRequest::RescanPaths { paths, done } => { + self.reload_entries_for_paths(paths, None).await; + self.send_status_update(false, Some(done)) + } + ScanRequest::SetDirExpanded { + entry_id, + replica_id, + is_expanded, + } => { + let mut state = self.state.lock(); + if is_expanded { + state.expanded_dirs.insert((entry_id, replica_id)); + } else { + state.expanded_dirs.remove(&(entry_id, replica_id)); + } + + // todo + + true + } + } } async fn process_events(&mut self, paths: Vec) { @@ -2995,9 +3048,9 @@ impl BackgroundScanner { select_biased! { // Process any path refresh requests before moving on to process // the scan queue, so that user operations are prioritized. - request = self.refresh_requests_rx.recv().fuse() => { - let Ok((paths, barrier)) = request else { break }; - if !self.process_refresh_request(paths, barrier).await { + request = self.scan_requests_rx.recv().fuse() => { + let Ok(request) = request else { break }; + if !self.process_scan_request(request).await { return; } } @@ -3487,9 +3540,9 @@ impl BackgroundScanner { select_biased! { // Process any path refresh requests before moving on to process // the queue of ignore statuses. - request = self.refresh_requests_rx.recv().fuse() => { - let Ok((paths, barrier)) = request else { break }; - if !self.process_refresh_request(paths, barrier).await { + request = self.scan_requests_rx.recv().fuse() => { + let Ok(request) = request else { break }; + if !self.process_scan_request(request).await { return; } } diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index dc592b7588..fd2ff1c6d5 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -431,18 +431,23 @@ impl ProjectPanel { fn collapse_selected_entry(&mut self, _: &CollapseSelectedEntry, cx: &mut ViewContext) { if let Some((worktree, mut entry)) = self.selected_entry(cx) { + let worktree_id = worktree.id(); let expanded_dir_ids = - if let Some(expanded_dir_ids) = self.expanded_dir_ids.get_mut(&worktree.id()) { + if let Some(expanded_dir_ids) = self.expanded_dir_ids.get_mut(&worktree_id) { expanded_dir_ids } else { return; }; loop { - match expanded_dir_ids.binary_search(&entry.id) { + let entry_id = entry.id; + match expanded_dir_ids.binary_search(&entry_id) { Ok(ix) => { expanded_dir_ids.remove(ix); - self.update_visible_entries(Some((worktree.id(), entry.id)), cx); + self.update_visible_entries(Some((worktree_id, entry_id)), cx); + self.project.update(cx, |project, cx| { + project.mark_entry_collapsed(worktree_id, entry_id, cx); + }); cx.notify(); break; } @@ -938,10 +943,19 @@ impl ProjectPanel { } fn selected_entry<'a>(&self, cx: &'a AppContext) -> Option<(&'a Worktree, &'a project::Entry)> { + let (worktree, entry) = self.selected_entry_handle(cx)?; + Some((worktree.read(cx), entry)) + } + + fn selected_entry_handle<'a>( + &self, + cx: &'a AppContext, + ) -> Option<(ModelHandle, &'a project::Entry)> { let selection = self.selection?; let project = self.project.read(cx); - let worktree = project.worktree_for_id(selection.worktree_id, cx)?.read(cx); - Some((worktree, worktree.entry_for_id(selection.entry_id)?)) + let worktree = project.worktree_for_id(selection.worktree_id, cx)?; + let entry = worktree.read(cx).entry_for_id(selection.entry_id)?; + Some((worktree, entry)) } fn update_visible_entries( @@ -1058,29 +1072,31 @@ impl ProjectPanel { entry_id: ProjectEntryId, cx: &mut ViewContext, ) { - let project = self.project.read(cx); - if let Some((worktree, expanded_dir_ids)) = project - .worktree_for_id(worktree_id, cx) - .zip(self.expanded_dir_ids.get_mut(&worktree_id)) - { - let worktree = worktree.read(cx); + self.project.update(cx, |project, cx| { + if let Some((worktree, expanded_dir_ids)) = project + .worktree_for_id(worktree_id, cx) + .zip(self.expanded_dir_ids.get_mut(&worktree_id)) + { + project.mark_entry_expanded(worktree_id, entry_id, cx); + let worktree = worktree.read(cx); - if let Some(mut entry) = worktree.entry_for_id(entry_id) { - loop { - if let Err(ix) = expanded_dir_ids.binary_search(&entry.id) { - expanded_dir_ids.insert(ix, entry.id); - } + if let Some(mut entry) = worktree.entry_for_id(entry_id) { + loop { + if let Err(ix) = expanded_dir_ids.binary_search(&entry.id) { + expanded_dir_ids.insert(ix, entry.id); + } - if let Some(parent_entry) = - entry.path.parent().and_then(|p| worktree.entry_for_path(p)) - { - entry = parent_entry; - } else { - break; + if let Some(parent_entry) = + entry.path.parent().and_then(|p| worktree.entry_for_path(p)) + { + entry = parent_entry; + } else { + break; + } } } } - } + }); } fn for_each_visible_entry( diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index ce4dd7f7cf..d7e8d16a0e 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -62,6 +62,8 @@ message Envelope { RenameProjectEntry rename_project_entry = 46; CopyProjectEntry copy_project_entry = 47; DeleteProjectEntry delete_project_entry = 48; + ExpandProjectEntry expand_project_entry = 114; + CollapseProjectEntry collapse_project_entry = 115; ProjectEntryResponse project_entry_response = 49; UpdateDiagnosticSummary update_diagnostic_summary = 50; @@ -372,6 +374,16 @@ message DeleteProjectEntry { uint64 entry_id = 2; } +message ExpandProjectEntry { + uint64 project_id = 1; + uint64 entry_id = 2; +} + +message CollapseProjectEntry { + uint64 project_id = 1; + uint64 entry_id = 2; +} + message ProjectEntryResponse { Entry entry = 1; uint64 worktree_scan_id = 2; diff --git a/crates/rpc/src/proto.rs b/crates/rpc/src/proto.rs index 13794ea64d..9f8e942492 100644 --- a/crates/rpc/src/proto.rs +++ b/crates/rpc/src/proto.rs @@ -150,6 +150,8 @@ messages!( (DeclineCall, Foreground), (DeleteProjectEntry, Foreground), (Error, Foreground), + (ExpandProjectEntry, Foreground), + (CollapseProjectEntry, Foreground), (Follow, Foreground), (FollowResponse, Foreground), (FormatBuffers, Foreground), @@ -255,6 +257,8 @@ request_messages!( (CreateRoom, CreateRoomResponse), (DeclineCall, Ack), (DeleteProjectEntry, ProjectEntryResponse), + (ExpandProjectEntry, Ack), + (CollapseProjectEntry, Ack), (Follow, FollowResponse), (FormatBuffers, FormatBuffersResponse), (GetChannelMessages, GetChannelMessagesResponse), @@ -311,6 +315,8 @@ entity_messages!( CreateBufferForPeer, CreateProjectEntry, DeleteProjectEntry, + ExpandProjectEntry, + CollapseProjectEntry, Follow, FormatBuffers, GetCodeActions, diff --git a/crates/rpc/src/rpc.rs b/crates/rpc/src/rpc.rs index 8b10167091..6b430d90e4 100644 --- a/crates/rpc/src/rpc.rs +++ b/crates/rpc/src/rpc.rs @@ -6,4 +6,4 @@ pub use conn::Connection; pub use peer::*; mod macros; -pub const PROTOCOL_VERSION: u32 = 58; +pub const PROTOCOL_VERSION: u32 = 59; From 55f1a6647fa2c5e9c191130b35aa8e0f431fe44c Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 13 Jun 2023 17:16:11 -0700 Subject: [PATCH 02/21] Model symlinks better in FakeFs, add read_link Fs method --- crates/fs/Cargo.toml | 3 ++ crates/fs/src/fs.rs | 86 +++++++++++++++++++++++++++++++------------- 2 files changed, 65 insertions(+), 24 deletions(-) diff --git a/crates/fs/Cargo.toml b/crates/fs/Cargo.toml index 7dda3f7273..cb738f567c 100644 --- a/crates/fs/Cargo.toml +++ b/crates/fs/Cargo.toml @@ -32,5 +32,8 @@ serde_json.workspace = true log.workspace = true libc = "0.2" +[dev-dependencies] +gpui = { path = "../gpui", features = ["test-support"] } + [features] test-support = [] diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index fee7765d49..f5abe35dbd 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -108,6 +108,7 @@ pub trait Fs: Send + Sync { async fn canonicalize(&self, path: &Path) -> Result; async fn is_file(&self, path: &Path) -> bool; async fn metadata(&self, path: &Path) -> Result>; + async fn read_link(&self, path: &Path) -> Result; async fn read_dir( &self, path: &Path, @@ -323,6 +324,11 @@ impl Fs for RealFs { })) } + async fn read_link(&self, path: &Path) -> Result { + let path = smol::fs::read_link(path).await?; + Ok(path) + } + async fn read_dir( &self, path: &Path, @@ -407,46 +413,51 @@ enum FakeFsEntry { impl FakeFsState { fn read_path<'a>(&'a self, target: &Path) -> Result>> { Ok(self - .try_read_path(target) + .try_read_path(target, true) .ok_or_else(|| anyhow!("path does not exist: {}", target.display()))? .0) } - fn try_read_path<'a>(&'a self, target: &Path) -> Option<(Arc>, PathBuf)> { + fn try_read_path<'a>( + &'a self, + target: &Path, + follow_symlink: bool, + ) -> Option<(Arc>, PathBuf)> { let mut path = target.to_path_buf(); - let mut real_path = PathBuf::new(); + let mut canonical_path = PathBuf::new(); let mut entry_stack = Vec::new(); 'outer: loop { - let mut path_components = path.components().collect::>(); - while let Some(component) = path_components.pop_front() { + let mut path_components = path.components().peekable(); + while let Some(component) = path_components.next() { match component { Component::Prefix(_) => panic!("prefix paths aren't supported"), Component::RootDir => { entry_stack.clear(); entry_stack.push(self.root.clone()); - real_path.clear(); - real_path.push("/"); + canonical_path.clear(); + canonical_path.push("/"); } Component::CurDir => {} Component::ParentDir => { entry_stack.pop()?; - real_path.pop(); + canonical_path.pop(); } Component::Normal(name) => { let current_entry = entry_stack.last().cloned()?; let current_entry = current_entry.lock(); if let FakeFsEntry::Dir { entries, .. } = &*current_entry { let entry = entries.get(name.to_str().unwrap()).cloned()?; - let _entry = entry.lock(); - if let FakeFsEntry::Symlink { target, .. } = &*_entry { - let mut target = target.clone(); - target.extend(path_components); - path = target; - continue 'outer; - } else { - entry_stack.push(entry.clone()); - real_path.push(name); + if path_components.peek().is_some() || follow_symlink { + let entry = entry.lock(); + if let FakeFsEntry::Symlink { target, .. } = &*entry { + let mut target = target.clone(); + target.extend(path_components); + path = target; + continue 'outer; + } } + entry_stack.push(entry.clone()); + canonical_path.push(name); } else { return None; } @@ -455,7 +466,7 @@ impl FakeFsState { } break; } - entry_stack.pop().map(|entry| (entry, real_path)) + Some((entry_stack.pop()?, canonical_path)) } fn write_path(&self, path: &Path, callback: Fn) -> Result @@ -776,6 +787,10 @@ impl FakeFsEntry { matches!(self, Self::File { .. }) } + fn is_symlink(&self) -> bool { + matches!(self, Self::Symlink { .. }) + } + fn file_content(&self, path: &Path) -> Result<&String> { if let Self::File { content, .. } = self { Ok(content) @@ -1056,8 +1071,8 @@ impl Fs for FakeFs { let path = normalize_path(path); self.simulate_random_delay().await; let state = self.state.lock(); - if let Some((_, real_path)) = state.try_read_path(&path) { - Ok(real_path) + if let Some((_, canonical_path)) = state.try_read_path(&path, true) { + Ok(canonical_path) } else { Err(anyhow!("path does not exist: {}", path.display())) } @@ -1067,7 +1082,7 @@ impl Fs for FakeFs { let path = normalize_path(path); self.simulate_random_delay().await; let state = self.state.lock(); - if let Some((entry, _)) = state.try_read_path(&path) { + if let Some((entry, _)) = state.try_read_path(&path, true) { entry.lock().is_file() } else { false @@ -1078,10 +1093,17 @@ impl Fs for FakeFs { self.simulate_random_delay().await; let path = normalize_path(path); let state = self.state.lock(); - if let Some((entry, real_path)) = state.try_read_path(&path) { - let entry = entry.lock(); - let is_symlink = real_path != path; + if let Some((mut entry, _)) = state.try_read_path(&path, false) { + let is_symlink = entry.lock().is_symlink(); + if is_symlink { + if let Some(e) = state.try_read_path(&path, true).map(|e| e.0) { + entry = e; + } else { + return Ok(None); + } + } + let entry = entry.lock(); Ok(Some(match &*entry { FakeFsEntry::File { inode, mtime, .. } => Metadata { inode: *inode, @@ -1102,6 +1124,22 @@ impl Fs for FakeFs { } } + async fn read_link(&self, path: &Path) -> Result { + self.simulate_random_delay().await; + let path = normalize_path(path); + let state = self.state.lock(); + if let Some((entry, _)) = state.try_read_path(&path, false) { + let entry = entry.lock(); + if let FakeFsEntry::Symlink { target } = &*entry { + Ok(target.clone()) + } else { + Err(anyhow!("not a symlink: {}", path.display())) + } + } else { + Err(anyhow!("path does not exist: {}", path.display())) + } + } + async fn read_dir( &self, path: &Path, From f910d8fe3ea71be80b961f4c1e35e8e8d14dcd38 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 13 Jun 2023 17:16:19 -0700 Subject: [PATCH 03/21] Only scan ignored or externals paths if expanded in a project panel --- crates/project/src/worktree.rs | 106 +++++++++++++++++++--- crates/project/src/worktree_tests.rs | 68 ++++++++++++++ crates/project_panel/src/project_panel.rs | 34 ++++--- 3 files changed, 182 insertions(+), 26 deletions(-) diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 5ffbbe5b83..1ce08a9a47 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -5,7 +5,7 @@ use ::ignore::gitignore::{Gitignore, GitignoreBuilder}; use anyhow::{anyhow, Context, Result}; use client::{proto, Client}; use clock::ReplicaId; -use collections::{HashMap, HashSet, VecDeque}; +use collections::{BTreeSet, HashMap, VecDeque}; use fs::{ repository::{GitFileStatus, GitRepository, RepoPath}, Fs, LineEnding, @@ -226,7 +226,7 @@ pub struct LocalSnapshot { struct BackgroundScannerState { snapshot: LocalSnapshot, - expanded_dirs: HashSet<(ProjectEntryId, ReplicaId)>, + expanded_dirs: BTreeSet<(ProjectEntryId, ReplicaId)>, /// The ids of all of the entries that were removed from the snapshot /// as part of the current update. These entry ids may be re-used /// if the same inode is discovered at a new path, or if the given @@ -2209,6 +2209,15 @@ impl LocalSnapshot { } impl BackgroundScannerState { + fn is_entry_expanded(&self, entry: &Entry) -> bool { + let expanded = self + .expanded_dirs + .range((entry.id, 0)..=(entry.id, ReplicaId::MAX)) + .next() + .is_some(); + expanded + } + fn reuse_entry_id(&mut self, entry: &mut Entry) { if let Some(removed_entry_id) = self.removed_entry_ids.remove(&entry.inode) { entry.id = removed_entry_id; @@ -2661,11 +2670,11 @@ impl Entry { } pub fn is_dir(&self) -> bool { - matches!(self.kind, EntryKind::Dir | EntryKind::PendingDir) + self.kind.is_dir() } pub fn is_file(&self) -> bool { - matches!(self.kind, EntryKind::File(_)) + self.kind.is_file() } pub fn git_status(&self) -> Option { @@ -2673,6 +2682,16 @@ impl Entry { } } +impl EntryKind { + pub fn is_dir(&self) -> bool { + matches!(self, EntryKind::Dir | EntryKind::PendingDir) + } + + pub fn is_file(&self) -> bool { + matches!(self, EntryKind::File(_)) + } +} + impl sum_tree::Item for Entry { type Summary = EntrySummary; @@ -2901,6 +2920,7 @@ impl BackgroundScanner { path: Arc::from(Path::new("")), ignore_stack, ancestor_inodes: TreeSet::from_ordered_entries(root_inode), + is_outside_root: false, scan_queue: scan_job_tx.clone(), })) .unwrap(); @@ -2961,15 +2981,27 @@ impl BackgroundScanner { replica_id, is_expanded, } => { - let mut state = self.state.lock(); - if is_expanded { - state.expanded_dirs.insert((entry_id, replica_id)); - } else { - state.expanded_dirs.remove(&(entry_id, replica_id)); + let path = { + let mut state = self.state.lock(); + if is_expanded { + state.expanded_dirs.insert((entry_id, replica_id)); + } else { + state.expanded_dirs.remove(&(entry_id, replica_id)); + } + state + .snapshot + .entry_for_id(entry_id) + .map(|e| state.snapshot.absolutize(&e.path)) + }; + if let Some(path) = path { + let (scan_job_tx, mut scan_job_rx) = channel::unbounded(); + self.reload_entries_for_paths(vec![path.clone()], Some(scan_job_tx)) + .await; + if let Some(job) = scan_job_rx.next().await { + self.scan_dir(&job).await.log_err(); + self.send_status_update(false, None); + } } - - // todo - true } } @@ -3120,7 +3152,16 @@ impl BackgroundScanner { let mut ignore_stack = job.ignore_stack.clone(); let mut new_ignore = None; let (root_abs_path, root_char_bag, next_entry_id, repository) = { - let snapshot = &self.state.lock().snapshot; + let state = self.state.lock(); + if job.is_outside_root || job.ignore_stack.is_all() { + if let Some(entry) = state.snapshot.entry_for_path(&job.path) { + if !state.is_entry_expanded(entry) { + return Ok(()); + } + } + } + + let snapshot = &state.snapshot; ( snapshot.abs_path().clone(), snapshot.root_char_bag, @@ -3131,6 +3172,7 @@ impl BackgroundScanner { ) }; + let mut root_canonical_path = None; let mut child_paths = self.fs.read_dir(&job.abs_path).await?; while let Some(child_abs_path) = child_paths.next().await { let child_abs_path: Arc = match child_abs_path { @@ -3198,6 +3240,38 @@ impl BackgroundScanner { root_char_bag, ); + let mut is_outside_root = false; + if child_metadata.is_symlink { + let canonical_path = match self.fs.canonicalize(&child_abs_path).await { + Ok(path) => path, + Err(err) => { + log::error!( + "error reading target of symlink {:?}: {:?}", + child_abs_path, + err + ); + continue; + } + }; + + // lazily canonicalize the root path in order to determine if + // symlinks point outside of the worktree. + let root_canonical_path = match &root_canonical_path { + Some(path) => path, + None => match self.fs.canonicalize(&root_abs_path).await { + Ok(path) => root_canonical_path.insert(path), + Err(err) => { + log::error!("error canonicalizing root {:?}: {:?}", root_abs_path, err); + continue; + } + }, + }; + + if !canonical_path.starts_with(root_canonical_path) { + is_outside_root = true; + } + } + if child_entry.is_dir() { let is_ignored = ignore_stack.is_abs_path_ignored(&child_abs_path, true); child_entry.is_ignored = is_ignored; @@ -3210,6 +3284,7 @@ impl BackgroundScanner { new_jobs.push(Some(ScanJob { abs_path: child_abs_path, path: child_path, + is_outside_root, ignore_stack: if is_ignored { IgnoreStack::all() } else { @@ -3259,7 +3334,7 @@ impl BackgroundScanner { for new_job in new_jobs { if let Some(new_job) = new_job { - job.scan_queue.send(new_job).await.unwrap(); + job.scan_queue.send(new_job).await.ok(); } } @@ -3354,12 +3429,14 @@ impl BackgroundScanner { if let Some(scan_queue_tx) = &scan_queue_tx { let mut ancestor_inodes = state.snapshot.ancestor_inodes_for_path(&path); if metadata.is_dir && !ancestor_inodes.contains(&metadata.inode) { + let is_outside_root = !abs_path.starts_with(&root_canonical_path); ancestor_inodes.insert(metadata.inode); smol::block_on(scan_queue_tx.send(ScanJob { abs_path, path, ignore_stack, ancestor_inodes, + is_outside_root, scan_queue: scan_queue_tx.clone(), })) .unwrap(); @@ -3748,6 +3825,7 @@ struct ScanJob { ignore_stack: Arc, scan_queue: Sender, ancestor_inodes: TreeSet, + is_outside_root: bool, } struct UpdateIgnoreStatusJob { diff --git a/crates/project/src/worktree_tests.rs b/crates/project/src/worktree_tests.rs index 3abf660282..6468ac9a16 100644 --- a/crates/project/src/worktree_tests.rs +++ b/crates/project/src/worktree_tests.rs @@ -256,6 +256,74 @@ async fn test_circular_symlinks(executor: Arc, cx: &mut TestAppCo }); } +#[gpui::test(iterations = 10)] +async fn test_symlinks_pointing_outside(cx: &mut TestAppContext) { + let fs = FakeFs::new(cx.background()); + fs.insert_tree( + "/root", + json!({ + "dir1": { + "deps": { + // symlinks here + }, + "src": { + "a.rs": "", + "b.rs": "", + }, + }, + "dir2": { + "src": { + "c.rs": "", + "d.rs": "", + } + }, + "dir3": { + "src": { + "e.rs": "", + "f.rs": "", + } + } + }), + ) + .await; + fs.insert_symlink("/root/dir1/deps/dep-dir2", "../../dir2".into()) + .await; + fs.insert_symlink("/root/dir1/deps/dep-dir3", "../../dir3".into()) + .await; + + let client = cx.read(|cx| Client::new(FakeHttpClient::with_404_response(), cx)); + let tree = Worktree::local( + client, + Path::new("/root/dir1"), + true, + fs.clone(), + Default::default(), + &mut cx.to_async(), + ) + .await + .unwrap(); + + cx.read(|cx| tree.read(cx).as_local().unwrap().scan_complete()) + .await; + + tree.read_with(cx, |tree, _| { + assert_eq!( + tree.entries(false) + .map(|entry| entry.path.as_ref()) + .collect::>(), + vec![ + Path::new(""), + Path::new("deps"), + Path::new("deps/dep-dir2"), + Path::new("deps/dep-dir3"), + Path::new("src"), + Path::new("src/a.rs"), + Path::new("src/b.rs"), + ] + ); + }); +} + #[gpui::test] async fn test_rescan_with_gitignore(cx: &mut TestAppContext) { // .gitignores are handled explicitly by Zed and do not use the git diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index fd2ff1c6d5..78a9fb032f 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -410,17 +410,23 @@ impl ProjectPanel { fn expand_selected_entry(&mut self, _: &ExpandSelectedEntry, cx: &mut ViewContext) { if let Some((worktree, entry)) = self.selected_entry(cx) { if entry.is_dir() { + let worktree_id = worktree.id(); + let entry_id = entry.id; let expanded_dir_ids = - if let Some(expanded_dir_ids) = self.expanded_dir_ids.get_mut(&worktree.id()) { + if let Some(expanded_dir_ids) = self.expanded_dir_ids.get_mut(&worktree_id) { expanded_dir_ids } else { return; }; - match expanded_dir_ids.binary_search(&entry.id) { + match expanded_dir_ids.binary_search(&entry_id) { Ok(_) => self.select_next(&SelectNext, cx), Err(ix) => { - expanded_dir_ids.insert(ix, entry.id); + self.project.update(cx, |project, cx| { + project.mark_entry_expanded(worktree_id, entry_id, cx); + }); + + expanded_dir_ids.insert(ix, entry_id); self.update_visible_entries(None, cx); cx.notify(); } @@ -468,14 +474,18 @@ impl ProjectPanel { fn toggle_expanded(&mut self, entry_id: ProjectEntryId, cx: &mut ViewContext) { if let Some(worktree_id) = self.project.read(cx).worktree_id_for_entry(entry_id, cx) { if let Some(expanded_dir_ids) = self.expanded_dir_ids.get_mut(&worktree_id) { - match expanded_dir_ids.binary_search(&entry_id) { - Ok(ix) => { - expanded_dir_ids.remove(ix); + self.project.update(cx, |project, cx| { + match expanded_dir_ids.binary_search(&entry_id) { + Ok(ix) => { + project.mark_entry_collapsed(worktree_id, entry_id, cx); + expanded_dir_ids.remove(ix); + } + Err(ix) => { + project.mark_entry_expanded(worktree_id, entry_id, cx); + expanded_dir_ids.insert(ix, entry_id); + } } - Err(ix) => { - expanded_dir_ids.insert(ix, entry_id); - } - } + }); self.update_visible_entries(Some((worktree_id, entry_id)), cx); cx.focus_self(); cx.notify(); @@ -1206,7 +1216,7 @@ impl ProjectPanel { Flex::row() .with_child( - if kind == EntryKind::Dir { + if kind.is_dir() { if details.is_expanded { Svg::new("icons/chevron_down_8.svg").with_color(style.icon_color) } else { @@ -1303,7 +1313,7 @@ impl ProjectPanel { }) .on_click(MouseButton::Left, move |event, this, cx| { if !show_editor { - if kind == EntryKind::Dir { + if kind.is_dir() { this.toggle_expanded(entry_id, cx); } else { this.open_entry(entry_id, event.click_count > 1, cx); From aa6f2f1816079bb15b942b60b8579a7f7002c37a Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 15 Jun 2023 15:58:29 -0700 Subject: [PATCH 04/21] Remove logic for marking worktree entries as collapsed --- crates/collab/src/rpc.rs | 1 - crates/project/src/project.rs | 84 +++-------------------- crates/project/src/worktree.rs | 35 ++-------- crates/project_panel/src/project_panel.rs | 4 -- crates/rpc/proto/zed.proto | 6 -- crates/rpc/src/proto.rs | 3 - 6 files changed, 15 insertions(+), 118 deletions(-) diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index fd27557041..a5be6e7d62 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -225,7 +225,6 @@ impl Server { .add_request_handler(forward_project_request::) .add_request_handler(forward_project_request::) .add_request_handler(forward_project_request::) - .add_request_handler(forward_project_request::) .add_request_handler(forward_project_request::) .add_message_handler(create_buffer_for_peer) .add_request_handler(update_buffer) diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 9242168754..79f3e743bd 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -51,7 +51,6 @@ use lsp_command::*; use postage::watch; use project_settings::ProjectSettings; use rand::prelude::*; -use rpc::proto::PeerId; use search::SearchQuery; use serde::Serialize; use settings::SettingsStore; @@ -480,7 +479,6 @@ impl Project { client.add_model_request_handler(Self::handle_copy_project_entry); client.add_model_request_handler(Self::handle_delete_project_entry); client.add_model_request_handler(Self::handle_expand_project_entry); - client.add_model_request_handler(Self::handle_collapse_project_entry); client.add_model_request_handler(Self::handle_apply_additional_edits_for_completion); client.add_model_request_handler(Self::handle_apply_code_action); client.add_model_request_handler(Self::handle_on_type_formatting); @@ -5418,7 +5416,7 @@ impl Project { worktree .as_local_mut() .unwrap() - .mark_entry_expanded(entry_id, true, 0, cx); + .mark_entry_expanded(entry_id, cx); }); } else if let Some(project_id) = self.remote_id() { cx.background() @@ -5431,31 +5429,6 @@ impl Project { Some(()) } - pub fn mark_entry_collapsed( - &mut self, - worktree_id: WorktreeId, - entry_id: ProjectEntryId, - cx: &mut ModelContext, - ) -> Option<()> { - if self.is_local() { - let worktree = self.worktree_for_id(worktree_id, cx)?; - worktree.update(cx, |worktree, cx| { - worktree - .as_local_mut() - .unwrap() - .mark_entry_expanded(entry_id, false, 0, cx); - }); - } else if let Some(project_id) = self.remote_id() { - cx.background() - .spawn(self.client.request(proto::CollapseProjectEntry { - project_id, - entry_id: entry_id.to_proto(), - })) - .log_err(); - } - Some(()) - } - pub fn absolute_path(&self, project_path: &ProjectPath, cx: &AppContext) -> Option { let workspace_root = self .worktree_for_id(project_path.worktree_id, cx)? @@ -5762,58 +5735,17 @@ impl Project { this: ModelHandle, envelope: TypedEnvelope, _: Arc, - cx: AsyncAppContext, - ) -> Result { - Self::handle_expand_or_collapse_project_entry( - this, - envelope.payload.entry_id, - envelope.original_sender_id, - true, - cx, - ) - .await - } - - async fn handle_collapse_project_entry( - this: ModelHandle, - envelope: TypedEnvelope, - _: Arc, - cx: AsyncAppContext, - ) -> Result { - Self::handle_expand_or_collapse_project_entry( - this, - envelope.payload.entry_id, - envelope.original_sender_id, - false, - cx, - ) - .await - } - - async fn handle_expand_or_collapse_project_entry( - this: ModelHandle, - entry_id: u64, - original_sender_id: Option, - is_expanded: bool, mut cx: AsyncAppContext, ) -> Result { - let entry_id = ProjectEntryId::from_proto(entry_id); - let (worktree, replica_id) = this - .read_with(&cx, |this, cx| { - let replica_id = original_sender_id - .and_then(|peer_id| this.collaborators.get(&peer_id))? - .replica_id; - let worktree = this.worktree_for_entry(entry_id, cx)?; - Some((worktree, replica_id)) - }) + let entry_id = ProjectEntryId::from_proto(envelope.payload.entry_id); + let worktree = this + .read_with(&cx, |this, cx| this.worktree_for_entry(entry_id, cx)) .ok_or_else(|| anyhow!("invalid request"))?; worktree.update(&mut cx, |worktree, cx| { - worktree.as_local_mut().unwrap().mark_entry_expanded( - entry_id, - is_expanded, - replica_id, - cx, - ) + worktree + .as_local_mut() + .unwrap() + .mark_entry_expanded(entry_id, cx) }); Ok(proto::Ack {}) } diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 1ce08a9a47..4e5a828165 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -5,7 +5,7 @@ use ::ignore::gitignore::{Gitignore, GitignoreBuilder}; use anyhow::{anyhow, Context, Result}; use client::{proto, Client}; use clock::ReplicaId; -use collections::{BTreeSet, HashMap, VecDeque}; +use collections::{HashMap, HashSet, VecDeque}; use fs::{ repository::{GitFileStatus, GitRepository, RepoPath}, Fs, LineEnding, @@ -89,10 +89,8 @@ enum ScanRequest { paths: Vec, done: barrier::Sender, }, - SetDirExpanded { + ExpandDir { entry_id: ProjectEntryId, - replica_id: ReplicaId, - is_expanded: bool, }, } @@ -226,7 +224,7 @@ pub struct LocalSnapshot { struct BackgroundScannerState { snapshot: LocalSnapshot, - expanded_dirs: BTreeSet<(ProjectEntryId, ReplicaId)>, + expanded_dirs: HashSet, /// The ids of all of the entries that were removed from the snapshot /// as part of the current update. These entry ids may be re-used /// if the same inode is discovered at a new path, or if the given @@ -1154,16 +1152,10 @@ impl LocalWorktree { pub fn mark_entry_expanded( &mut self, entry_id: ProjectEntryId, - is_expanded: bool, - replica_id: ReplicaId, _cx: &mut ModelContext, ) { self.scan_requests_tx - .try_send(ScanRequest::SetDirExpanded { - entry_id, - replica_id, - is_expanded, - }) + .try_send(ScanRequest::ExpandDir { entry_id }) .ok(); } @@ -2210,12 +2202,7 @@ impl LocalSnapshot { impl BackgroundScannerState { fn is_entry_expanded(&self, entry: &Entry) -> bool { - let expanded = self - .expanded_dirs - .range((entry.id, 0)..=(entry.id, ReplicaId::MAX)) - .next() - .is_some(); - expanded + self.expanded_dirs.contains(&entry.id) } fn reuse_entry_id(&mut self, entry: &mut Entry) { @@ -2976,18 +2963,10 @@ impl BackgroundScanner { self.reload_entries_for_paths(paths, None).await; self.send_status_update(false, Some(done)) } - ScanRequest::SetDirExpanded { - entry_id, - replica_id, - is_expanded, - } => { + ScanRequest::ExpandDir { entry_id } => { let path = { let mut state = self.state.lock(); - if is_expanded { - state.expanded_dirs.insert((entry_id, replica_id)); - } else { - state.expanded_dirs.remove(&(entry_id, replica_id)); - } + state.expanded_dirs.insert(entry_id); state .snapshot .entry_for_id(entry_id) diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 78a9fb032f..1291835ae9 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -451,9 +451,6 @@ impl ProjectPanel { Ok(ix) => { expanded_dir_ids.remove(ix); self.update_visible_entries(Some((worktree_id, entry_id)), cx); - self.project.update(cx, |project, cx| { - project.mark_entry_collapsed(worktree_id, entry_id, cx); - }); cx.notify(); break; } @@ -477,7 +474,6 @@ impl ProjectPanel { self.project.update(cx, |project, cx| { match expanded_dir_ids.binary_search(&entry_id) { Ok(ix) => { - project.mark_entry_collapsed(worktree_id, entry_id, cx); expanded_dir_ids.remove(ix); } Err(ix) => { diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index d7e8d16a0e..f3a9f7eab5 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -63,7 +63,6 @@ message Envelope { CopyProjectEntry copy_project_entry = 47; DeleteProjectEntry delete_project_entry = 48; ExpandProjectEntry expand_project_entry = 114; - CollapseProjectEntry collapse_project_entry = 115; ProjectEntryResponse project_entry_response = 49; UpdateDiagnosticSummary update_diagnostic_summary = 50; @@ -379,11 +378,6 @@ message ExpandProjectEntry { uint64 entry_id = 2; } -message CollapseProjectEntry { - uint64 project_id = 1; - uint64 entry_id = 2; -} - message ProjectEntryResponse { Entry entry = 1; uint64 worktree_scan_id = 2; diff --git a/crates/rpc/src/proto.rs b/crates/rpc/src/proto.rs index 9f8e942492..6311b043d3 100644 --- a/crates/rpc/src/proto.rs +++ b/crates/rpc/src/proto.rs @@ -151,7 +151,6 @@ messages!( (DeleteProjectEntry, Foreground), (Error, Foreground), (ExpandProjectEntry, Foreground), - (CollapseProjectEntry, Foreground), (Follow, Foreground), (FollowResponse, Foreground), (FormatBuffers, Foreground), @@ -258,7 +257,6 @@ request_messages!( (DeclineCall, Ack), (DeleteProjectEntry, ProjectEntryResponse), (ExpandProjectEntry, Ack), - (CollapseProjectEntry, Ack), (Follow, FollowResponse), (FormatBuffers, FormatBuffersResponse), (GetChannelMessages, GetChannelMessagesResponse), @@ -316,7 +314,6 @@ entity_messages!( CreateProjectEntry, DeleteProjectEntry, ExpandProjectEntry, - CollapseProjectEntry, Follow, FormatBuffers, GetCodeActions, From 205c758e4e5d8c5ef67794023ce61d6ef663d2b1 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 16 Jun 2023 09:42:13 -0700 Subject: [PATCH 05/21] Wait for ignored directory to be expanded in descendant entries test --- crates/project/src/project.rs | 10 ++-------- crates/project/src/worktree.rs | 13 ++++++++----- crates/project/src/worktree_tests.rs | 11 +++++++++++ 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 79f3e743bd..a360e5dd72 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -5413,10 +5413,7 @@ impl Project { if self.is_local() { let worktree = self.worktree_for_id(worktree_id, cx)?; worktree.update(cx, |worktree, cx| { - worktree - .as_local_mut() - .unwrap() - .mark_entry_expanded(entry_id, cx); + worktree.as_local_mut().unwrap().expand_dir(entry_id, cx); }); } else if let Some(project_id) = self.remote_id() { cx.background() @@ -5742,10 +5739,7 @@ impl Project { .read_with(&cx, |this, cx| this.worktree_for_entry(entry_id, cx)) .ok_or_else(|| anyhow!("invalid request"))?; worktree.update(&mut cx, |worktree, cx| { - worktree - .as_local_mut() - .unwrap() - .mark_entry_expanded(entry_id, cx) + worktree.as_local_mut().unwrap().expand_dir(entry_id, cx) }); Ok(proto::Ack {}) } diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 4e5a828165..0480461556 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -91,6 +91,7 @@ enum ScanRequest { }, ExpandDir { entry_id: ProjectEntryId, + done: barrier::Sender, }, } @@ -1149,14 +1150,16 @@ impl LocalWorktree { })) } - pub fn mark_entry_expanded( + pub fn expand_dir( &mut self, entry_id: ProjectEntryId, _cx: &mut ModelContext, - ) { + ) -> barrier::Receiver { + let (tx, rx) = barrier::channel(); self.scan_requests_tx - .try_send(ScanRequest::ExpandDir { entry_id }) + .try_send(ScanRequest::ExpandDir { entry_id, done: tx }) .ok(); + rx } fn refresh_entry( @@ -2963,7 +2966,7 @@ impl BackgroundScanner { self.reload_entries_for_paths(paths, None).await; self.send_status_update(false, Some(done)) } - ScanRequest::ExpandDir { entry_id } => { + ScanRequest::ExpandDir { entry_id, done } => { let path = { let mut state = self.state.lock(); state.expanded_dirs.insert(entry_id); @@ -2978,7 +2981,7 @@ impl BackgroundScanner { .await; if let Some(job) = scan_job_rx.next().await { self.scan_dir(&job).await.log_err(); - self.send_status_update(false, None); + self.send_status_update(false, Some(done)); } } true diff --git a/crates/project/src/worktree_tests.rs b/crates/project/src/worktree_tests.rs index 6468ac9a16..00697d04a2 100644 --- a/crates/project/src/worktree_tests.rs +++ b/crates/project/src/worktree_tests.rs @@ -8,6 +8,7 @@ use fs::{repository::GitFileStatus, FakeFs, Fs, RealFs, RemoveOptions}; use git::GITIGNORE; use gpui::{executor::Deterministic, ModelContext, Task, TestAppContext}; use parking_lot::Mutex; +use postage::stream::Stream; use pretty_assertions::assert_eq; use rand::prelude::*; use serde_json::json; @@ -154,7 +155,17 @@ async fn test_descendent_entries(cx: &mut TestAppContext) { .collect::>(), vec![Path::new("g"), Path::new("g/h"),] ); + }); + // Expand gitignored directory. + tree.update(cx, |tree, cx| { + let tree = tree.as_local_mut().unwrap(); + tree.expand_dir(tree.entry_for_path("i/j").unwrap().id, cx) + }) + .recv() + .await; + + tree.read_with(cx, |tree, _| { assert_eq!( tree.descendent_entries(false, false, Path::new("i")) .map(|entry| entry.path.as_ref()) From 6fe74602ac0e1dce7843bd10635a444700ec3c56 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 16 Jun 2023 09:42:47 -0700 Subject: [PATCH 06/21] Fix detection of when refreshed paths are outside of worktree root --- crates/project/src/worktree.rs | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 0480461556..c508b84f6c 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -3338,7 +3338,19 @@ impl BackgroundScanner { let metadata = futures::future::join_all( abs_paths .iter() - .map(|abs_path| self.fs.metadata(&abs_path)) + .map(|abs_path| async move { + let metadata = self.fs.metadata(&abs_path).await?; + anyhow::Ok(if let Some(metadata) = metadata { + let canonical_path = if metadata.is_symlink { + self.fs.canonicalize(&abs_path).await? + } else { + abs_path.clone() + }; + Some((metadata, canonical_path)) + } else { + None + }) + }) .collect::>(), ) .await; @@ -3376,7 +3388,7 @@ impl BackgroundScanner { let abs_path: Arc = root_abs_path.join(&path).into(); match metadata { - Ok(Some(metadata)) => { + Ok(Some((metadata, canonical_path))) => { let ignore_stack = state .snapshot .ignore_stack_for_abs_path(&abs_path, metadata.is_dir); @@ -3411,7 +3423,7 @@ impl BackgroundScanner { if let Some(scan_queue_tx) = &scan_queue_tx { let mut ancestor_inodes = state.snapshot.ancestor_inodes_for_path(&path); if metadata.is_dir && !ancestor_inodes.contains(&metadata.inode) { - let is_outside_root = !abs_path.starts_with(&root_canonical_path); + let is_outside_root = !canonical_path.starts_with(&root_canonical_path); ancestor_inodes.insert(metadata.inode); smol::block_on(scan_queue_tx.send(ScanJob { abs_path, From 3c06bd056a15499fe410f1d3a8fd7c639df9a1ee Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 16 Jun 2023 10:46:14 -0700 Subject: [PATCH 07/21] Load git repositories when inserting the entry for the .git Don't wait until populating that directory entry, for two reasons: * In the case of submodules, .git is not a directory * We don't eagerly populate .git directories, since their contents are automatically ignored. --- crates/project/src/worktree.rs | 57 +++++++++++++++++----------------- 1 file changed, 29 insertions(+), 28 deletions(-) diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index c508b84f6c..1ae0f6067a 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -2218,17 +2218,23 @@ impl BackgroundScannerState { fn insert_entry(&mut self, mut entry: Entry, fs: &dyn Fs) -> Entry { self.reuse_entry_id(&mut entry); - self.snapshot.insert_entry(entry, fs) + let entry = self.snapshot.insert_entry(entry, fs); + if entry.path.file_name() == Some(&DOT_GIT) { + let changed_paths = self.snapshot.build_repo(entry.path.clone(), fs); + if let Some(changed_paths) = changed_paths { + util::extend_sorted(&mut self.changed_paths, changed_paths, usize::MAX, Ord::cmp) + } + } + entry } - #[must_use = "Changed paths must be used for diffing later"] fn populate_dir( &mut self, - parent_path: Arc, + parent_path: &Arc, entries: impl IntoIterator, ignore: Option>, fs: &dyn Fs, - ) -> Option>> { + ) { let mut parent_entry = if let Some(parent_entry) = self .snapshot .entries_by_path @@ -2240,15 +2246,13 @@ impl BackgroundScannerState { "populating a directory {:?} that has been removed", parent_path ); - return None; + return; }; match parent_entry.kind { - EntryKind::PendingDir => { - parent_entry.kind = EntryKind::Dir; - } + EntryKind::PendingDir => parent_entry.kind = EntryKind::Dir, EntryKind::Dir => {} - _ => return None, + _ => return, } if let Some(ignore) = ignore { @@ -2260,8 +2264,13 @@ impl BackgroundScannerState { let mut entries_by_path_edits = vec![Edit::Insert(parent_entry)]; let mut entries_by_id_edits = Vec::new(); + let mut dotgit_path = None; for mut entry in entries { + if entry.path.file_name() == Some(&DOT_GIT) { + dotgit_path = Some(entry.path.clone()); + } + self.reuse_entry_id(&mut entry); entries_by_id_edits.push(Edit::Insert(PathEntry { id: entry.id, @@ -2277,10 +2286,15 @@ impl BackgroundScannerState { .edit(entries_by_path_edits, &()); self.snapshot.entries_by_id.edit(entries_by_id_edits, &()); - if parent_path.file_name() == Some(&DOT_GIT) { - return self.snapshot.build_repo(parent_path, fs); + if let Some(dotgit_path) = dotgit_path { + let changed_paths = self.snapshot.build_repo(dotgit_path, fs); + if let Some(changed_paths) = changed_paths { + util::extend_sorted(&mut self.changed_paths, changed_paths, usize::MAX, Ord::cmp) + } + } + if let Err(ix) = self.changed_paths.binary_search(parent_path) { + self.changed_paths.insert(ix, parent_path.clone()); } - None } fn remove_path(&mut self, path: &Path) { @@ -3297,22 +3311,9 @@ impl BackgroundScanner { new_entries.push(child_entry); } - { - let mut state = self.state.lock(); - let changed_paths = - state.populate_dir(job.path.clone(), new_entries, new_ignore, self.fs.as_ref()); - if let Err(ix) = state.changed_paths.binary_search(&job.path) { - state.changed_paths.insert(ix, job.path.clone()); - } - if let Some(changed_paths) = changed_paths { - util::extend_sorted( - &mut state.changed_paths, - changed_paths, - usize::MAX, - Ord::cmp, - ) - } - } + self.state + .lock() + .populate_dir(&job.path, new_entries, new_ignore, self.fs.as_ref()); for new_job in new_jobs { if let Some(new_job) = new_job { From 1b71589514a047115c66a8601b898defb6c87d0e Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 16 Jun 2023 11:53:05 -0700 Subject: [PATCH 08/21] Fix confusion between canonical vs non-canonical paths when rescanning, expanding paths --- crates/project/src/worktree.rs | 125 +++++++++++++++++---------- crates/project/src/worktree_tests.rs | 9 +- 2 files changed, 87 insertions(+), 47 deletions(-) diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 1ae0f6067a..5693dd8f12 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -86,7 +86,7 @@ pub struct LocalWorktree { enum ScanRequest { RescanPaths { - paths: Vec, + relative_paths: Vec>, done: barrier::Sender, }, ExpandDir { @@ -1051,14 +1051,10 @@ impl LocalWorktree { cx: &mut ModelContext, ) -> Option>> { let entry = self.entry_for_id(entry_id)?.clone(); - let abs_path = self.abs_path.clone(); + let abs_path = self.absolutize(&entry.path); let fs = self.fs.clone(); let delete = cx.background().spawn(async move { - let mut abs_path = fs.canonicalize(&abs_path).await?; - if entry.path.file_name().is_some() { - abs_path = abs_path.join(&entry.path); - } if entry.is_file() { fs.remove_file(&abs_path, Default::default()).await?; } else { @@ -1071,18 +1067,18 @@ impl LocalWorktree { ) .await?; } - anyhow::Ok(abs_path) + anyhow::Ok(entry.path) }); Some(cx.spawn(|this, mut cx| async move { - let abs_path = delete.await?; + let path = delete.await?; let (tx, mut rx) = barrier::channel(); this.update(&mut cx, |this, _| { this.as_local_mut() .unwrap() .scan_requests_tx .try_send(ScanRequest::RescanPaths { - paths: vec![abs_path], + relative_paths: vec![path], done: tx, }) })?; @@ -1168,27 +1164,19 @@ impl LocalWorktree { old_path: Option>, cx: &mut ModelContext, ) -> Task> { - let fs = self.fs.clone(); - let abs_root_path = self.abs_path.clone(); let path_changes_tx = self.scan_requests_tx.clone(); cx.spawn_weak(move |this, mut cx| async move { - let abs_path = fs.canonicalize(&abs_root_path).await?; - let mut paths = Vec::with_capacity(2); - paths.push(if path.file_name().is_some() { - abs_path.join(&path) + let relative_paths = if let Some(old_path) = old_path.as_ref() { + vec![old_path.clone(), path.clone()] } else { - abs_path.clone() - }); - if let Some(old_path) = old_path { - paths.push(if old_path.file_name().is_some() { - abs_path.join(&old_path) - } else { - abs_path.clone() - }); - } + vec![path.clone()] + }; let (tx, mut rx) = barrier::channel(); - path_changes_tx.try_send(ScanRequest::RescanPaths { paths, done: tx })?; + path_changes_tx.try_send(ScanRequest::RescanPaths { + relative_paths, + done: tx, + })?; rx.recv().await; this.upgrade(&cx) .ok_or_else(|| anyhow!("worktree was dropped"))? @@ -2975,38 +2963,81 @@ impl BackgroundScanner { } async fn process_scan_request(&self, request: ScanRequest) -> bool { + let root_path = self.state.lock().snapshot.abs_path.clone(); + let root_canonical_path = match self.fs.canonicalize(&root_path).await { + Ok(path) => path, + Err(err) => { + log::error!("failed to canonicalize root path: {}", err); + return false; + } + }; + match request { - ScanRequest::RescanPaths { paths, done } => { - self.reload_entries_for_paths(paths, None).await; + ScanRequest::RescanPaths { + relative_paths, + done, + } => { + let abs_paths = relative_paths + .into_iter() + .map(|path| { + if path.file_name().is_some() { + root_canonical_path.join(path) + } else { + root_canonical_path.clone() + } + }) + .collect::>(); + self.reload_entries_for_paths(root_path, root_canonical_path, abs_paths, None) + .await; self.send_status_update(false, Some(done)) } ScanRequest::ExpandDir { entry_id, done } => { - let path = { + let abs_path; + { let mut state = self.state.lock(); - state.expanded_dirs.insert(entry_id); - state - .snapshot - .entry_for_id(entry_id) - .map(|e| state.snapshot.absolutize(&e.path)) - }; - if let Some(path) = path { - let (scan_job_tx, mut scan_job_rx) = channel::unbounded(); - self.reload_entries_for_paths(vec![path.clone()], Some(scan_job_tx)) - .await; - if let Some(job) = scan_job_rx.next().await { - self.scan_dir(&job).await.log_err(); - self.send_status_update(false, Some(done)); + if let Some(entry) = state.snapshot.entry_for_id(entry_id) { + abs_path = root_canonical_path.join(&entry.path); + state.expanded_dirs.insert(entry_id); + } else { + return true; } + }; + + let (scan_job_tx, mut scan_job_rx) = channel::unbounded(); + self.reload_entries_for_paths( + root_path, + root_canonical_path, + vec![abs_path], + Some(scan_job_tx), + ) + .await; + if let Some(job) = scan_job_rx.next().await { + self.scan_dir(&job).await.log_err(); + self.send_status_update(false, Some(done)); } true } } } - async fn process_events(&mut self, paths: Vec) { + async fn process_events(&mut self, abs_paths: Vec) { + let root_path = self.state.lock().snapshot.abs_path.clone(); + let root_canonical_path = match self.fs.canonicalize(&root_path).await { + Ok(path) => path, + Err(err) => { + log::error!("failed to canonicalize root path: {}", err); + return; + } + }; + let (scan_job_tx, scan_job_rx) = channel::unbounded(); let paths = self - .reload_entries_for_paths(paths, Some(scan_job_tx.clone())) + .reload_entries_for_paths( + root_path, + root_canonical_path, + abs_paths, + Some(scan_job_tx.clone()), + ) .await; drop(scan_job_tx); self.scan_dirs(false, scan_job_rx).await; @@ -3152,6 +3183,7 @@ impl BackgroundScanner { if job.is_outside_root || job.ignore_stack.is_all() { if let Some(entry) = state.snapshot.entry_for_path(&job.path) { if !state.is_entry_expanded(entry) { + log::debug!("defer scanning directory {:?}", job.path); return Ok(()); } } @@ -3168,6 +3200,8 @@ impl BackgroundScanner { ) }; + log::debug!("scan directory {:?}", job.path); + let mut root_canonical_path = None; let mut child_paths = self.fs.read_dir(&job.abs_path).await?; while let Some(child_abs_path) = child_paths.next().await { @@ -3326,16 +3360,15 @@ impl BackgroundScanner { async fn reload_entries_for_paths( &self, + root_abs_path: Arc, + root_canonical_path: PathBuf, mut abs_paths: Vec, scan_queue_tx: Option>, ) -> Option>> { let doing_recursive_update = scan_queue_tx.is_some(); - abs_paths.sort_unstable(); abs_paths.dedup_by(|a, b| a.starts_with(&b)); - let root_abs_path = self.state.lock().snapshot.abs_path.clone(); - let root_canonical_path = self.fs.canonicalize(&root_abs_path).await.log_err()?; let metadata = futures::future::join_all( abs_paths .iter() diff --git a/crates/project/src/worktree_tests.rs b/crates/project/src/worktree_tests.rs index 00697d04a2..216e14f51c 100644 --- a/crates/project/src/worktree_tests.rs +++ b/crates/project/src/worktree_tests.rs @@ -369,7 +369,14 @@ async fn test_rescan_with_gitignore(cx: &mut TestAppContext) { .unwrap(); cx.read(|cx| tree.read(cx).as_local().unwrap().scan_complete()) .await; - tree.flush_fs_events(cx).await; + + tree.update(cx, |tree, cx| { + let tree = tree.as_local_mut().unwrap(); + tree.expand_dir(tree.entry_for_path("ignored-dir").unwrap().id, cx) + }) + .recv() + .await; + cx.read(|cx| { let tree = tree.read(cx); assert!( From cd823ede4d8b714de1ee3fd0e996973c9e4829e3 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 16 Jun 2023 14:19:48 -0700 Subject: [PATCH 09/21] Add a bit to each entry indicating if it's outside of the worktree root --- .../20221109000000_test_schema.sql | 1 + ...35_add_is_external_to_worktree_entries.sql | 2 + crates/collab/src/db.rs | 3 + crates/collab/src/db/worktree_entry.rs | 1 + crates/project/src/worktree.rs | 50 ++++++----- crates/project/src/worktree_tests.rs | 83 ++++++++++++++++--- crates/project_panel/src/project_panel.rs | 1 + crates/rpc/proto/zed.proto | 3 +- 8 files changed, 110 insertions(+), 34 deletions(-) create mode 100644 crates/collab/migrations/20230616134535_add_is_external_to_worktree_entries.sql diff --git a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql index 595d841d07..c690b6148a 100644 --- a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql +++ b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql @@ -74,6 +74,7 @@ CREATE TABLE "worktree_entries" ( "mtime_seconds" INTEGER NOT NULL, "mtime_nanos" INTEGER NOT NULL, "is_symlink" BOOL NOT NULL, + "is_external" BOOL NOT NULL, "is_ignored" BOOL NOT NULL, "is_deleted" BOOL NOT NULL, "git_status" INTEGER, diff --git a/crates/collab/migrations/20230616134535_add_is_external_to_worktree_entries.sql b/crates/collab/migrations/20230616134535_add_is_external_to_worktree_entries.sql new file mode 100644 index 0000000000..e4348af0cc --- /dev/null +++ b/crates/collab/migrations/20230616134535_add_is_external_to_worktree_entries.sql @@ -0,0 +1,2 @@ +ALTER TABLE "worktree_entries" +ADD "is_external" BOOL NOT NULL DEFAULT FALSE; diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index 7e2c376bc2..208da22efe 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -1539,6 +1539,7 @@ impl Database { }), is_symlink: db_entry.is_symlink, is_ignored: db_entry.is_ignored, + is_external: db_entry.is_external, git_status: db_entry.git_status.map(|status| status as i32), }); } @@ -2349,6 +2350,7 @@ impl Database { mtime_nanos: ActiveValue::set(mtime.nanos as i32), is_symlink: ActiveValue::set(entry.is_symlink), is_ignored: ActiveValue::set(entry.is_ignored), + is_external: ActiveValue::set(entry.is_external), git_status: ActiveValue::set(entry.git_status.map(|status| status as i64)), is_deleted: ActiveValue::set(false), scan_id: ActiveValue::set(update.scan_id as i64), @@ -2705,6 +2707,7 @@ impl Database { }), is_symlink: db_entry.is_symlink, is_ignored: db_entry.is_ignored, + is_external: db_entry.is_external, git_status: db_entry.git_status.map(|status| status as i32), }); } diff --git a/crates/collab/src/db/worktree_entry.rs b/crates/collab/src/db/worktree_entry.rs index f2df808ee3..cf5090ab6d 100644 --- a/crates/collab/src/db/worktree_entry.rs +++ b/crates/collab/src/db/worktree_entry.rs @@ -18,6 +18,7 @@ pub struct Model { pub git_status: Option, pub is_symlink: bool, pub is_ignored: bool, + pub is_external: bool, pub is_deleted: bool, pub scan_id: i64, } diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 5693dd8f12..6be61e8b9c 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -2604,6 +2604,7 @@ pub struct Entry { pub mtime: SystemTime, pub is_symlink: bool, pub is_ignored: bool, + pub is_external: bool, pub git_status: Option, } @@ -2657,6 +2658,7 @@ impl Entry { mtime: metadata.mtime, is_symlink: metadata.is_symlink, is_ignored: false, + is_external: false, git_status: None, } } @@ -2912,7 +2914,7 @@ impl BackgroundScanner { path: Arc::from(Path::new("")), ignore_stack, ancestor_inodes: TreeSet::from_ordered_entries(root_inode), - is_outside_root: false, + is_external: false, scan_queue: scan_job_tx.clone(), })) .unwrap(); @@ -3180,7 +3182,7 @@ impl BackgroundScanner { let mut new_ignore = None; let (root_abs_path, root_char_bag, next_entry_id, repository) = { let state = self.state.lock(); - if job.is_outside_root || job.ignore_stack.is_all() { + if job.is_external || job.ignore_stack.is_all() { if let Some(entry) = state.snapshot.entry_for_path(&job.path) { if !state.is_entry_expanded(entry) { log::debug!("defer scanning directory {:?}", job.path); @@ -3200,7 +3202,11 @@ impl BackgroundScanner { ) }; - log::debug!("scan directory {:?}", job.path); + log::debug!( + "scan directory {:?}. external: {}", + job.path, + job.is_external + ); let mut root_canonical_path = None; let mut child_paths = self.fs.read_dir(&job.abs_path).await?; @@ -3270,8 +3276,9 @@ impl BackgroundScanner { root_char_bag, ); - let mut is_outside_root = false; - if child_metadata.is_symlink { + if job.is_external { + child_entry.is_external = true; + } else if child_metadata.is_symlink { let canonical_path = match self.fs.canonicalize(&child_abs_path).await { Ok(path) => path, Err(err) => { @@ -3298,13 +3305,12 @@ impl BackgroundScanner { }; if !canonical_path.starts_with(root_canonical_path) { - is_outside_root = true; + child_entry.is_external = true; } } if child_entry.is_dir() { - let is_ignored = ignore_stack.is_abs_path_ignored(&child_abs_path, true); - child_entry.is_ignored = is_ignored; + child_entry.is_ignored = ignore_stack.is_abs_path_ignored(&child_abs_path, true); // Avoid recursing until crash in the case of a recursive symlink if !job.ancestor_inodes.contains(&child_entry.inode) { @@ -3314,8 +3320,8 @@ impl BackgroundScanner { new_jobs.push(Some(ScanJob { abs_path: child_abs_path, path: child_path, - is_outside_root, - ignore_stack: if is_ignored { + is_external: child_entry.is_external, + ignore_stack: if child_entry.is_ignored { IgnoreStack::all() } else { ignore_stack.clone() @@ -3374,16 +3380,12 @@ impl BackgroundScanner { .iter() .map(|abs_path| async move { let metadata = self.fs.metadata(&abs_path).await?; - anyhow::Ok(if let Some(metadata) = metadata { - let canonical_path = if metadata.is_symlink { - self.fs.canonicalize(&abs_path).await? - } else { - abs_path.clone() - }; - Some((metadata, canonical_path)) + if let Some(metadata) = metadata { + let canonical_path = self.fs.canonicalize(&abs_path).await?; + anyhow::Ok(Some((metadata, canonical_path))) } else { - None - }) + Ok(None) + } }) .collect::>(), ) @@ -3434,6 +3436,7 @@ impl BackgroundScanner { state.snapshot.root_char_bag, ); fs_entry.is_ignored = ignore_stack.is_all(); + fs_entry.is_external = !canonical_path.starts_with(&root_canonical_path); if !fs_entry.is_ignored { if !fs_entry.is_dir() { @@ -3452,19 +3455,18 @@ impl BackgroundScanner { } } - state.insert_entry(fs_entry, self.fs.as_ref()); + let fs_entry = state.insert_entry(fs_entry, self.fs.as_ref()); if let Some(scan_queue_tx) = &scan_queue_tx { let mut ancestor_inodes = state.snapshot.ancestor_inodes_for_path(&path); if metadata.is_dir && !ancestor_inodes.contains(&metadata.inode) { - let is_outside_root = !canonical_path.starts_with(&root_canonical_path); ancestor_inodes.insert(metadata.inode); smol::block_on(scan_queue_tx.send(ScanJob { abs_path, path, ignore_stack, ancestor_inodes, - is_outside_root, + is_external: fs_entry.is_external, scan_queue: scan_queue_tx.clone(), })) .unwrap(); @@ -3853,7 +3855,7 @@ struct ScanJob { ignore_stack: Arc, scan_queue: Sender, ancestor_inodes: TreeSet, - is_outside_root: bool, + is_external: bool, } struct UpdateIgnoreStatusJob { @@ -4141,6 +4143,7 @@ impl<'a> From<&'a Entry> for proto::Entry { mtime: Some(entry.mtime.into()), is_symlink: entry.is_symlink, is_ignored: entry.is_ignored, + is_external: entry.is_external, git_status: entry.git_status.map(|status| status.to_proto()), } } @@ -4167,6 +4170,7 @@ impl<'a> TryFrom<(&'a CharBag, proto::Entry)> for Entry { mtime: mtime.into(), is_symlink: entry.is_symlink, is_ignored: entry.is_ignored, + is_external: entry.is_external, git_status: GitFileStatus::from_proto(entry.git_status), }) } else { diff --git a/crates/project/src/worktree_tests.rs b/crates/project/src/worktree_tests.rs index 216e14f51c..83f416d784 100644 --- a/crates/project/src/worktree_tests.rs +++ b/crates/project/src/worktree_tests.rs @@ -267,7 +267,7 @@ async fn test_circular_symlinks(executor: Arc, cx: &mut TestAppCo }); } -#[gpui::test(iterations = 10)] +#[gpui::test] async fn test_symlinks_pointing_outside(cx: &mut TestAppContext) { let fs = FakeFs::new(cx.background()); fs.insert_tree( @@ -289,14 +289,17 @@ async fn test_symlinks_pointing_outside(cx: &mut TestAppContext) { } }, "dir3": { + "deps": {}, "src": { "e.rs": "", "f.rs": "", - } + }, } }), ) .await; + + // These symlinks point to directories outside of the worktree's root, dir1. fs.insert_symlink("/root/dir1/deps/dep-dir2", "../../dir2".into()) .await; fs.insert_symlink("/root/dir1/deps/dep-dir3", "../../dir3".into()) @@ -317,19 +320,79 @@ async fn test_symlinks_pointing_outside(cx: &mut TestAppContext) { cx.read(|cx| tree.read(cx).as_local().unwrap().scan_complete()) .await; + // The symlinked directories are not scanned by default. tree.read_with(cx, |tree, _| { assert_eq!( tree.entries(false) - .map(|entry| entry.path.as_ref()) + .map(|entry| (entry.path.as_ref(), entry.is_external)) .collect::>(), vec![ - Path::new(""), - Path::new("deps"), - Path::new("deps/dep-dir2"), - Path::new("deps/dep-dir3"), - Path::new("src"), - Path::new("src/a.rs"), - Path::new("src/b.rs"), + (Path::new(""), false), + (Path::new("deps"), false), + (Path::new("deps/dep-dir2"), true), + (Path::new("deps/dep-dir3"), true), + (Path::new("src"), false), + (Path::new("src/a.rs"), false), + (Path::new("src/b.rs"), false), + ] + ); + }); + + // Expand one of the symlinked directories. + tree.update(cx, |tree, cx| { + let tree = tree.as_local_mut().unwrap(); + tree.expand_dir(tree.entry_for_path("deps/dep-dir3").unwrap().id, cx) + }) + .recv() + .await; + + // The expanded directory's contents are loaded. Subdirectories are + // not scanned yet. + tree.read_with(cx, |tree, _| { + assert_eq!( + tree.entries(false) + .map(|entry| (entry.path.as_ref(), entry.is_external)) + .collect::>(), + vec![ + (Path::new(""), false), + (Path::new("deps"), false), + (Path::new("deps/dep-dir2"), true), + (Path::new("deps/dep-dir3"), true), + (Path::new("deps/dep-dir3/deps"), true), + (Path::new("deps/dep-dir3/src"), true), + (Path::new("src"), false), + (Path::new("src/a.rs"), false), + (Path::new("src/b.rs"), false), + ] + ); + }); + + // Expand a subdirectory of one of the symlinked directories. + tree.update(cx, |tree, cx| { + let tree = tree.as_local_mut().unwrap(); + tree.expand_dir(tree.entry_for_path("deps/dep-dir3/src").unwrap().id, cx) + }) + .recv() + .await; + + // The expanded subdirectory's contents are loaded. + tree.read_with(cx, |tree, _| { + assert_eq!( + tree.entries(false) + .map(|entry| (entry.path.as_ref(), entry.is_external)) + .collect::>(), + vec![ + (Path::new(""), false), + (Path::new("deps"), false), + (Path::new("deps/dep-dir2"), true), + (Path::new("deps/dep-dir3"), true), + (Path::new("deps/dep-dir3/deps"), true), + (Path::new("deps/dep-dir3/src"), true), + (Path::new("deps/dep-dir3/src/e.rs"), true), + (Path::new("deps/dep-dir3/src/f.rs"), true), + (Path::new("src"), false), + (Path::new("src/a.rs"), false), + (Path::new("src/b.rs"), false), ] ); }); diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 1291835ae9..c8dfb7cb38 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -1022,6 +1022,7 @@ impl ProjectPanel { mtime: entry.mtime, is_symlink: false, is_ignored: false, + is_external: false, git_status: entry.git_status, }); } diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index f3a9f7eab5..78aab23a44 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -1011,7 +1011,8 @@ message Entry { Timestamp mtime = 5; bool is_symlink = 6; bool is_ignored = 7; - optional GitStatus git_status = 8; + bool is_external = 8; + optional GitStatus git_status = 9; } message RepositoryEntry { From 3e6aedfc694894966ae89b2c85ab338c83b8f7a9 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 16 Jun 2023 15:49:27 -0700 Subject: [PATCH 10/21] Expand dirs on-demand when opening buffers inside unloaded dirs --- crates/project/src/project.rs | 10 +- crates/project/src/worktree.rs | 170 ++++++++++++++++----------- crates/project/src/worktree_tests.rs | 100 +++++++++++++++- 3 files changed, 206 insertions(+), 74 deletions(-) diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index a360e5dd72..5c71202956 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -5413,7 +5413,10 @@ impl Project { if self.is_local() { let worktree = self.worktree_for_id(worktree_id, cx)?; worktree.update(cx, |worktree, cx| { - worktree.as_local_mut().unwrap().expand_dir(entry_id, cx); + worktree + .as_local_mut() + .unwrap() + .expand_entry_for_id(entry_id, cx); }); } else if let Some(project_id) = self.remote_id() { cx.background() @@ -5739,7 +5742,10 @@ impl Project { .read_with(&cx, |this, cx| this.worktree_for_entry(entry_id, cx)) .ok_or_else(|| anyhow!("invalid request"))?; worktree.update(&mut cx, |worktree, cx| { - worktree.as_local_mut().unwrap().expand_dir(entry_id, cx) + worktree + .as_local_mut() + .unwrap() + .expand_entry_for_id(entry_id, cx) }); Ok(proto::Ack {}) } diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 6be61e8b9c..bbf63901d1 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -89,8 +89,8 @@ enum ScanRequest { relative_paths: Vec>, done: barrier::Sender, }, - ExpandDir { - entry_id: ProjectEntryId, + ExpandPath { + path: Arc, done: barrier::Sender, }, } @@ -879,26 +879,31 @@ impl LocalWorktree { path: &Path, cx: &mut ModelContext, ) -> Task)>> { - let handle = cx.handle(); let path = Arc::from(path); let abs_path = self.absolutize(&path); let fs = self.fs.clone(); - let snapshot = self.snapshot(); - - let mut index_task = None; - - if let Some(repo) = snapshot.repository_for_path(&path) { - let repo_path = repo.work_directory.relativize(self, &path).unwrap(); - if let Some(repo) = self.git_repositories.get(&*repo.work_directory) { - let repo = repo.repo_ptr.to_owned(); - index_task = Some( - cx.background() - .spawn(async move { repo.lock().load_index_text(&repo_path) }), - ); - } - } + let expand = path + .parent() + .map(|path| self.expand_entry_for_path(path, cx)); cx.spawn(|this, mut cx| async move { + if let Some(mut expand) = expand { + expand.recv().await; + } + + let mut index_task = None; + let snapshot = this.read_with(&cx, |this, _| this.as_local().unwrap().snapshot()); + if let Some(repo) = snapshot.repository_for_path(&path) { + let repo_path = repo.work_directory.relativize(&snapshot, &path).unwrap(); + if let Some(repo) = snapshot.git_repositories.get(&*repo.work_directory) { + let repo = repo.repo_ptr.clone(); + index_task = Some( + cx.background() + .spawn(async move { repo.lock().load_index_text(&repo_path) }), + ); + } + } + let text = fs.load(&abs_path).await?; let diff_base = if let Some(index_task) = index_task { @@ -917,7 +922,7 @@ impl LocalWorktree { Ok(( File { entry_id: entry.id, - worktree: handle, + worktree: this, path: entry.path, mtime: entry.mtime, is_local: true, @@ -1146,14 +1151,34 @@ impl LocalWorktree { })) } - pub fn expand_dir( + pub fn expand_entry_for_id( &mut self, entry_id: ProjectEntryId, _cx: &mut ModelContext, + ) -> barrier::Receiver { + let (tx, rx) = barrier::channel(); + if let Some(entry) = self.entry_for_id(entry_id) { + self.scan_requests_tx + .try_send(ScanRequest::ExpandPath { + path: entry.path.clone(), + done: tx, + }) + .ok(); + } + rx + } + + pub fn expand_entry_for_path( + &self, + path: impl Into>, + _cx: &mut ModelContext, ) -> barrier::Receiver { let (tx, rx) = barrier::channel(); self.scan_requests_tx - .try_send(ScanRequest::ExpandDir { entry_id, done: tx }) + .try_send(ScanRequest::ExpandPath { + path: path.into(), + done: tx, + }) .ok(); rx } @@ -2192,10 +2217,6 @@ impl LocalSnapshot { } impl BackgroundScannerState { - fn is_entry_expanded(&self, entry: &Entry) -> bool { - self.expanded_dirs.contains(&entry.id) - } - fn reuse_entry_id(&mut self, entry: &mut Entry) { if let Some(removed_entry_id) = self.removed_entry_ids.remove(&entry.inode) { entry.id = removed_entry_id; @@ -2989,34 +3010,39 @@ impl BackgroundScanner { } }) .collect::>(); - self.reload_entries_for_paths(root_path, root_canonical_path, abs_paths, None) - .await; - self.send_status_update(false, Some(done)) - } - ScanRequest::ExpandDir { entry_id, done } => { - let abs_path; - { - let mut state = self.state.lock(); - if let Some(entry) = state.snapshot.entry_for_id(entry_id) { - abs_path = root_canonical_path.join(&entry.path); - state.expanded_dirs.insert(entry_id); - } else { - return true; - } - }; - - let (scan_job_tx, mut scan_job_rx) = channel::unbounded(); self.reload_entries_for_paths( root_path, root_canonical_path, - vec![abs_path], - Some(scan_job_tx), + abs_paths, + false, + None, ) .await; - if let Some(job) = scan_job_rx.next().await { - self.scan_dir(&job).await.log_err(); - self.send_status_update(false, Some(done)); + self.send_status_update(false, Some(done)) + } + ScanRequest::ExpandPath { path, done } => { + let (scan_job_tx, mut scan_job_rx) = channel::unbounded(); + let mut abs_path = root_canonical_path.clone(); + for component in path.iter() { + abs_path.push(component); + self.reload_entries_for_paths( + root_path.clone(), + root_canonical_path.clone(), + vec![abs_path.clone()], + true, + Some(scan_job_tx.clone()), + ) + .await; } + drop(scan_job_tx); + + // Scan the expanded directories serially. This is ok, because only + // the direct ancestors of the expanded path need to be scanned. + while let Some(job) = scan_job_rx.next().await { + self.scan_dir(&job).await.log_err(); + } + + self.send_status_update(false, Some(done)); true } } @@ -3038,6 +3064,7 @@ impl BackgroundScanner { root_path, root_canonical_path, abs_paths, + false, Some(scan_job_tx.clone()), ) .await; @@ -3176,22 +3203,18 @@ impl BackgroundScanner { } async fn scan_dir(&self, job: &ScanJob) -> Result<()> { + log::debug!( + "scan directory {:?}. external: {}", + job.path, + job.is_external + ); + let mut new_entries: Vec = Vec::new(); let mut new_jobs: Vec> = Vec::new(); let mut ignore_stack = job.ignore_stack.clone(); let mut new_ignore = None; let (root_abs_path, root_char_bag, next_entry_id, repository) = { - let state = self.state.lock(); - if job.is_external || job.ignore_stack.is_all() { - if let Some(entry) = state.snapshot.entry_for_path(&job.path) { - if !state.is_entry_expanded(entry) { - log::debug!("defer scanning directory {:?}", job.path); - return Ok(()); - } - } - } - - let snapshot = &state.snapshot; + let snapshot = &self.state.lock().snapshot; ( snapshot.abs_path().clone(), snapshot.root_char_bag, @@ -3202,12 +3225,6 @@ impl BackgroundScanner { ) }; - log::debug!( - "scan directory {:?}. external: {}", - job.path, - job.is_external - ); - let mut root_canonical_path = None; let mut child_paths = self.fs.read_dir(&job.abs_path).await?; while let Some(child_abs_path) = child_paths.next().await { @@ -3258,7 +3275,7 @@ impl BackgroundScanner { ignore_stack.is_abs_path_ignored(&entry_abs_path, entry.is_dir()); if entry.is_dir() { - if let Some(job) = new_jobs.next().expect("Missing scan job for entry") { + if let Some(job) = new_jobs.next().expect("missing scan job for entry") { job.ignore_stack = if entry.is_ignored { IgnoreStack::all() } else { @@ -3351,13 +3368,26 @@ impl BackgroundScanner { new_entries.push(child_entry); } - self.state - .lock() - .populate_dir(&job.path, new_entries, new_ignore, self.fs.as_ref()); + let mut state = self.state.lock(); + state.populate_dir(&job.path, new_entries, new_ignore, self.fs.as_ref()); for new_job in new_jobs { if let Some(new_job) = new_job { - job.scan_queue.send(new_job).await.ok(); + // If a subdirectory is ignored, or is external to the worktree, don't scan + // it unless it is marked as expanded. + if (new_job.is_external || new_job.ignore_stack.is_all()) + && !state + .snapshot + .entry_for_path(&new_job.path) + .map_or(true, |entry| state.expanded_dirs.contains(&entry.id)) + { + log::debug!("defer scanning directory {:?}", new_job.path); + continue; + } + + job.scan_queue + .try_send(new_job) + .expect("channel is unbounded"); } } @@ -3369,6 +3399,7 @@ impl BackgroundScanner { root_abs_path: Arc, root_canonical_path: PathBuf, mut abs_paths: Vec, + expand: bool, scan_queue_tx: Option>, ) -> Option>> { let doing_recursive_update = scan_queue_tx.is_some(); @@ -3456,6 +3487,9 @@ impl BackgroundScanner { } let fs_entry = state.insert_entry(fs_entry, self.fs.as_ref()); + if expand { + state.expanded_dirs.insert(fs_entry.id); + } if let Some(scan_queue_tx) = &scan_queue_tx { let mut ancestor_inodes = state.snapshot.ancestor_inodes_for_path(&path); diff --git a/crates/project/src/worktree_tests.rs b/crates/project/src/worktree_tests.rs index 83f416d784..92eded0a99 100644 --- a/crates/project/src/worktree_tests.rs +++ b/crates/project/src/worktree_tests.rs @@ -160,7 +160,7 @@ async fn test_descendent_entries(cx: &mut TestAppContext) { // Expand gitignored directory. tree.update(cx, |tree, cx| { let tree = tree.as_local_mut().unwrap(); - tree.expand_dir(tree.entry_for_path("i/j").unwrap().id, cx) + tree.expand_entry_for_path("i/j".as_ref(), cx) }) .recv() .await; @@ -341,7 +341,7 @@ async fn test_symlinks_pointing_outside(cx: &mut TestAppContext) { // Expand one of the symlinked directories. tree.update(cx, |tree, cx| { let tree = tree.as_local_mut().unwrap(); - tree.expand_dir(tree.entry_for_path("deps/dep-dir3").unwrap().id, cx) + tree.expand_entry_for_path("deps/dep-dir3".as_ref(), cx) }) .recv() .await; @@ -370,7 +370,7 @@ async fn test_symlinks_pointing_outside(cx: &mut TestAppContext) { // Expand a subdirectory of one of the symlinked directories. tree.update(cx, |tree, cx| { let tree = tree.as_local_mut().unwrap(); - tree.expand_dir(tree.entry_for_path("deps/dep-dir3/src").unwrap().id, cx) + tree.expand_entry_for_path("deps/dep-dir3/src".as_ref(), cx) }) .recv() .await; @@ -398,6 +398,98 @@ async fn test_symlinks_pointing_outside(cx: &mut TestAppContext) { }); } +#[gpui::test] +async fn test_open_gitignored_files(cx: &mut TestAppContext) { + let fs = FakeFs::new(cx.background()); + fs.insert_tree( + "/root", + json!({ + ".gitignore": "node_modules\n", + "node_modules": { + "a": { + "a1.js": "a1", + "a2.js": "a2", + }, + "b": { + "b1.js": "b1", + "b2.js": "b2", + }, + }, + "src": { + "x.js": "", + "y.js": "", + }, + }), + ) + .await; + + let client = cx.read(|cx| Client::new(FakeHttpClient::with_404_response(), cx)); + let tree = Worktree::local( + client, + Path::new("/root"), + true, + fs.clone(), + Default::default(), + &mut cx.to_async(), + ) + .await + .unwrap(); + + cx.read(|cx| tree.read(cx).as_local().unwrap().scan_complete()) + .await; + + tree.read_with(cx, |tree, _| { + assert_eq!( + tree.entries(true) + .map(|entry| (entry.path.as_ref(), entry.is_ignored)) + .collect::>(), + vec![ + (Path::new(""), false), + (Path::new(".gitignore"), false), + (Path::new("node_modules"), true), + (Path::new("src"), false), + (Path::new("src/x.js"), false), + (Path::new("src/y.js"), false), + ] + ); + }); + + let buffer = tree + .update(cx, |tree, cx| { + tree.as_local_mut() + .unwrap() + .load_buffer(0, "node_modules/b/b1.js".as_ref(), cx) + }) + .await + .unwrap(); + + tree.read_with(cx, |tree, cx| { + assert_eq!( + tree.entries(true) + .map(|entry| (entry.path.as_ref(), entry.is_ignored)) + .collect::>(), + vec![ + (Path::new(""), false), + (Path::new(".gitignore"), false), + (Path::new("node_modules"), true), + (Path::new("node_modules/a"), true), + (Path::new("node_modules/b"), true), + (Path::new("node_modules/b/b1.js"), true), + (Path::new("node_modules/b/b2.js"), true), + (Path::new("src"), false), + (Path::new("src/x.js"), false), + (Path::new("src/y.js"), false), + ] + ); + + let buffer = buffer.read(cx); + assert_eq!( + buffer.file().unwrap().path().as_ref(), + Path::new("node_modules/b/b1.js") + ); + }); +} + #[gpui::test] async fn test_rescan_with_gitignore(cx: &mut TestAppContext) { // .gitignores are handled explicitly by Zed and do not use the git @@ -435,7 +527,7 @@ async fn test_rescan_with_gitignore(cx: &mut TestAppContext) { tree.update(cx, |tree, cx| { let tree = tree.as_local_mut().unwrap(); - tree.expand_dir(tree.entry_for_path("ignored-dir").unwrap().id, cx) + tree.expand_entry_for_path("ignored-dir".as_ref(), cx) }) .recv() .await; From 4424dafcd789ec8dde248c07e6a8666a60265628 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Mon, 19 Jun 2023 12:20:54 -0700 Subject: [PATCH 11/21] Fix expansion of ancestor directories when refreshing a path --- crates/fs/src/fs.rs | 9 +- crates/project/src/worktree.rs | 263 +++++++++++++-------------- crates/project/src/worktree_tests.rs | 168 +++++++++++++---- 3 files changed, 265 insertions(+), 175 deletions(-) diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index f5abe35dbd..e487b64c4e 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -388,6 +388,7 @@ struct FakeFsState { event_txs: Vec>>, events_paused: bool, buffered_events: Vec, + read_dir_call_count: usize, } #[cfg(any(test, feature = "test-support"))] @@ -536,6 +537,7 @@ impl FakeFs { event_txs: Default::default(), buffered_events: Vec::new(), events_paused: false, + read_dir_call_count: 0, }), }) } @@ -772,6 +774,10 @@ impl FakeFs { result } + pub fn read_dir_call_count(&self) -> usize { + self.state.lock().read_dir_call_count + } + async fn simulate_random_delay(&self) { self.executor .upgrade() @@ -1146,7 +1152,8 @@ impl Fs for FakeFs { ) -> Result>>>> { self.simulate_random_delay().await; let path = normalize_path(path); - let state = self.state.lock(); + let mut state = self.state.lock(); + state.read_dir_call_count += 1; let entry = state.read_path(&path)?; let mut entry = entry.lock(); let children = entry.dir_entries(&path)?; diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index bbf63901d1..76a1030134 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -84,15 +84,9 @@ pub struct LocalWorktree { visible: bool, } -enum ScanRequest { - RescanPaths { - relative_paths: Vec>, - done: barrier::Sender, - }, - ExpandPath { - path: Arc, - done: barrier::Sender, - }, +struct ScanRequest { + relative_paths: Vec>, + done: barrier::Sender, } pub struct RemoteWorktree { @@ -226,6 +220,7 @@ pub struct LocalSnapshot { struct BackgroundScannerState { snapshot: LocalSnapshot, expanded_dirs: HashSet, + expanded_paths: HashSet>, /// The ids of all of the entries that were removed from the snapshot /// as part of the current update. These entry ids may be re-used /// if the same inode is discovered at a new path, or if the given @@ -882,14 +877,11 @@ impl LocalWorktree { let path = Arc::from(path); let abs_path = self.absolutize(&path); let fs = self.fs.clone(); - let expand = path - .parent() - .map(|path| self.expand_entry_for_path(path, cx)); + let entry = self.refresh_entry(path.clone(), None, cx); - cx.spawn(|this, mut cx| async move { - if let Some(mut expand) = expand { - expand.recv().await; - } + cx.spawn(|this, cx| async move { + let text = fs.load(&abs_path).await?; + let entry = entry.await?; let mut index_task = None; let snapshot = this.read_with(&cx, |this, _| this.as_local().unwrap().snapshot()); @@ -904,21 +896,12 @@ impl LocalWorktree { } } - let text = fs.load(&abs_path).await?; - let diff_base = if let Some(index_task) = index_task { index_task.await } else { None }; - // Eagerly populate the snapshot with an updated entry for the loaded file - let entry = this - .update(&mut cx, |this, cx| { - this.as_local().unwrap().refresh_entry(path, None, cx) - }) - .await?; - Ok(( File { entry_id: entry.id, @@ -1082,7 +1065,7 @@ impl LocalWorktree { this.as_local_mut() .unwrap() .scan_requests_tx - .try_send(ScanRequest::RescanPaths { + .try_send(ScanRequest { relative_paths: vec![path], done: tx, }) @@ -1159,8 +1142,8 @@ impl LocalWorktree { let (tx, rx) = barrier::channel(); if let Some(entry) = self.entry_for_id(entry_id) { self.scan_requests_tx - .try_send(ScanRequest::ExpandPath { - path: entry.path.clone(), + .try_send(ScanRequest { + relative_paths: vec![entry.path.clone()], done: tx, }) .ok(); @@ -1168,15 +1151,11 @@ impl LocalWorktree { rx } - pub fn expand_entry_for_path( - &self, - path: impl Into>, - _cx: &mut ModelContext, - ) -> barrier::Receiver { + pub fn refresh_entries_for_paths(&self, paths: Vec>) -> barrier::Receiver { let (tx, rx) = barrier::channel(); self.scan_requests_tx - .try_send(ScanRequest::ExpandPath { - path: path.into(), + .try_send(ScanRequest { + relative_paths: paths, done: tx, }) .ok(); @@ -1189,20 +1168,14 @@ impl LocalWorktree { old_path: Option>, cx: &mut ModelContext, ) -> Task> { - let path_changes_tx = self.scan_requests_tx.clone(); + let paths = if let Some(old_path) = old_path.as_ref() { + vec![old_path.clone(), path.clone()] + } else { + vec![path.clone()] + }; + let mut refresh = self.refresh_entries_for_paths(paths); cx.spawn_weak(move |this, mut cx| async move { - let relative_paths = if let Some(old_path) = old_path.as_ref() { - vec![old_path.clone(), path.clone()] - } else { - vec![path.clone()] - }; - - let (tx, mut rx) = barrier::channel(); - path_changes_tx.try_send(ScanRequest::RescanPaths { - relative_paths, - done: tx, - })?; - rx.recv().await; + refresh.recv().await; this.upgrade(&cx) .ok_or_else(|| anyhow!("worktree was dropped"))? .update(&mut cx, |this, _| { @@ -2138,9 +2111,14 @@ impl LocalSnapshot { ignore_stack } -} -impl LocalSnapshot { + #[cfg(test)] + pub(crate) fn expanded_entries(&self) -> impl Iterator { + self.entries_by_path + .cursor::<()>() + .filter(|entry| entry.kind == EntryKind::Dir && (entry.is_external || entry.is_ignored)) + } + #[cfg(test)] pub fn check_invariants(&self) { assert_eq!( @@ -2217,6 +2195,14 @@ impl LocalSnapshot { } impl BackgroundScannerState { + fn is_path_expanded(&self, path: &Path) -> bool { + self.expanded_paths.iter().any(|p| p.starts_with(path)) + || self + .snapshot + .entry_for_path(&path) + .map_or(true, |entry| self.expanded_dirs.contains(&entry.id)) + } + fn reuse_entry_id(&mut self, entry: &mut Entry) { if let Some(removed_entry_id) = self.removed_entry_ids.remove(&entry.inode) { entry.id = removed_entry_id; @@ -2271,6 +2257,7 @@ impl BackgroundScannerState { .insert(abs_parent_path, (ignore, false)); } + self.expanded_dirs.insert(parent_entry.id); let mut entries_by_path_edits = vec![Edit::Insert(parent_entry)]; let mut entries_by_id_edits = Vec::new(); let mut dotgit_path = None; @@ -2883,6 +2870,7 @@ impl BackgroundScanner { expanded_dirs: Default::default(), removed_entry_ids: Default::default(), changed_paths: Default::default(), + expanded_paths: Default::default(), }), phase: BackgroundScannerPhase::InitialScan, } @@ -2986,7 +2974,7 @@ impl BackgroundScanner { } async fn process_scan_request(&self, request: ScanRequest) -> bool { - let root_path = self.state.lock().snapshot.abs_path.clone(); + let root_path = self.expand_paths(&request.relative_paths).await; let root_canonical_path = match self.fs.canonicalize(&root_path).await { Ok(path) => path, Err(err) => { @@ -2995,57 +2983,20 @@ impl BackgroundScanner { } }; - match request { - ScanRequest::RescanPaths { - relative_paths, - done, - } => { - let abs_paths = relative_paths - .into_iter() - .map(|path| { - if path.file_name().is_some() { - root_canonical_path.join(path) - } else { - root_canonical_path.clone() - } - }) - .collect::>(); - self.reload_entries_for_paths( - root_path, - root_canonical_path, - abs_paths, - false, - None, - ) - .await; - self.send_status_update(false, Some(done)) - } - ScanRequest::ExpandPath { path, done } => { - let (scan_job_tx, mut scan_job_rx) = channel::unbounded(); - let mut abs_path = root_canonical_path.clone(); - for component in path.iter() { - abs_path.push(component); - self.reload_entries_for_paths( - root_path.clone(), - root_canonical_path.clone(), - vec![abs_path.clone()], - true, - Some(scan_job_tx.clone()), - ) - .await; + let abs_paths = request + .relative_paths + .into_iter() + .map(|path| { + if path.file_name().is_some() { + root_canonical_path.join(path) + } else { + root_canonical_path.clone() } - drop(scan_job_tx); - - // Scan the expanded directories serially. This is ok, because only - // the direct ancestors of the expanded path need to be scanned. - while let Some(job) = scan_job_rx.next().await { - self.scan_dir(&job).await.log_err(); - } - - self.send_status_update(false, Some(done)); - true - } - } + }) + .collect::>(); + self.reload_entries_for_paths(root_path, root_canonical_path, abs_paths, None) + .await; + self.send_status_update(false, Some(request.done)) } async fn process_events(&mut self, abs_paths: Vec) { @@ -3060,15 +3011,8 @@ impl BackgroundScanner { let (scan_job_tx, scan_job_rx) = channel::unbounded(); let paths = self - .reload_entries_for_paths( - root_path, - root_canonical_path, - abs_paths, - false, - Some(scan_job_tx.clone()), - ) + .reload_entries_for_paths(root_path, root_canonical_path, abs_paths, Some(scan_job_tx)) .await; - drop(scan_job_tx); self.scan_dirs(false, scan_job_rx).await; self.update_ignore_statuses().await; @@ -3108,6 +3052,46 @@ impl BackgroundScanner { self.send_status_update(false, None); } + async fn expand_paths(&self, paths: &[Arc]) -> Arc { + let root_path; + let (scan_job_tx, mut scan_job_rx) = channel::unbounded(); + { + let mut state = self.state.lock(); + root_path = state.snapshot.abs_path.clone(); + for path in paths { + for ancestor in path.ancestors() { + if let Some(entry) = state.snapshot.entry_for_path(ancestor) { + if entry.kind == EntryKind::PendingDir { + let abs_path = root_path.join(ancestor); + let ignore_stack = + state.snapshot.ignore_stack_for_abs_path(&abs_path, true); + let ancestor_inodes = + state.snapshot.ancestor_inodes_for_path(&ancestor); + scan_job_tx + .try_send(ScanJob { + abs_path: abs_path.into(), + path: ancestor.into(), + ignore_stack, + scan_queue: scan_job_tx.clone(), + ancestor_inodes, + is_external: entry.is_external, + }) + .unwrap(); + state.expanded_paths.insert(path.clone()); + break; + } + } + } + } + drop(scan_job_tx); + } + while let Some(job) = scan_job_rx.next().await { + self.scan_dir(&job).await.log_err(); + } + self.state.lock().expanded_paths.clear(); + root_path + } + async fn scan_dirs( &self, enable_progress_updates: bool, @@ -3376,10 +3360,7 @@ impl BackgroundScanner { // If a subdirectory is ignored, or is external to the worktree, don't scan // it unless it is marked as expanded. if (new_job.is_external || new_job.ignore_stack.is_all()) - && !state - .snapshot - .entry_for_path(&new_job.path) - .map_or(true, |entry| state.expanded_dirs.contains(&entry.id)) + && !state.is_path_expanded(&new_job.path) { log::debug!("defer scanning directory {:?}", new_job.path); continue; @@ -3399,12 +3380,40 @@ impl BackgroundScanner { root_abs_path: Arc, root_canonical_path: PathBuf, mut abs_paths: Vec, - expand: bool, scan_queue_tx: Option>, ) -> Option>> { - let doing_recursive_update = scan_queue_tx.is_some(); + let mut event_paths = Vec::>::with_capacity(abs_paths.len()); abs_paths.sort_unstable(); abs_paths.dedup_by(|a, b| a.starts_with(&b)); + { + let state = self.state.lock(); + abs_paths.retain(|abs_path| { + let path = if let Ok(path) = abs_path.strip_prefix(&root_canonical_path) { + path + } else { + log::error!( + "unexpected event {:?} for root path {:?}", + abs_path, + root_canonical_path + ); + return false; + }; + + if let Some(parent) = path.parent() { + if state + .snapshot + .entry_for_path(parent) + .map_or(true, |entry| entry.kind != EntryKind::Dir) + { + log::info!("ignoring event within unloaded directory {:?}", parent); + return false; + } + } + + event_paths.push(path.into()); + return true; + }); + } let metadata = futures::future::join_all( abs_paths @@ -3424,6 +3433,7 @@ impl BackgroundScanner { let mut state = self.state.lock(); let snapshot = &mut state.snapshot; + let doing_recursive_update = scan_queue_tx.is_some(); let is_idle = snapshot.completed_scan_id == snapshot.scan_id; snapshot.scan_id += 1; if is_idle && !doing_recursive_update { @@ -3433,25 +3443,13 @@ impl BackgroundScanner { // Remove any entries for paths that no longer exist or are being recursively // refreshed. Do this before adding any new entries, so that renames can be // detected regardless of the order of the paths. - let mut event_paths = Vec::>::with_capacity(abs_paths.len()); - let mut event_metadata = Vec::<_>::with_capacity(abs_paths.len()); - for (abs_path, metadata) in abs_paths.iter().zip(metadata.iter()) { - if let Ok(path) = abs_path.strip_prefix(&root_canonical_path) { - if matches!(metadata, Ok(None)) || doing_recursive_update { - state.remove_path(path); - } - event_paths.push(path.into()); - event_metadata.push(metadata); - } else { - log::error!( - "unexpected event {:?} for root path {:?}", - abs_path, - root_canonical_path - ); + for (path, metadata) in event_paths.iter().zip(metadata.iter()) { + if matches!(metadata, Ok(None)) || doing_recursive_update { + state.remove_path(path); } } - for (path, metadata) in event_paths.iter().cloned().zip(event_metadata.into_iter()) { + for (path, metadata) in event_paths.iter().cloned().zip(metadata.into_iter()) { let abs_path: Arc = root_abs_path.join(&path).into(); match metadata { @@ -3487,9 +3485,6 @@ impl BackgroundScanner { } let fs_entry = state.insert_entry(fs_entry, self.fs.as_ref()); - if expand { - state.expanded_dirs.insert(fs_entry.id); - } if let Some(scan_queue_tx) = &scan_queue_tx { let mut ancestor_inodes = state.snapshot.ancestor_inodes_for_path(&path); diff --git a/crates/project/src/worktree_tests.rs b/crates/project/src/worktree_tests.rs index 92eded0a99..0b8e02dc49 100644 --- a/crates/project/src/worktree_tests.rs +++ b/crates/project/src/worktree_tests.rs @@ -1,6 +1,6 @@ use crate::{ worktree::{Event, Snapshot, WorktreeHandle}, - EntryKind, PathChange, Worktree, + Entry, EntryKind, PathChange, Worktree, }; use anyhow::Result; use client::Client; @@ -158,9 +158,10 @@ async fn test_descendent_entries(cx: &mut TestAppContext) { }); // Expand gitignored directory. - tree.update(cx, |tree, cx| { - let tree = tree.as_local_mut().unwrap(); - tree.expand_entry_for_path("i/j".as_ref(), cx) + tree.read_with(cx, |tree, _| { + tree.as_local() + .unwrap() + .refresh_entries_for_paths(vec![Path::new("i/j").into()]) }) .recv() .await; @@ -336,12 +337,18 @@ async fn test_symlinks_pointing_outside(cx: &mut TestAppContext) { (Path::new("src/b.rs"), false), ] ); + + assert_eq!( + tree.entry_for_path("deps/dep-dir2").unwrap().kind, + EntryKind::PendingDir + ); }); // Expand one of the symlinked directories. - tree.update(cx, |tree, cx| { - let tree = tree.as_local_mut().unwrap(); - tree.expand_entry_for_path("deps/dep-dir3".as_ref(), cx) + tree.read_with(cx, |tree, _| { + tree.as_local() + .unwrap() + .refresh_entries_for_paths(vec![Path::new("deps/dep-dir3").into()]) }) .recv() .await; @@ -368,9 +375,10 @@ async fn test_symlinks_pointing_outside(cx: &mut TestAppContext) { }); // Expand a subdirectory of one of the symlinked directories. - tree.update(cx, |tree, cx| { - let tree = tree.as_local_mut().unwrap(); - tree.expand_entry_for_path("deps/dep-dir3/src".as_ref(), cx) + tree.read_with(cx, |tree, _| { + tree.as_local() + .unwrap() + .refresh_entries_for_paths(vec![Path::new("deps/dep-dir3/src").into()]) }) .recv() .await; @@ -405,17 +413,19 @@ async fn test_open_gitignored_files(cx: &mut TestAppContext) { "/root", json!({ ".gitignore": "node_modules\n", - "node_modules": { - "a": { - "a1.js": "a1", - "a2.js": "a2", - }, - "b": { - "b1.js": "b1", - "b2.js": "b2", + "one": { + "node_modules": { + "a": { + "a1.js": "a1", + "a2.js": "a2", + }, + "b": { + "b1.js": "b1", + "b2.js": "b2", + }, }, }, - "src": { + "two": { "x.js": "", "y.js": "", }, @@ -446,19 +456,23 @@ async fn test_open_gitignored_files(cx: &mut TestAppContext) { vec![ (Path::new(""), false), (Path::new(".gitignore"), false), - (Path::new("node_modules"), true), - (Path::new("src"), false), - (Path::new("src/x.js"), false), - (Path::new("src/y.js"), false), + (Path::new("one"), false), + (Path::new("one/node_modules"), true), + (Path::new("two"), false), + (Path::new("two/x.js"), false), + (Path::new("two/y.js"), false), ] ); }); + // Open a file that is nested inside of a gitignored directory that + // has not yet been expanded. + let prev_read_dir_count = fs.read_dir_call_count(); let buffer = tree .update(cx, |tree, cx| { tree.as_local_mut() .unwrap() - .load_buffer(0, "node_modules/b/b1.js".as_ref(), cx) + .load_buffer(0, "one/node_modules/b/b1.js".as_ref(), cx) }) .await .unwrap(); @@ -471,22 +485,68 @@ async fn test_open_gitignored_files(cx: &mut TestAppContext) { vec![ (Path::new(""), false), (Path::new(".gitignore"), false), - (Path::new("node_modules"), true), - (Path::new("node_modules/a"), true), - (Path::new("node_modules/b"), true), - (Path::new("node_modules/b/b1.js"), true), - (Path::new("node_modules/b/b2.js"), true), - (Path::new("src"), false), - (Path::new("src/x.js"), false), - (Path::new("src/y.js"), false), + (Path::new("one"), false), + (Path::new("one/node_modules"), true), + (Path::new("one/node_modules/a"), true), + (Path::new("one/node_modules/b"), true), + (Path::new("one/node_modules/b/b1.js"), true), + (Path::new("one/node_modules/b/b2.js"), true), + (Path::new("two"), false), + (Path::new("two/x.js"), false), + (Path::new("two/y.js"), false), ] ); - let buffer = buffer.read(cx); assert_eq!( - buffer.file().unwrap().path().as_ref(), - Path::new("node_modules/b/b1.js") + buffer.read(cx).file().unwrap().path().as_ref(), + Path::new("one/node_modules/b/b1.js") ); + + // Only the newly-expanded directories are scanned. + assert_eq!(fs.read_dir_call_count() - prev_read_dir_count, 2); + }); + + // Open another file in a different subdirectory of the same + // gitignored directory. + let prev_read_dir_count = fs.read_dir_call_count(); + let buffer = tree + .update(cx, |tree, cx| { + tree.as_local_mut() + .unwrap() + .load_buffer(0, "one/node_modules/a/a2.js".as_ref(), cx) + }) + .await + .unwrap(); + + tree.read_with(cx, |tree, cx| { + assert_eq!( + tree.entries(true) + .map(|entry| (entry.path.as_ref(), entry.is_ignored)) + .collect::>(), + vec![ + (Path::new(""), false), + (Path::new(".gitignore"), false), + (Path::new("one"), false), + (Path::new("one/node_modules"), true), + (Path::new("one/node_modules/a"), true), + (Path::new("one/node_modules/a/a1.js"), true), + (Path::new("one/node_modules/a/a2.js"), true), + (Path::new("one/node_modules/b"), true), + (Path::new("one/node_modules/b/b1.js"), true), + (Path::new("one/node_modules/b/b2.js"), true), + (Path::new("two"), false), + (Path::new("two/x.js"), false), + (Path::new("two/y.js"), false), + ] + ); + + assert_eq!( + buffer.read(cx).file().unwrap().path().as_ref(), + Path::new("one/node_modules/a/a2.js") + ); + + // Only the newly-expanded directory is scanned. + assert_eq!(fs.read_dir_call_count() - prev_read_dir_count, 1); }); } @@ -525,9 +585,10 @@ async fn test_rescan_with_gitignore(cx: &mut TestAppContext) { cx.read(|cx| tree.read(cx).as_local().unwrap().scan_complete()) .await; - tree.update(cx, |tree, cx| { - let tree = tree.as_local_mut().unwrap(); - tree.expand_entry_for_path("ignored-dir".as_ref(), cx) + tree.read_with(cx, |tree, _| { + tree.as_local() + .unwrap() + .refresh_entries_for_paths(vec![Path::new("ignored-dir").into()]) }) .recv() .await; @@ -868,8 +929,13 @@ async fn test_random_worktree_changes(cx: &mut TestAppContext, mut rng: StdRng) log::info!("quiescing"); fs.as_fake().flush_events(usize::MAX); cx.foreground().run_until_parked(); + let snapshot = worktree.read_with(cx, |tree, _| tree.as_local().unwrap().snapshot()); snapshot.check_invariants(); + let expanded_paths = snapshot + .expanded_entries() + .map(|e| e.path.clone()) + .collect::>(); { let new_worktree = Worktree::local( @@ -885,6 +951,14 @@ async fn test_random_worktree_changes(cx: &mut TestAppContext, mut rng: StdRng) new_worktree .update(cx, |tree, _| tree.as_local_mut().unwrap().scan_complete()) .await; + new_worktree + .update(cx, |tree, _| { + tree.as_local_mut() + .unwrap() + .refresh_entries_for_paths(expanded_paths) + }) + .recv() + .await; let new_snapshot = new_worktree.read_with(cx, |tree, _| tree.as_local().unwrap().snapshot()); assert_eq!( @@ -901,11 +975,25 @@ async fn test_random_worktree_changes(cx: &mut TestAppContext, mut rng: StdRng) } assert_eq!( - prev_snapshot.entries(true).collect::>(), - snapshot.entries(true).collect::>(), + prev_snapshot + .entries(true) + .map(ignore_pending_dir) + .collect::>(), + snapshot + .entries(true) + .map(ignore_pending_dir) + .collect::>(), "wrong updates after snapshot {i}: {updates:#?}", ); } + + fn ignore_pending_dir(entry: &Entry) -> Entry { + let mut entry = entry.clone(); + if entry.kind == EntryKind::PendingDir { + entry.kind = EntryKind::Dir + } + entry + } } // The worktree's `UpdatedEntries` event can be used to follow along with From d3477f75ac6cce63d7631107b2abd9bc334c12bd Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 20 Jun 2023 16:14:47 -0700 Subject: [PATCH 12/21] Fix reloading of git repositories Also, clean up logic for reloading git repositories. --- crates/project/src/worktree.rs | 372 +++++++++++++++------------------ 1 file changed, 164 insertions(+), 208 deletions(-) diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 76a1030134..d3ff0e93d2 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -239,13 +239,6 @@ pub struct LocalRepositoryEntry { pub(crate) git_dir_path: Arc, } -impl LocalRepositoryEntry { - // Note that this path should be relative to the worktree root. - pub(crate) fn in_dot_git(&self, path: &Path) -> bool { - path.starts_with(self.git_dir_path.as_ref()) - } -} - impl Deref for LocalSnapshot { type Target = Snapshot; @@ -1850,15 +1843,6 @@ impl LocalSnapshot { Some((path, self.git_repositories.get(&repo.work_directory_id())?)) } - pub(crate) fn repo_for_metadata( - &self, - path: &Path, - ) -> Option<(&ProjectEntryId, &LocalRepositoryEntry)> { - self.git_repositories - .iter() - .find(|(_, repo)| repo.in_dot_git(path)) - } - fn build_update( &self, project_id: u64, @@ -1994,57 +1978,6 @@ impl LocalSnapshot { entry } - #[must_use = "Changed paths must be used for diffing later"] - fn build_repo(&mut self, parent_path: Arc, fs: &dyn Fs) -> Option>> { - let abs_path = self.abs_path.join(&parent_path); - let work_dir: Arc = parent_path.parent().unwrap().into(); - - // Guard against repositories inside the repository metadata - if work_dir - .components() - .find(|component| component.as_os_str() == *DOT_GIT) - .is_some() - { - return None; - }; - - let work_dir_id = self - .entry_for_path(work_dir.clone()) - .map(|entry| entry.id)?; - - if self.git_repositories.get(&work_dir_id).is_some() { - return None; - } - - let repo = fs.open_repo(abs_path.as_path())?; - let work_directory = RepositoryWorkDirectory(work_dir.clone()); - - let repo_lock = repo.lock(); - - self.repository_entries.insert( - work_directory.clone(), - RepositoryEntry { - work_directory: work_dir_id.into(), - branch: repo_lock.branch_name().map(Into::into), - }, - ); - - let changed_paths = self.scan_statuses(repo_lock.deref(), &work_directory); - - drop(repo_lock); - - self.git_repositories.insert( - work_dir_id, - LocalRepositoryEntry { - git_dir_scan_id: 0, - repo_ptr: repo, - git_dir_path: parent_path.clone(), - }, - ); - - Some(changed_paths) - } - #[must_use = "Changed paths must be used for diffing later"] fn scan_statuses( &mut self, @@ -2215,10 +2148,7 @@ impl BackgroundScannerState { self.reuse_entry_id(&mut entry); let entry = self.snapshot.insert_entry(entry, fs); if entry.path.file_name() == Some(&DOT_GIT) { - let changed_paths = self.snapshot.build_repo(entry.path.clone(), fs); - if let Some(changed_paths) = changed_paths { - util::extend_sorted(&mut self.changed_paths, changed_paths, usize::MAX, Ord::cmp) - } + self.build_repository(entry.path.clone(), fs); } entry } @@ -2283,10 +2213,7 @@ impl BackgroundScannerState { self.snapshot.entries_by_id.edit(entries_by_id_edits, &()); if let Some(dotgit_path) = dotgit_path { - let changed_paths = self.snapshot.build_repo(dotgit_path, fs); - if let Some(changed_paths) = changed_paths { - util::extend_sorted(&mut self.changed_paths, changed_paths, usize::MAX, Ord::cmp) - } + self.build_repository(dotgit_path, fs); } if let Err(ix) = self.changed_paths.binary_search(parent_path) { self.changed_paths.insert(ix, parent_path.clone()); @@ -2326,6 +2253,134 @@ impl BackgroundScannerState { } } } + + fn reload_repositories(&mut self, changed_paths: &[Arc], fs: &dyn Fs) { + let scan_id = self.snapshot.scan_id; + + // Find each of the .git directories that contain any of the given paths. + let mut prev_dot_git_dir = None; + for changed_path in changed_paths { + let Some(dot_git_dir) = changed_path + .ancestors() + .find(|ancestor| ancestor.file_name() == Some(&*DOT_GIT)) else { + continue; + }; + + // Avoid processing the same repository multiple times, if multiple paths + // within it have changed. + if prev_dot_git_dir == Some(dot_git_dir) { + continue; + } + prev_dot_git_dir = Some(dot_git_dir); + + // If there is already a repository for this .git directory, reload + // the status for all of its files. + let repository = self + .snapshot + .git_repositories + .iter() + .find_map(|(entry_id, repo)| { + (repo.git_dir_path.as_ref() == dot_git_dir).then(|| (*entry_id, repo.clone())) + }); + match repository { + None => { + self.build_repository(dot_git_dir.into(), fs); + } + Some((entry_id, repository)) => { + if repository.git_dir_scan_id == scan_id { + continue; + } + let Some(work_dir) = self + .snapshot + .entry_for_id(entry_id) + .map(|entry| RepositoryWorkDirectory(entry.path.clone())) else { continue }; + + let repository = repository.repo_ptr.lock(); + let branch = repository.branch_name(); + repository.reload_index(); + + self.snapshot + .git_repositories + .update(&entry_id, |entry| entry.git_dir_scan_id = scan_id); + self.snapshot + .snapshot + .repository_entries + .update(&work_dir, |entry| entry.branch = branch.map(Into::into)); + + let changed_paths = self.snapshot.scan_statuses(&*repository, &work_dir); + util::extend_sorted( + &mut self.changed_paths, + changed_paths, + usize::MAX, + Ord::cmp, + ) + } + } + } + + // Remove any git repositories whose .git entry no longer exists. + let mut snapshot = &mut self.snapshot; + let mut repositories = mem::take(&mut snapshot.git_repositories); + let mut repository_entries = mem::take(&mut snapshot.repository_entries); + repositories.retain(|work_directory_id, _| { + snapshot + .entry_for_id(*work_directory_id) + .map_or(false, |entry| { + snapshot.entry_for_path(entry.path.join(*DOT_GIT)).is_some() + }) + }); + repository_entries.retain(|_, entry| repositories.get(&entry.work_directory.0).is_some()); + snapshot.git_repositories = repositories; + snapshot.repository_entries = repository_entries; + } + + fn build_repository(&mut self, dot_git_path: Arc, fs: &dyn Fs) -> Option<()> { + let work_dir_path: Arc = dot_git_path.parent().unwrap().into(); + + // Guard against repositories inside the repository metadata + if work_dir_path.iter().any(|component| component == *DOT_GIT) { + return None; + }; + + let work_dir_id = self + .snapshot + .entry_for_path(work_dir_path.clone()) + .map(|entry| entry.id)?; + + if self.snapshot.git_repositories.get(&work_dir_id).is_some() { + return None; + } + + let abs_path = self.snapshot.abs_path.join(&dot_git_path); + let repository = fs.open_repo(abs_path.as_path())?; + let work_directory = RepositoryWorkDirectory(work_dir_path.clone()); + + let repo_lock = repository.lock(); + self.snapshot.repository_entries.insert( + work_directory.clone(), + RepositoryEntry { + work_directory: work_dir_id.into(), + branch: repo_lock.branch_name().map(Into::into), + }, + ); + + let changed_paths = self + .snapshot + .scan_statuses(repo_lock.deref(), &work_directory); + drop(repo_lock); + + self.snapshot.git_repositories.insert( + work_dir_id, + LocalRepositoryEntry { + git_dir_scan_id: 0, + repo_ptr: repository, + git_dir_path: dot_git_path.clone(), + }, + ); + + util::extend_sorted(&mut self.changed_paths, changed_paths, usize::MAX, Ord::cmp); + Some(()) + } } async fn build_gitignore(abs_path: &Path, fs: &dyn Fs) -> Result { @@ -3019,34 +3074,8 @@ impl BackgroundScanner { { let mut state = self.state.lock(); - - if let Some(paths) = paths { - for path in paths { - self.reload_git_repo(&path, &mut *state, self.fs.as_ref()); - } - } - - let mut snapshot = &mut state.snapshot; - - let mut git_repositories = mem::take(&mut snapshot.git_repositories); - git_repositories.retain(|work_directory_id, _| { - snapshot - .entry_for_id(*work_directory_id) - .map_or(false, |entry| { - snapshot.entry_for_path(entry.path.join(*DOT_GIT)).is_some() - }) - }); - snapshot.git_repositories = git_repositories; - - let mut git_repository_entries = mem::take(&mut snapshot.snapshot.repository_entries); - git_repository_entries.retain(|_, entry| { - snapshot - .git_repositories - .get(&entry.work_directory.0) - .is_some() - }); - snapshot.snapshot.repository_entries = git_repository_entries; - snapshot.completed_scan_id = snapshot.scan_id; + state.reload_repositories(&paths, self.fs.as_ref()); + state.snapshot.completed_scan_id = state.snapshot.scan_id; } self.send_status_update(false, None); @@ -3381,39 +3410,23 @@ impl BackgroundScanner { root_canonical_path: PathBuf, mut abs_paths: Vec, scan_queue_tx: Option>, - ) -> Option>> { + ) -> Vec> { let mut event_paths = Vec::>::with_capacity(abs_paths.len()); abs_paths.sort_unstable(); abs_paths.dedup_by(|a, b| a.starts_with(&b)); - { - let state = self.state.lock(); - abs_paths.retain(|abs_path| { - let path = if let Ok(path) = abs_path.strip_prefix(&root_canonical_path) { - path - } else { - log::error!( - "unexpected event {:?} for root path {:?}", - abs_path, - root_canonical_path - ); - return false; - }; - - if let Some(parent) = path.parent() { - if state - .snapshot - .entry_for_path(parent) - .map_or(true, |entry| entry.kind != EntryKind::Dir) - { - log::info!("ignoring event within unloaded directory {:?}", parent); - return false; - } - } - + abs_paths.retain(|abs_path| { + if let Ok(path) = abs_path.strip_prefix(&root_canonical_path) { event_paths.push(path.into()); - return true; - }); - } + true + } else { + log::error!( + "unexpected event {:?} for root path {:?}", + abs_path, + root_canonical_path + ); + false + } + }); let metadata = futures::future::join_all( abs_paths @@ -3433,8 +3446,8 @@ impl BackgroundScanner { let mut state = self.state.lock(); let snapshot = &mut state.snapshot; - let doing_recursive_update = scan_queue_tx.is_some(); let is_idle = snapshot.completed_scan_id == snapshot.scan_id; + let doing_recursive_update = scan_queue_tx.is_some(); snapshot.scan_id += 1; if is_idle && !doing_recursive_update { snapshot.completed_scan_id = snapshot.scan_id; @@ -3449,7 +3462,21 @@ impl BackgroundScanner { } } - for (path, metadata) in event_paths.iter().cloned().zip(metadata.into_iter()) { + for (path, metadata) in event_paths.iter().zip(metadata.iter()) { + if let (Some(parent), true) = (path.parent(), doing_recursive_update) { + if state + .snapshot + .entry_for_path(parent) + .map_or(true, |entry| entry.kind != EntryKind::Dir) + { + log::debug!( + "ignoring event {path:?} within unloaded directory {:?}", + parent + ); + continue; + } + } + let abs_path: Arc = root_abs_path.join(&path).into(); match metadata { @@ -3460,7 +3487,7 @@ impl BackgroundScanner { let mut fs_entry = Entry::new( path.clone(), - &metadata, + metadata, self.next_entry_id.as_ref(), state.snapshot.root_char_bag, ); @@ -3492,7 +3519,7 @@ impl BackgroundScanner { ancestor_inodes.insert(metadata.inode); smol::block_on(scan_queue_tx.send(ScanJob { abs_path, - path, + path: path.clone(), ignore_stack, ancestor_inodes, is_external: fs_entry.is_external, @@ -3519,7 +3546,7 @@ impl BackgroundScanner { Ord::cmp, ); - Some(event_paths) + event_paths } fn remove_repo_path(&self, path: &Path, snapshot: &mut LocalSnapshot) -> Option<()> { @@ -3544,77 +3571,6 @@ impl BackgroundScanner { Some(()) } - fn reload_git_repo( - &self, - path: &Path, - state: &mut BackgroundScannerState, - fs: &dyn Fs, - ) -> Option<()> { - let scan_id = state.snapshot.scan_id; - - if path - .components() - .any(|component| component.as_os_str() == *DOT_GIT) - { - let (entry_id, repo_ptr) = { - let Some((entry_id, repo)) = state.snapshot.repo_for_metadata(&path) else { - let dot_git_dir = path.ancestors() - .skip_while(|ancestor| ancestor.file_name() != Some(&*DOT_GIT)) - .next()?; - - let changed_paths = state.snapshot.build_repo(dot_git_dir.into(), fs); - if let Some(changed_paths) = changed_paths { - util::extend_sorted( - &mut state.changed_paths, - changed_paths, - usize::MAX, - Ord::cmp, - ); - } - - return None; - }; - if repo.git_dir_scan_id == scan_id { - return None; - } - - (*entry_id, repo.repo_ptr.to_owned()) - }; - - let work_dir = state - .snapshot - .entry_for_id(entry_id) - .map(|entry| RepositoryWorkDirectory(entry.path.clone()))?; - - let repo = repo_ptr.lock(); - repo.reload_index(); - let branch = repo.branch_name(); - - state.snapshot.git_repositories.update(&entry_id, |entry| { - entry.git_dir_scan_id = scan_id; - }); - - state - .snapshot - .snapshot - .repository_entries - .update(&work_dir, |entry| { - entry.branch = branch.map(Into::into); - }); - - let changed_paths = state.snapshot.scan_statuses(repo.deref(), &work_dir); - - util::extend_sorted( - &mut state.changed_paths, - changed_paths, - usize::MAX, - Ord::cmp, - ) - } - - Some(()) - } - async fn update_ignore_statuses(&self) { use futures::FutureExt as _; From bfc90f45020a5ca051a04d289b8794e7ec03855e Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 20 Jun 2023 17:56:27 -0700 Subject: [PATCH 13/21] Add failing test for changing a gitignore so a pending dir is no longer ignored --- crates/project/src/worktree_tests.rs | 182 +++++++++++++++++++++------ 1 file changed, 142 insertions(+), 40 deletions(-) diff --git a/crates/project/src/worktree_tests.rs b/crates/project/src/worktree_tests.rs index 0b8e02dc49..b544181292 100644 --- a/crates/project/src/worktree_tests.rs +++ b/crates/project/src/worktree_tests.rs @@ -35,11 +35,8 @@ async fn test_traversal(cx: &mut TestAppContext) { ) .await; - let http_client = FakeHttpClient::with_404_response(); - let client = cx.read(|cx| Client::new(http_client, cx)); - let tree = Worktree::local( - client, + build_client(cx), Path::new("/root"), true, fs, @@ -108,11 +105,8 @@ async fn test_descendent_entries(cx: &mut TestAppContext) { ) .await; - let http_client = FakeHttpClient::with_404_response(); - let client = cx.read(|cx| Client::new(http_client, cx)); - let tree = Worktree::local( - client, + build_client(cx), Path::new("/root"), true, fs, @@ -208,9 +202,8 @@ async fn test_circular_symlinks(executor: Arc, cx: &mut TestAppCo fs.insert_symlink("/root/lib/a/lib", "..".into()).await; fs.insert_symlink("/root/lib/b/lib", "..".into()).await; - let client = cx.read(|cx| Client::new(FakeHttpClient::with_404_response(), cx)); let tree = Worktree::local( - client, + build_client(cx), Path::new("/root"), true, fs.clone(), @@ -306,9 +299,8 @@ async fn test_symlinks_pointing_outside(cx: &mut TestAppContext) { fs.insert_symlink("/root/dir1/deps/dep-dir3", "../../dir3".into()) .await; - let client = cx.read(|cx| Client::new(FakeHttpClient::with_404_response(), cx)); let tree = Worktree::local( - client, + build_client(cx), Path::new("/root/dir1"), true, fs.clone(), @@ -433,9 +425,8 @@ async fn test_open_gitignored_files(cx: &mut TestAppContext) { ) .await; - let client = cx.read(|cx| Client::new(FakeHttpClient::with_404_response(), cx)); let tree = Worktree::local( - client, + build_client(cx), Path::new("/root"), true, fs.clone(), @@ -550,6 +541,128 @@ async fn test_open_gitignored_files(cx: &mut TestAppContext) { }); } +#[gpui::test] +async fn test_dirs_no_longer_ignored(cx: &mut TestAppContext) { + let fs = FakeFs::new(cx.background()); + fs.insert_tree( + "/root", + json!({ + ".gitignore": "node_modules\n", + "a": { + "a.js": "", + }, + "b": { + "b.js": "", + }, + "node_modules": { + "c": { + "c.js": "", + }, + "d": { + "d.js": "", + "e": { + "e1.js": "", + "e2.js": "", + }, + "f": { + "f1.js": "", + "f2.js": "", + } + }, + }, + }), + ) + .await; + + let tree = Worktree::local( + build_client(cx), + Path::new("/root"), + true, + fs.clone(), + Default::default(), + &mut cx.to_async(), + ) + .await + .unwrap(); + + cx.read(|cx| tree.read(cx).as_local().unwrap().scan_complete()) + .await; + + // Open a file within the gitignored directory, forcing some of its + // subdirectories to be read, but not all. + let read_dir_count_1 = fs.read_dir_call_count(); + tree.read_with(cx, |tree, _| { + tree.as_local() + .unwrap() + .refresh_entries_for_paths(vec![Path::new("node_modules/d/d.js").into()]) + }) + .recv() + .await; + + // Those subdirectories are now loaded. + tree.read_with(cx, |tree, _| { + assert_eq!( + tree.entries(true) + .map(|e| (e.path.as_ref(), e.is_ignored)) + .collect::>(), + &[ + (Path::new(""), false), + (Path::new(".gitignore"), false), + (Path::new("a"), false), + (Path::new("a/a.js"), false), + (Path::new("b"), false), + (Path::new("b/b.js"), false), + (Path::new("node_modules"), true), + (Path::new("node_modules/c"), true), + (Path::new("node_modules/d"), true), + (Path::new("node_modules/d/d.js"), true), + (Path::new("node_modules/d/e"), true), + (Path::new("node_modules/d/f"), true), + ] + ); + }); + let read_dir_count_2 = fs.read_dir_call_count(); + assert_eq!(read_dir_count_2 - read_dir_count_1, 2); + + // Update the gitignore so that node_modules is no longer ignored, + // but a subdirectory is ignored + fs.save("/root/.gitignore".as_ref(), &"e".into(), Default::default()) + .await + .unwrap(); + cx.foreground().run_until_parked(); + + // All of the directories that are no longer ignored are now loaded. + tree.read_with(cx, |tree, _| { + assert_eq!( + tree.entries(true) + .map(|e| (e.path.as_ref(), e.is_ignored)) + .collect::>(), + &[ + (Path::new(""), false), + (Path::new(".gitignore"), false), + (Path::new("a"), false), + (Path::new("a/a.js"), false), + (Path::new("b"), false), + (Path::new("b/b.js"), false), + (Path::new("node_modules"), false), + (Path::new("node_modules/c"), false), + (Path::new("node_modules/c/c.js"), false), + (Path::new("node_modules/d"), false), + (Path::new("node_modules/d/d.js"), false), + // This subdirectory is now ignored + (Path::new("node_modules/d/e"), true), + (Path::new("node_modules/d/f"), false), + (Path::new("node_modules/d/f/f1.js"), false), + (Path::new("node_modules/d/f/f2.js"), false), + ] + ); + }); + + // Each of the newly-loaded directories is scanned only once. + let read_dir_count_3 = fs.read_dir_call_count(); + assert_eq!(read_dir_count_3 - read_dir_count_2, 4); +} + #[gpui::test] async fn test_rescan_with_gitignore(cx: &mut TestAppContext) { // .gitignores are handled explicitly by Zed and do not use the git @@ -570,10 +683,8 @@ async fn test_rescan_with_gitignore(cx: &mut TestAppContext) { })); let dir = parent_dir.path().join("tree"); - let client = cx.read(|cx| Client::new(FakeHttpClient::with_404_response(), cx)); - let tree = Worktree::local( - client, + build_client(cx), dir.as_path(), true, Arc::new(RealFs), @@ -648,10 +759,8 @@ async fn test_write_file(cx: &mut TestAppContext) { "ignored-dir": {} })); - let client = cx.read(|cx| Client::new(FakeHttpClient::with_404_response(), cx)); - let tree = Worktree::local( - client, + build_client(cx), dir.path(), true, Arc::new(RealFs), @@ -695,8 +804,6 @@ async fn test_write_file(cx: &mut TestAppContext) { #[gpui::test(iterations = 30)] async fn test_create_directory_during_initial_scan(cx: &mut TestAppContext) { - let client = cx.read(|cx| Client::new(FakeHttpClient::with_404_response(), cx)); - let fs = FakeFs::new(cx.background()); fs.insert_tree( "/root", @@ -709,7 +816,7 @@ async fn test_create_directory_during_initial_scan(cx: &mut TestAppContext) { .await; let tree = Worktree::local( - client, + build_client(cx), "/root".as_ref(), true, fs, @@ -774,9 +881,8 @@ async fn test_random_worktree_operations_during_initial_scan( } log::info!("generated initial tree"); - let client = cx.read(|cx| Client::new(FakeHttpClient::with_404_response(), cx)); let worktree = Worktree::local( - client.clone(), + build_client(cx), root_dir, true, fs.clone(), @@ -864,9 +970,8 @@ async fn test_random_worktree_changes(cx: &mut TestAppContext, mut rng: StdRng) } log::info!("generated initial tree"); - let client = cx.read(|cx| Client::new(FakeHttpClient::with_404_response(), cx)); let worktree = Worktree::local( - client.clone(), + build_client(cx), root_dir, true, fs.clone(), @@ -939,7 +1044,7 @@ async fn test_random_worktree_changes(cx: &mut TestAppContext, mut rng: StdRng) { let new_worktree = Worktree::local( - client.clone(), + build_client(cx), root_dir, true, fs.clone(), @@ -1276,10 +1381,8 @@ async fn test_rename_work_directory(cx: &mut TestAppContext) { })); let root_path = root.path(); - let http_client = FakeHttpClient::with_404_response(); - let client = cx.read(|cx| Client::new(http_client, cx)); let tree = Worktree::local( - client, + build_client(cx), root_path, true, Arc::new(RealFs), @@ -1355,10 +1458,8 @@ async fn test_git_repository_for_path(cx: &mut TestAppContext) { }, })); - let http_client = FakeHttpClient::with_404_response(); - let client = cx.read(|cx| Client::new(http_client, cx)); let tree = Worktree::local( - client, + build_client(cx), root.path(), true, Arc::new(RealFs), @@ -1479,10 +1580,8 @@ async fn test_git_status(deterministic: Arc, cx: &mut TestAppCont })); - let http_client = FakeHttpClient::with_404_response(); - let client = cx.read(|cx| Client::new(http_client, cx)); let tree = Worktree::local( - client, + build_client(cx), root.path(), true, Arc::new(RealFs), @@ -1686,10 +1785,8 @@ async fn test_propagate_git_statuses(cx: &mut TestAppContext) { ], ); - let http_client = FakeHttpClient::with_404_response(); - let client = cx.read(|cx| Client::new(http_client, cx)); let tree = Worktree::local( - client, + build_client(cx), Path::new("/root"), true, fs.clone(), @@ -1768,6 +1865,11 @@ async fn test_propagate_git_statuses(cx: &mut TestAppContext) { } } +fn build_client(cx: &mut TestAppContext) -> Arc { + let http_client = FakeHttpClient::with_404_response(); + cx.read(|cx| Client::new(http_client, cx)) +} + #[track_caller] fn git_init(path: &Path) -> git2::Repository { git2::Repository::init(path).expect("Failed to initialize git repository") From 400e3cda322086ef8a33ed231807e65947a31268 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Wed, 21 Jun 2023 10:07:29 -0700 Subject: [PATCH 14/21] Scan directories when they stop being ignored --- crates/project/src/worktree.rs | 44 ++++++++++++++++++++++++---- crates/project/src/worktree_tests.rs | 3 +- 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index d3ff0e93d2..39350f1c8f 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -3066,11 +3066,16 @@ impl BackgroundScanner { let (scan_job_tx, scan_job_rx) = channel::unbounded(); let paths = self - .reload_entries_for_paths(root_path, root_canonical_path, abs_paths, Some(scan_job_tx)) + .reload_entries_for_paths( + root_path, + root_canonical_path, + abs_paths, + Some(scan_job_tx.clone()), + ) .await; - self.scan_dirs(false, scan_job_rx).await; - self.update_ignore_statuses().await; + self.update_ignore_statuses(scan_job_tx).await; + self.scan_dirs(false, scan_job_rx).await; { let mut state = self.state.lock(); @@ -3571,7 +3576,7 @@ impl BackgroundScanner { Some(()) } - async fn update_ignore_statuses(&self) { + async fn update_ignore_statuses(&self, scan_job_tx: Sender) { use futures::FutureExt as _; let mut snapshot = self.state.lock().snapshot.clone(); @@ -3619,6 +3624,7 @@ impl BackgroundScanner { abs_path: parent_abs_path, ignore_stack, ignore_queue: ignore_queue_tx.clone(), + scan_queue: scan_job_tx.clone(), })) .unwrap(); } @@ -3663,7 +3669,7 @@ impl BackgroundScanner { let path = job.abs_path.strip_prefix(&snapshot.abs_path).unwrap(); for mut entry in snapshot.child_entries(path).cloned() { let was_ignored = entry.is_ignored; - let abs_path = snapshot.abs_path().join(&entry.path); + let abs_path: Arc = snapshot.abs_path().join(&entry.path).into(); entry.is_ignored = ignore_stack.is_abs_path_ignored(&abs_path, entry.is_dir()); if entry.is_dir() { let child_ignore_stack = if entry.is_ignored { @@ -3671,11 +3677,36 @@ impl BackgroundScanner { } else { ignore_stack.clone() }; + + // Scan any directories that were previously ignored and weren't + // previously scanned. + if was_ignored + && !entry.is_ignored + && !entry.is_external + && entry.kind == EntryKind::PendingDir + { + job.scan_queue + .try_send(ScanJob { + abs_path: abs_path.clone(), + path: entry.path.clone(), + ignore_stack: child_ignore_stack.clone(), + scan_queue: job.scan_queue.clone(), + ancestor_inodes: self + .state + .lock() + .snapshot + .ancestor_inodes_for_path(&entry.path), + is_external: false, + }) + .unwrap(); + } + job.ignore_queue .send(UpdateIgnoreStatusJob { - abs_path: abs_path.into(), + abs_path: abs_path.clone(), ignore_stack: child_ignore_stack, ignore_queue: job.ignore_queue.clone(), + scan_queue: job.scan_queue.clone(), }) .await .unwrap(); @@ -3847,6 +3878,7 @@ struct UpdateIgnoreStatusJob { abs_path: Arc, ignore_stack: Arc, ignore_queue: Sender, + scan_queue: Sender, } pub trait WorktreeHandle { diff --git a/crates/project/src/worktree_tests.rs b/crates/project/src/worktree_tests.rs index b544181292..5cab52a21a 100644 --- a/crates/project/src/worktree_tests.rs +++ b/crates/project/src/worktree_tests.rs @@ -644,6 +644,7 @@ async fn test_dirs_no_longer_ignored(cx: &mut TestAppContext) { (Path::new("a/a.js"), false), (Path::new("b"), false), (Path::new("b/b.js"), false), + // This directory is no longer ignored (Path::new("node_modules"), false), (Path::new("node_modules/c"), false), (Path::new("node_modules/c/c.js"), false), @@ -660,7 +661,7 @@ async fn test_dirs_no_longer_ignored(cx: &mut TestAppContext) { // Each of the newly-loaded directories is scanned only once. let read_dir_count_3 = fs.read_dir_call_count(); - assert_eq!(read_dir_count_3 - read_dir_count_2, 4); + assert_eq!(read_dir_count_3 - read_dir_count_2, 2); } #[gpui::test] From 5350164db91b1efa2d88534729e83071346ec4cc Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Wed, 21 Jun 2023 13:38:30 -0700 Subject: [PATCH 15/21] Get integration test passing. Wait for expand entry on remote projects. --- crates/collab/src/tests/integration_tests.rs | 21 ++++++ crates/project/src/project.rs | 79 ++++++++++++-------- crates/project/src/worktree.rs | 24 +++--- crates/project_panel/src/project_panel.rs | 6 +- crates/rpc/proto/zed.proto | 7 +- crates/rpc/src/proto.rs | 3 +- 6 files changed, 88 insertions(+), 52 deletions(-) diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index 92b63478cb..2211e53263 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/crates/collab/src/tests/integration_tests.rs @@ -1266,6 +1266,27 @@ async fn test_share_project( let client_b_collaborator = project.collaborators().get(&client_b_peer_id).unwrap(); assert_eq!(client_b_collaborator.replica_id, replica_id_b); }); + project_b.read_with(cx_b, |project, cx| { + let worktree = project.worktrees(cx).next().unwrap().read(cx); + assert_eq!( + worktree.paths().map(AsRef::as_ref).collect::>(), + [ + Path::new(".gitignore"), + Path::new("a.txt"), + Path::new("b.txt"), + Path::new("ignored-dir"), + ] + ); + }); + + project_b + .update(cx_b, |project, cx| { + let worktree = project.worktrees(cx).next().unwrap(); + let entry = worktree.read(cx).entry_for_path("ignored-dir").unwrap(); + project.expand_entry(worktree_id, entry.id, cx).unwrap() + }) + .await + .unwrap(); project_b.read_with(cx_b, |project, cx| { let worktree = project.worktrees(cx).next().unwrap().read(cx); assert_eq!( diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 5c71202956..1c9336e2ae 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -1073,6 +1073,40 @@ impl Project { } } + pub fn expand_entry( + &mut self, + worktree_id: WorktreeId, + entry_id: ProjectEntryId, + cx: &mut ModelContext, + ) -> Option>> { + let worktree = self.worktree_for_id(worktree_id, cx)?; + if self.is_local() { + worktree.update(cx, |worktree, cx| { + worktree.as_local_mut().unwrap().expand_entry(entry_id, cx) + }) + } else { + let worktree = worktree.downgrade(); + let request = self.client.request(proto::ExpandProjectEntry { + project_id: self.remote_id().unwrap(), + entry_id: entry_id.to_proto(), + }); + Some(cx.spawn_weak(|_, mut cx| async move { + let response = request.await?; + if let Some(worktree) = worktree.upgrade(&cx) { + worktree + .update(&mut cx, |worktree, _| { + worktree + .as_remote_mut() + .unwrap() + .wait_for_snapshot(response.worktree_scan_id as usize) + }) + .await?; + } + Ok(()) + })) + } + } + pub fn shared(&mut self, project_id: u64, cx: &mut ModelContext) -> Result<()> { if self.client_state.is_some() { return Err(anyhow!("project was already shared")); @@ -5404,31 +5438,6 @@ impl Project { Some(ProjectPath { worktree_id, path }) } - pub fn mark_entry_expanded( - &mut self, - worktree_id: WorktreeId, - entry_id: ProjectEntryId, - cx: &mut ModelContext, - ) -> Option<()> { - if self.is_local() { - let worktree = self.worktree_for_id(worktree_id, cx)?; - worktree.update(cx, |worktree, cx| { - worktree - .as_local_mut() - .unwrap() - .expand_entry_for_id(entry_id, cx); - }); - } else if let Some(project_id) = self.remote_id() { - cx.background() - .spawn(self.client.request(proto::ExpandProjectEntry { - project_id, - entry_id: entry_id.to_proto(), - })) - .log_err(); - } - Some(()) - } - pub fn absolute_path(&self, project_path: &ProjectPath, cx: &AppContext) -> Option { let workspace_root = self .worktree_for_id(project_path.worktree_id, cx)? @@ -5736,18 +5745,22 @@ impl Project { envelope: TypedEnvelope, _: Arc, mut cx: AsyncAppContext, - ) -> Result { + ) -> Result { let entry_id = ProjectEntryId::from_proto(envelope.payload.entry_id); let worktree = this .read_with(&cx, |this, cx| this.worktree_for_entry(entry_id, cx)) .ok_or_else(|| anyhow!("invalid request"))?; - worktree.update(&mut cx, |worktree, cx| { - worktree - .as_local_mut() - .unwrap() - .expand_entry_for_id(entry_id, cx) - }); - Ok(proto::Ack {}) + worktree + .update(&mut cx, |worktree, cx| { + worktree + .as_local_mut() + .unwrap() + .expand_entry(entry_id, cx) + .ok_or_else(|| anyhow!("invalid entry")) + })? + .await?; + let worktree_scan_id = worktree.read_with(&cx, |worktree, _| worktree.scan_id()) as u64; + Ok(proto::ExpandProjectEntryResponse { worktree_scan_id }) } async fn handle_update_diagnostic_summary( diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 39350f1c8f..2f57b84bba 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -1127,21 +1127,17 @@ impl LocalWorktree { })) } - pub fn expand_entry_for_id( + pub fn expand_entry( &mut self, entry_id: ProjectEntryId, - _cx: &mut ModelContext, - ) -> barrier::Receiver { - let (tx, rx) = barrier::channel(); - if let Some(entry) = self.entry_for_id(entry_id) { - self.scan_requests_tx - .try_send(ScanRequest { - relative_paths: vec![entry.path.clone()], - done: tx, - }) - .ok(); - } - rx + cx: &mut ModelContext, + ) -> Option>> { + let path = self.entry_for_id(entry_id)?.path.clone(); + let mut refresh = self.refresh_entries_for_paths(vec![path]); + Some(cx.background().spawn(async move { + refresh.next().await; + Ok(()) + })) } pub fn refresh_entries_for_paths(&self, paths: Vec>) -> barrier::Receiver { @@ -1337,7 +1333,7 @@ impl RemoteWorktree { self.completed_scan_id >= scan_id } - fn wait_for_snapshot(&mut self, scan_id: usize) -> impl Future> { + pub(crate) fn wait_for_snapshot(&mut self, scan_id: usize) -> impl Future> { let (tx, rx) = oneshot::channel(); if self.observed_snapshot(scan_id) { let _ = tx.send(()); diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index c8dfb7cb38..57934c4adb 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -423,7 +423,7 @@ impl ProjectPanel { Ok(_) => self.select_next(&SelectNext, cx), Err(ix) => { self.project.update(cx, |project, cx| { - project.mark_entry_expanded(worktree_id, entry_id, cx); + project.expand_entry(worktree_id, entry_id, cx); }); expanded_dir_ids.insert(ix, entry_id); @@ -477,7 +477,7 @@ impl ProjectPanel { expanded_dir_ids.remove(ix); } Err(ix) => { - project.mark_entry_expanded(worktree_id, entry_id, cx); + project.expand_entry(worktree_id, entry_id, cx); expanded_dir_ids.insert(ix, entry_id); } } @@ -1084,7 +1084,7 @@ impl ProjectPanel { .worktree_for_id(worktree_id, cx) .zip(self.expanded_dir_ids.get_mut(&worktree_id)) { - project.mark_entry_expanded(worktree_id, entry_id, cx); + project.expand_entry(worktree_id, entry_id, cx); let worktree = worktree.read(cx); if let Some(mut entry) = worktree.entry_for_id(entry_id) { diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index 78aab23a44..2bce1ce1e3 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -62,8 +62,9 @@ message Envelope { RenameProjectEntry rename_project_entry = 46; CopyProjectEntry copy_project_entry = 47; DeleteProjectEntry delete_project_entry = 48; - ExpandProjectEntry expand_project_entry = 114; ProjectEntryResponse project_entry_response = 49; + ExpandProjectEntry expand_project_entry = 114; + ExpandProjectEntryResponse expand_project_entry_response = 115; UpdateDiagnosticSummary update_diagnostic_summary = 50; StartLanguageServer start_language_server = 51; @@ -378,6 +379,10 @@ message ExpandProjectEntry { uint64 entry_id = 2; } +message ExpandProjectEntryResponse { + uint64 worktree_scan_id = 1; +} + message ProjectEntryResponse { Entry entry = 1; uint64 worktree_scan_id = 2; diff --git a/crates/rpc/src/proto.rs b/crates/rpc/src/proto.rs index 6311b043d3..4532e798e7 100644 --- a/crates/rpc/src/proto.rs +++ b/crates/rpc/src/proto.rs @@ -201,6 +201,7 @@ messages!( (Ping, Foreground), (PrepareRename, Background), (PrepareRenameResponse, Background), + (ExpandProjectEntryResponse, Foreground), (ProjectEntryResponse, Foreground), (RejoinRoom, Foreground), (RejoinRoomResponse, Foreground), @@ -256,7 +257,7 @@ request_messages!( (CreateRoom, CreateRoomResponse), (DeclineCall, Ack), (DeleteProjectEntry, ProjectEntryResponse), - (ExpandProjectEntry, Ack), + (ExpandProjectEntry, ExpandProjectEntryResponse), (Follow, FollowResponse), (FormatBuffers, FormatBuffersResponse), (GetChannelMessages, GetChannelMessagesResponse), From ffb0a215ea1aa7f2c1b5a6c00545a6fb8778780c Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 22 Jun 2023 16:31:04 -0700 Subject: [PATCH 16/21] Fix randomized worktree test failures * Distinguish between unloaded and pending directories via separate entry kind. * Scan directories before updating ignore statuses after fs events. --- crates/project/src/worktree.rs | 110 +++++++++++++++++---------- crates/project/src/worktree_tests.rs | 10 +-- 2 files changed, 73 insertions(+), 47 deletions(-) diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 2f57b84bba..4e72f2d689 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -1053,17 +1053,13 @@ impl LocalWorktree { Some(cx.spawn(|this, mut cx| async move { let path = delete.await?; - let (tx, mut rx) = barrier::channel(); this.update(&mut cx, |this, _| { this.as_local_mut() .unwrap() - .scan_requests_tx - .try_send(ScanRequest { - relative_paths: vec![path], - done: tx, - }) - })?; - rx.recv().await; + .refresh_entries_for_paths(vec![path]) + }) + .recv() + .await; Ok(()) })) } @@ -2049,7 +2045,9 @@ impl LocalSnapshot { } #[cfg(test)] - pub fn check_invariants(&self) { + pub fn check_invariants(&self, git_state: bool) { + use pretty_assertions::assert_eq; + assert_eq!( self.entries_by_path .cursor::<()>() @@ -2079,7 +2077,11 @@ impl LocalSnapshot { assert!(visible_files.next().is_none()); let mut bfs_paths = Vec::new(); - let mut stack = vec![Path::new("")]; + let mut stack = self + .root_entry() + .map(|e| e.path.as_ref()) + .into_iter() + .collect::>(); while let Some(path) = stack.pop() { bfs_paths.push(path); let ix = stack.len(); @@ -2101,12 +2103,15 @@ impl LocalSnapshot { .collect::>(); assert_eq!(dfs_paths_via_traversal, dfs_paths_via_iter); - for ignore_parent_abs_path in self.ignores_by_parent_abs_path.keys() { - let ignore_parent_path = ignore_parent_abs_path.strip_prefix(&self.abs_path).unwrap(); - assert!(self.entry_for_path(&ignore_parent_path).is_some()); - assert!(self - .entry_for_path(ignore_parent_path.join(&*GITIGNORE)) - .is_some()); + if git_state { + for ignore_parent_abs_path in self.ignores_by_parent_abs_path.keys() { + let ignore_parent_path = + ignore_parent_abs_path.strip_prefix(&self.abs_path).unwrap(); + assert!(self.entry_for_path(&ignore_parent_path).is_some()); + assert!(self + .entry_for_path(ignore_parent_path.join(&*GITIGNORE)) + .is_some()); + } } } @@ -2129,7 +2134,7 @@ impl BackgroundScannerState { || self .snapshot .entry_for_path(&path) - .map_or(true, |entry| self.expanded_dirs.contains(&entry.id)) + .map_or(false, |entry| self.expanded_dirs.contains(&entry.id)) } fn reuse_entry_id(&mut self, entry: &mut Entry) { @@ -2146,6 +2151,10 @@ impl BackgroundScannerState { if entry.path.file_name() == Some(&DOT_GIT) { self.build_repository(entry.path.clone(), fs); } + + #[cfg(test)] + self.snapshot.check_invariants(false); + entry } @@ -2171,7 +2180,7 @@ impl BackgroundScannerState { }; match parent_entry.kind { - EntryKind::PendingDir => parent_entry.kind = EntryKind::Dir, + EntryKind::PendingDir | EntryKind::UnloadedDir => parent_entry.kind = EntryKind::Dir, EntryKind::Dir => {} _ => return, } @@ -2214,6 +2223,9 @@ impl BackgroundScannerState { if let Err(ix) = self.changed_paths.binary_search(parent_path) { self.changed_paths.insert(ix, parent_path.clone()); } + + #[cfg(test)] + self.snapshot.check_invariants(false); } fn remove_path(&mut self, path: &Path) { @@ -2248,6 +2260,9 @@ impl BackgroundScannerState { *needs_update = true; } } + + #[cfg(test)] + self.snapshot.check_invariants(false); } fn reload_repositories(&mut self, changed_paths: &[Arc], fs: &dyn Fs) { @@ -2669,6 +2684,7 @@ pub struct Entry { #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum EntryKind { + UnloadedDir, PendingDir, Dir, File(CharBag), @@ -2737,7 +2753,10 @@ impl Entry { impl EntryKind { pub fn is_dir(&self) -> bool { - matches!(self, EntryKind::Dir | EntryKind::PendingDir) + matches!( + self, + EntryKind::Dir | EntryKind::PendingDir | EntryKind::UnloadedDir + ) } pub fn is_file(&self) -> bool { @@ -3025,6 +3044,8 @@ impl BackgroundScanner { } async fn process_scan_request(&self, request: ScanRequest) -> bool { + log::debug!("rescanning paths {:?}", request.relative_paths); + let root_path = self.expand_paths(&request.relative_paths).await; let root_canonical_path = match self.fs.canonicalize(&root_path).await { Ok(path) => path, @@ -3051,6 +3072,8 @@ impl BackgroundScanner { } async fn process_events(&mut self, abs_paths: Vec) { + log::debug!("received fs events {:?}", abs_paths); + let root_path = self.state.lock().snapshot.abs_path.clone(); let root_canonical_path = match self.fs.canonicalize(&root_path).await { Ok(path) => path, @@ -3069,7 +3092,10 @@ impl BackgroundScanner { Some(scan_job_tx.clone()), ) .await; + drop(scan_job_tx); + self.scan_dirs(false, scan_job_rx).await; + let (scan_job_tx, scan_job_rx) = channel::unbounded(); self.update_ignore_statuses(scan_job_tx).await; self.scan_dirs(false, scan_job_rx).await; @@ -3091,7 +3117,7 @@ impl BackgroundScanner { for path in paths { for ancestor in path.ancestors() { if let Some(entry) = state.snapshot.entry_for_path(ancestor) { - if entry.kind == EntryKind::PendingDir { + if entry.kind == EntryKind::UnloadedDir { let abs_path = root_path.join(ancestor); let ignore_stack = state.snapshot.ignore_stack_for_abs_path(&abs_path, true); @@ -3217,14 +3243,8 @@ impl BackgroundScanner { } async fn scan_dir(&self, job: &ScanJob) -> Result<()> { - log::debug!( - "scan directory {:?}. external: {}", - job.path, - job.is_external - ); + log::debug!("scan directory {:?}", job.path); - let mut new_entries: Vec = Vec::new(); - let mut new_jobs: Vec> = Vec::new(); let mut ignore_stack = job.ignore_stack.clone(); let mut new_ignore = None; let (root_abs_path, root_char_bag, next_entry_id, repository) = { @@ -3240,6 +3260,8 @@ impl BackgroundScanner { }; let mut root_canonical_path = None; + let mut new_entries: Vec = Vec::new(); + let mut new_jobs: Vec> = Vec::new(); let mut child_paths = self.fs.read_dir(&job.abs_path).await?; while let Some(child_abs_path) = child_paths.next().await { let child_abs_path: Arc = match child_abs_path { @@ -3383,25 +3405,26 @@ impl BackgroundScanner { } let mut state = self.state.lock(); - state.populate_dir(&job.path, new_entries, new_ignore, self.fs.as_ref()); - - for new_job in new_jobs { - if let Some(new_job) = new_job { - // If a subdirectory is ignored, or is external to the worktree, don't scan - // it unless it is marked as expanded. - if (new_job.is_external || new_job.ignore_stack.is_all()) - && !state.is_path_expanded(&new_job.path) + let mut new_jobs = new_jobs.into_iter(); + for entry in &mut new_entries { + if entry.is_dir() { + let new_job = new_jobs.next().expect("missing scan job for entry"); + if (entry.is_external || entry.is_ignored) + && entry.kind == EntryKind::PendingDir + && !state.is_path_expanded(&entry.path) { - log::debug!("defer scanning directory {:?}", new_job.path); - continue; + log::debug!("defer scanning directory {:?} {:?}", entry.path, entry.kind); + entry.kind = EntryKind::UnloadedDir; + } else if let Some(new_job) = new_job { + job.scan_queue + .try_send(new_job) + .expect("channel is unbounded"); } - - job.scan_queue - .try_send(new_job) - .expect("channel is unbounded"); } } + assert!(new_jobs.next().is_none()); + state.populate_dir(&job.path, new_entries, new_ignore, self.fs.as_ref()); Ok(()) } @@ -3459,6 +3482,7 @@ impl BackgroundScanner { // detected regardless of the order of the paths. for (path, metadata) in event_paths.iter().zip(metadata.iter()) { if matches!(metadata, Ok(None)) || doing_recursive_update { + log::trace!("remove path {:?}", path); state.remove_path(path); } } @@ -3655,6 +3679,8 @@ impl BackgroundScanner { } async fn update_ignore_status(&self, job: UpdateIgnoreStatusJob, snapshot: &LocalSnapshot) { + log::trace!("update ignore status {:?}", job.abs_path); + let mut ignore_stack = job.ignore_stack; if let Some((ignore, _)) = snapshot.ignores_by_parent_abs_path.get(&job.abs_path) { ignore_stack = ignore_stack.append(job.abs_path.clone(), ignore.clone()); @@ -3679,7 +3705,7 @@ impl BackgroundScanner { if was_ignored && !entry.is_ignored && !entry.is_external - && entry.kind == EntryKind::PendingDir + && entry.kind == EntryKind::UnloadedDir { job.scan_queue .try_send(ScanJob { diff --git a/crates/project/src/worktree_tests.rs b/crates/project/src/worktree_tests.rs index 5cab52a21a..0a228c6830 100644 --- a/crates/project/src/worktree_tests.rs +++ b/crates/project/src/worktree_tests.rs @@ -332,7 +332,7 @@ async fn test_symlinks_pointing_outside(cx: &mut TestAppContext) { assert_eq!( tree.entry_for_path("deps/dep-dir2").unwrap().kind, - EntryKind::PendingDir + EntryKind::UnloadedDir ); }); @@ -915,7 +915,7 @@ async fn test_random_worktree_operations_during_initial_scan( .await .log_err(); worktree.read_with(cx, |tree, _| { - tree.as_local().unwrap().snapshot().check_invariants() + tree.as_local().unwrap().snapshot().check_invariants(true) }); if rng.gen_bool(0.6) { @@ -932,7 +932,7 @@ async fn test_random_worktree_operations_during_initial_scan( let final_snapshot = worktree.read_with(cx, |tree, _| { let tree = tree.as_local().unwrap(); let snapshot = tree.snapshot(); - snapshot.check_invariants(); + snapshot.check_invariants(true); snapshot }); @@ -1037,7 +1037,7 @@ async fn test_random_worktree_changes(cx: &mut TestAppContext, mut rng: StdRng) cx.foreground().run_until_parked(); let snapshot = worktree.read_with(cx, |tree, _| tree.as_local().unwrap().snapshot()); - snapshot.check_invariants(); + snapshot.check_invariants(true); let expanded_paths = snapshot .expanded_entries() .map(|e| e.path.clone()) @@ -1095,7 +1095,7 @@ async fn test_random_worktree_changes(cx: &mut TestAppContext, mut rng: StdRng) fn ignore_pending_dir(entry: &Entry) -> Entry { let mut entry = entry.clone(); - if entry.kind == EntryKind::PendingDir { + if entry.kind.is_dir() { entry.kind = EntryKind::Dir } entry From 9ad1ebf387e0fe31a8a4a898c123dbe43d9d5870 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 22 Jun 2023 17:02:01 -0700 Subject: [PATCH 17/21] Fix project panel test helper --- crates/project_panel/src/project_panel.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 57934c4adb..16876106f4 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -2371,7 +2371,7 @@ mod tests { } let indent = " ".repeat(details.depth); - let icon = if matches!(details.kind, EntryKind::Dir | EntryKind::PendingDir) { + let icon = if details.kind.is_dir() { if details.is_expanded { "v " } else { From b22a18345e2745b4c1f0a52223d80efc8df9162b Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 23 Jun 2023 09:39:37 -0700 Subject: [PATCH 18/21] Emit loaded events for lazily loaded paths in worktree --- crates/project/src/worktree.rs | 30 ++++++++++++++++---- crates/project/src/worktree_tests.rs | 42 ++++++++++++++++++++++++++-- 2 files changed, 64 insertions(+), 8 deletions(-) diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 4e72f2d689..811098540f 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -2759,6 +2759,10 @@ impl EntryKind { ) } + pub fn is_unloaded(&self) -> bool { + matches!(self, EntryKind::UnloadedDir) + } + pub fn is_file(&self) -> bool { matches!(self, EntryKind::File(_)) } @@ -3773,6 +3777,7 @@ impl BackgroundScanner { let mut changes = Vec::new(); let mut old_paths = old_snapshot.entries_by_path.cursor::(); let mut new_paths = new_snapshot.entries_by_path.cursor::(); + let mut last_newly_loaded_dir_path = None; old_paths.next(&()); new_paths.next(&()); for path in event_paths { @@ -3820,20 +3825,33 @@ impl BackgroundScanner { changes.push((old_entry.path.clone(), old_entry.id, Removed)); changes.push((new_entry.path.clone(), new_entry.id, Added)); } else if old_entry != new_entry { - changes.push((new_entry.path.clone(), new_entry.id, Updated)); + if old_entry.kind.is_unloaded() { + last_newly_loaded_dir_path = Some(&new_entry.path); + changes.push(( + new_entry.path.clone(), + new_entry.id, + Loaded, + )); + } else { + changes.push(( + new_entry.path.clone(), + new_entry.id, + Updated, + )); + } } old_paths.next(&()); new_paths.next(&()); } Ordering::Greater => { + let is_newly_loaded = self.phase == InitialScan + || last_newly_loaded_dir_path + .as_ref() + .map_or(false, |dir| new_entry.path.starts_with(&dir)); changes.push(( new_entry.path.clone(), new_entry.id, - if self.phase == InitialScan { - Loaded - } else { - Added - }, + if is_newly_loaded { Loaded } else { Added }, )); new_paths.next(&()); } diff --git a/crates/project/src/worktree_tests.rs b/crates/project/src/worktree_tests.rs index 0a228c6830..a539e138bf 100644 --- a/crates/project/src/worktree_tests.rs +++ b/crates/project/src/worktree_tests.rs @@ -15,6 +15,7 @@ use serde_json::json; use std::{ env, fmt::Write, + mem, path::{Path, PathBuf}, sync::Arc, }; @@ -313,6 +314,21 @@ async fn test_symlinks_pointing_outside(cx: &mut TestAppContext) { cx.read(|cx| tree.read(cx).as_local().unwrap().scan_complete()) .await; + let tree_updates = Arc::new(Mutex::new(Vec::new())); + tree.update(cx, |_, cx| { + let tree_updates = tree_updates.clone(); + cx.subscribe(&tree, move |_, _, event, _| { + if let Event::UpdatedEntries(update) = event { + tree_updates.lock().extend( + update + .iter() + .map(|(path, _, change)| (path.clone(), *change)), + ); + } + }) + .detach(); + }); + // The symlinked directories are not scanned by default. tree.read_with(cx, |tree, _| { assert_eq!( @@ -365,6 +381,14 @@ async fn test_symlinks_pointing_outside(cx: &mut TestAppContext) { ] ); }); + assert_eq!( + mem::take(&mut *tree_updates.lock()), + &[ + (Path::new("deps/dep-dir3").into(), PathChange::Loaded), + (Path::new("deps/dep-dir3/deps").into(), PathChange::Loaded), + (Path::new("deps/dep-dir3/src").into(), PathChange::Loaded) + ] + ); // Expand a subdirectory of one of the symlinked directories. tree.read_with(cx, |tree, _| { @@ -396,6 +420,21 @@ async fn test_symlinks_pointing_outside(cx: &mut TestAppContext) { ] ); }); + + assert_eq!( + mem::take(&mut *tree_updates.lock()), + &[ + (Path::new("deps/dep-dir3/src").into(), PathChange::Loaded), + ( + Path::new("deps/dep-dir3/src/e.rs").into(), + PathChange::Loaded + ), + ( + Path::new("deps/dep-dir3/src/f.rs").into(), + PathChange::Loaded + ) + ] + ); } #[gpui::test] @@ -1114,7 +1153,6 @@ fn check_worktree_change_events(tree: &mut Worktree, cx: &mut ModelContext ix, }; match change_type { - PathChange::Loaded => entries.insert(ix, entry.unwrap()), PathChange::Added => entries.insert(ix, entry.unwrap()), PathChange::Removed => drop(entries.remove(ix)), PathChange::Updated => { @@ -1123,7 +1161,7 @@ fn check_worktree_change_events(tree: &mut Worktree, cx: &mut ModelContext { + PathChange::AddedOrUpdated | PathChange::Loaded => { let entry = entry.unwrap(); if entries.get(ix).map(|e| &e.path) == Some(&entry.path) { *entries.get_mut(ix).unwrap() = entry; From 27b74e9ea151aa2df801d669948ec250c978a64f Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 23 Jun 2023 10:23:21 -0700 Subject: [PATCH 19/21] Prune the set of expanded dir ids as entries are removed --- crates/project/src/worktree.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 811098540f..b627c644a7 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -3030,7 +3030,7 @@ impl BackgroundScanner { // these before handling changes reported by the filesystem. request = self.scan_requests_rx.recv().fuse() => { let Ok(request) = request else { break }; - if !self.process_scan_request(request).await { + if !self.process_scan_request(request, false).await { return; } } @@ -3047,7 +3047,7 @@ impl BackgroundScanner { } } - async fn process_scan_request(&self, request: ScanRequest) -> bool { + async fn process_scan_request(&self, request: ScanRequest, scanning: bool) -> bool { log::debug!("rescanning paths {:?}", request.relative_paths); let root_path = self.expand_paths(&request.relative_paths).await; @@ -3072,7 +3072,7 @@ impl BackgroundScanner { .collect::>(); self.reload_entries_for_paths(root_path, root_canonical_path, abs_paths, None) .await; - self.send_status_update(false, Some(request.done)) + self.send_status_update(scanning, Some(request.done)) } async fn process_events(&mut self, abs_paths: Vec) { @@ -3107,6 +3107,9 @@ impl BackgroundScanner { let mut state = self.state.lock(); state.reload_repositories(&paths, self.fs.as_ref()); state.snapshot.completed_scan_id = state.snapshot.scan_id; + for (_, entry_id) in mem::take(&mut state.removed_entry_ids) { + state.expanded_dirs.remove(&entry_id); + } } self.send_status_update(false, None); @@ -3182,7 +3185,7 @@ impl BackgroundScanner { // the scan queue, so that user operations are prioritized. request = self.scan_requests_rx.recv().fuse() => { let Ok(request) = request else { break }; - if !self.process_scan_request(request).await { + if !self.process_scan_request(request, true).await { return; } } @@ -3664,7 +3667,7 @@ impl BackgroundScanner { // the queue of ignore statuses. request = self.scan_requests_rx.recv().fuse() => { let Ok(request) = request else { break }; - if !self.process_scan_request(request).await { + if !self.process_scan_request(request, true).await { return; } } From 91f87bb31fb01d5effa0c50655c8dded5a3a8329 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 23 Jun 2023 12:53:25 -0700 Subject: [PATCH 20/21] Scan any external/ignored directories needed for LSP watchers Also, don't include "external" files in project searches. Treat them the same as ignored files. Co-authored-by: Nathan Sobo --- crates/project/src/project.rs | 69 ++++++++--- crates/project/src/project_tests.rs | 124 +++++++++++++++++--- crates/project/src/worktree.rs | 165 ++++++++++++++++++--------- crates/project/src/worktree_tests.rs | 6 +- 4 files changed, 273 insertions(+), 91 deletions(-) diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 1c9336e2ae..91c92edb6a 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -64,7 +64,7 @@ use std::{ mem, num::NonZeroU32, ops::Range, - path::{Component, Path, PathBuf}, + path::{self, Component, Path, PathBuf}, rc::Rc, str, sync::{ @@ -3116,23 +3116,44 @@ impl Project { for watcher in params.watchers { for worktree in &self.worktrees { if let Some(worktree) = worktree.upgrade(cx) { - let worktree = worktree.read(cx); - if let Some(abs_path) = worktree.abs_path().to_str() { - if let Some(suffix) = match &watcher.glob_pattern { - lsp::GlobPattern::String(s) => s, - lsp::GlobPattern::Relative(rp) => &rp.pattern, - } - .strip_prefix(abs_path) - .and_then(|s| s.strip_prefix(std::path::MAIN_SEPARATOR)) - { - if let Some(glob) = Glob::new(suffix).log_err() { - builders - .entry(worktree.id()) - .or_insert_with(|| GlobSetBuilder::new()) - .add(glob); + let glob_is_inside_worktree = worktree.update(cx, |tree, _| { + if let Some(abs_path) = tree.abs_path().to_str() { + let relative_glob_pattern = match &watcher.glob_pattern { + lsp::GlobPattern::String(s) => s + .strip_prefix(abs_path) + .and_then(|s| s.strip_prefix(std::path::MAIN_SEPARATOR)), + lsp::GlobPattern::Relative(rp) => { + let base_uri = match &rp.base_uri { + lsp::OneOf::Left(workspace_folder) => { + &workspace_folder.uri + } + lsp::OneOf::Right(base_uri) => base_uri, + }; + base_uri.to_file_path().ok().and_then(|file_path| { + (file_path.to_str() == Some(abs_path)) + .then_some(rp.pattern.as_str()) + }) + } + }; + if let Some(relative_glob_pattern) = relative_glob_pattern { + let literal_prefix = + glob_literal_prefix(&relative_glob_pattern); + tree.as_local_mut() + .unwrap() + .add_path_prefix_to_scan(Path::new(literal_prefix).into()); + if let Some(glob) = Glob::new(relative_glob_pattern).log_err() { + builders + .entry(tree.id()) + .or_insert_with(|| GlobSetBuilder::new()) + .add(glob); + } + return true; } - break; } + false + }); + if glob_is_inside_worktree { + break; } } } @@ -7105,6 +7126,22 @@ impl Project { } } +fn glob_literal_prefix<'a>(glob: &'a str) -> &'a str { + let mut literal_end = 0; + for (i, part) in glob.split(path::MAIN_SEPARATOR).enumerate() { + if part.contains(&['*', '?', '{', '}']) { + break; + } else { + if i > 0 { + // Acount for separator prior to this part + literal_end += path::MAIN_SEPARATOR.len_utf8(); + } + literal_end += part.len(); + } + } + &glob[..literal_end] +} + impl WorktreeHandle { pub fn upgrade(&self, cx: &AppContext) -> Option> { match self { diff --git a/crates/project/src/project_tests.rs b/crates/project/src/project_tests.rs index 3c23c30ab9..478fad74a9 100644 --- a/crates/project/src/project_tests.rs +++ b/crates/project/src/project_tests.rs @@ -535,8 +535,28 @@ async fn test_reporting_fs_changes_to_language_servers(cx: &mut gpui::TestAppCon fs.insert_tree( "/the-root", json!({ - "a.rs": "", - "b.rs": "", + ".gitignore": "target\n", + "src": { + "a.rs": "", + "b.rs": "", + }, + "target": { + "x": { + "out": { + "x.rs": "" + } + }, + "y": { + "out": { + "y.rs": "", + } + }, + "z": { + "out": { + "z.rs": "" + } + } + } }), ) .await; @@ -550,11 +570,32 @@ async fn test_reporting_fs_changes_to_language_servers(cx: &mut gpui::TestAppCon // Start the language server by opening a buffer with a compatible file extension. let _buffer = project .update(cx, |project, cx| { - project.open_local_buffer("/the-root/a.rs", cx) + project.open_local_buffer("/the-root/src/a.rs", cx) }) .await .unwrap(); + // Initially, we don't load ignored files because the language server has not explicitly asked us to watch them. + project.read_with(cx, |project, cx| { + let worktree = project.worktrees(cx).next().unwrap(); + assert_eq!( + worktree + .read(cx) + .snapshot() + .entries(true) + .map(|entry| (entry.path.as_ref(), entry.is_ignored)) + .collect::>(), + &[ + (Path::new(""), false), + (Path::new(".gitignore"), false), + (Path::new("src"), false), + (Path::new("src/a.rs"), false), + (Path::new("src/b.rs"), false), + (Path::new("target"), true), + ] + ); + }); + // Keep track of the FS events reported to the language server. let fake_server = fake_servers.next().await.unwrap(); let file_changes = Arc::new(Mutex::new(Vec::new())); @@ -565,12 +606,20 @@ async fn test_reporting_fs_changes_to_language_servers(cx: &mut gpui::TestAppCon method: "workspace/didChangeWatchedFiles".to_string(), register_options: serde_json::to_value( lsp::DidChangeWatchedFilesRegistrationOptions { - watchers: vec![lsp::FileSystemWatcher { - glob_pattern: lsp::GlobPattern::String( - "/the-root/*.{rs,c}".to_string(), - ), - kind: None, - }], + watchers: vec![ + lsp::FileSystemWatcher { + glob_pattern: lsp::GlobPattern::String( + "/the-root/src/*.{rs,c}".to_string(), + ), + kind: None, + }, + lsp::FileSystemWatcher { + glob_pattern: lsp::GlobPattern::String( + "/the-root/target/y/**/*.rs".to_string(), + ), + kind: None, + }, + ], }, ) .ok(), @@ -588,17 +637,50 @@ async fn test_reporting_fs_changes_to_language_servers(cx: &mut gpui::TestAppCon }); cx.foreground().run_until_parked(); - assert_eq!(file_changes.lock().len(), 0); + assert_eq!(mem::take(&mut *file_changes.lock()), &[]); + + // Now the language server has asked us to watch an ignored directory path, + // so we recursively load it. + project.read_with(cx, |project, cx| { + let worktree = project.worktrees(cx).next().unwrap(); + assert_eq!( + worktree + .read(cx) + .snapshot() + .entries(true) + .map(|entry| (entry.path.as_ref(), entry.is_ignored)) + .collect::>(), + &[ + (Path::new(""), false), + (Path::new(".gitignore"), false), + (Path::new("src"), false), + (Path::new("src/a.rs"), false), + (Path::new("src/b.rs"), false), + (Path::new("target"), true), + (Path::new("target/x"), true), + (Path::new("target/y"), true), + (Path::new("target/y/out"), true), + (Path::new("target/y/out/y.rs"), true), + (Path::new("target/z"), true), + ] + ); + }); // Perform some file system mutations, two of which match the watched patterns, // and one of which does not. - fs.create_file("/the-root/c.rs".as_ref(), Default::default()) + fs.create_file("/the-root/src/c.rs".as_ref(), Default::default()) .await .unwrap(); - fs.create_file("/the-root/d.txt".as_ref(), Default::default()) + fs.create_file("/the-root/src/d.txt".as_ref(), Default::default()) .await .unwrap(); - fs.remove_file("/the-root/b.rs".as_ref(), Default::default()) + fs.remove_file("/the-root/src/b.rs".as_ref(), Default::default()) + .await + .unwrap(); + fs.create_file("/the-root/target/x/out/x2.rs".as_ref(), Default::default()) + .await + .unwrap(); + fs.create_file("/the-root/target/y/out/y2.rs".as_ref(), Default::default()) .await .unwrap(); @@ -608,11 +690,15 @@ async fn test_reporting_fs_changes_to_language_servers(cx: &mut gpui::TestAppCon &*file_changes.lock(), &[ lsp::FileEvent { - uri: lsp::Url::from_file_path("/the-root/b.rs").unwrap(), + uri: lsp::Url::from_file_path("/the-root/src/b.rs").unwrap(), typ: lsp::FileChangeType::DELETED, }, lsp::FileEvent { - uri: lsp::Url::from_file_path("/the-root/c.rs").unwrap(), + uri: lsp::Url::from_file_path("/the-root/src/c.rs").unwrap(), + typ: lsp::FileChangeType::CREATED, + }, + lsp::FileEvent { + uri: lsp::Url::from_file_path("/the-root/target/y/out/y2.rs").unwrap(), typ: lsp::FileChangeType::CREATED, }, ] @@ -3846,6 +3932,14 @@ async fn test_search_with_exclusions_and_inclusions(cx: &mut gpui::TestAppContex ); } +#[test] +fn test_glob_literal_prefix() { + assert_eq!(glob_literal_prefix("**/*.js"), ""); + assert_eq!(glob_literal_prefix("node_modules/**/*.js"), "node_modules"); + assert_eq!(glob_literal_prefix("foo/{bar,baz}.js"), "foo"); + assert_eq!(glob_literal_prefix("foo/bar/baz.js"), "foo/bar/baz.js"); +} + async fn search( project: &ModelHandle, query: SearchQuery, diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index b627c644a7..be3bcd05fa 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -68,6 +68,7 @@ pub enum Worktree { pub struct LocalWorktree { snapshot: LocalSnapshot, scan_requests_tx: channel::Sender, + path_prefixes_to_scan_tx: channel::Sender>, is_scanning: (watch::Sender, watch::Receiver), _background_scanner_task: Task<()>, share: Option, @@ -219,8 +220,9 @@ pub struct LocalSnapshot { struct BackgroundScannerState { snapshot: LocalSnapshot, - expanded_dirs: HashSet, - expanded_paths: HashSet>, + scanned_dirs: HashSet, + path_prefixes_to_scan: HashSet>, + paths_to_scan: HashSet>, /// The ids of all of the entries that were removed from the snapshot /// as part of the current update. These entry ids may be re-used /// if the same inode is discovered at a new path, or if the given @@ -331,6 +333,7 @@ impl Worktree { } let (scan_requests_tx, scan_requests_rx) = channel::unbounded(); + let (path_prefixes_to_scan_tx, path_prefixes_to_scan_rx) = channel::unbounded(); let (scan_states_tx, mut scan_states_rx) = mpsc::unbounded(); cx.spawn_weak(|this, mut cx| async move { @@ -371,6 +374,7 @@ impl Worktree { scan_states_tx, background, scan_requests_rx, + path_prefixes_to_scan_rx, ) .run(events) .await; @@ -382,6 +386,7 @@ impl Worktree { is_scanning: watch::channel_with(true), share: None, scan_requests_tx, + path_prefixes_to_scan_tx, _background_scanner_task: background_scanner_task, diagnostics: Default::default(), diagnostic_summaries: Default::default(), @@ -1147,6 +1152,10 @@ impl LocalWorktree { rx } + pub fn add_path_prefix_to_scan(&self, path_prefix: Arc) { + self.path_prefixes_to_scan_tx.try_send(path_prefix).ok(); + } + fn refresh_entry( &self, path: Arc, @@ -1566,7 +1575,7 @@ impl Snapshot { } pub fn visible_file_count(&self) -> usize { - self.entries_by_path.summary().visible_file_count + self.entries_by_path.summary().non_ignored_file_count } fn traverse_from_offset( @@ -2067,7 +2076,7 @@ impl LocalSnapshot { for entry in self.entries_by_path.cursor::<()>() { if entry.is_file() { assert_eq!(files.next().unwrap().inode, entry.inode); - if !entry.is_ignored { + if !entry.is_ignored && !entry.is_external { assert_eq!(visible_files.next().unwrap().inode, entry.inode); } } @@ -2129,12 +2138,17 @@ impl LocalSnapshot { } impl BackgroundScannerState { - fn is_path_expanded(&self, path: &Path) -> bool { - self.expanded_paths.iter().any(|p| p.starts_with(path)) + fn should_scan_directory(&self, entry: &Entry) -> bool { + (!entry.is_external && !entry.is_ignored) + || self.scanned_dirs.contains(&entry.id) // If we've ever scanned it, keep scanning || self - .snapshot - .entry_for_path(&path) - .map_or(false, |entry| self.expanded_dirs.contains(&entry.id)) + .paths_to_scan + .iter() + .any(|p| p.starts_with(&entry.path)) + || self + .path_prefixes_to_scan + .iter() + .any(|p| entry.path.starts_with(p)) } fn reuse_entry_id(&mut self, entry: &mut Entry) { @@ -2192,17 +2206,16 @@ impl BackgroundScannerState { .insert(abs_parent_path, (ignore, false)); } - self.expanded_dirs.insert(parent_entry.id); + self.scanned_dirs.insert(parent_entry.id); let mut entries_by_path_edits = vec![Edit::Insert(parent_entry)]; let mut entries_by_id_edits = Vec::new(); let mut dotgit_path = None; - for mut entry in entries { + for entry in entries { if entry.path.file_name() == Some(&DOT_GIT) { dotgit_path = Some(entry.path.clone()); } - self.reuse_entry_id(&mut entry); entries_by_id_edits.push(Edit::Insert(PathEntry { id: entry.id, path: entry.path.clone(), @@ -2677,7 +2690,20 @@ pub struct Entry { pub inode: u64, pub mtime: SystemTime, pub is_symlink: bool, + + /// Whether this entry is ignored by Git. + /// + /// We only scan ignored entries once the directory is expanded and + /// exclude them from searches. pub is_ignored: bool, + + /// Whether this entry's canonical path is outside of the worktree. + /// This means the entry is only accessible from the worktree root via a + /// symlink. + /// + /// We only scan entries outside of the worktree once the symlinked + /// directory is expanded. External entries are treated like gitignored + /// entries in that they are not included in searches. pub is_external: bool, pub git_status: Option, } @@ -2772,15 +2798,19 @@ impl sum_tree::Item for Entry { type Summary = EntrySummary; fn summary(&self) -> Self::Summary { - let visible_count = if self.is_ignored { 0 } else { 1 }; + let non_ignored_count = if self.is_ignored || self.is_external { + 0 + } else { + 1 + }; let file_count; - let visible_file_count; + let non_ignored_file_count; if self.is_file() { file_count = 1; - visible_file_count = visible_count; + non_ignored_file_count = non_ignored_count; } else { file_count = 0; - visible_file_count = 0; + non_ignored_file_count = 0; } let mut statuses = GitStatuses::default(); @@ -2796,9 +2826,9 @@ impl sum_tree::Item for Entry { EntrySummary { max_path: self.path.clone(), count: 1, - visible_count, + non_ignored_count, file_count, - visible_file_count, + non_ignored_file_count, statuses, } } @@ -2816,9 +2846,9 @@ impl sum_tree::KeyedItem for Entry { pub struct EntrySummary { max_path: Arc, count: usize, - visible_count: usize, + non_ignored_count: usize, file_count: usize, - visible_file_count: usize, + non_ignored_file_count: usize, statuses: GitStatuses, } @@ -2827,9 +2857,9 @@ impl Default for EntrySummary { Self { max_path: Arc::from(Path::new("")), count: 0, - visible_count: 0, + non_ignored_count: 0, file_count: 0, - visible_file_count: 0, + non_ignored_file_count: 0, statuses: Default::default(), } } @@ -2841,9 +2871,9 @@ impl sum_tree::Summary for EntrySummary { fn add_summary(&mut self, rhs: &Self, _: &()) { self.max_path = rhs.max_path.clone(); self.count += rhs.count; - self.visible_count += rhs.visible_count; + self.non_ignored_count += rhs.non_ignored_count; self.file_count += rhs.file_count; - self.visible_file_count += rhs.visible_file_count; + self.non_ignored_file_count += rhs.non_ignored_file_count; self.statuses += rhs.statuses; } } @@ -2912,6 +2942,7 @@ struct BackgroundScanner { status_updates_tx: UnboundedSender, executor: Arc, scan_requests_rx: channel::Receiver, + path_prefixes_to_scan_rx: channel::Receiver>, next_entry_id: Arc, phase: BackgroundScannerPhase, } @@ -2931,20 +2962,23 @@ impl BackgroundScanner { status_updates_tx: UnboundedSender, executor: Arc, scan_requests_rx: channel::Receiver, + path_prefixes_to_scan_rx: channel::Receiver>, ) -> Self { Self { fs, status_updates_tx, executor, scan_requests_rx, + path_prefixes_to_scan_rx, next_entry_id, state: Mutex::new(BackgroundScannerState { prev_snapshot: snapshot.snapshot.clone(), snapshot, - expanded_dirs: Default::default(), + scanned_dirs: Default::default(), + path_prefixes_to_scan: Default::default(), + paths_to_scan: Default::default(), removed_entry_ids: Default::default(), changed_paths: Default::default(), - expanded_paths: Default::default(), }), phase: BackgroundScannerPhase::InitialScan, } @@ -2952,7 +2986,7 @@ impl BackgroundScanner { async fn run( &mut self, - mut events_rx: Pin>>>, + mut fs_events_rx: Pin>>>, ) { use futures::FutureExt as _; @@ -3014,9 +3048,9 @@ impl BackgroundScanner { // For these events, update events cannot be as precise, because we didn't // have the previous state loaded yet. self.phase = BackgroundScannerPhase::EventsReceivedDuringInitialScan; - if let Poll::Ready(Some(events)) = futures::poll!(events_rx.next()) { + if let Poll::Ready(Some(events)) = futures::poll!(fs_events_rx.next()) { let mut paths = events.into_iter().map(|e| e.path).collect::>(); - while let Poll::Ready(Some(more_events)) = futures::poll!(events_rx.next()) { + while let Poll::Ready(Some(more_events)) = futures::poll!(fs_events_rx.next()) { paths.extend(more_events.into_iter().map(|e| e.path)); } self.process_events(paths).await; @@ -3035,10 +3069,26 @@ impl BackgroundScanner { } } - events = events_rx.next().fuse() => { + path_prefix = self.path_prefixes_to_scan_rx.recv().fuse() => { + let Ok(path_prefix) = path_prefix else { break }; + + self.forcibly_load_paths(&[path_prefix.clone()]).await; + + let abs_path = + { + let mut state = self.state.lock(); + state.path_prefixes_to_scan.insert(path_prefix.clone()); + state.snapshot.abs_path.join(path_prefix) + }; + if let Some(abs_path) = self.fs.canonicalize(&abs_path).await.log_err() { + self.process_events(vec![abs_path]).await; + } + } + + events = fs_events_rx.next().fuse() => { let Some(events) = events else { break }; let mut paths = events.into_iter().map(|e| e.path).collect::>(); - while let Poll::Ready(Some(more_events)) = futures::poll!(events_rx.next()) { + while let Poll::Ready(Some(more_events)) = futures::poll!(fs_events_rx.next()) { paths.extend(more_events.into_iter().map(|e| e.path)); } self.process_events(paths.clone()).await; @@ -3050,7 +3100,7 @@ impl BackgroundScanner { async fn process_scan_request(&self, request: ScanRequest, scanning: bool) -> bool { log::debug!("rescanning paths {:?}", request.relative_paths); - let root_path = self.expand_paths(&request.relative_paths).await; + let root_path = self.forcibly_load_paths(&request.relative_paths).await; let root_canonical_path = match self.fs.canonicalize(&root_path).await { Ok(path) => path, Err(err) => { @@ -3108,14 +3158,14 @@ impl BackgroundScanner { state.reload_repositories(&paths, self.fs.as_ref()); state.snapshot.completed_scan_id = state.snapshot.scan_id; for (_, entry_id) in mem::take(&mut state.removed_entry_ids) { - state.expanded_dirs.remove(&entry_id); + state.scanned_dirs.remove(&entry_id); } } self.send_status_update(false, None); } - async fn expand_paths(&self, paths: &[Arc]) -> Arc { + async fn forcibly_load_paths(&self, paths: &[Arc]) -> Arc { let root_path; let (scan_job_tx, mut scan_job_rx) = channel::unbounded(); { @@ -3140,7 +3190,7 @@ impl BackgroundScanner { is_external: entry.is_external, }) .unwrap(); - state.expanded_paths.insert(path.clone()); + state.paths_to_scan.insert(path.clone()); break; } } @@ -3151,7 +3201,7 @@ impl BackgroundScanner { while let Some(job) = scan_job_rx.next().await { self.scan_dir(&job).await.log_err(); } - self.state.lock().expanded_paths.clear(); + self.state.lock().paths_to_scan.clear(); root_path } @@ -3414,18 +3464,19 @@ impl BackgroundScanner { let mut state = self.state.lock(); let mut new_jobs = new_jobs.into_iter(); for entry in &mut new_entries { + state.reuse_entry_id(entry); + if entry.is_dir() { let new_job = new_jobs.next().expect("missing scan job for entry"); - if (entry.is_external || entry.is_ignored) - && entry.kind == EntryKind::PendingDir - && !state.is_path_expanded(&entry.path) - { + if state.should_scan_directory(&entry) { + if let Some(new_job) = new_job { + job.scan_queue + .try_send(new_job) + .expect("channel is unbounded"); + } + } else { log::debug!("defer scanning directory {:?} {:?}", entry.path, entry.kind); entry.kind = EntryKind::UnloadedDir; - } else if let Some(new_job) = new_job { - job.scan_queue - .try_send(new_job) - .expect("channel is unbounded"); } } } @@ -3865,14 +3916,14 @@ impl BackgroundScanner { old_paths.next(&()); } (None, Some(new_entry)) => { + let is_newly_loaded = self.phase == InitialScan + || last_newly_loaded_dir_path + .as_ref() + .map_or(false, |dir| new_entry.path.starts_with(&dir)); changes.push(( new_entry.path.clone(), new_entry.id, - if self.phase == InitialScan { - Loaded - } else { - Added - }, + if is_newly_loaded { Loaded } else { Added }, )); new_paths.next(&()); } @@ -3975,9 +4026,9 @@ impl WorktreeHandle for ModelHandle { struct TraversalProgress<'a> { max_path: &'a Path, count: usize, - visible_count: usize, + non_ignored_count: usize, file_count: usize, - visible_file_count: usize, + non_ignored_file_count: usize, } impl<'a> TraversalProgress<'a> { @@ -3985,8 +4036,8 @@ impl<'a> TraversalProgress<'a> { match (include_ignored, include_dirs) { (true, true) => self.count, (true, false) => self.file_count, - (false, true) => self.visible_count, - (false, false) => self.visible_file_count, + (false, true) => self.non_ignored_count, + (false, false) => self.non_ignored_file_count, } } } @@ -3995,9 +4046,9 @@ impl<'a> sum_tree::Dimension<'a, EntrySummary> for TraversalProgress<'a> { fn add_summary(&mut self, summary: &'a EntrySummary, _: &()) { self.max_path = summary.max_path.as_ref(); self.count += summary.count; - self.visible_count += summary.visible_count; + self.non_ignored_count += summary.non_ignored_count; self.file_count += summary.file_count; - self.visible_file_count += summary.visible_file_count; + self.non_ignored_file_count += summary.non_ignored_file_count; } } @@ -4006,9 +4057,9 @@ impl<'a> Default for TraversalProgress<'a> { Self { max_path: Path::new(""), count: 0, - visible_count: 0, + non_ignored_count: 0, file_count: 0, - visible_file_count: 0, + non_ignored_file_count: 0, } } } diff --git a/crates/project/src/worktree_tests.rs b/crates/project/src/worktree_tests.rs index a539e138bf..39c6e19d56 100644 --- a/crates/project/src/worktree_tests.rs +++ b/crates/project/src/worktree_tests.rs @@ -332,7 +332,7 @@ async fn test_symlinks_pointing_outside(cx: &mut TestAppContext) { // The symlinked directories are not scanned by default. tree.read_with(cx, |tree, _| { assert_eq!( - tree.entries(false) + tree.entries(true) .map(|entry| (entry.path.as_ref(), entry.is_external)) .collect::>(), vec![ @@ -365,7 +365,7 @@ async fn test_symlinks_pointing_outside(cx: &mut TestAppContext) { // not scanned yet. tree.read_with(cx, |tree, _| { assert_eq!( - tree.entries(false) + tree.entries(true) .map(|entry| (entry.path.as_ref(), entry.is_external)) .collect::>(), vec![ @@ -402,7 +402,7 @@ async fn test_symlinks_pointing_outside(cx: &mut TestAppContext) { // The expanded subdirectory's contents are loaded. tree.read_with(cx, |tree, _| { assert_eq!( - tree.entries(false) + tree.entries(true) .map(|entry| (entry.path.as_ref(), entry.is_external)) .collect::>(), vec![ From 201188fdaa284debcd445b1d70d6918d800a3861 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 27 Jun 2023 15:40:36 -0700 Subject: [PATCH 21/21] Use FakeFs in worktree gitignore test --- crates/project/src/worktree_tests.rs | 64 ++++++++++++++++++---------- 1 file changed, 41 insertions(+), 23 deletions(-) diff --git a/crates/project/src/worktree_tests.rs b/crates/project/src/worktree_tests.rs index 39c6e19d56..553c5e2cca 100644 --- a/crates/project/src/worktree_tests.rs +++ b/crates/project/src/worktree_tests.rs @@ -703,31 +703,33 @@ async fn test_dirs_no_longer_ignored(cx: &mut TestAppContext) { assert_eq!(read_dir_count_3 - read_dir_count_2, 2); } -#[gpui::test] +#[gpui::test(iterations = 10)] async fn test_rescan_with_gitignore(cx: &mut TestAppContext) { - // .gitignores are handled explicitly by Zed and do not use the git - // machinery that the git_tests module checks - let parent_dir = temp_tree(json!({ - ".gitignore": "ancestor-ignored-file1\nancestor-ignored-file2\n", - "tree": { - ".git": {}, - ".gitignore": "ignored-dir\n", - "tracked-dir": { - "tracked-file1": "", - "ancestor-ignored-file1": "", - }, - "ignored-dir": { - "ignored-file1": "" + let fs = FakeFs::new(cx.background()); + fs.insert_tree( + "/root", + json!({ + ".gitignore": "ancestor-ignored-file1\nancestor-ignored-file2\n", + "tree": { + ".git": {}, + ".gitignore": "ignored-dir\n", + "tracked-dir": { + "tracked-file1": "", + "ancestor-ignored-file1": "", + }, + "ignored-dir": { + "ignored-file1": "" + } } - } - })); - let dir = parent_dir.path().join("tree"); + }), + ) + .await; let tree = Worktree::local( build_client(cx), - dir.as_path(), + "/root/tree".as_ref(), true, - Arc::new(RealFs), + fs.clone(), Default::default(), &mut cx.to_async(), ) @@ -764,10 +766,26 @@ async fn test_rescan_with_gitignore(cx: &mut TestAppContext) { ); }); - std::fs::write(dir.join("tracked-dir/tracked-file2"), "").unwrap(); - std::fs::write(dir.join("tracked-dir/ancestor-ignored-file2"), "").unwrap(); - std::fs::write(dir.join("ignored-dir/ignored-file2"), "").unwrap(); - tree.flush_fs_events(cx).await; + fs.create_file( + "/root/tree/tracked-dir/tracked-file2".as_ref(), + Default::default(), + ) + .await + .unwrap(); + fs.create_file( + "/root/tree/tracked-dir/ancestor-ignored-file2".as_ref(), + Default::default(), + ) + .await + .unwrap(); + fs.create_file( + "/root/tree/ignored-dir/ignored-file2".as_ref(), + Default::default(), + ) + .await + .unwrap(); + + cx.foreground().run_until_parked(); cx.read(|cx| { let tree = tree.read(cx); assert!(