From 017b2db6309fc72e852193e208df2c8e6f2484ff Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Wed, 14 Feb 2024 16:00:31 +0100 Subject: [PATCH] Fix case-only renaming of files (#7768) This fixes #5211 and #7732 by fixing the case-only file renaming. The fix here works by checking hooking into function that produces the data to populate the project panel. It checks whether we're on a case-insensitive file system (default on macOS, but you can have case-sensitive FS on macOS too) and if so, it ignores the metadata for files for which the absolute path (returned by the FS scanner) and canonicalized path do NOT match. That's the case for (a) symlinks and (b) case-only renames of files. It only does this check for case-only renames. Release Notes: - Fixed case-only renaming of files producing duplicate entries in project panel. ([#5211](https://github.com/zed-industries/zed/issues/5211)). Co-authored-by: Antonio --- crates/fs/src/fs.rs | 45 ++++++++++++++++++++- crates/project/src/worktree.rs | 26 ++++++++++++ crates/project/src/worktree_tests.rs | 59 ++++++++++++++++++++++++++++ 3 files changed, 129 insertions(+), 1 deletion(-) diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index 571de91a17..885b94e3a3 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -26,7 +26,7 @@ use std::{ pin::Pin, time::{Duration, SystemTime}, }; -use tempfile::NamedTempFile; +use tempfile::{NamedTempFile, TempDir}; use text::LineEnding; use util::ResultExt; @@ -66,6 +66,7 @@ pub trait Fs: Send + Sync { fn open_repo(&self, abs_dot_git: &Path) -> Option>>; fn is_fake(&self) -> bool; + async fn is_case_sensitive(&self) -> Result; #[cfg(any(test, feature = "test-support"))] fn as_fake(&self) -> &FakeFs; } @@ -339,6 +340,44 @@ impl Fs for RealFs { fn is_fake(&self) -> bool { false } + + /// Checks whether the file system is case sensitive by attempting to create two files + /// that have the same name except for the casing. + /// + /// It creates both files in a temporary directory it removes at the end. + async fn is_case_sensitive(&self) -> Result { + let temp_dir = TempDir::new()?; + let test_file_1 = temp_dir.path().join("case_sensitivity_test.tmp"); + let test_file_2 = temp_dir.path().join("CASE_SENSITIVITY_TEST.TMP"); + + let create_opts = CreateOptions { + overwrite: false, + ignore_if_exists: false, + }; + + // Create file1 + self.create_file(&test_file_1, create_opts).await?; + + // Now check whether it's possible to create file2 + let case_sensitive = match self.create_file(&test_file_2, create_opts).await { + Ok(_) => Ok(true), + Err(e) => { + if let Some(io_error) = e.downcast_ref::() { + if io_error.kind() == io::ErrorKind::AlreadyExists { + Ok(false) + } else { + Err(e) + } + } else { + Err(e) + } + } + }; + + temp_dir.close()?; + case_sensitive + } + #[cfg(any(test, feature = "test-support"))] fn as_fake(&self) -> &FakeFs { panic!("called `RealFs::as_fake`") @@ -1186,6 +1225,10 @@ impl Fs for FakeFs { true } + async fn is_case_sensitive(&self) -> Result { + Ok(true) + } + #[cfg(any(test, feature = "test-support"))] fn as_fake(&self) -> &FakeFs { self diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 21994397fe..054a5b9933 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -638,10 +638,18 @@ fn start_background_scan_tasks( let background = cx.background_executor().clone(); async move { let events = fs.watch(&abs_path, Duration::from_millis(100)).await; + let case_sensitive = fs.is_case_sensitive().await.unwrap_or_else(|e| { + log::error!( + "Failed to determine whether filesystem is case sensitive (falling back to true) due to error: {e:#}" + ); + true + }); + BackgroundScanner::new( snapshot, next_entry_id, fs, + case_sensitive, scan_states_tx, background, scan_requests_rx, @@ -3229,6 +3237,7 @@ impl<'a> sum_tree::Dimension<'a, EntrySummary> for PathKey { struct BackgroundScanner { state: Mutex, fs: Arc, + fs_case_sensitive: bool, status_updates_tx: UnboundedSender, executor: BackgroundExecutor, scan_requests_rx: channel::Receiver, @@ -3249,6 +3258,7 @@ impl BackgroundScanner { snapshot: LocalSnapshot, next_entry_id: Arc, fs: Arc, + fs_case_sensitive: bool, status_updates_tx: UnboundedSender, executor: BackgroundExecutor, scan_requests_rx: channel::Receiver, @@ -3256,6 +3266,7 @@ impl BackgroundScanner { ) -> Self { Self { fs, + fs_case_sensitive, status_updates_tx, executor, scan_requests_rx, @@ -3884,6 +3895,21 @@ impl BackgroundScanner { let metadata = self.fs.metadata(abs_path).await?; if let Some(metadata) = metadata { let canonical_path = self.fs.canonicalize(abs_path).await?; + + // If we're on a case-insensitive filesystem (default on macOS), we want + // to only ignore metadata for non-symlink files if their absolute-path matches + // the canonical-path. + // Because if not, this might be a case-only-renaming (`mv test.txt TEST.TXT`) + // and we want to ignore the metadata for the old path (`test.txt`) so it's + // treated as removed. + if !self.fs_case_sensitive && !metadata.is_symlink { + let canonical_file_name = canonical_path.file_name(); + let file_name = abs_path.file_name(); + if canonical_file_name != file_name { + return Ok(None); + } + } + anyhow::Ok(Some((metadata, canonical_path))) } else { Ok(None) diff --git a/crates/project/src/worktree_tests.rs b/crates/project/src/worktree_tests.rs index 26a180e5f9..e3725d43d5 100644 --- a/crates/project/src/worktree_tests.rs +++ b/crates/project/src/worktree_tests.rs @@ -444,6 +444,65 @@ async fn test_symlinks_pointing_outside(cx: &mut TestAppContext) { ); } +#[cfg(target_os = "macos")] +#[gpui::test] +async fn test_renaming_case_only(cx: &mut TestAppContext) { + cx.executor().allow_parking(); + init_test(cx); + + const OLD_NAME: &str = "aaa.rs"; + const NEW_NAME: &str = "AAA.rs"; + + let fs = Arc::new(RealFs); + let temp_root = temp_tree(json!({ + OLD_NAME: "", + })); + + let tree = Worktree::local( + build_client(cx), + temp_root.path(), + 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()) + .collect::>(), + vec![Path::new(""), Path::new(OLD_NAME)] + ); + }); + + fs.rename( + &temp_root.path().join(OLD_NAME), + &temp_root.path().join(NEW_NAME), + fs::RenameOptions { + overwrite: true, + ignore_if_exists: true, + }, + ) + .await + .unwrap(); + + tree.flush_fs_events(cx).await; + + tree.read_with(cx, |tree, _| { + assert_eq!( + tree.entries(true) + .map(|entry| entry.path.as_ref()) + .collect::>(), + vec![Path::new(""), Path::new(NEW_NAME)] + ); + }); +} + #[gpui::test] async fn test_open_gitignored_files(cx: &mut TestAppContext) { init_test(cx);