From 7620b7833722279aaced4255517cf85b8000153e Mon Sep 17 00:00:00 2001 From: prayansh_chhablani <135210710+prayanshchh@users.noreply.github.com> Date: Tue, 21 Apr 2026 13:52:49 +0530 Subject: [PATCH] Fix zed irresponsive on symlinked directory events outside the editor (#50746) Closes #48729, closes #27263, closes #45954 This PR aims to make zed responsive on symlinked directory events outside the editor. Before you mark this PR as ready for review, make sure that you have: - [ ] Added a solid test coverage and/or screenshots from doing manual testing - [x] Done a self-review taking into account security and performance aspects - [ ] Aligned any UI changes with the [UI checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) new-linked-folder is inside /zed-test/zed-project output of ls -ld new-linked-folder `lrwxr-xr-x 1 prayanshchhablani staff 42 28 Mar 23:20 new-linked-folder -> /Users/prayanshchhablani/new-target-folder` this shows new-linked-folder is a symlink folder whose target is new-target-folder which is outside the root dir of the project opened in zed. https://github.com/user-attachments/assets/ffebafc3-2fc4-4293-bdbf-3a894a45e276 Release Notes: - Fixed file watching of symlinks that point outside of the project/watched directory. Zed should now properly respond to changes in files in symlinked directories --- crates/settings/src/settings_file.rs | 36 ++++ crates/worktree/src/worktree.rs | 109 +++++++++- crates/worktree/tests/integration/main.rs | 249 ++++++++++++++++++++++ 3 files changed, 392 insertions(+), 2 deletions(-) diff --git a/crates/settings/src/settings_file.rs b/crates/settings/src/settings_file.rs index efc5e45130e244b5da457b3f1f1f687800fe3b33..258deaa9ee4c64119601fabfdae38b79c654c81b 100644 --- a/crates/settings/src/settings_file.rs +++ b/crates/settings/src/settings_file.rs @@ -58,6 +58,41 @@ mod tests { fs.unpause_events_and_flush(); assert_eq!(rx.next().await.as_deref(), Some("A")); } + + #[gpui::test] + async fn test_watch_config_file_reloads_when_parent_dir_is_symlink(cx: &mut TestAppContext) { + cx.executor().allow_parking(); + let fs = FakeFs::new(cx.background_executor.clone()); + let config_settings_path = PathBuf::from("/root/.config/zed/settings.json"); + let target_settings_path = PathBuf::from("/root/dotfiles/zed/settings.json"); + + fs.insert_tree( + Path::new("/root"), + json!({ + ".config": {}, + "dotfiles": { + "zed": { + "settings.json": "A" + } + } + }), + ) + .await; + + fs.create_symlink( + Path::new("/root/.config/zed"), + PathBuf::from("/root/dotfiles/zed"), + ) + .await + .unwrap(); + + let (mut rx, _task) = + watch_config_file(&cx.background_executor, fs.clone(), config_settings_path); + assert_eq!(rx.next().await.as_deref(), Some("A")); + + fs.insert_file(&target_settings_path, b"B".to_vec()).await; + assert_eq!(rx.next().await.as_deref(), Some("B")); + } } pub const EMPTY_THEME_NAME: &str = "empty-theme"; @@ -134,6 +169,7 @@ pub fn watch_config_file( ) -> (mpsc::UnboundedReceiver, gpui::Task<()>) { let (tx, rx) = mpsc::unbounded(); let task = executor.spawn(async move { + let path = fs.canonicalize(&path).await.unwrap_or_else(|_| path); let (events, _) = fs.watch(&path, Duration::from_millis(100)).await; futures::pin_mut!(events); diff --git a/crates/worktree/src/worktree.rs b/crates/worktree/src/worktree.rs index 55d40e1416307622f71c244dc12e79884a76bbac..690ba6887c6fc24dec0338206e46b87b22611e90 100644 --- a/crates/worktree/src/worktree.rs +++ b/crates/worktree/src/worktree.rs @@ -260,7 +260,9 @@ pub struct LocalSnapshot { struct BackgroundScannerState { snapshot: LocalSnapshot, + symlink_paths_by_target: HashMap, SmallVec<[Arc; 1]>>, scanned_dirs: HashSet, + watched_dir_abs_paths_by_entry_id: HashMap>, path_prefixes_to_scan: HashSet>, paths_to_scan: HashSet>, /// The ids of all of the entries that were removed from the snapshot @@ -1171,7 +1173,9 @@ impl LocalWorktree { state: async_lock::Mutex::new(BackgroundScannerState { prev_snapshot: snapshot.snapshot.clone(), snapshot, + symlink_paths_by_target: Default::default(), scanned_dirs: Default::default(), + watched_dir_abs_paths_by_entry_id: Default::default(), scanning_enabled, path_prefixes_to_scan: Default::default(), paths_to_scan: Default::default(), @@ -3151,7 +3155,12 @@ impl BackgroundScannerState { let mut removed_dir_abs_paths = Vec::new(); for entry in removed_entries.cursor::<()>(()) { if entry.is_dir() { - removed_dir_abs_paths.push(self.snapshot.absolutize(&entry.path)); + let watch_path = self + .watched_dir_abs_paths_by_entry_id + .remove(&entry.id) + .map(|path| path.as_ref().to_path_buf()) + .unwrap_or_else(|| self.snapshot.absolutize(&entry.path)); + removed_dir_abs_paths.push(watch_path); } match self.removed_entries.entry(entry.inode) { @@ -4194,6 +4203,67 @@ impl BackgroundScanner { self.send_status_update(scanning, request.done, &[]).await } + fn normalized_events_for_worktree( + state: &BackgroundScannerState, + root_canonical_path: &SanitizedPath, + mut events: Vec, + ) -> Vec { + if state.symlink_paths_by_target.is_empty() { + return events; + } + let mut mapped_events = Vec::new(); + + events.retain(|event| { + let abs_path = SanitizedPath::new(&event.path); + + let mut best_match: Option<(&Arc, &SmallVec<[Arc; 1]>)> = None; + let mut best_depth = 0; + for (target_root, symlink_paths) in &state.symlink_paths_by_target { + if abs_path.as_path().starts_with(target_root.as_ref()) { + let depth = target_root.as_ref().components().count(); + if depth > best_depth { + best_depth = depth; + best_match = Some((target_root, symlink_paths)); + } + } + } + + let Some((target_root, symlink_paths)) = best_match else { + return true; + }; + + let Ok(suffix) = abs_path.as_path().strip_prefix(target_root.as_ref()) else { + return true; + }; + + // If the symlink's real target is outside this worktree, the original path + // isn't visible to the worktree. Keep only the remapped symlink events. + let keep_original = target_root.starts_with(root_canonical_path.as_path()); + + for symlink_path in symlink_paths { + let mapped_path = if suffix.as_os_str().is_empty() { + root_canonical_path + .as_path() + .join(symlink_path.as_std_path()) + } else { + root_canonical_path + .as_path() + .join(symlink_path.as_std_path()) + .join(suffix) + }; + if mapped_path != event.path { + mapped_events.push(PathEvent { + path: mapped_path, + kind: event.kind, + }); + } + } + keep_original + }); + events.extend(mapped_events); + events + } + 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; @@ -4245,6 +4315,11 @@ impl BackgroundScanner { } }; + { + let state = self.state.lock().await; + events = Self::normalized_events_for_worktree(&state, &root_canonical_path, events); + } + // Certain directories may have FS changes, but do not lead to git data changes that Zed cares about. // Ignore these, to avoid Zed unnecessarily rescanning git metadata. let skipped_files_in_dot_git = [COMMIT_MESSAGE, INDEX_LOCK]; @@ -4513,7 +4588,15 @@ impl BackgroundScanner { if let Some(entry) = state.snapshot.entry_for_path(ancestor) && entry.kind == EntryKind::UnloadedDir { - let abs_path = root_path.join(ancestor.as_std_path()); + let abs_path = if entry.is_external { + entry + .canonical_path + .as_ref() + .map(|path| path.as_ref().to_path_buf()) + .unwrap_or_else(|| root_path.join(ancestor.as_std_path())) + } else { + root_path.join(ancestor.as_std_path()) + }; state .enqueue_scan_dir( abs_path.into(), @@ -4777,6 +4860,17 @@ impl BackgroundScanner { child_entry.is_external = true; } + if child_metadata.is_dir { + let mut state = self.state.lock().await; + let paths = state + .symlink_paths_by_target + .entry(Arc::from(canonical_path.clone())) + .or_default(); + if !paths.iter().any(|path| path == &child_path) { + paths.push(child_path.clone()); + } + } + child_entry.canonical_path = Some(canonical_path.into()); } @@ -4852,8 +4946,19 @@ impl BackgroundScanner { } state.populate_dir(job.path.clone(), new_entries, new_ignore); + self.watcher.add(job.abs_path.as_ref()).log_err(); + let entry_id = state + .snapshot + .entry_for_path(&job.path) + .map(|entry| entry.id); + if let Some(entry_id) = entry_id { + state + .watched_dir_abs_paths_by_entry_id + .insert(entry_id, job.abs_path.clone()); + } + for new_job in new_jobs.into_iter().flatten() { job.scan_queue .try_send(new_job) diff --git a/crates/worktree/tests/integration/main.rs b/crates/worktree/tests/integration/main.rs index 8b6a03fd1f02e725dc119283007798745ffdcaed..922da7f1bf2b73f0e0494c3d03e6f50005435851 100644 --- a/crates/worktree/tests/integration/main.rs +++ b/crates/worktree/tests/integration/main.rs @@ -199,6 +199,9 @@ async fn test_symlinks_pointing_outside(cx: &mut TestAppContext) { "src": { "e.rs": "", "f.rs": "", + "nested": { + "deep.rs": "" + } }, } }), @@ -212,6 +215,18 @@ async fn test_symlinks_pointing_outside(cx: &mut TestAppContext) { fs.create_symlink("/root/dir1/deps/dep-dir3".as_ref(), "../../dir3".into()) .await .unwrap(); + fs.create_symlink( + "/root/dir1/deps/dep-dir3-alias".as_ref(), + "../../dir3".into(), + ) + .await + .unwrap(); + fs.create_symlink( + "/root/dir1/deps/dep-dir3-nested".as_ref(), + "../../dir3/src/nested".into(), + ) + .await + .unwrap(); let tree = Worktree::local( Path::new("/root/dir1"), @@ -254,6 +269,8 @@ async fn test_symlinks_pointing_outside(cx: &mut TestAppContext) { (rel_path("deps"), false), (rel_path("deps/dep-dir2"), true), (rel_path("deps/dep-dir3"), true), + (rel_path("deps/dep-dir3-alias"), true), + (rel_path("deps/dep-dir3-nested"), true), (rel_path("src"), false), (rel_path("src/a.rs"), false), (rel_path("src/b.rs"), false), @@ -289,6 +306,8 @@ async fn test_symlinks_pointing_outside(cx: &mut TestAppContext) { (rel_path("deps/dep-dir3"), true), (rel_path("deps/dep-dir3/deps"), true), (rel_path("deps/dep-dir3/src"), true), + (rel_path("deps/dep-dir3-alias"), true), + (rel_path("deps/dep-dir3-nested"), true), (rel_path("src"), false), (rel_path("src/a.rs"), false), (rel_path("src/b.rs"), false), @@ -328,6 +347,9 @@ async fn test_symlinks_pointing_outside(cx: &mut TestAppContext) { (rel_path("deps/dep-dir3/src"), true), (rel_path("deps/dep-dir3/src/e.rs"), true), (rel_path("deps/dep-dir3/src/f.rs"), true), + (rel_path("deps/dep-dir3/src/nested"), true), + (rel_path("deps/dep-dir3-alias"), true), + (rel_path("deps/dep-dir3-nested"), true), (rel_path("src"), false), (rel_path("src/a.rs"), false), (rel_path("src/b.rs"), false), @@ -346,9 +368,220 @@ async fn test_symlinks_pointing_outside(cx: &mut TestAppContext) { ( rel_path("deps/dep-dir3/src/f.rs").into(), PathChange::Loaded + ), + ( + rel_path("deps/dep-dir3/src/nested").into(), + PathChange::Loaded ) ] ); + + // After an external symlink subtree is loaded, changes in the target should be reflected. + fs.insert_file(Path::new("/root/dir3/src/new.rs"), b"".to_vec()) + .await; + + wait_for_condition(cx, |cx| { + tree.read_with(cx, |tree, _| { + tree.entry_for_path(rel_path("deps/dep-dir3/src/new.rs")) + .is_some() + }) + }) + .await; + + tree.read_with(cx, |tree, _| { + assert!( + tree.entry_for_path(rel_path("deps/dep-dir3/src/new.rs")) + .is_some() + ); + }); + + tree.read_with(cx, |tree, _| { + tree.as_local() + .unwrap() + .refresh_entries_for_paths(vec![rel_path("deps/dep-dir3-alias").into()]) + }) + .recv() + .await; + + tree.read_with(cx, |tree, _| { + tree.as_local() + .unwrap() + .refresh_entries_for_paths(vec![rel_path("deps/dep-dir3-alias/src").into()]) + }) + .recv() + .await; + + tree.read_with(cx, |tree, _| { + tree.as_local() + .unwrap() + .refresh_entries_for_paths(vec![rel_path("deps/dep-dir3-nested").into()]) + }) + .recv() + .await; + // Create a file in the shared target subtree. Because dep-dir3 and dep-dir3-alias both + // point to the same target, both logical paths should observe the new file. + fs.insert_file(Path::new("/root/dir3/src/shared-new.rs"), b"".to_vec()) + .await; + + wait_for_condition(cx, |cx| { + tree.read_with(cx, |tree, _| { + tree.entry_for_path(rel_path("deps/dep-dir3/src/shared-new.rs")) + .is_some() + && tree + .entry_for_path(rel_path("deps/dep-dir3-alias/src/shared-new.rs")) + .is_some() + }) + }) + .await; + + tree.read_with(cx, |tree, _| { + assert!( + tree.entry_for_path(rel_path("deps/dep-dir3/src/shared-new.rs")) + .is_some() + ); + assert!( + tree.entry_for_path(rel_path("deps/dep-dir3-alias/src/shared-new.rs")) + .is_some() + ); + }); + + // Create a file under the more specific nested target. Longest-prefix matching means this should appear under dep-dir3-nested + fs.insert_file( + Path::new("/root/dir3/src/nested/longest-prefix.rs"), + b"".to_vec(), + ) + .await; + + wait_for_condition(cx, |cx| { + tree.read_with(cx, |tree, _| { + tree.entry_for_path(rel_path("deps/dep-dir3-nested/longest-prefix.rs")) + .is_some() + }) + }) + .await; + + tree.read_with(cx, |tree, _| { + assert!( + tree.entry_for_path(rel_path("deps/dep-dir3-nested/longest-prefix.rs")) + .is_some() + ); + assert!( + tree.entry_for_path(rel_path("deps/dep-dir3/src/nested/longest-prefix.rs")) + .is_none() + ); + assert!( + tree.entry_for_path(rel_path("deps/dep-dir3-alias/src/nested/longest-prefix.rs")) + .is_none() + ); + }); +} + +#[gpui::test] +async fn test_symlinked_dir_inside_project(cx: &mut TestAppContext) { + init_test(cx); + let fs = FakeFs::new(cx.background_executor.clone()); + + fs.insert_tree( + "/root", + json!({ + "project": { + "real-dir": { + "existing.rs": "", + "nested": { + "deep.rs": "" + } + }, + "links": {} + } + }), + ) + .await; + + fs.create_symlink( + "/root/project/links/internal".as_ref(), + "../real-dir".into(), + ) + .await + .unwrap(); + + let tree = Worktree::local( + Path::new("/root/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; + + tree.read_with(cx, |tree, _| { + assert_eq!( + tree.entries(true, 0) + .map(|entry| (entry.path.as_ref(), entry.is_external)) + .collect::>(), + vec![ + (rel_path(""), false), + (rel_path("links"), false), + (rel_path("links/internal"), false), + (rel_path("links/internal/existing.rs"), false), + (rel_path("links/internal/nested"), false), + (rel_path("links/internal/nested/deep.rs"), false), + (rel_path("real-dir"), false), + (rel_path("real-dir/existing.rs"), false), + (rel_path("real-dir/nested"), false), + (rel_path("real-dir/nested/deep.rs"), false), + ] + ); + + assert_eq!( + tree.entry_for_path(rel_path("links/internal")) + .unwrap() + .kind, + EntryKind::Dir + ); + }); + + fs.insert_file(Path::new("/root/project/real-dir/new.txt"), b"".to_vec()) + .await; + wait_for_condition(cx, |cx| { + tree.read_with(cx, |tree, _| { + tree.entry_for_path(rel_path("links/internal/new.txt")) + .is_some() + }) + }) + .await; + + tree.read_with(cx, |tree, _| { + assert!( + tree.entry_for_path(rel_path("links/internal/new.txt")) + .is_some() + ); + }); + + fs.insert_file( + Path::new("/root/project/real-dir/nested/inner.txt"), + b"".to_vec(), + ) + .await; + wait_for_condition(cx, |cx| { + tree.read_with(cx, |tree, _| { + tree.entry_for_path(rel_path("links/internal/nested/inner.txt")) + .is_some() + }) + }) + .await; + + tree.read_with(cx, |tree, _| { + assert!( + tree.entry_for_path(rel_path("links/internal/nested/inner.txt")) + .is_some() + ); + }); } #[cfg(target_os = "macos")] @@ -2994,6 +3227,22 @@ fn init_test(cx: &mut gpui::TestAppContext) { }); } +async fn wait_for_condition( + cx: &mut TestAppContext, + mut condition: impl FnMut(&mut TestAppContext) -> bool, +) { + for _ in 0..50 { + if condition(cx) { + return; + } + cx.executor().run_until_parked(); + cx.background_executor + .timer(std::time::Duration::from_millis(10)) + .await; + } + panic!("timed out waiting for test condition"); +} + #[gpui::test] async fn test_load_file_encoding(cx: &mut TestAppContext) { init_test(cx);