windows: Fix FS-related issues (#23369)

张小白 created

I've noticed an occasional error: `ignoring event C:\some\path\to\file
outside of root path \\?\C:\some\path`. This happens because UNC paths
always fail to match with non-UNC paths during operations like
`strip_prefix` or `starts_with`. To address this, I changed the types of
some key parameters to `SanitizedPath`. With this adjustment, FS events
are now correctly identified, and under the changes in this PR, the
`test_rescan_and_remote_updates` test also passes successfully on
Windows.

Release Notes:

- N/A

Change summary

crates/fs/src/fs_watcher.rs         |  7 ++++---
crates/project/src/project_tests.rs | 12 ++++++++++--
crates/util/src/paths.rs            | 17 +++++++++++++++++
crates/worktree/src/worktree.rs     | 25 +++++++++++++------------
4 files changed, 44 insertions(+), 17 deletions(-)

Detailed changes

crates/fs/src/fs_watcher.rs 🔗

@@ -1,7 +1,7 @@
 use notify::EventKind;
 use parking_lot::Mutex;
 use std::sync::{Arc, OnceLock};
-use util::ResultExt;
+use util::{paths::SanitizedPath, ResultExt};
 
 use crate::{PathEvent, PathEventKind, Watcher};
 
@@ -24,7 +24,7 @@ impl FsWatcher {
 
 impl Watcher for FsWatcher {
     fn add(&self, path: &std::path::Path) -> gpui::Result<()> {
-        let root_path = path.to_path_buf();
+        let root_path = SanitizedPath::from(path);
 
         let tx = self.tx.clone();
         let pending_paths = self.pending_path_events.clone();
@@ -44,8 +44,9 @@ impl Watcher for FsWatcher {
                         .paths
                         .iter()
                         .filter_map(|event_path| {
+                            let event_path = SanitizedPath::from(event_path);
                             event_path.starts_with(&root_path).then(|| PathEvent {
-                                path: event_path.clone(),
+                                path: event_path.as_path().to_path_buf(),
                                 kind,
                             })
                         })

crates/project/src/project_tests.rs 🔗

@@ -24,7 +24,12 @@ use std::{str::FromStr, sync::OnceLock};
 use std::{mem, num::NonZeroU32, ops::Range, task::Poll};
 use task::{ResolvedTask, TaskContext};
 use unindent::Unindent as _;
-use util::{assert_set_eq, paths::PathMatcher, test::temp_tree, TryFutureExt as _};
+use util::{
+    assert_set_eq,
+    paths::{replace_path_separator, PathMatcher},
+    test::temp_tree,
+    TryFutureExt as _,
+};
 
 #[gpui::test]
 async fn test_block_via_channel(cx: &mut gpui::TestAppContext) {
@@ -3265,7 +3270,10 @@ async fn test_rescan_and_remote_updates(cx: &mut gpui::TestAppContext) {
         "d",
         "d/file3",
         "d/file4",
-    ];
+    ]
+    .into_iter()
+    .map(replace_path_separator)
+    .collect::<Vec<_>>();
 
     cx.update(|app| {
         assert_eq!(

crates/util/src/paths.rs 🔗

@@ -1,4 +1,5 @@
 use std::cmp;
+use std::path::StripPrefixError;
 use std::sync::{Arc, OnceLock};
 use std::{
     ffi::OsStr,
@@ -113,6 +114,14 @@ impl SanitizedPath {
     pub fn to_string(&self) -> String {
         self.0.to_string_lossy().to_string()
     }
+
+    pub fn join(&self, path: &Self) -> Self {
+        self.0.join(&path.0).into()
+    }
+
+    pub fn strip_prefix(&self, base: &Self) -> Result<&Path, StripPrefixError> {
+        self.0.strip_prefix(base.as_path())
+    }
 }
 
 impl From<SanitizedPath> for Arc<Path> {
@@ -439,6 +448,14 @@ pub fn compare_paths(
     }
 }
 
+#[cfg(any(test, feature = "test-support"))]
+pub fn replace_path_separator(path: &str) -> String {
+    #[cfg(target_os = "windows")]
+    return path.replace("/", std::path::MAIN_SEPARATOR_STR);
+    #[cfg(not(target_os = "windows"))]
+    return path.to_string();
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

crates/worktree/src/worktree.rs 🔗

@@ -4175,7 +4175,7 @@ impl BackgroundScanner {
 
         let root_path = self.state.lock().snapshot.abs_path.clone();
         let root_canonical_path = match self.fs.canonicalize(root_path.as_path()).await {
-            Ok(path) => path,
+            Ok(path) => SanitizedPath::from(path),
             Err(err) => {
                 log::error!("failed to canonicalize root path: {}", err);
                 return true;
@@ -4186,9 +4186,9 @@ impl BackgroundScanner {
             .iter()
             .map(|path| {
                 if path.file_name().is_some() {
-                    root_canonical_path.join(path)
+                    root_canonical_path.as_path().join(path).to_path_buf()
                 } else {
-                    root_canonical_path.clone()
+                    root_canonical_path.as_path().to_path_buf()
                 }
             })
             .collect::<Vec<_>>();
@@ -4203,7 +4203,7 @@ impl BackgroundScanner {
         }
 
         self.reload_entries_for_paths(
-            root_path.into(),
+            root_path,
             root_canonical_path,
             &request.relative_paths,
             abs_paths,
@@ -4217,7 +4217,7 @@ impl BackgroundScanner {
     async fn process_events(&self, mut abs_paths: Vec<PathBuf>) {
         let root_path = self.state.lock().snapshot.abs_path.clone();
         let root_canonical_path = match self.fs.canonicalize(root_path.as_path()).await {
-            Ok(path) => path,
+            Ok(path) => SanitizedPath::from(path),
             Err(err) => {
                 let new_path = self
                     .state
@@ -4250,6 +4250,7 @@ impl BackgroundScanner {
         abs_paths.sort_unstable();
         abs_paths.dedup_by(|a, b| a.starts_with(b));
         abs_paths.retain(|abs_path| {
+            let abs_path = SanitizedPath::from(abs_path);
             let snapshot = &self.state.lock().snapshot;
             {
                 let mut is_git_related = false;
@@ -4261,7 +4262,7 @@ impl BackgroundScanner {
                     FsMonitor
                 }
                 let mut fsmonitor_parse_state = None;
-                if let Some(dot_git_abs_path) = abs_path
+                if let Some(dot_git_abs_path) = abs_path.as_path()
                     .ancestors()
                     .find(|ancestor| {
                         let file_name = ancestor.file_name();
@@ -4334,7 +4335,7 @@ impl BackgroundScanner {
         let (scan_job_tx, scan_job_rx) = channel::unbounded();
         log::debug!("received fs events {:?}", relative_paths);
         self.reload_entries_for_paths(
-            root_path.into(),
+            root_path,
             root_canonical_path,
             &relative_paths,
             abs_paths,
@@ -4693,8 +4694,8 @@ impl BackgroundScanner {
     /// All list arguments should be sorted before calling this function
     async fn reload_entries_for_paths(
         &self,
-        root_abs_path: Arc<Path>,
-        root_canonical_path: PathBuf,
+        root_abs_path: SanitizedPath,
+        root_canonical_path: SanitizedPath,
         relative_paths: &[Arc<Path>],
         abs_paths: Vec<PathBuf>,
         scan_queue_tx: Option<Sender<ScanJob>>,
@@ -4722,7 +4723,7 @@ impl BackgroundScanner {
                             }
                         }
 
-                        anyhow::Ok(Some((metadata, canonical_path)))
+                        anyhow::Ok(Some((metadata, SanitizedPath::from(canonical_path))))
                     } else {
                         Ok(None)
                     }
@@ -4819,7 +4820,7 @@ impl BackgroundScanner {
         }
 
         for (path, metadata) in relative_paths.iter().zip(metadata.into_iter()) {
-            let abs_path: Arc<Path> = root_abs_path.join(path).into();
+            let abs_path: Arc<Path> = root_abs_path.as_path().join(path).into();
             match metadata {
                 Ok(Some((metadata, canonical_path))) => {
                     let ignore_stack = state
@@ -4832,7 +4833,7 @@ impl BackgroundScanner {
                         self.next_entry_id.as_ref(),
                         state.snapshot.root_char_bag,
                         if metadata.is_symlink {
-                            Some(canonical_path.into())
+                            Some(canonical_path.as_path().to_path_buf().into())
                         } else {
                             None
                         },