diff --git a/Cargo.lock b/Cargo.lock index 2f63722843..4029b426c5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11410,7 +11410,7 @@ dependencies = [ [[package]] name = "notify" version = "9.0.0-rc.4" -source = "git+https://github.com/zed-industries/notify?rev=0890bbb8ca40a4b5d1f67031698dd7918b37d991#0890bbb8ca40a4b5d1f67031698dd7918b37d991" +source = "git+https://github.com/zed-industries/notify?rev=d842f16b2716bd60f09caf3ae3a894237ab38f54#d842f16b2716bd60f09caf3ae3a894237ab38f54" dependencies = [ "bitflags 2.13.1", "inotify 0.11.0", @@ -11454,7 +11454,7 @@ dependencies = [ [[package]] name = "notify-types" version = "2.1.0" -source = "git+https://github.com/zed-industries/notify?rev=0890bbb8ca40a4b5d1f67031698dd7918b37d991#0890bbb8ca40a4b5d1f67031698dd7918b37d991" +source = "git+https://github.com/zed-industries/notify?rev=d842f16b2716bd60f09caf3ae3a894237ab38f54#d842f16b2716bd60f09caf3ae3a894237ab38f54" dependencies = [ "bitflags 2.13.1", ] diff --git a/Cargo.toml b/Cargo.toml index 8c4c71f044..f3d5d49741 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -982,8 +982,8 @@ windows-capture = { git = "https://github.com/zed-industries/windows-capture.git calloop = { git = "https://github.com/zed-industries/calloop" } livekit = { git = "https://github.com/zed-industries/livekit-rust-sdks", rev = "0a1c519cfce9b365229026b55de9b9dbdb6fed3c" } libwebrtc = { git = "https://github.com/zed-industries/livekit-rust-sdks", rev = "0a1c519cfce9b365229026b55de9b9dbdb6fed3c" } -notify = { git = "https://github.com/zed-industries/notify", rev = "0890bbb8ca40a4b5d1f67031698dd7918b37d991" } -notify-types = { git = "https://github.com/zed-industries/notify", rev = "0890bbb8ca40a4b5d1f67031698dd7918b37d991" } +notify = { git = "https://github.com/zed-industries/notify", rev = "d842f16b2716bd60f09caf3ae3a894237ab38f54" } +notify-types = { git = "https://github.com/zed-industries/notify", rev = "d842f16b2716bd60f09caf3ae3a894237ab38f54" } webrtc-sys = { git = "https://github.com/zed-industries/livekit-rust-sdks", rev = "0a1c519cfce9b365229026b55de9b9dbdb6fed3c" } # corgi runs build.rs/proc-macros in a sandbox with no write access outside the diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index e592934a4a..f128b7e579 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -1438,7 +1438,6 @@ struct FakeFsState { metadata_call_count: usize, read_dir_call_count: usize, path_write_counts: std::collections::HashMap, - moves: std::collections::HashMap, job_event_subscribers: Arc>>, trash: Mutex>, file_to_create_before_watch_add: Option<(PathBuf, PathBuf)>, @@ -1772,7 +1771,6 @@ impl FakeFs { read_dir_call_count: 0, metadata_call_count: 0, path_write_counts: Default::default(), - moves: Default::default(), job_event_subscribers: Arc::new(Mutex::new(Vec::new())), trash: Mutex::new(SlotMap::with_key()), file_to_create_before_watch_add: None, @@ -2869,13 +2867,23 @@ struct FakeHandle { impl FileHandle for FakeHandle { fn current_path(&self, fs: &Arc) -> Result { let fs = fs.as_fake(); - let mut state = fs.state.lock(); - let Some(target) = state.moves.get(&self.inode).cloned() else { - anyhow::bail!("fake fd not moved") - }; - - if state.try_entry(&target, false).is_some() { - return Ok(target); + let state = fs.state.lock(); + let mut queue = collections::VecDeque::new(); + queue.push_back((PathBuf::from(util::path!("/")), &state.root)); + while let Some((path, entry)) = queue.pop_front() { + match entry { + FakeFsEntry::File { inode, .. } | FakeFsEntry::Dir { inode, .. } + if *inode == self.inode => + { + return Ok(path); + } + FakeFsEntry::Dir { entries, .. } => { + for (name, entry) in entries { + queue.push_back((path.join(name), entry)); + } + } + _ => {} + } } anyhow::bail!("fake fd target not found") } @@ -3030,12 +3038,6 @@ impl Fs for FakeFs { return Ok(()); } - let inode = match moved_entry { - FakeFsEntry::File { inode, .. } => inode, - FakeFsEntry::Dir { inode, .. } => inode, - _ => 0, - }; - let mut moved = true; state.write_path(&new_path, |e| { match e { @@ -3062,8 +3064,6 @@ impl Fs for FakeFs { return Ok(()); } - state.moves.insert(inode, new_path.clone()); - state .write_path(&old_path, |e| { if let btree_map::Entry::Occupied(e) = e { diff --git a/crates/fs/tests/integration/fs_tests.rs b/crates/fs/tests/integration/fs_tests.rs index a18aef6edd..9b7fc979cd 100644 --- a/crates/fs/tests/integration/fs_tests.rs +++ b/crates/fs/tests/integration/fs_tests.rs @@ -631,11 +631,11 @@ async fn test_fake_fs_rename_ignore_if_exists_leaves_source_and_target_unchanged "from target" ); - // An ignored rename must not be recorded as a move either, or a handle held - // across it reports a path its file never went to. - assert!( - handle.current_path(&(fs.clone() as Arc)).is_err(), - "an ignored rename should not record a move" + // A handle held across an ignored rename must keep reporting the path the + // file is actually at, not the one it never went to. + assert_eq!( + handle.current_path(&(fs.clone() as Arc)).unwrap(), + PathBuf::from(path!("/root/source.txt")) ); } diff --git a/crates/worktree/src/worktree.rs b/crates/worktree/src/worktree.rs index bffbe72449..9d218c7741 100644 --- a/crates/worktree/src/worktree.rs +++ b/crates/worktree/src/worktree.rs @@ -82,6 +82,12 @@ pub use worktree_settings::WorktreeSettings; pub const FS_WATCH_LATENCY: Duration = Duration::from_millis(100); +/// How often the background scanner verifies that the worktree root still +/// exists at its recorded path. Native watchers report the root itself being +/// renamed or deleted, but not a rename of one of its ancestors, which leaves +/// the watcher silently attached to a path that no longer exists. +pub const ROOT_PATH_CHECK_INTERVAL: Duration = Duration::from_secs(5); + /// A set of local or remote files that are being opened as part of a project. /// Responsible for tracking related FS (for local)/collab (for remote) events and corresponding updates. /// Stores git repositories data and the diagnostics for the file(s). @@ -4558,6 +4564,10 @@ impl BackgroundScanner { // Continue processing events until the worktree is dropped. self.phase = BackgroundScannerPhase::Events; + let root_path_check_timer = self.executor.timer(ROOT_PATH_CHECK_INTERVAL).fuse(); + futures::pin_mut!(root_path_check_timer); + let mut root_path_missing = false; + loop { select_biased! { // Process any path refresh requests from the worktree. Prioritize @@ -4612,6 +4622,20 @@ impl BackgroundScanner { self.update_global_gitignore(&path).await; } } + + _ = root_path_check_timer => { + let root_path = self.state.lock().await.snapshot.abs_path.clone(); + match self.fs.canonicalize(root_path.as_path()).await { + Ok(_) => root_path_missing = false, + Err(error) => { + if !root_path_missing { + root_path_missing = true; + self.report_root_moved_or_deleted(&root_path, &error).await; + } + } + } + root_path_check_timer.set(self.executor.timer(ROOT_PATH_CHECK_INTERVAL).fuse()); + } } } } @@ -4746,56 +4770,67 @@ impl BackgroundScanner { events } + /// Called when the worktree root can no longer be canonicalized. Uses the + /// open handle on the root to find where it moved to, and reports the new + /// location (or, for single-file worktrees, the deletion) to the worktree. + async fn report_root_moved_or_deleted( + &self, + root_path: &Arc, + canonicalize_error: &anyhow::Error, + ) { + let new_path = self + .state + .lock() + .await + .snapshot + .root_file_handle + .clone() + .and_then(|handle| match handle.current_path(&self.fs) { + Ok(new_path) => Some(new_path), + Err(e) => { + log::error!("Failed to refresh worktree root path: {e:#}"); + None + } + }) + .map(|path| SanitizedPath::new_arc(&path)) + .filter(|new_path| new_path != root_path); + + if let Some(new_path) = new_path { + log::info!( + "root renamed from {:?} to {:?}", + root_path.as_path(), + new_path.as_path(), + ); + self.status_updates_tx + .unbounded_send(ScanState::RootUpdated { new_path }) + .ok(); + } else { + log::error!("root path could not be canonicalized: {canonicalize_error:#}"); + + // For single-file worktrees, if we can't canonicalize and the file handle + // fallback also failed, the file is gone - close the worktree + if self.is_single_file { + log::info!( + "single-file worktree root {:?} no longer exists, marking as deleted", + root_path.as_path() + ); + self.status_updates_tx + .unbounded_send(ScanState::RootDeleted) + .ok(); + } + } + } + async fn process_events(&self, mut events: Vec) { let root_path = self.state.lock().await.snapshot.abs_path.clone(); - let root_canonical_path = self.fs.canonicalize(root_path.as_path()).await; - let root_canonical_path = match &root_canonical_path { - Ok(path) => SanitizedPath::new(path), - Err(err) => { - let new_path = self - .state - .lock() - .await - .snapshot - .root_file_handle - .clone() - .and_then(|handle| match handle.current_path(&self.fs) { - Ok(new_path) => Some(new_path), - Err(e) => { - log::error!("Failed to refresh worktree root path: {e:#}"); - None - } - }) - .map(|path| SanitizedPath::new_arc(&path)) - .filter(|new_path| *new_path != root_path); - - if let Some(new_path) = new_path { - log::info!( - "root renamed from {:?} to {:?}", - root_path.as_path(), - new_path.as_path(), - ); - self.status_updates_tx - .unbounded_send(ScanState::RootUpdated { new_path }) - .ok(); - } else { - log::error!("root path could not be canonicalized: {err:#}"); - - // For single-file worktrees, if we can't canonicalize and the file handle - // fallback also failed, the file is gone - close the worktree - if self.is_single_file { - log::info!( - "single-file worktree root {:?} no longer exists, marking as deleted", - root_path.as_path() - ); - self.status_updates_tx - .unbounded_send(ScanState::RootDeleted) - .ok(); - } - } + let root_canonical_path = match self.fs.canonicalize(root_path.as_path()).await { + Ok(path) => path, + Err(error) => { + self.report_root_moved_or_deleted(&root_path, &error).await; return; } }; + let root_canonical_path = SanitizedPath::new(&root_canonical_path); { let state = self.state.lock().await; diff --git a/crates/worktree/tests/integration/worktree_tests.rs b/crates/worktree/tests/integration/worktree_tests.rs index be84ed1436..8f6476bbde 100644 --- a/crates/worktree/tests/integration/worktree_tests.rs +++ b/crates/worktree/tests/integration/worktree_tests.rs @@ -5949,6 +5949,70 @@ async fn test_single_file_worktree_deleted(cx: &mut TestAppContext) { ); } +#[gpui::test] +async fn test_root_ancestor_rename_is_detected_without_fs_events(cx: &mut TestAppContext) { + init_test(cx); + let fs = FakeFs::new(cx.background_executor.clone()); + fs.insert_tree( + path!("/code"), + json!({ + "project": { + "src": { + "main.rs": "fn main() {}", + }, + }, + }), + ) + .await; + + let tree = Worktree::local( + Path::new(path!("/code/project")), + 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; + + // Renaming an ancestor of the root produces no event under the watched + // path, so only the periodic root check can notice it. + fs.rename( + Path::new(path!("/code")), + Path::new(path!("/src")), + Default::default(), + ) + .await + .unwrap(); + cx.background_executor.run_until_parked(); + tree.read_with(cx, |tree, _| { + assert_eq!(tree.abs_path().as_ref(), Path::new(path!("/code/project"))); + }); + + cx.background_executor + .advance_clock(worktree::ROOT_PATH_CHECK_INTERVAL); + cx.background_executor.run_until_parked(); + + tree.read_with(cx, |tree, _| { + assert_eq!(tree.abs_path().as_ref(), Path::new(path!("/src/project"))); + assert_eq!(tree.root_name(), "project"); + assert_eq!( + tree.entries(false, 0) + .map(|entry| entry.path.clone()) + .collect::>(), + vec![ + rel_path("").into(), + rel_path("src").into(), + rel_path("src/main.rs").into(), + ] + ); + }); +} + #[gpui::test] async fn test_remote_worktree_without_git_emits_root_repo_event_after_first_update( cx: &mut TestAppContext,