From 92df76e632f7f76a5f064043d67ed09c4bf931b5 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 30 Jun 2023 11:20:50 -0700 Subject: [PATCH] Fix accidental ignoring of git FS events --- crates/project/src/worktree.rs | 80 +++++++++++++++------------- crates/project/src/worktree_tests.rs | 50 ++++++++--------- 2 files changed, 69 insertions(+), 61 deletions(-) diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 20e693770f..2f793c84c9 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -2140,6 +2140,7 @@ impl LocalSnapshot { impl BackgroundScannerState { fn should_scan_directory(&self, entry: &Entry) -> bool { (!entry.is_external && !entry.is_ignored) + || entry.path.file_name() == Some(&*DOT_GIT) || self.scanned_dirs.contains(&entry.id) // If we've ever scanned it, keep scanning || self .paths_to_scan @@ -2319,6 +2320,7 @@ impl BackgroundScannerState { .entry_for_id(entry_id) .map(|entry| RepositoryWorkDirectory(entry.path.clone())) else { continue }; + log::info!("reload git repository {:?}", dot_git_dir); let repository = repository.repo_ptr.lock(); let branch = repository.branch_name(); repository.reload_index(); @@ -2359,6 +2361,8 @@ impl BackgroundScannerState { } fn build_repository(&mut self, dot_git_path: Arc, fs: &dyn Fs) -> Option<()> { + log::info!("build git repository {:?}", dot_git_path); + let work_dir_path: Arc = dot_git_path.parent().unwrap().into(); // Guard against repositories inside the repository metadata @@ -3138,8 +3142,6 @@ impl BackgroundScanner { } async fn process_events(&mut self, mut abs_paths: Vec) { - log::debug!("received fs events {:?}", abs_paths); - let root_path = self.state.lock().snapshot.abs_path.clone(); let root_canonical_path = match self.fs.canonicalize(&root_path).await { Ok(path) => path, @@ -3150,7 +3152,6 @@ impl BackgroundScanner { }; let mut relative_paths = Vec::with_capacity(abs_paths.len()); - let mut unloaded_relative_paths = Vec::new(); abs_paths.sort_unstable(); abs_paths.dedup_by(|a, b| a.starts_with(&b)); abs_paths.retain(|abs_path| { @@ -3173,7 +3174,6 @@ impl BackgroundScanner { }); if !parent_dir_is_loaded { log::debug!("ignoring event {relative_path:?} within unloaded directory"); - unloaded_relative_paths.push(relative_path); return false; } @@ -3182,27 +3182,30 @@ impl BackgroundScanner { } }); - if !relative_paths.is_empty() { - let (scan_job_tx, scan_job_rx) = channel::unbounded(); - self.reload_entries_for_paths( - root_path, - root_canonical_path, - &relative_paths, - abs_paths, - Some(scan_job_tx.clone()), - ) - .await; - drop(scan_job_tx); - self.scan_dirs(false, scan_job_rx).await; - - let (scan_job_tx, scan_job_rx) = channel::unbounded(); - self.update_ignore_statuses(scan_job_tx).await; - self.scan_dirs(false, scan_job_rx).await; + if relative_paths.is_empty() { + return; } + log::debug!("received fs events {:?}", relative_paths); + + let (scan_job_tx, scan_job_rx) = channel::unbounded(); + self.reload_entries_for_paths( + root_path, + root_canonical_path, + &relative_paths, + abs_paths, + Some(scan_job_tx.clone()), + ) + .await; + drop(scan_job_tx); + self.scan_dirs(false, scan_job_rx).await; + + let (scan_job_tx, scan_job_rx) = channel::unbounded(); + self.update_ignore_statuses(scan_job_tx).await; + self.scan_dirs(false, scan_job_rx).await; + { let mut state = self.state.lock(); - relative_paths.extend(unloaded_relative_paths); state.reload_repositories(&relative_paths, self.fs.as_ref()); state.snapshot.completed_scan_id = state.snapshot.scan_id; for (_, entry_id) in mem::take(&mut state.removed_entry_ids) { @@ -3610,23 +3613,28 @@ impl BackgroundScanner { } } - let fs_entry = state.insert_entry(fs_entry, self.fs.as_ref()); - - if let Some(scan_queue_tx) = &scan_queue_tx { - let mut ancestor_inodes = state.snapshot.ancestor_inodes_for_path(&path); - if metadata.is_dir && !ancestor_inodes.contains(&metadata.inode) { - ancestor_inodes.insert(metadata.inode); - smol::block_on(scan_queue_tx.send(ScanJob { - abs_path, - path: path.clone(), - ignore_stack, - ancestor_inodes, - is_external: fs_entry.is_external, - scan_queue: scan_queue_tx.clone(), - })) - .unwrap(); + if let (Some(scan_queue_tx), true) = (&scan_queue_tx, fs_entry.is_dir()) { + if state.should_scan_directory(&fs_entry) { + let mut ancestor_inodes = + state.snapshot.ancestor_inodes_for_path(&path); + if !ancestor_inodes.contains(&metadata.inode) { + ancestor_inodes.insert(metadata.inode); + smol::block_on(scan_queue_tx.send(ScanJob { + abs_path, + path: path.clone(), + ignore_stack, + ancestor_inodes, + is_external: fs_entry.is_external, + scan_queue: scan_queue_tx.clone(), + })) + .unwrap(); + } + } else { + fs_entry.kind = EntryKind::UnloadedDir; } } + + state.insert_entry(fs_entry, self.fs.as_ref()); } Ok(None) => { self.remove_repo_path(&path, &mut state.snapshot); diff --git a/crates/project/src/worktree_tests.rs b/crates/project/src/worktree_tests.rs index f908d702eb..c9ed06dea7 100644 --- a/crates/project/src/worktree_tests.rs +++ b/crates/project/src/worktree_tests.rs @@ -1654,6 +1654,23 @@ async fn test_git_status(deterministic: Arc, cx: &mut TestAppCont })); + const A_TXT: &'static str = "a.txt"; + const B_TXT: &'static str = "b.txt"; + const E_TXT: &'static str = "c/d/e.txt"; + const F_TXT: &'static str = "f.txt"; + const DOTGITIGNORE: &'static str = ".gitignore"; + const BUILD_FILE: &'static str = "target/build_file"; + let project_path = Path::new("project"); + + // Set up git repository before creating the worktree. + let work_dir = root.path().join("project"); + let mut repo = git_init(work_dir.as_path()); + repo.add_ignore_rule(IGNORE_RULE).unwrap(); + git_add(A_TXT, &repo); + git_add(E_TXT, &repo); + git_add(DOTGITIGNORE, &repo); + git_commit("Initial commit", &repo); + let tree = Worktree::local( build_client(cx), root.path(), @@ -1665,26 +1682,9 @@ async fn test_git_status(deterministic: Arc, cx: &mut TestAppCont .await .unwrap(); + tree.flush_fs_events(cx).await; cx.read(|cx| tree.read(cx).as_local().unwrap().scan_complete()) .await; - - const A_TXT: &'static str = "a.txt"; - const B_TXT: &'static str = "b.txt"; - const E_TXT: &'static str = "c/d/e.txt"; - const F_TXT: &'static str = "f.txt"; - const DOTGITIGNORE: &'static str = ".gitignore"; - const BUILD_FILE: &'static str = "target/build_file"; - let project_path: &Path = &Path::new("project"); - - let work_dir = root.path().join("project"); - let mut repo = git_init(work_dir.as_path()); - repo.add_ignore_rule(IGNORE_RULE).unwrap(); - git_add(Path::new(A_TXT), &repo); - git_add(Path::new(E_TXT), &repo); - git_add(Path::new(DOTGITIGNORE), &repo); - git_commit("Initial commit", &repo); - - tree.flush_fs_events(cx).await; deterministic.run_until_parked(); // Check that the right git state is observed on startup @@ -1704,39 +1704,39 @@ async fn test_git_status(deterministic: Arc, cx: &mut TestAppCont ); }); + // Modify a file in the working copy. std::fs::write(work_dir.join(A_TXT), "aa").unwrap(); - tree.flush_fs_events(cx).await; deterministic.run_until_parked(); + // The worktree detects that the file's git status has changed. tree.read_with(cx, |tree, _cx| { let snapshot = tree.snapshot(); - assert_eq!( snapshot.status_for_file(project_path.join(A_TXT)), Some(GitFileStatus::Modified) ); }); - git_add(Path::new(A_TXT), &repo); - git_add(Path::new(B_TXT), &repo); + // Create a commit in the git repository. + git_add(A_TXT, &repo); + git_add(B_TXT, &repo); git_commit("Committing modified and added", &repo); tree.flush_fs_events(cx).await; deterministic.run_until_parked(); - // Check that repo only changes are tracked + // The worktree detects that the files' git status have changed. tree.read_with(cx, |tree, _cx| { let snapshot = tree.snapshot(); - assert_eq!( snapshot.status_for_file(project_path.join(F_TXT)), Some(GitFileStatus::Added) ); - assert_eq!(snapshot.status_for_file(project_path.join(B_TXT)), None); assert_eq!(snapshot.status_for_file(project_path.join(A_TXT)), None); }); + // Modify files in the working copy and perform git operations on other files. git_reset(0, &repo); git_remove_index(Path::new(B_TXT), &repo); git_stash(&mut repo);