From da50d3fbe385469e8c9e05e4e51c828bcdffea39 Mon Sep 17 00:00:00 2001 From: Dmitriy Kovalenko Date: Fri, 31 Jul 2026 12:03:57 -0700 Subject: [PATCH 1/3] fix: Correctly handle empty directories during the scan Closes #725 Before we have completely ignored empty directories partially as a feature cause usually they do not contain anything useful but there is a bug #725 that we need to fix and it definetely makes sense to show empty directories in the dir search --- crates/fff-core/src/file_picker.rs | 224 ++++++++++++++---- crates/fff-core/src/walk/mod.rs | 4 + crates/fff-core/src/walk/ripgrep.rs | 13 + crates/fff-core/src/walk/zlob.rs | 20 +- .../src/watcher/background_watcher.rs | 120 ++++++---- .../tests/dir_index_consistency_test.rs | 45 ++++ .../tests/new_directory_watcher_test.rs | 84 +++++++ 7 files changed, 414 insertions(+), 96 deletions(-) diff --git a/crates/fff-core/src/file_picker.rs b/crates/fff-core/src/file_picker.rs index 6f9055fc..e17694fb 100644 --- a/crates/fff-core/src/file_picker.rs +++ b/crates/fff-core/src/file_picker.rs @@ -45,6 +45,7 @@ use crate::types::{ ContentCacheBudget, DirItem, DirSearchResult, FileItem, MixedItemRef, MixedSearchResult, PaginationArgs, Score, ScoringContext, SearchResult, }; +use crate::walk::WalkOutput; use crate::watch::BackgroundWatcher; use fff_query_parser::FFFQuery; use git2::{Repository, Status}; @@ -728,6 +729,19 @@ impl FilePicker { &self.sync_data.dirs } + /// Whether the absolute `path` is a directory known to the index + /// (including dirs that were empty at scan time). + pub(crate) fn has_indexed_dir(&self, path: &Path) -> bool { + let Ok(rel) = path.strip_prefix(&self.base_path) else { + return false; + }; + let mut rel = crate::path_utils::to_canonical_slashes(&rel.to_string_lossy()).into_owned(); + if !rel.is_empty() && !rel.ends_with('/') { + rel.push('/'); + } + self.sync_data.find_dir_index(&rel).is_some() + } + /// Actual heap bytes used: (chunked_path_store, 0, 0). /// The second element is 0 because leaked overflow stores aren't tracked. pub fn arena_bytes(&self) -> (usize, usize, usize) { @@ -2020,30 +2034,40 @@ impl FileSync { let is_git_repo = git_workdir.is_some(); let bg_threads = BACKGROUND_THREAD_POOL.current_num_threads(); - let mut walk_output = crate::walk::walk_collect_files( + let WalkOutput { + dirs: mut walked_dirs, + mut pairs, + ignore_rules, + } = crate::walk::walk_collect_files( base_path, is_git_repo, follow_symlinks, bg_threads, synced_files_count, )?; - let ignore_rules = walk_output.ignore_rules.take().map(Arc::new); - let mut pairs = walk_output.pairs; + let ignore_rules = ignore_rules.map(Arc::new); - // Sort by (dir_part, filename). This groups files by their directory - // into contiguous runs so the linear dir-extraction pass below can - // dedupe by comparing only against the previous dir. + // Sort files by (dir_part, filename) — grouping them into contiguous + // per-dir runs — and the walked dirs by the same '/'-terminated form, + // so the build pass below can merge both in a single sweep. BACKGROUND_THREAD_POOL.install(|| { - pairs.par_sort_unstable_by(|(a, path_a), (b, path_b)| { - // SAFETY: `filename_offset` is always at a character boundary - let (a_dir, a_file) = path_a.split_at(a.path.filename_offset as usize); - let (b_dir, b_file) = path_b.split_at(b.path.filename_offset as usize); - a_dir.cmp(b_dir).then_with(|| a_file.cmp(b_file)) - }); + rayon::join( + || { + pairs.par_sort_unstable_by(|(a, path_a), (b, path_b)| { + // SAFETY: `filename_offset` is always at a character boundary + let (a_dir, a_file) = path_a.split_at(a.path.filename_offset as usize); + let (b_dir, b_file) = path_b.split_at(b.path.filename_offset as usize); + a_dir.cmp(b_dir).then_with(|| a_file.cmp(b_file)) + }); + }, + || walked_dirs.par_sort_unstable(), + ); }); + walked_dirs.dedup(); let mut builder = crate::simd_path::ChunkedPathStoreBuilder::new(pairs.len()); - let dirs = populates_dirs_files_chunked_storage(&mut pairs, &mut builder); + let dirs = populates_dirs_files_chunked_storage(&mut pairs, &walked_dirs, &mut builder); + drop(walked_dirs); let mut files: Vec = pairs.into_iter().map(|(file, _)| file).collect(); let chunked_paths = builder.finish(); @@ -2164,12 +2188,17 @@ pub(crate) fn warmup_mmaps( } /// This does both thing (yes sorry all the OOP morons) -/// in one go: populates files chunked storage and creates new directories +/// in one go: populates files chunked storage and builds the dir table from +/// `walked_dirs` (every dir the walker visited: sorted, '/'-terminated, +/// deduped), merging file parents in a single lockstep sweep so dirs with no +/// files (empty subtrees, pure ancestors) are indexed and searchable too. fn populates_dirs_files_chunked_storage<'a>( pairs: &'a mut [(FileItem, String)], + walked_dirs: &[String], chunk_storage: &mut crate::simd_path::ChunkedPathStoreBuilder, ) -> Vec { - let mut dirs: Vec = Vec::new(); + let mut dirs: Vec = Vec::with_capacity(walked_dirs.len() + 1); + let mut dir_iter = walked_dirs.iter().peekable(); let mut prev_dir: &'a str = ""; let mut prev_dir_valid = false; @@ -2180,20 +2209,22 @@ fn populates_dirs_files_chunked_storage<'a>( let dir_part: &'a str = &rel[..file.path.filename_offset as usize]; if !prev_dir_valid || prev_dir != dir_part { - let dir_string = chunk_storage.add_dir_immediate(dir_part); - - // Compute last-segment offset: for "src/components/" -> 4 (points to "components/") - let last_seg = if dir_part.is_empty() { - 0 - } else { - let trimmed = dir_part.trim_end_matches(std::path::is_separator); - trimmed - .rfind(std::path::is_separator) - .map(|i| i + 1) - .unwrap_or(0) as u16 - }; + // Flush walked dirs up to and including this file's parent, + // keeping the table sorted for the find_dir_index binary search. + let mut matched = false; + while let Some(dir) = dir_iter.peek() + && dir.as_str() <= dir_part + { + matched = dir.as_str() == dir_part; + push_dir_item(&mut dirs, chunk_storage, dir); + dir_iter.next(); + } - dirs.push(DirItem::new(dir_string, last_seg)); + // Root-level files ("" dir part) and parents the walker reported + // with a non-dir kind (e.g. followed symlinks) aren't in the list. + if !matched { + push_dir_item(&mut dirs, chunk_storage, dir_part); + } current_dir_idx = (dirs.len() - 1) as u32; prev_dir = dir_part; @@ -2204,9 +2235,34 @@ fn populates_dirs_files_chunked_storage<'a>( file.parent_dir_index = current_dir_idx; } + for dir in dir_iter { + push_dir_item(&mut dirs, chunk_storage, dir); + } + dirs } +fn push_dir_item( + dirs: &mut Vec, + chunk_storage: &mut crate::simd_path::ChunkedPathStoreBuilder, + dir_part: &str, +) { + let dir_string = chunk_storage.add_dir_immediate(dir_part); + + // Compute last-segment offset: for "src/components/" -> 4 (points to "components/") + let last_seg = if dir_part.is_empty() { + 0 + } else { + let trimmed = dir_part.trim_end_matches(std::path::is_separator); + trimmed + .rfind(std::path::is_separator) + .map(|i| i + 1) + .unwrap_or(0) as u16 + }; + + dirs.push(DirItem::new(dir_string, last_seg)); +} + /// Fast extension-based binary detection. Avoids opening files during scan. /// Covers the vast majority of binary files in typical repositories. #[inline] @@ -2341,13 +2397,9 @@ mod tests { use super::*; /// The watcher must watch every ancestor directory up to `base_path`, - /// not just the immediate parents of indexed files. Intermediate dirs - /// that contain only subdirectories (no direct files) are NOT in - /// `sync_data.dirs` — yet they must still appear in `extract_watch_dirs` - /// so Create events on new subdirectories below them fire. - /// - /// Correctness regression guard for any refactor that replaces the - /// ancestor walk with a direct `sync_data.dirs` iteration. + /// not just the immediate parents of indexed files. The dir table is + /// built from the walker's visited dirs, so pure ancestors (dirs that + /// contain only subdirectories) must be present and emitted exactly once. #[test] fn extract_watch_dirs_includes_pure_ancestor_dirs() { let dir = tempfile::tempdir().unwrap(); @@ -2361,17 +2413,6 @@ mod tests { // base/src/components/button.txt (src/components has a file) // base/src/routes/home.txt (src/routes has a file) // base/lib/deep/nested/util.txt (lib and lib/deep have no files) - // - // `sync_data.dirs` will only contain: - // src/components/ - // src/routes/ - // lib/deep/nested/ - // - // But the watcher also needs: - // src/ (pure ancestor — no direct files) - // lib/ (pure ancestor) - // lib/deep/ (pure ancestor) - // otherwise new siblings like `src/NewDir/x.txt` are missed. for rel in [ "src/components/button.txt", "src/routes/home.txt", @@ -2429,6 +2470,97 @@ mod tests { ); } + /// Regression guard for #725: dirs that are EMPTY at scan time are merged + /// into `sync_data.dirs` so they are searchable and get an inotify watch; + /// files created in them later must be detected. + #[test] + fn for_each_dir_includes_empty_directories() { + let dir = tempfile::tempdir().unwrap(); + let base_buf = crate::path_utils::canonicalize(dir.path()).unwrap(); + let base = base_buf.as_path(); + + // Tree: + // base/init.lua (file directly under base) + // base/commands/ (empty at scan — the #725 repro) + // base/src/main.rs (src is indexed) + // base/src/plugins/extra/ (empty chain under an indexed dir) + std::fs::create_dir_all(base.join("commands")).unwrap(); + std::fs::create_dir_all(base.join("src/plugins/extra")).unwrap(); + std::fs::write(base.join("init.lua"), b"x").unwrap(); + std::fs::write(base.join("src/main.rs"), b"x").unwrap(); + + let mut picker = FilePicker::new(FilePickerOptions { + base_path: base.to_str().unwrap().into(), + watch: false, + ..Default::default() + }) + .unwrap(); + picker.collect_files().unwrap(); + + let mut watch_dirs: Vec = Vec::new(); + picker.for_each_dir(|p| { + watch_dirs.push(p.to_path_buf()); + std::ops::ControlFlow::Continue(()) + }); + let watch_set: std::collections::HashSet = watch_dirs.iter().cloned().collect(); + + for rel in ["commands", "src/plugins", "src/plugins/extra", "src"] { + assert!( + watch_set.contains(&base.join(rel)), + "expected {rel} in watch dirs, got {watch_set:?}", + ); + } + + // Dirs covered by indexed files must not be duplicated. + assert_eq!( + watch_dirs.len(), + watch_set.len(), + "duplicate watch dir emitted: {watch_dirs:?}", + ); + } + + #[test] + fn dir_table_merges_walked_dirs_with_file_parents() { + let mut pairs: Vec<(FileItem, String)> = ["src/main.rs", "src/deep/lib.rs", "root.txt"] + .iter() + .map(|p| { + let (item, rel) = FileItem::new(PathBuf::from(p), Path::new(""), None); + (item, rel) + }) + .collect(); + pairs.sort_by(|(a, pa), (b, pb)| { + pa[..a.path.filename_offset as usize] + .cmp(&pb[..b.path.filename_offset as usize]) + .then_with(|| pa.cmp(pb)) + }); + + // Sorted '/'-terminated walker output: file parents + an empty dir + + // a sibling sharing a prefix with a file parent. + let walked: Vec = ["empty/", "src/", "src/deep/", "src/deeper/"] + .iter() + .map(|s| s.to_string()) + .collect(); + + let mut builder = crate::simd_path::ChunkedPathStoreBuilder::new(pairs.len()); + let dirs = populates_dirs_files_chunked_storage(&mut pairs, &walked, &mut builder); + let store = builder.finish(); + let arena = store.as_arena_ptr(); + + let table: Vec = dirs.iter().map(|d| d.relative_path(arena)).collect(); + // Sorted: "" (root files) first, all walked dirs present exactly once. + assert_eq!(table, ["", "empty/", "src/", "src/deep/", "src/deeper/"]); + + // Every file's parent_dir_index points at its own dir entry. + for (file, _) in &pairs { + let dir = &dirs[file.parent_dir_index as usize]; + let rel = file.relative_path(arena); + assert!( + rel.starts_with(&dir.relative_path(arena)), + "file {rel} must live under its parent dir", + ); + } + } + #[test] fn common_dir_prefix_len_cases() { assert_eq!(common_dir_prefix_len("", ""), 0); diff --git a/crates/fff-core/src/walk/mod.rs b/crates/fff-core/src/walk/mod.rs index a533f0bb..2ea3db08 100644 --- a/crates/fff-core/src/walk/mod.rs +++ b/crates/fff-core/src/walk/mod.rs @@ -20,6 +20,10 @@ pub(crate) use ripgrep::walk_collect_files; pub(crate) struct WalkOutput { pub(crate) pairs: Vec<(FileItem, String)>, + /// Every non-ignored directory the walk visited, as '/'-canonical + /// '/'-terminated paths relative to the base (base itself excluded). + /// This is the source of the picker's dir table; unsorted (parallel walk). + pub(crate) dirs: Vec, pub(crate) ignore_rules: Option, } diff --git a/crates/fff-core/src/walk/ripgrep.rs b/crates/fff-core/src/walk/ripgrep.rs index def4599b..e6f019db 100644 --- a/crates/fff-core/src/walk/ripgrep.rs +++ b/crates/fff-core/src/walk/ripgrep.rs @@ -35,8 +35,10 @@ pub(crate) fn walk_collect_files( let walker = walk_builder.build_parallel(); let pairs = parking_lot::Mutex::new(Vec::<(FileItem, String)>::new()); + let dirs = parking_lot::Mutex::new(Vec::::new()); walker.run(|| { let pairs = &pairs; + let dirs = &dirs; let counter = Arc::clone(synced_files_count); let base_path = base_path.to_path_buf(); @@ -60,6 +62,16 @@ pub(crate) fn walk_collect_files( pairs.lock().push((file_item, rel_path)); counter.fetch_add(1, Ordering::Relaxed); + } else if entry.depth() > 0 && entry.file_type().is_some_and(|ft| ft.is_dir()) { + let path = entry.path(); + if !is_git_file(path) + && let Ok(rel) = path.strip_prefix(&base_path) + { + let mut rel = crate::path_utils::to_canonical_slashes(&rel.to_string_lossy()) + .into_owned(); + rel.push('/'); + dirs.lock().push(rel); + } } ignore::WalkState::Continue }) @@ -67,6 +79,7 @@ pub(crate) fn walk_collect_files( Ok(WalkOutput { pairs: pairs.into_inner(), + dirs: dirs.into_inner(), ignore_rules: None, }) } diff --git a/crates/fff-core/src/walk/zlob.rs b/crates/fff-core/src/walk/zlob.rs index 0978ad1b..d2cdb7da 100644 --- a/crates/fff-core/src/walk/zlob.rs +++ b/crates/fff-core/src/walk/zlob.rs @@ -1,10 +1,8 @@ -//! Filesystem traversal backed by zlob's native parallel walker. -//! Active when the `zlob` feature is enabled (requires the Zig toolchain). - use crate::file_picker::is_known_binary_extension_basename; use crate::ignore::IGNORED_DIRS; use crate::types::FileItem; use crate::walk::{WalkIgnoreRules, WalkOutput}; +use parking_lot::Mutex; use std::path::Path; use std::sync::Arc; use std::sync::atomic::{AtomicUsize, Ordering}; @@ -48,12 +46,25 @@ pub(crate) fn walk_collect_files( tracing::warn!(?e, "zlob extra_ignore rejected; walking without it"); } - let pairs = parking_lot::Mutex::new(Vec::<(FileItem, String)>::new()); + let pairs = Mutex::new(Vec::new()); + let dirs = Mutex::new(Vec::new()); let outcome = match builder.run(|entry| { if !entry.is_file() { + // `.git` and ignored dirs are pruned by zlob itself (GITIGNORE + // flag); empty rel path is the walk root, covered separately. + if entry.is_dir() { + let rel_bytes = entry.relative_path_bytes(); + if !rel_bytes.is_empty() { + let mut rel = String::from_utf8_lossy(rel_bytes).into_owned(); + rel.push('/'); + dirs.lock().push(rel); + } + } + return WalkState::Continue; } + let rel_bytes = entry.relative_path_bytes(); // `basename()` returns `&str` for files only. @@ -106,6 +117,7 @@ pub(crate) fn walk_collect_files( Ok(WalkOutput { pairs, + dirs: dirs.into_inner(), ignore_rules, }) } diff --git a/crates/fff-core/src/watcher/background_watcher.rs b/crates/fff-core/src/watcher/background_watcher.rs index 36a9e124..8c8ba3f7 100644 --- a/crates/fff-core/src/watcher/background_watcher.rs +++ b/crates/fff-core/src/watcher/background_watcher.rs @@ -108,48 +108,57 @@ impl BackgroundWatcher { .name("fff-watcher-own".into()) .spawn(move || { let _g = owner_span.enter(); - while let Ok(dir) = watch_rx.recv() { - // if the picker is dropped we do need to exit the loop - let Some(strong_picker) = owner_weak_picker.upgrade() else { - break; - }; - - // Only inotify (Linux) has no kernel-level recursion, so - // it's the only platform that needs a per-subdir watch to - // be registered at runtime. macOS FSEvents and Windows - // ReadDirectoryChangesW are already watching recursively - // from the base path (see `create_debouncer`), and - // registering a second overlapping stream there produces - // duplicate/out-of-order events. - #[cfg(target_os = "linux")] - { - // Register the new directory with the debouncer, then - // drop the mutex BEFORE doing picker-side work — see - // the comment on `BackgroundWatcher::stop` for the - // lock-ordering rationale. - let mut guard = owner_debouncer.lock(); - let Some(debouncer) = guard.as_mut() else { - break; + // Local work queue: enumerating a dir can surface subdirs + // unknown to the index (empty at scan time) that also need + // watches; we must not self-send on `watch_tx` or `recv()` + // would never disconnect on stop. + let mut queue = std::collections::VecDeque::new(); + 'recv: while let Ok(dir) = watch_rx.recv() { + queue.push_back(dir); + while let Some(dir) = queue.pop_front() { + // if the picker is dropped we do need to exit the loop + let Some(strong_picker) = owner_weak_picker.upgrade() else { + break 'recv; }; - if let Err(e) = debouncer.watch(&dir, RecursiveMode::NonRecursive) { - warn!( - ?e, - dir = %dir.display(), - "Failed to init watcher for new directory" - ); + // Only inotify (Linux) has no kernel-level recursion, so + // it's the only platform that needs a per-subdir watch to + // be registered at runtime. macOS FSEvents and Windows + // ReadDirectoryChangesW are already watching recursively + // from the base path (see `create_debouncer`), and + // registering a second overlapping stream there produces + // duplicate/out-of-order events. + #[cfg(target_os = "linux")] + { + // Register the new directory with the debouncer, then + // drop the mutex BEFORE doing picker-side work — see + // the comment on `BackgroundWatcher::stop` for the + // lock-ordering rationale. + let mut guard = owner_debouncer.lock(); + let Some(debouncer) = guard.as_mut() else { + break 'recv; + }; + + if let Err(e) = debouncer.watch(&dir, RecursiveMode::NonRecursive) { + warn!( + ?e, + dir = %dir.display(), + "Failed to init watcher for new directory" + ); + } } - } - track_files_from_new_directories( - &dir, - &strong_picker, - &owner_git_workdir, - &owner_git_worker, - ); + let unindexed_subdirs = track_files_from_new_directories( + &dir, + &strong_picker, + &owner_git_workdir, + &owner_git_worker, + ); + queue.extend(unindexed_subdirs); - // Transient strong ref drops here, back - // to weak-only before the next `recv()`. + // Transient strong ref drops here, back + // to weak-only before the next `recv()`. + } } tracing::info!("Background watcher is stopped"); @@ -671,15 +680,17 @@ fn handle_debounced_events( } /// After registering a watch on a newly created directory, list its -/// immediate children and add any files to the picker. +/// immediate children and add any files to the picker. Returns subdirs +/// unknown to the index (empty at scan time or freshly moved in) so the +/// caller can register watches for them too. fn track_files_from_new_directories( dir: &Path, shared_picker: &SharedFilePicker, git_workdir: &Option, git_status_worker: &Arc, -) { +) -> Vec { let Ok(entries) = std::fs::read_dir(dir) else { - return; + return Vec::new(); }; let repo = git_workdir.as_ref().and_then(|p| Repository::open(p).ok()); @@ -689,34 +700,49 @@ fn track_files_from_new_directories( .map(|p| (p.base_path().to_path_buf(), p.ignore_rules())) }) { Some(pair) => pair, - None => return, + None => return Vec::new(), }; let filter = IgnoreFilter::new(&base_path, walker_rules, repo.as_ref()); let mut files_to_add = Vec::new(); + let mut subdirs = Vec::new(); for entry in entries.flatten() { - if entry.file_type().is_ok_and(|ft| ft.is_file()) { - let path = entry.path(); + let Ok(file_type) = entry.file_type() else { + continue; + }; + let path = entry.path(); + if file_type.is_file() { // file_type() already ruled out directories — only ignore rules left if !filter.is_ignored(&path) { files_to_add.push(path); } + } else if file_type.is_dir() && !is_git_file(&path) && !filter.is_ignored(&path) { + subdirs.push(path); } } + // Indexed dirs already have watches (initial setup / post-scan + // resubscribe); only recurse into dirs the index doesn't know about. + if !subdirs.is_empty() + && let Ok(guard) = shared_picker.read() + && let Some(picker) = guard.as_ref() + { + subdirs.retain(|d| !picker.has_indexed_dir(d)); + } + if files_to_add.is_empty() { - return; + return subdirs; } let mut indexed_files = Vec::with_capacity(files_to_add.len()); { let Ok(mut guard) = shared_picker.write() else { - return; + return subdirs; }; let Some(ref mut picker) = *guard else { - return; + return subdirs; }; for path in &files_to_add { @@ -750,6 +776,8 @@ fn track_files_from_new_directories( added, dir.display(), ); + + subdirs } struct IgnoreFilter<'a> { diff --git a/crates/fff-core/tests/dir_index_consistency_test.rs b/crates/fff-core/tests/dir_index_consistency_test.rs index 0fd766e1..145913dc 100644 --- a/crates/fff-core/tests/dir_index_consistency_test.rs +++ b/crates/fff-core/tests/dir_index_consistency_test.rs @@ -254,3 +254,48 @@ fn recreated_directory_reappears_in_dir_search() { search_dirs(&picker, "phoenix") ); } + +/// Regression for #725: a dir that is EMPTY at scan time must be indexed — +/// searchable in dir search and watched so later file creations are seen. +#[test] +fn empty_directory_at_scan_is_searchable_and_watched() { + let tmp = TempDir::new().unwrap(); + let base = fff_search::path_utils::canonicalize(tmp.path()).unwrap(); + fs::create_dir_all(base.join("commands")).unwrap(); + fs::write(base.join("keep.rs"), "x").unwrap(); + + let (picker, _frecency) = make_watched_picker(&base); + + assert!( + search_dirs(&picker, "commands") + .iter() + .any(|d| d.starts_with("commands")), + "empty dir must be searchable right after the scan, got: {:?}", + search_dirs(&picker, "commands") + ); + + // The empty dir must reuse its scan-built DirItem when a file lands in it + // and the watcher must have registered a watch on it (the #725 repro). + fs::write(base.join("commands/review.md"), "# review").unwrap(); + assert!( + wait_until( + || { + let guard = picker.read().unwrap(); + let p = guard.as_ref().unwrap(); + p.get_file_by_path(base.join("commands/review.md")) + .is_some() + }, + Duration::from_secs(10) + ), + "file created in a scan-time-empty dir must be indexed" + ); + + let guard = picker.read().unwrap(); + let p = guard.as_ref().unwrap(); + let commands_dirs = p + .get_dirs() + .iter() + .filter(|d| d.relative_path(p).starts_with("commands")) + .count(); + assert_eq!(commands_dirs, 1, "no duplicate DirItem for the empty dir"); +} diff --git a/crates/fff-core/tests/new_directory_watcher_test.rs b/crates/fff-core/tests/new_directory_watcher_test.rs index a32d1738..ce1cc6bf 100644 --- a/crates/fff-core/tests/new_directory_watcher_test.rs +++ b/crates/fff-core/tests/new_directory_watcher_test.rs @@ -469,6 +469,90 @@ fn burst_file_creation_in_new_directory() { } } +/// bug pinning #725: a directory that already exists but is EMPTY at +/// initial scan time is absent from `sync_data.dirs` and missing watch events +#[test] +fn file_created_in_preexisting_empty_directory() { + let tmp = TempDir::new().unwrap(); + let base = tmp.path().canonicalize().unwrap(); + + // `commands/` is empty during the initial scan — only `init.lua` is indexed. + fs::create_dir_all(base.join("commands")).unwrap(); + fs::write(base.join("init.lua"), "-- init\n").unwrap(); + + let (shared_picker, _frecency) = make_watched_picker(&base); + wait_ready(&shared_picker); + + // Now write a file into the directory that was empty at scan time. + fs::write( + base.join("commands/review.md"), + "# Review\nEMPTY_DIR_REVIEW_TOKEN\n", + ) + .unwrap(); + + let elapsed = poll_until( + &shared_picker, + WATCHER_TIMEOUT, + "file commands/review.md created in a pre-existing empty directory", + |picker| { + picker + .get_files() + .iter() + .any(|f| f.relative_path(picker).contains("review.md")) + }, + ); + eprintln!( + " File in pre-existing empty directory detected in {:.0}ms", + elapsed.as_secs_f64() * 1000.0 + ); +} + +/// Same as above but with a nested chain of empty directories under an +/// indexed one: every level of the empty subtree must be watched. +#[test] +fn file_created_in_nested_preexisting_empty_directories() { + let tmp = TempDir::new().unwrap(); + let base = tmp.path().canonicalize().unwrap(); + + // `src/` is indexed (has a file); `src/plugins/extra/` is an empty chain. + fs::create_dir_all(base.join("src/plugins/extra")).unwrap(); + fs::write(base.join("src/main.rs"), "fn main() {}\n").unwrap(); + + git_init_and_commit(&base); + + let (shared_picker, _frecency) = make_watched_picker(&base); + wait_ready(&shared_picker); + + fs::write( + base.join("src/plugins/extra/loader.rs"), + "pub fn load() {}\nconst TOKEN: &str = \"NESTED_EMPTY_DIR_TOKEN\";\n", + ) + .unwrap(); + + let elapsed = poll_until( + &shared_picker, + WATCHER_TIMEOUT, + "file src/plugins/extra/loader.rs created in nested empty directories", + |picker| { + picker + .get_files() + .iter() + .any(|f| f.relative_path(picker).contains("loader.rs")) + }, + ); + eprintln!( + " File in nested empty directories detected in {:.0}ms", + elapsed.as_secs_f64() * 1000.0 + ); + + poll_until( + &shared_picker, + WATCHER_TIMEOUT, + "grep finds NESTED_EMPTY_DIR_TOKEN", + |picker| grep_plain_count(picker, "NESTED_EMPTY_DIR_TOKEN") >= 1, + ); +} + /// Verify that gitignored directories created at runtime are NOT watched /// and their files do NOT appear in the index. #[test] From 8443c49ad90516ed524b367055db124100d495c6 Mon Sep 17 00:00:00 2001 From: Dmitriy Kovalenko Date: Tue, 4 Aug 2026 20:16:37 -0700 Subject: [PATCH 2/3] fix: Gitignore incompatbility Closes https://github.com/dmtrKovalenko/fff/issues/723 fixed in zlob --- Cargo.lock | 4 ++-- Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bfe636b1..6f6c65df 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3235,9 +3235,9 @@ dependencies = [ [[package]] name = "zlob" -version = "1.6.1" +version = "1.6.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e41cb327ac1b7e7e0d4514658500cb5734cd655edbe4e5ffeda69955da9028ee" +checksum = "57e7ca1588981ea66f5ac470915c4cb44a47ecfb5797576f65fca1bafacedf4e" dependencies = [ "bindgen", "bitflags 2.11.0", diff --git a/Cargo.toml b/Cargo.toml index 4aa7459c..385d2ff2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,7 +36,7 @@ ignore = "0.4.22" memmap2 = "0.9" mimalloc = "0.1.47" signal-hook-registry = "1.4" -zlob = { version = "=1.6.1" } +zlob = { version = "=1.6.2" } mlua = { version = "0.11.1", features = ["module", "luajit"] } neo_frizbee = { version = "0.11.0", features = ["match_end_col"] } From 795ee9c7d8acf7b0eaa19658f5f6eb36a30f6166 Mon Sep 17 00:00:00 2001 From: Dmitriy Kovalenko Date: Wed, 5 Aug 2026 18:09:52 -0700 Subject: [PATCH 3/3] more efficient way to track subdirs --- .github/workflows/external-tests.yml | 22 +- crates/fff-core/src/file_picker.rs | 49 ++--- crates/fff-core/src/walk/mod.rs | 4 +- crates/fff-core/src/walk/ripgrep.rs | 18 +- crates/fff-core/src/walk/zlob.rs | 20 +- .../src/watcher/background_watcher.rs | 204 ++++++++++-------- .../tests/new_directory_watcher_test.rs | 75 ++++++- 7 files changed, 237 insertions(+), 155 deletions(-) diff --git a/.github/workflows/external-tests.yml b/.github/workflows/external-tests.yml index 8cf071f4..480fd2e2 100644 --- a/.github/workflows/external-tests.yml +++ b/.github/workflows/external-tests.yml @@ -18,6 +18,10 @@ env: # Force Node 24 for all JS-based actions to avoid the libuv # process_title assertion crash on Windows (known Node 20 bug). FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true + # e2e only needs a working binary, so skip fat LTO (same settings as the `ci` + # profile releases ship). Overriding release keeps artifacts in target/release. + CARGO_PROFILE_RELEASE_LTO: thin + CARGO_PROFILE_RELEASE_CODEGEN_UNITS: 16 jobs: lua-tests: @@ -32,7 +36,6 @@ jobs: - os: ubuntu-latest - os: macos-latest - os: windows-latest - target: x86_64-pc-windows-msvc steps: - uses: actions/checkout@v5 - uses: oven-sh/setup-bun@v2 @@ -49,20 +52,13 @@ jobs: cache-on-failure: false cache-key: "v2-lua-e2e" rustflags: "" - target: ${{ matrix.target || '' }} - - name: Build Rust binary (Windows) - if: matrix.target - run: cargo build --release --target ${{ matrix.target }} -p fff-nvim --no-default-features --features zlob - - - name: Copy binary to target/release (Windows) - if: matrix.target + - name: Build Rust binary shell: bash - run: | - cp target/${{ matrix.target }}/release/fff_nvim.dll target/release/fff_nvim.dll + run: make build - name: Verify Windows DLL has no unexpected dependencies - if: matrix.target + if: matrix.os == 'windows-latest' shell: pwsh run: | # Find dumpbin via vswhere (always available on GitHub Actions Windows runners) @@ -78,10 +74,6 @@ jobs: exit 1 } - - name: Build Rust binary - if: ${{ !matrix.target }} - run: make build - - name: Install Neovim uses: rhysd/action-setup-vim@v1 with: diff --git a/crates/fff-core/src/file_picker.rs b/crates/fff-core/src/file_picker.rs index e17694fb..36b93269 100644 --- a/crates/fff-core/src/file_picker.rs +++ b/crates/fff-core/src/file_picker.rs @@ -729,19 +729,6 @@ impl FilePicker { &self.sync_data.dirs } - /// Whether the absolute `path` is a directory known to the index - /// (including dirs that were empty at scan time). - pub(crate) fn has_indexed_dir(&self, path: &Path) -> bool { - let Ok(rel) = path.strip_prefix(&self.base_path) else { - return false; - }; - let mut rel = crate::path_utils::to_canonical_slashes(&rel.to_string_lossy()).into_owned(); - if !rel.is_empty() && !rel.ends_with('/') { - rel.push('/'); - } - self.sync_data.find_dir_index(&rel).is_some() - } - /// Actual heap bytes used: (chunked_path_store, 0, 0). /// The second element is 0 because leaked overflow stores aren't tracked. pub fn arena_bytes(&self) -> (usize, usize, usize) { @@ -2047,9 +2034,7 @@ impl FileSync { )?; let ignore_rules = ignore_rules.map(Arc::new); - // Sort files by (dir_part, filename) — grouping them into contiguous - // per-dir runs — and the walked dirs by the same '/'-terminated form, - // so the build pass below can merge both in a single sweep. + // group walked dirs and files with a dir part to the same order BACKGROUND_THREAD_POOL.install(|| { rayon::join( || { @@ -2200,35 +2185,45 @@ fn populates_dirs_files_chunked_storage<'a>( let mut dirs: Vec = Vec::with_capacity(walked_dirs.len() + 1); let mut dir_iter = walked_dirs.iter().peekable(); + // Root-level files sort first and their "" parent is never a walker dir. + if pairs + .first() + .is_some_and(|(f, _)| f.path.filename_offset == 0) + { + push_dir_item(&mut dirs, chunk_storage, ""); + } + + // Detects contiguous same-dir runs (pairs are sorted by dir) so the + // merge below runs once per directory, not once per file. let mut prev_dir: &'a str = ""; - let mut prev_dir_valid = false; let mut current_dir_idx: u32 = 0; for (file, rel) in pairs.iter_mut() { let rel: &'a str = rel; let dir_part: &'a str = &rel[..file.path.filename_offset as usize]; - if !prev_dir_valid || prev_dir != dir_part { + if prev_dir != dir_part { // Flush walked dirs up to and including this file's parent, // keeping the table sorted for the find_dir_index binary search. - let mut matched = false; while let Some(dir) = dir_iter.peek() - && dir.as_str() <= dir_part + && dir.as_str() < dir_part { - matched = dir.as_str() == dir_part; push_dir_item(&mut dirs, chunk_storage, dir); dir_iter.next(); } - // Root-level files ("" dir part) and parents the walker reported - // with a non-dir kind (e.g. followed symlinks) aren't in the list. - if !matched { - push_dir_item(&mut dirs, chunk_storage, dir_part); + match dir_iter.peek() { + Some(dir) if dir.as_str() == dir_part => { + push_dir_item(&mut dirs, chunk_storage, dir); + dir_iter.next(); + } + // Parents the walker reported with a non-dir kind + // (e.g. followed symlinks) aren't in the list. + _ => push_dir_item(&mut dirs, chunk_storage, dir_part), } - current_dir_idx = (dirs.len() - 1) as u32; + current_dir_idx = (dirs.len() - 1) as u32; prev_dir = dir_part; - prev_dir_valid = true; } file.path = chunk_storage.add_file_immediate(rel, file.path.filename_offset); diff --git a/crates/fff-core/src/walk/mod.rs b/crates/fff-core/src/walk/mod.rs index 2ea3db08..e163d3a6 100644 --- a/crates/fff-core/src/walk/mod.rs +++ b/crates/fff-core/src/walk/mod.rs @@ -20,9 +20,7 @@ pub(crate) use ripgrep::walk_collect_files; pub(crate) struct WalkOutput { pub(crate) pairs: Vec<(FileItem, String)>, - /// Every non-ignored directory the walk visited, as '/'-canonical - /// '/'-terminated paths relative to the base (base itself excluded). - /// This is the source of the picker's dir table; unsorted (parallel walk). + /// Every non-ignored directory the walk visited, relative, ending with / pub(crate) dirs: Vec, pub(crate) ignore_rules: Option, } diff --git a/crates/fff-core/src/walk/ripgrep.rs b/crates/fff-core/src/walk/ripgrep.rs index e6f019db..3f3ca1bc 100644 --- a/crates/fff-core/src/walk/ripgrep.rs +++ b/crates/fff-core/src/walk/ripgrep.rs @@ -34,11 +34,12 @@ pub(crate) fn walk_collect_files( let walker = walk_builder.build_parallel(); - let pairs = parking_lot::Mutex::new(Vec::<(FileItem, String)>::new()); - let dirs = parking_lot::Mutex::new(Vec::::new()); + // Single lock for both collections: every entry is either a file or a + // dir, so this keeps one mutex acquisition per entry. + let collected = + parking_lot::Mutex::new((Vec::<(FileItem, String)>::new(), Vec::::new())); walker.run(|| { - let pairs = &pairs; - let dirs = &dirs; + let collected = &collected; let counter = Arc::clone(synced_files_count); let base_path = base_path.to_path_buf(); @@ -60,7 +61,7 @@ pub(crate) fn walk_collect_files( let (file_item, rel_path) = FileItem::new_from_walk(path, &base_path, None, metadata.as_ref()); - pairs.lock().push((file_item, rel_path)); + collected.lock().0.push((file_item, rel_path)); counter.fetch_add(1, Ordering::Relaxed); } else if entry.depth() > 0 && entry.file_type().is_some_and(|ft| ft.is_dir()) { let path = entry.path(); @@ -70,16 +71,17 @@ pub(crate) fn walk_collect_files( let mut rel = crate::path_utils::to_canonical_slashes(&rel.to_string_lossy()) .into_owned(); rel.push('/'); - dirs.lock().push(rel); + collected.lock().1.push(rel); } } ignore::WalkState::Continue }) }); + let (pairs, dirs) = collected.into_inner(); Ok(WalkOutput { - pairs: pairs.into_inner(), - dirs: dirs.into_inner(), + pairs, + dirs, ignore_rules: None, }) } diff --git a/crates/fff-core/src/walk/zlob.rs b/crates/fff-core/src/walk/zlob.rs index d2cdb7da..9dc2f350 100644 --- a/crates/fff-core/src/walk/zlob.rs +++ b/crates/fff-core/src/walk/zlob.rs @@ -46,19 +46,19 @@ pub(crate) fn walk_collect_files( tracing::warn!(?e, "zlob extra_ignore rejected; walking without it"); } - let pairs = Mutex::new(Vec::new()); - let dirs = Mutex::new(Vec::new()); + // Single lock for both collections: every entry is either a file or a + // dir, so this keeps one mutex acquisition per entry. + let collected = Mutex::new((Vec::new(), Vec::new())); let outcome = match builder.run(|entry| { if !entry.is_file() { - // `.git` and ignored dirs are pruned by zlob itself (GITIGNORE - // flag); empty rel path is the walk root, covered separately. + // unlike ripgrep walker zlob doesnt show .git files if entry.is_dir() { let rel_bytes = entry.relative_path_bytes(); if !rel_bytes.is_empty() { let mut rel = String::from_utf8_lossy(rel_bytes).into_owned(); rel.push('/'); - dirs.lock().push(rel); + collected.lock().1.push(rel); } } @@ -84,9 +84,9 @@ pub(crate) fn walk_collect_files( let rel_str = String::from_utf8_lossy(rel_bytes).into_owned(); let item = FileItem::new_raw(basename_offset, size, modified, None, is_binary); - let mut guard = pairs.lock(); - guard.push((item, rel_str)); - let n = guard.len(); + let mut guard = collected.lock(); + guard.0.push((item, rel_str)); + let n = guard.0.len(); drop(guard); if n % PROGRESS_STEP == 0 { @@ -104,7 +104,7 @@ pub(crate) fn walk_collect_files( } }; - let pairs = pairs.into_inner(); + let (pairs, dirs) = collected.into_inner(); // Always report the exact final total regardless of the last step. synced_files_count.store(pairs.len(), Ordering::Relaxed); @@ -117,7 +117,7 @@ pub(crate) fn walk_collect_files( Ok(WalkOutput { pairs, - dirs: dirs.into_inner(), + dirs, ignore_rules, }) } diff --git a/crates/fff-core/src/watcher/background_watcher.rs b/crates/fff-core/src/watcher/background_watcher.rs index 8c8ba3f7..73568a87 100644 --- a/crates/fff-core/src/watcher/background_watcher.rs +++ b/crates/fff-core/src/watcher/background_watcher.rs @@ -22,10 +22,19 @@ type Debouncer = notify_debouncer_full::Debouncer>>, - watch_tx: Option>, + watch_tx: Option>, owner_thread: Option>, } +enum WatchTask { + /// Only subscribe to a specific path, this is happening when we did rescun and have to update + /// the watcher only + Subscribe(PathBuf), + /// This is requires a separate walk of the new directory copies or created within a scan + /// window because it might contain subdirectories we have to walk, prune, and add to index + IndexNewDir(PathBuf), +} + const DEBOUNCE_TIMEOUT: Duration = Duration::from_millis(50); /// Minimum seconds between frecency tracks of the same file in AI mode. /// Prevents score inflation from rapid burst edits by AI agents. @@ -77,7 +86,7 @@ impl BackgroundWatcher { // spare watcher (configurable by the user, usually 100k - 1m) let use_recursive = cfg!(any(target_os = "macos", target_os = "windows")); - let (watch_tx, watch_rx) = mpsc::channel::(); + let (watch_tx, watch_rx) = mpsc::channel::(); let watch_tx_for_debouncer = watch_tx.clone(); let owner_weak_picker = shared_picker.weaken(); @@ -108,57 +117,46 @@ impl BackgroundWatcher { .name("fff-watcher-own".into()) .spawn(move || { let _g = owner_span.enter(); - // Local work queue: enumerating a dir can surface subdirs - // unknown to the index (empty at scan time) that also need - // watches; we must not self-send on `watch_tx` or `recv()` - // would never disconnect on stop. - let mut queue = std::collections::VecDeque::new(); - 'recv: while let Ok(dir) = watch_rx.recv() { - queue.push_back(dir); - while let Some(dir) = queue.pop_front() { - // if the picker is dropped we do need to exit the loop - let Some(strong_picker) = owner_weak_picker.upgrade() else { - break 'recv; - }; + while let Ok(task) = watch_rx.recv() { + // if the picker is dropped we do need to exit the loop + let Some(strong_picker) = owner_weak_picker.upgrade() else { + break; + }; - // Only inotify (Linux) has no kernel-level recursion, so - // it's the only platform that needs a per-subdir watch to - // be registered at runtime. macOS FSEvents and Windows - // ReadDirectoryChangesW are already watching recursively - // from the base path (see `create_debouncer`), and - // registering a second overlapping stream there produces - // duplicate/out-of-order events. - #[cfg(target_os = "linux")] - { - // Register the new directory with the debouncer, then - // drop the mutex BEFORE doing picker-side work — see - // the comment on `BackgroundWatcher::stop` for the - // lock-ordering rationale. - let mut guard = owner_debouncer.lock(); - let Some(debouncer) = guard.as_mut() else { - break 'recv; - }; - - if let Err(e) = debouncer.watch(&dir, RecursiveMode::NonRecursive) { - warn!( - ?e, - dir = %dir.display(), - "Failed to init watcher for new directory" - ); - } - } + let (dir, is_new_dir) = match task { + WatchTask::Subscribe(dir) => (dir, false), + WatchTask::IndexNewDir(dir) => (dir, true), + }; + + // Register the watch BEFORE walking so files created mid-walk still handled + #[cfg(target_os = "linux")] + if !watch_dirs_nonrecursive(&owner_debouncer, std::iter::once(dir.as_path())) { + break; + } - let unindexed_subdirs = track_files_from_new_directories( + if is_new_dir { + // need to call this on every platform to add subdirectories from the + // new folders to the picker, but on linux we have to handle the subdirs + let subdirs = index_new_directory( &dir, &strong_picker, &owner_git_workdir, &owner_git_worker, ); - queue.extend(unindexed_subdirs); - // Transient strong ref drops here, back - // to weak-only before the next `recv()`. + // on linux we manually resubscribe for new inodes + #[cfg(target_os = "linux")] + if !watch_dirs_nonrecursive( + &owner_debouncer, + subdirs.iter().map(|p| p.as_path()), + ) { + break; + } + + drop(subdirs); // need it cause subdirs is unused on non-linux target' } + + drop(strong_picker); } tracing::info!("Background watcher is stopped"); @@ -180,7 +178,7 @@ impl BackgroundWatcher { shared_frecency: SharedFrecency, mode: FFFMode, use_recursive: bool, - watch_tx: mpsc::Sender, + watch_tx: mpsc::Sender, git_status_worker: Arc, ) -> Result { let config = Config::default() @@ -215,7 +213,7 @@ impl BackgroundWatcher { // every new directory created has to be reflected in the picker state for dir in new_dirs { - if let Err(e) = watch_tx.send(dir) { + if let Err(e) = watch_tx.send(WatchTask::IndexNewDir(dir)) { error!(?e, "Failed to send directory update error"); } } @@ -313,7 +311,7 @@ impl BackgroundWatcher { pub(crate) fn request_watch_dir(&self, dir: PathBuf) -> bool { match self.watch_tx.as_ref() { - Some(tx) => tx.send(dir).is_ok(), + Some(tx) => tx.send(WatchTask::Subscribe(dir)).is_ok(), None => false, } } @@ -679,57 +677,60 @@ fn handle_debounced_events( new_dirs_to_watch } -/// After registering a watch on a newly created directory, list its -/// immediate children and add any files to the picker. Returns subdirs -/// unknown to the index (empty at scan time or freshly moved in) so the -/// caller can register watches for them too. -fn track_files_from_new_directories( +fn index_new_directory( dir: &Path, shared_picker: &SharedFilePicker, git_workdir: &Option, git_status_worker: &Arc, ) -> Vec { - let Ok(entries) = std::fs::read_dir(dir) else { - return Vec::new(); - }; - let repo = git_workdir.as_ref().and_then(|p| Repository::open(p).ok()); // Prefer the walker's ignore rules; read base_path + rules from the picker. - let (base_path, walker_rules) = match shared_picker.read().ok().and_then(|g| { - g.as_ref() - .map(|p| (p.base_path().to_path_buf(), p.ignore_rules())) + let (base_path, walker_rules, follow_symlinks) = match shared_picker.read().ok().and_then(|g| { + g.as_ref().map(|p| { + ( + p.base_path().to_path_buf(), + p.ignore_rules(), + p.follows_symlinks(), + ) + }) }) { - Some(pair) => pair, + Some(triple) => triple, None => return Vec::new(), }; + let walk = match crate::walk::walk_collect_files( + dir, + repo.is_some(), + follow_symlinks, + 1, + &Arc::new(std::sync::atomic::AtomicUsize::new(0)), + ) { + Ok(walk) => walk, + Err(e) => { + warn!(?e, dir = %dir.display(), "Failed to walk new directory"); + return Vec::new(); + } + }; + + // TODO: figure out a better optimized way for zlob to rerun the directory walk using existing + // ignore rules, but currently we have to filter out ignored files on our own let filter = IgnoreFilter::new(&base_path, walker_rules, repo.as_ref()); - let mut files_to_add = Vec::new(); - let mut subdirs = Vec::new(); + let join_unless_ignored = |relative_path: &str| -> Option { + let path = dir.join(relative_path); + (!filter.is_ignored(&path)).then_some(path) + }; - for entry in entries.flatten() { - let Ok(file_type) = entry.file_type() else { - continue; - }; - let path = entry.path(); - if file_type.is_file() { - // file_type() already ruled out directories — only ignore rules left - if !filter.is_ignored(&path) { - files_to_add.push(path); - } - } else if file_type.is_dir() && !is_git_file(&path) && !filter.is_ignored(&path) { - subdirs.push(path); - } - } + let files_to_add: Vec = walk + .pairs + .iter() + .filter_map(|(_, path)| join_unless_ignored(path)) + .collect(); - // Indexed dirs already have watches (initial setup / post-scan - // resubscribe); only recurse into dirs the index doesn't know about. - if !subdirs.is_empty() - && let Ok(guard) = shared_picker.read() - && let Some(picker) = guard.as_ref() - { - subdirs.retain(|d| !picker.has_indexed_dir(d)); - } + let subdirs: Vec = walk + .dirs + .iter() + .filter_map(|path| join_unless_ignored(path.trim_end_matches('/'))) + .collect(); if files_to_add.is_empty() { return subdirs; @@ -745,9 +746,9 @@ fn track_files_from_new_directories( return subdirs; }; - for path in &files_to_add { - if picker.handle_create_or_modify(path).is_some() { - indexed_files.push(path.clone()); + for path in files_to_add { + if picker.handle_create_or_modify(&path).is_some() { + indexed_files.push(path); } } } @@ -772,7 +773,7 @@ fn track_files_from_new_directories( } debug!( - "Injected {} existing files from new directory {}", + "Indexed new {} files from new directory {}", added, dir.display(), ); @@ -780,6 +781,29 @@ fn track_files_from_new_directories( subdirs } +#[cfg(target_os = "linux")] +fn watch_dirs_nonrecursive<'a>( + debouncer: &Mutex>, + dirs: impl Iterator, +) -> bool { + let mut guard = debouncer.lock(); + let Some(debouncer) = guard.as_mut() else { + return false; + }; + + for dir in dirs { + if let Err(e) = debouncer.watch(dir, RecursiveMode::NonRecursive) { + warn!( + ?e, + dir = %dir.display(), + "Failed to init watcher for new directory" + ); + } + } + + true +} + struct IgnoreFilter<'a> { base_path: &'a Path, /// Reusable ignore rules from the last walk (zlob backend only). @@ -805,13 +829,13 @@ impl<'a> IgnoreFilter<'a> { /// Whether `path` (absolute) is ignored. fn is_ignored(&self, path: &Path) -> bool { if let Some(rules) = self.rules.as_ref() { - let Ok(rel) = path.strip_prefix(self.base_path) else { + let Ok(relative) = path.strip_prefix(self.base_path) else { return false; }; // `IgnoreRules::is_ignored` enumerates every ancestor .gitignore // layer internally, so a leaf under an ignored directory (rule // `build/`, path `build/out.rs`) is caught in one call. - return rules.is_ignored(rel); + return rules.is_ignored(relative); } match self.repo { Some(repo) => repo.is_path_ignored(path) == Ok(true), diff --git a/crates/fff-core/tests/new_directory_watcher_test.rs b/crates/fff-core/tests/new_directory_watcher_test.rs index ce1cc6bf..e76b71b5 100644 --- a/crates/fff-core/tests/new_directory_watcher_test.rs +++ b/crates/fff-core/tests/new_directory_watcher_test.rs @@ -8,8 +8,9 @@ //! 3. The watcher's event handler detects the directory Create event, //! collects it, and sends it to the owner thread via `watch_tx`. //! 4. The owner thread adds a NonRecursive watch on the new directory and -//! does a flat (non-recursive) read_dir to inject files that already -//! exist (race-window coverage). +//! walks its subtree (`index_new_directory`) to inject files that +//! already exist (race-window + burst/mv-in coverage) and to watch +//! nested subdirectories. //! 5. Files created *after* the watch is established are picked up via //! normal event delivery. //! @@ -553,6 +554,76 @@ fn file_created_in_nested_preexisting_empty_directories() { ); } +#[test] +fn nested_tree_created_in_one_burst_detected() { + let tmp = TempDir::new().unwrap(); + let base = tmp.path().canonicalize().unwrap(); + + fs::write(base.join("root.txt"), "root file\n").unwrap(); + git_init_and_commit(&base); + + let (shared_picker, _frecency) = make_watched_picker(&base); + wait_ready(&shared_picker); + + // No sleeps between levels: the watcher sees one Create for `pkg` and + // must index the whole subtree from it. + fs::create_dir_all(base.join("pkg/src/nested")).unwrap(); + fs::write(base.join("pkg/Cargo.toml"), "[package]\n").unwrap(); + fs::write( + base.join("pkg/src/lib.rs"), + "const TOKEN: &str = \"BURST_TREE_LIB_TOKEN\";\n", + ) + .unwrap(); + fs::write( + base.join("pkg/src/nested/deep.rs"), + "const TOKEN: &str = \"BURST_TREE_DEEP_TOKEN\";\n", + ) + .unwrap(); + + for rel in ["pkg/Cargo.toml", "pkg/src/lib.rs", "pkg/src/nested/deep.rs"] { + let elapsed = poll_until( + &shared_picker, + WATCHER_TIMEOUT, + &format!("burst-created file {rel}"), + |picker| { + picker + .get_files() + .iter() + .any(|f| f.relative_path(picker) == rel) + }, + ); + eprintln!( + " Burst file {rel} detected in {:.0}ms", + elapsed.as_secs_f64() * 1000.0 + ); + } + + // Files created later at the deepest level need the nested watches too. + fs::write( + base.join("pkg/src/nested/late.rs"), + "const TOKEN: &str = \"BURST_TREE_LATE_TOKEN\";\n", + ) + .unwrap(); + poll_until( + &shared_picker, + WATCHER_TIMEOUT, + "late file in burst-created nested dir", + |picker| { + picker + .get_files() + .iter() + .any(|f| f.relative_path(picker).ends_with("late.rs")) + }, + ); + + poll_until( + &shared_picker, + WATCHER_TIMEOUT, + "grep finds BURST_TREE_DEEP_TOKEN", + |picker| grep_plain_count(picker, "BURST_TREE_DEEP_TOKEN") >= 1, + ); +} + /// Verify that gitignored directories created at runtime are NOT watched /// and their files do NOT appear in the index. #[test]