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/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"] } diff --git a/crates/fff-core/src/file_picker.rs b/crates/fff-core/src/file_picker.rs index 6f9055fc..36b93269 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}; @@ -2020,30 +2021,38 @@ 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. + // group walked dirs and files with a dir part to the same order 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,49 +2173,91 @@ 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(); + // 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 { - let dir_string = chunk_storage.add_dir_immediate(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. + while let Some(dir) = dir_iter.peek() + && dir.as_str() < dir_part + { + push_dir_item(&mut dirs, chunk_storage, dir); + dir_iter.next(); + } - // 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 - }; + 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), + } - dirs.push(DirItem::new(dir_string, last_seg)); 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); 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 +2392,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 +2408,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 +2465,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..e163d3a6 100644 --- a/crates/fff-core/src/walk/mod.rs +++ b/crates/fff-core/src/walk/mod.rs @@ -20,6 +20,8 @@ pub(crate) use ripgrep::walk_collect_files; pub(crate) struct WalkOutput { pub(crate) pairs: Vec<(FileItem, String)>, + /// 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 def4599b..3f3ca1bc 100644 --- a/crates/fff-core/src/walk/ripgrep.rs +++ b/crates/fff-core/src/walk/ripgrep.rs @@ -34,9 +34,12 @@ pub(crate) fn walk_collect_files( let walker = walk_builder.build_parallel(); - let pairs = parking_lot::Mutex::new(Vec::<(FileItem, String)>::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 collected = &collected; let counter = Arc::clone(synced_files_count); let base_path = base_path.to_path_buf(); @@ -58,15 +61,27 @@ 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(); + 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('/'); + collected.lock().1.push(rel); + } } ignore::WalkState::Continue }) }); + let (pairs, dirs) = collected.into_inner(); Ok(WalkOutput { - pairs: pairs.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 0978ad1b..9dc2f350 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()); + // 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() { + // 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('/'); + collected.lock().1.push(rel); + } + } + return WalkState::Continue; } + let rel_bytes = entry.relative_path_bytes(); // `basename()` returns `&str` for files only. @@ -73,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 { @@ -93,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); @@ -106,6 +117,7 @@ pub(crate) fn walk_collect_files( Ok(WalkOutput { pairs, + dirs, ignore_rules, }) } diff --git a/crates/fff-core/src/watcher/background_watcher.rs b/crates/fff-core/src/watcher/background_watcher.rs index 36a9e124..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,48 +117,46 @@ impl BackgroundWatcher { .name("fff-watcher-own".into()) .spawn(move || { let _g = owner_span.enter(); - while let Ok(dir) = watch_rx.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. + 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")] - { - // 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; - }; + if !watch_dirs_nonrecursive(&owner_debouncer, std::iter::once(dir.as_path())) { + break; + } - if let Err(e) = debouncer.watch(&dir, RecursiveMode::NonRecursive) { - warn!( - ?e, - dir = %dir.display(), - "Failed to init watcher for new directory" - ); + 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, + ); + + // 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; } - } - track_files_from_new_directories( - &dir, - &strong_picker, - &owner_git_workdir, - &owner_git_worker, - ); + drop(subdirs); // need it cause subdirs is unused on non-linux target' + } - // Transient strong ref drops here, back - // to weak-only before the next `recv()`. + drop(strong_picker); } tracing::info!("Background watcher is stopped"); @@ -171,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() @@ -206,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"); } } @@ -304,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, } } @@ -670,58 +677,78 @@ 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. -fn track_files_from_new_directories( +fn index_new_directory( dir: &Path, shared_picker: &SharedFilePicker, git_workdir: &Option, git_status_worker: &Arc, -) { - let Ok(entries) = std::fs::read_dir(dir) else { - return; - }; - +) -> Vec { 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, - None => return, + Some(triple) => triple, + None => return Vec::new(), }; - let filter = IgnoreFilter::new(&base_path, walker_rules, repo.as_ref()); - let mut files_to_add = Vec::new(); - - for entry in entries.flatten() { - if entry.file_type().is_ok_and(|ft| ft.is_file()) { - let path = entry.path(); - // file_type() already ruled out directories — only ignore rules left - if !filter.is_ignored(&path) { - files_to_add.push(path); - } + 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 join_unless_ignored = |relative_path: &str| -> Option { + let path = dir.join(relative_path); + (!filter.is_ignored(&path)).then_some(path) + }; + + let files_to_add: Vec = walk + .pairs + .iter() + .filter_map(|(_, path)| join_unless_ignored(path)) + .collect(); + + let subdirs: Vec = walk + .dirs + .iter() + .filter_map(|path| join_unless_ignored(path.trim_end_matches('/'))) + .collect(); 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 { - 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); } } } @@ -746,10 +773,35 @@ fn track_files_from_new_directories( } debug!( - "Injected {} existing files from new directory {}", + "Indexed new {} files from new directory {}", added, dir.display(), ); + + 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> { @@ -777,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/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..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. //! @@ -469,6 +470,160 @@ 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, + ); +} + +#[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]