From e645aa9d20280716c2098b0824e6c243fe82ccfe Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Wed, 6 Nov 2024 20:36:59 -0700 Subject: [PATCH] Root rename detection (#20313) Closes #5349 Release Notes: - Fixed Zed when the directory that you opened is renamed. --- crates/fs/src/fs.rs | 105 +++++++++++++++++- .../remote_server/src/remote_editing_tests.rs | 39 +++++++ crates/worktree/src/worktree.rs | 56 +++++++++- 3 files changed, 196 insertions(+), 4 deletions(-) diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index 6cfaf82a29..de43dc8e97 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -10,7 +10,11 @@ use git::GitHostingProviderRegistry; #[cfg(target_os = "linux")] use ashpd::desktop::trash; #[cfg(target_os = "linux")] -use std::{fs::File, os::fd::AsFd}; +use std::fs::File; +#[cfg(unix)] +use std::os::fd::AsFd; +#[cfg(unix)] +use std::os::fd::AsRawFd; #[cfg(unix)] use std::os::unix::fs::MetadataExt; @@ -51,14 +55,14 @@ pub trait Watcher: Send + Sync { fn remove(&self, path: &Path) -> Result<()>; } -#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord)] +#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Debug)] pub enum PathEventKind { Removed, Created, Changed, } -#[derive(Clone, PartialEq, Eq, PartialOrd, Ord)] +#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Debug)] pub struct PathEvent { pub path: PathBuf, pub kind: Option, @@ -95,6 +99,7 @@ pub trait Fs: Send + Sync { async fn trash_file(&self, path: &Path, options: RemoveOptions) -> Result<()> { self.remove_file(path, options).await } + async fn open_handle(&self, path: &Path) -> Result>; async fn open_sync(&self, path: &Path) -> Result>; async fn load(&self, path: &Path) -> Result { Ok(String::from_utf8(self.load_bytes(path).await?)?) @@ -187,6 +192,52 @@ pub struct RealFs { git_binary_path: Option, } +pub trait FileHandle: Send + Sync + std::fmt::Debug { + fn current_path(&self, fs: &Arc) -> Result; +} + +impl FileHandle for std::fs::File { + #[cfg(target_os = "macos")] + fn current_path(&self, _: &Arc) -> Result { + use std::{ + ffi::{CStr, OsStr}, + os::unix::ffi::OsStrExt, + }; + + let fd = self.as_fd(); + let mut path_buf: [libc::c_char; libc::PATH_MAX as usize] = [0; libc::PATH_MAX as usize]; + + let result = unsafe { libc::fcntl(fd.as_raw_fd(), libc::F_GETPATH, path_buf.as_mut_ptr()) }; + if result == -1 { + anyhow::bail!("fcntl returned -1".to_string()); + } + + let c_str = unsafe { CStr::from_ptr(path_buf.as_ptr()) }; + let path = PathBuf::from(OsStr::from_bytes(c_str.to_bytes())); + Ok(path) + } + + #[cfg(target_os = "linux")] + fn current_path(&self, _: &Arc) -> Result { + let fd = self.as_fd(); + let fd_path = format!("/proc/self/fd/{}", fd.as_raw_fd()); + let new_path = std::fs::read_link(fd_path)?; + if new_path + .file_name() + .is_some_and(|f| f.to_string_lossy().ends_with(" (deleted)")) + { + anyhow::bail!("file was deleted") + }; + + Ok(new_path) + } + + #[cfg(target_os = "windows")] + fn current_path(&self, _: &Arc) -> Result { + anyhow::bail!("unimplemented") + } +} + pub struct RealWatcher {} impl RealFs { @@ -400,6 +451,10 @@ impl Fs for RealFs { Ok(Box::new(std::fs::File::open(path)?)) } + async fn open_handle(&self, path: &Path) -> Result> { + Ok(Arc::new(std::fs::File::open(path)?)) + } + async fn load(&self, path: &Path) -> Result { let path = path.to_path_buf(); let text = smol::unblock(|| std::fs::read_to_string(path)).await?; @@ -755,6 +810,7 @@ struct FakeFsState { buffered_events: Vec, metadata_call_count: usize, read_dir_call_count: usize, + moves: std::collections::HashMap, } #[cfg(any(test, feature = "test-support"))] @@ -926,6 +982,7 @@ impl FakeFs { events_paused: false, read_dir_call_count: 0, metadata_call_count: 0, + moves: Default::default(), }), }); @@ -1362,6 +1419,27 @@ impl Watcher for FakeWatcher { } } +#[cfg(any(test, feature = "test-support"))] +#[derive(Debug)] +struct FakeHandle { + inode: u64, +} + +#[cfg(any(test, feature = "test-support"))] +impl FileHandle for FakeHandle { + fn current_path(&self, fs: &Arc) -> Result { + let state = fs.as_fake().state.lock(); + let Some(target) = state.moves.get(&self.inode) else { + anyhow::bail!("fake fd not moved") + }; + + if state.try_read_path(&target, false).is_some() { + return Ok(target.clone()); + } + anyhow::bail!("fake fd target not found") + } +} + #[cfg(any(test, feature = "test-support"))] #[async_trait::async_trait] impl Fs for FakeFs { @@ -1500,6 +1578,14 @@ impl Fs for FakeFs { } })?; + let inode = match *moved_entry.lock() { + FakeFsEntry::File { inode, .. } => inode, + FakeFsEntry::Dir { inode, .. } => inode, + _ => 0, + }; + + state.moves.insert(inode, new_path.clone()); + state.write_path(&new_path, |e| { match e { btree_map::Entry::Occupied(mut e) => { @@ -1644,6 +1730,19 @@ impl Fs for FakeFs { Ok(Box::new(io::Cursor::new(bytes))) } + async fn open_handle(&self, path: &Path) -> Result> { + self.simulate_random_delay().await; + let state = self.state.lock(); + let entry = state.read_path(&path)?; + let entry = entry.lock(); + let inode = match *entry { + FakeFsEntry::File { inode, .. } => inode, + FakeFsEntry::Dir { inode, .. } => inode, + _ => unreachable!(), + }; + Ok(Arc::new(FakeHandle { inode })) + } + async fn load(&self, path: &Path) -> Result { let content = self.load_internal(path).await?; Ok(String::from_utf8(content.clone())?) diff --git a/crates/remote_server/src/remote_editing_tests.rs b/crates/remote_server/src/remote_editing_tests.rs index 530e1c43e3..0ced691939 100644 --- a/crates/remote_server/src/remote_editing_tests.rs +++ b/crates/remote_server/src/remote_editing_tests.rs @@ -1086,6 +1086,45 @@ async fn test_reconnect(cx: &mut TestAppContext, server_cx: &mut TestAppContext) ); } +#[gpui::test] +async fn test_remote_root_rename(cx: &mut TestAppContext, server_cx: &mut TestAppContext) { + let fs = FakeFs::new(server_cx.executor()); + fs.insert_tree( + "/code", + json!({ + "project1": { + ".git": {}, + "README.md": "# project 1", + }, + }), + ) + .await; + + let (project, _) = init_test(&fs, cx, server_cx).await; + + let (worktree, _) = project + .update(cx, |project, cx| { + project.find_or_create_worktree("/code/project1", true, cx) + }) + .await + .unwrap(); + + cx.run_until_parked(); + + fs.rename( + &PathBuf::from("/code/project1"), + &PathBuf::from("/code/project2"), + Default::default(), + ) + .await + .unwrap(); + + cx.run_until_parked(); + worktree.update(cx, |worktree, _| { + assert_eq!(worktree.root_name(), "project2") + }) +} + #[gpui::test] async fn test_remote_git_branches(cx: &mut TestAppContext, server_cx: &mut TestAppContext) { let fs = FakeFs::new(server_cx.executor()); diff --git a/crates/worktree/src/worktree.rs b/crates/worktree/src/worktree.rs index 0d9f359b3b..1d1ad7da83 100644 --- a/crates/worktree/src/worktree.rs +++ b/crates/worktree/src/worktree.rs @@ -285,6 +285,9 @@ pub struct LocalSnapshot { /// All of the git repositories in the worktree, indexed by the project entry /// id of their parent directory. git_repositories: TreeMap, + /// The file handle of the root dir + /// (so we can find it after it's been moved) + root_file_handle: Option>, } struct BackgroundScannerState { @@ -341,6 +344,9 @@ enum ScanState { barrier: SmallVec<[barrier::Sender; 1]>, scanning: bool, }, + RootUpdated { + new_path: Option>, + }, } struct UpdateObservationState { @@ -382,6 +388,8 @@ impl Worktree { true }); + let root_file_handle = fs.open_handle(&abs_path).await.log_err(); + cx.new_model(move |cx: &mut ModelContext| { let mut snapshot = LocalSnapshot { ignores_by_parent_abs_path: Default::default(), @@ -393,6 +401,7 @@ impl Worktree { .map_or(String::new(), |f| f.to_string_lossy().to_string()), abs_path, ), + root_file_handle, }; if let Some(metadata) = metadata { @@ -1076,6 +1085,17 @@ impl LocalWorktree { this.set_snapshot(snapshot, changes, cx); drop(barrier); } + ScanState::RootUpdated { new_path } => { + if let Some(new_path) = new_path { + this.snapshot.git_repositories = Default::default(); + this.snapshot.ignores_by_parent_abs_path = Default::default(); + let root_name = new_path + .file_name() + .map_or(String::new(), |f| f.to_string_lossy().to_string()); + this.snapshot.update_abs_path(new_path, root_name); + } + this.restart_background_scanners(cx); + } } cx.notify(); }) @@ -2073,12 +2093,24 @@ impl Snapshot { .and_then(|entry| entry.git_status) } + fn update_abs_path(&mut self, abs_path: Arc, root_name: String) { + self.abs_path = abs_path; + if root_name != self.root_name { + self.root_char_bag = root_name.chars().map(|c| c.to_ascii_lowercase()).collect(); + self.root_name = root_name; + } + } + pub(crate) fn apply_remote_update(&mut self, mut update: proto::UpdateWorktree) -> Result<()> { log::trace!( "applying remote worktree update. {} entries updated, {} removed", update.updated_entries.len(), update.removed_entries.len() ); + self.update_abs_path( + Arc::from(PathBuf::from(update.abs_path).as_path()), + update.root_name, + ); let mut entries_by_path_edits = Vec::new(); let mut entries_by_id_edits = Vec::new(); @@ -3732,7 +3764,29 @@ impl BackgroundScanner { let root_canonical_path = match self.fs.canonicalize(&root_path).await { Ok(path) => path, Err(err) => { - log::error!("failed to canonicalize root path: {}", err); + let new_path = self + .state + .lock() + .snapshot + .root_file_handle + .clone() + .and_then(|handle| handle.current_path(&self.fs).log_err()) + .filter(|new_path| **new_path != *root_path); + + if let Some(new_path) = new_path.as_ref() { + log::info!( + "root renamed from {} to {}", + root_path.display(), + new_path.display() + ) + } else { + log::warn!("root path could not be canonicalized: {}", err); + } + self.status_updates_tx + .unbounded_send(ScanState::RootUpdated { + new_path: new_path.map(|p| p.into()), + }) + .ok(); return; } };