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 <antonio@zed.dev>
This commit is contained in:
Thorsten Ball 2024-02-14 16:00:31 +01:00 committed by GitHub
parent 75eac4783b
commit 017b2db630
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 129 additions and 1 deletions

View File

@ -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<Arc<Mutex<dyn GitRepository>>>;
fn is_fake(&self) -> bool;
async fn is_case_sensitive(&self) -> Result<bool>;
#[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<bool> {
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::<io::Error>() {
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<bool> {
Ok(true)
}
#[cfg(any(test, feature = "test-support"))]
fn as_fake(&self) -> &FakeFs {
self

View File

@ -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<BackgroundScannerState>,
fs: Arc<dyn Fs>,
fs_case_sensitive: bool,
status_updates_tx: UnboundedSender<ScanState>,
executor: BackgroundExecutor,
scan_requests_rx: channel::Receiver<ScanRequest>,
@ -3249,6 +3258,7 @@ impl BackgroundScanner {
snapshot: LocalSnapshot,
next_entry_id: Arc<AtomicUsize>,
fs: Arc<dyn Fs>,
fs_case_sensitive: bool,
status_updates_tx: UnboundedSender<ScanState>,
executor: BackgroundExecutor,
scan_requests_rx: channel::Receiver<ScanRequest>,
@ -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)

View File

@ -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<_>>(),
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<_>>(),
vec![Path::new(""), Path::new(NEW_NAME)]
);
});
}
#[gpui::test]
async fn test_open_gitignored_files(cx: &mut TestAppContext) {
init_test(cx);