diff --git a/crates/editor/src/hover_links.rs b/crates/editor/src/hover_links.rs index 00d02a11bd..0190aa788a 100644 --- a/crates/editor/src/hover_links.rs +++ b/crates/editor/src/hover_links.rs @@ -2180,6 +2180,7 @@ mod tests { "This is file2.rs".as_bytes().to_vec(), ) .await; + cx.run_until_parked(); // Base document with {ABS} placeholder for absolute path prefix. // Each test case replaces a specific line to add cursor (ˇ) or highlight («»ˇ) markers. @@ -2349,6 +2350,7 @@ Sentence ending file2.rs. .to_vec(), ) .await; + cx.run_until_parked(); // file2.rs:5:3 should be highlighted and clickable cx.set_state(indoc! {" @@ -2425,6 +2427,7 @@ Sentence ending file2.rs. .to_vec(), ) .await; + cx.run_until_parked(); // file2.rs:3 should be highlighted and clickable cx.set_state(indoc! {" @@ -2482,6 +2485,7 @@ Sentence ending file2.rs. "line 1\nline 2\nline 3\n".as_bytes().to_vec(), ) .await; + cx.run_until_parked(); // file2.rs:2:in should resolve to file2.rs line 2 (like Ruby backtraces) cx.set_state(indoc! {" @@ -2540,6 +2544,7 @@ Sentence ending file2.rs. .to_vec(), ) .await; + cx.run_until_parked(); // Markdown link [text](file2.rs:3:2) should highlight only the inner link, // not the surrounding markdown syntax. diff --git a/crates/git_ui/src/git_graph.rs b/crates/git_ui/src/git_graph.rs index d010200350..4e45583b4b 100644 --- a/crates/git_ui/src/git_graph.rs +++ b/crates/git_ui/src/git_graph.rs @@ -5399,7 +5399,7 @@ mod tests { let commits = generate_random_commit_dag(&mut rng, 10, false); fs.set_graph_commits(Path::new("/project/.git"), commits.clone()); - let project = Project::test(fs.clone(), [Path::new("/project")], cx).await; + let project = Project::test(fs.clone(), [], cx).await; let observed_repository_events = Arc::new(Mutex::new(Vec::new())); project.update(cx, |project, cx| { let observed_repository_events = observed_repository_events.clone(); @@ -5413,6 +5413,13 @@ mod tests { }) .detach(); }); + project + .update(cx, |project, cx| { + project.create_worktree("/project", true, cx) + }) + .await + .unwrap(); + cx.run_until_parked(); let repository = project.read_with(cx, |project, cx| { project diff --git a/crates/project/src/worktree_store.rs b/crates/project/src/worktree_store.rs index e0367051e0..2445ad57b0 100644 --- a/crates/project/src/worktree_store.rs +++ b/crates/project/src/worktree_store.rs @@ -797,27 +797,21 @@ impl WorktreeStore { } }; - self.loading_worktrees - .insert(abs_path.clone(), task.shared()); + let loading_task = cx.spawn({ + let abs_path = abs_path.clone(); + async move |this, cx| { + let result = task.await; + this.update(cx, |this, cx| { + this.loading_worktrees.remove(&abs_path); + if !visible || !this.scanning_enabled || result.is_err() { + this.update_initial_scan_state(cx); + } + }) + .ok(); - if visible && self.scanning_enabled { - *self.initial_scan_complete.0.borrow_mut() = false; - } - } - let task = self.loading_worktrees.get(&abs_path).unwrap().clone(); - cx.spawn(async move |this, cx| { - let result = task.await; - this.update(cx, |this, cx| { - this.loading_worktrees.remove(&abs_path); - if !visible || !this.scanning_enabled || result.is_err() { - this.update_initial_scan_state(cx); - } - }) - .ok(); - - match result { - Ok(worktree) => { - if !is_via_collab { + if let Ok(worktree) = &result + && !is_via_collab + { if let Some((trusted_worktrees, worktree_store)) = this .update(cx, |_, cx| { TrustedWorktrees::try_get_global(cx).zip(Some(cx.entity())) @@ -836,16 +830,25 @@ impl WorktreeStore { this.update(cx, |this, cx| { if this.scanning_enabled && visible { - this.observe_worktree_scan_completion(&worktree, cx); + this.observe_worktree_scan_completion(worktree, cx); } }) .ok(); } - Ok(worktree) + + result } - Err(err) => Err((*err).cloned()), + }); + + self.loading_worktrees + .insert(abs_path.clone(), loading_task.shared()); + + if visible && self.scanning_enabled { + *self.initial_scan_complete.0.borrow_mut() = false; } - }) + } + let task = self.loading_worktrees.get(&abs_path).unwrap().clone(); + cx.background_spawn(async move { task.await.map_err(|error| (*error).cloned()) }) } fn create_remote_worktree( diff --git a/crates/project/tests/integration/project_tests.rs b/crates/project/tests/integration/project_tests.rs index 6cefd2a231..967ed7e223 100644 --- a/crates/project/tests/integration/project_tests.rs +++ b/crates/project/tests/integration/project_tests.rs @@ -16252,6 +16252,81 @@ mod disable_ai_settings_tests { } } +#[gpui::test] +async fn test_worktree_released_when_creation_caller_is_cancelled(cx: &mut gpui::TestAppContext) { + init_test(cx); + let fs = FakeFs::new(cx.executor()); + fs.insert_tree(path!("/project"), serde_json::json!({ "a.rs": "" })) + .await; + fs.insert_tree(path!("/other"), serde_json::json!({ "b.rs": "" })) + .await; + let project = Project::test(fs, [path!("/project").as_ref()], cx).await; + cx.run_until_parked(); + + // Drop the returned task immediately, as a cancelled caller would; creation + // still completes through the clone held by `loading_worktrees`. + let task = project.update(cx, |project, cx| { + project.find_or_create_worktree(path!("/other"), true, cx) + }); + drop(task); + cx.run_until_parked(); + + let created = project.read_with(cx, |project, cx| { + project + .worktrees(cx) + .find(|worktree| worktree.read(cx).abs_path().ends_with("other")) + .map(|worktree| (worktree.read(cx).id(), worktree.downgrade())) + }); + + let Some((worktree_id, weak_worktree)) = created else { + panic!("worktree creation should complete even when the caller is cancelled"); + }; + + project.update(cx, |project, cx| project.remove_worktree(worktree_id, cx)); + cx.run_until_parked(); + + assert!( + weak_worktree.upgrade().is_none(), + "removed worktree should be released even when its creation task's caller was cancelled" + ); +} + +#[gpui::test] +async fn test_initial_scan_completes_when_creation_caller_is_cancelled( + cx: &mut gpui::TestAppContext, +) { + init_test(cx); + let fs = FakeFs::new(cx.executor()); + fs.insert_tree(path!("/project"), serde_json::json!({ "a.rs": "" })) + .await; + fs.insert_tree(path!("/other"), serde_json::json!({ "b.rs": "" })) + .await; + let project = Project::test(fs, [path!("/project").as_ref()], cx).await; + cx.run_until_parked(); + + let task = project.update(cx, |project, cx| { + project.find_or_create_worktree(path!("/other"), true, cx) + }); + drop(task); + cx.run_until_parked(); + + let (initial_scan_completed, loading_worktrees) = project.read_with(cx, |project, cx| { + let worktree_store = project.worktree_store().read(cx); + ( + worktree_store.initial_scan_completed(), + worktree_store.diagnostics(cx).loading_worktrees, + ) + }); + assert_eq!( + loading_worktrees, 0, + "no loading entry should remain after creation resolves" + ); + assert!( + initial_scan_completed, + "initial scan should complete even when the creation task's caller was cancelled" + ); +} + /// Regression test for https://github.com/zed-industries/zed/issues/60424. /// /// The committed text contains repeated `end\n\n` line runs, so the deletion diff --git a/crates/worktree/src/worktree.rs b/crates/worktree/src/worktree.rs index 5c7e336023..123debb348 100644 --- a/crates/worktree/src/worktree.rs +++ b/crates/worktree/src/worktree.rs @@ -288,11 +288,43 @@ struct BackgroundScannerState { /// inode-based rename heuristics in that case. #[derive(Default)] struct RemovedEntries { + current: RemovedEntriesGeneration, + previous: RemovedEntriesGeneration, +} + +#[derive(Default)] +struct RemovedEntriesGeneration { by_inode: HashMap, by_path: HashMap, Entry>, } impl RemovedEntries { + fn insert(&mut self, entry: &Entry) { + self.current.insert(entry); + } + + fn take_by_path(&mut self, path: &RelPath, inode: u64) -> Option { + self.current + .take_by_path(path, inode) + .or_else(|| self.previous.take_by_path(path, inode)) + } + + fn take_by_inode(&mut self, inode: u64) -> Option { + self.current + .take_by_inode(inode) + .or_else(|| self.previous.take_by_inode(inode)) + } + + fn rotate(&mut self) -> impl Iterator { + let dropped = mem::replace(&mut self.previous, mem::take(&mut self.current)); + dropped + .by_inode + .into_values() + .chain(dropped.by_path.into_values()) + } +} + +impl RemovedEntriesGeneration { fn insert(&mut self, entry: &Entry) { self.by_path.insert(entry.path.clone(), entry.clone()); match self.by_inode.entry(entry.inode) { @@ -4577,7 +4609,28 @@ impl BackgroundScanner { async fn process_scan_request(&self, mut request: ScanRequest, scanning: bool) -> bool { log::debug!("rescanning paths {:?}", request.relative_paths); + { + let state = self.state.lock().await; + let mut missing_ancestors = Vec::new(); + for path in &request.relative_paths { + let mut candidates = Vec::new(); + let mut covered_by_existing_ancestor = false; + for ancestor in path.ancestors().skip(1) { + if let Some(entry) = state.snapshot.entry_for_path(ancestor) { + covered_by_existing_ancestor = + entry.kind == EntryKind::UnloadedDir || entry.kind == EntryKind::File; + break; + } + candidates.push(ancestor.into_arc()); + } + if !covered_by_existing_ancestor { + missing_ancestors.extend(candidates); + } + } + request.relative_paths.extend(missing_ancestors); + } request.relative_paths.sort_unstable(); + request.relative_paths.dedup(); self.forcibly_load_paths(&request.relative_paths).await; let root_path = self.state.lock().await.snapshot.abs_path.clone(); @@ -5099,8 +5152,8 @@ impl BackgroundScanner { { let mut state = self.state.lock().await; state.snapshot.completed_scan_id = state.snapshot.scan_id; - let RemovedEntries { by_inode, by_path } = mem::take(&mut state.removed_entries); - for entry in by_inode.into_values().chain(by_path.into_values()) { + let dropped_entries = state.removed_entries.rotate().collect::>(); + for entry in dropped_entries { state.scanned_dirs.remove(&entry.id); } } diff --git a/crates/worktree/tests/integration/worktree_tests.rs b/crates/worktree/tests/integration/worktree_tests.rs index 71bd95b5bc..02e0e3dfb2 100644 --- a/crates/worktree/tests/integration/worktree_tests.rs +++ b/crates/worktree/tests/integration/worktree_tests.rs @@ -87,6 +87,140 @@ async fn test_traversal(cx: &mut TestAppContext) { }) } +#[gpui::test] +async fn test_entry_id_is_reused_when_rename_events_are_split(cx: &mut TestAppContext) { + init_test(cx); + let fs = FakeFs::new(cx.background_executor.clone()); + fs.insert_tree( + path!("/root"), + json!({ + "one.rs": "const ONE: usize = 1;", + }), + ) + .await; + + let tree = Worktree::local( + Path::new(path!("/root")), + true, + fs.clone(), + Default::default(), + true, + WorktreeId::from_proto(0), + &mut cx.to_async(), + ) + .await + .unwrap(); + cx.read(|cx| tree.read(cx).as_local().unwrap().scan_complete()) + .await; + + let old_entry_id = tree.read_with(cx, |tree, _| { + tree.entry_for_path(rel_path("one.rs")).unwrap().id + }); + + fs.pause_events(); + fs.rename( + Path::new(path!("/root/one.rs")), + Path::new(path!("/root/three.rs")), + Default::default(), + ) + .await + .unwrap(); + assert_eq!(fs.buffered_event_count(), 2); + fs.flush_events(1); + cx.executor().run_until_parked(); + fs.flush_events(1); + cx.executor().run_until_parked(); + + tree.read_with(cx, |tree, _| { + assert_eq!(tree.entry_for_path(rel_path("one.rs")), None); + assert_eq!( + tree.entry_for_path(rel_path("three.rs")).unwrap().id, + old_entry_id, + "a rename whose removal and creation events arrive in separate batches must reuse the entry id" + ); + }); +} + +#[gpui::test] +async fn test_rescan_requests_processed_before_root_creation_events(cx: &mut TestAppContext) { + init_test(cx); + let fs = FakeFs::new(cx.background_executor.clone()); + + let tree = Worktree::local( + Path::new(path!("/root")), + true, + fs.clone(), + Default::default(), + true, + WorktreeId::from_proto(0), + &mut cx.to_async(), + ) + .await + .unwrap(); + cx.read(|cx| tree.read(cx).as_local().unwrap().scan_complete()) + .await; + + fs.pause_events(); + fs.insert_tree( + path!("/root"), + json!({ + "a": { + "b": { "f1": "" }, + "c": { "f2": "" }, + } + }), + ) + .await; + + let mut refresh = tree.update(cx, |tree, _| { + tree.as_local() + .unwrap() + .refresh_entries_for_paths(vec![rel_path("a/b/f1").into()]) + }); + refresh.recv().await; + let mut refresh = tree.update(cx, |tree, _| { + tree.as_local() + .unwrap() + .refresh_entries_for_paths(vec![rel_path("a/c/f2").into()]) + }); + refresh.recv().await; + + tree.read_with(cx, |tree, _| { + assert_eq!( + tree.entries(true, 0) + .map(|entry| entry.path.as_ref()) + .collect::>(), + vec![ + rel_path(""), + rel_path("a"), + rel_path("a/b"), + rel_path("a/b/f1"), + rel_path("a/c"), + rel_path("a/c/f2"), + ] + ); + }); + + fs.unpause_events_and_flush(); + cx.executor().run_until_parked(); + + tree.read_with(cx, |tree, _| { + assert_eq!( + tree.entries(true, 0) + .map(|entry| entry.path.as_ref()) + .collect::>(), + vec![ + rel_path(""), + rel_path("a"), + rel_path("a/b"), + rel_path("a/b/f1"), + rel_path("a/c"), + rel_path("a/c/f2"), + ] + ); + }); +} + #[gpui::test(iterations = 10)] async fn test_circular_symlinks(cx: &mut TestAppContext) { init_test(cx);