Skip to content

Commit da50d3f

Browse files
committed
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
1 parent 1eb913e commit da50d3f

7 files changed

Lines changed: 414 additions & 96 deletions

File tree

crates/fff-core/src/file_picker.rs

Lines changed: 178 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ use crate::types::{
4545
ContentCacheBudget, DirItem, DirSearchResult, FileItem, MixedItemRef, MixedSearchResult,
4646
PaginationArgs, Score, ScoringContext, SearchResult,
4747
};
48+
use crate::walk::WalkOutput;
4849
use crate::watch::BackgroundWatcher;
4950
use fff_query_parser::FFFQuery;
5051
use git2::{Repository, Status};
@@ -728,6 +729,19 @@ impl FilePicker {
728729
&self.sync_data.dirs
729730
}
730731

732+
/// Whether the absolute `path` is a directory known to the index
733+
/// (including dirs that were empty at scan time).
734+
pub(crate) fn has_indexed_dir(&self, path: &Path) -> bool {
735+
let Ok(rel) = path.strip_prefix(&self.base_path) else {
736+
return false;
737+
};
738+
let mut rel = crate::path_utils::to_canonical_slashes(&rel.to_string_lossy()).into_owned();
739+
if !rel.is_empty() && !rel.ends_with('/') {
740+
rel.push('/');
741+
}
742+
self.sync_data.find_dir_index(&rel).is_some()
743+
}
744+
731745
/// Actual heap bytes used: (chunked_path_store, 0, 0).
732746
/// The second element is 0 because leaked overflow stores aren't tracked.
733747
pub fn arena_bytes(&self) -> (usize, usize, usize) {
@@ -2020,30 +2034,40 @@ impl FileSync {
20202034
let is_git_repo = git_workdir.is_some();
20212035
let bg_threads = BACKGROUND_THREAD_POOL.current_num_threads();
20222036

2023-
let mut walk_output = crate::walk::walk_collect_files(
2037+
let WalkOutput {
2038+
dirs: mut walked_dirs,
2039+
mut pairs,
2040+
ignore_rules,
2041+
} = crate::walk::walk_collect_files(
20242042
base_path,
20252043
is_git_repo,
20262044
follow_symlinks,
20272045
bg_threads,
20282046
synced_files_count,
20292047
)?;
2030-
let ignore_rules = walk_output.ignore_rules.take().map(Arc::new);
2031-
let mut pairs = walk_output.pairs;
2048+
let ignore_rules = ignore_rules.map(Arc::new);
20322049

2033-
// Sort by (dir_part, filename). This groups files by their directory
2034-
// into contiguous runs so the linear dir-extraction pass below can
2035-
// dedupe by comparing only against the previous dir.
2050+
// Sort files by (dir_part, filename) — grouping them into contiguous
2051+
// per-dir runs — and the walked dirs by the same '/'-terminated form,
2052+
// so the build pass below can merge both in a single sweep.
20362053
BACKGROUND_THREAD_POOL.install(|| {
2037-
pairs.par_sort_unstable_by(|(a, path_a), (b, path_b)| {
2038-
// SAFETY: `filename_offset` is always at a character boundary
2039-
let (a_dir, a_file) = path_a.split_at(a.path.filename_offset as usize);
2040-
let (b_dir, b_file) = path_b.split_at(b.path.filename_offset as usize);
2041-
a_dir.cmp(b_dir).then_with(|| a_file.cmp(b_file))
2042-
});
2054+
rayon::join(
2055+
|| {
2056+
pairs.par_sort_unstable_by(|(a, path_a), (b, path_b)| {
2057+
// SAFETY: `filename_offset` is always at a character boundary
2058+
let (a_dir, a_file) = path_a.split_at(a.path.filename_offset as usize);
2059+
let (b_dir, b_file) = path_b.split_at(b.path.filename_offset as usize);
2060+
a_dir.cmp(b_dir).then_with(|| a_file.cmp(b_file))
2061+
});
2062+
},
2063+
|| walked_dirs.par_sort_unstable(),
2064+
);
20432065
});
2066+
walked_dirs.dedup();
20442067

20452068
let mut builder = crate::simd_path::ChunkedPathStoreBuilder::new(pairs.len());
2046-
let dirs = populates_dirs_files_chunked_storage(&mut pairs, &mut builder);
2069+
let dirs = populates_dirs_files_chunked_storage(&mut pairs, &walked_dirs, &mut builder);
2070+
drop(walked_dirs);
20472071

20482072
let mut files: Vec<FileItem> = pairs.into_iter().map(|(file, _)| file).collect();
20492073
let chunked_paths = builder.finish();
@@ -2164,12 +2188,17 @@ pub(crate) fn warmup_mmaps(
21642188
}
21652189

21662190
/// This does both thing (yes sorry all the OOP morons)
2167-
/// in one go: populates files chunked storage and creates new directories
2191+
/// in one go: populates files chunked storage and builds the dir table from
2192+
/// `walked_dirs` (every dir the walker visited: sorted, '/'-terminated,
2193+
/// deduped), merging file parents in a single lockstep sweep so dirs with no
2194+
/// files (empty subtrees, pure ancestors) are indexed and searchable too.
21682195
fn populates_dirs_files_chunked_storage<'a>(
21692196
pairs: &'a mut [(FileItem, String)],
2197+
walked_dirs: &[String],
21702198
chunk_storage: &mut crate::simd_path::ChunkedPathStoreBuilder,
21712199
) -> Vec<DirItem> {
2172-
let mut dirs: Vec<DirItem> = Vec::new();
2200+
let mut dirs: Vec<DirItem> = Vec::with_capacity(walked_dirs.len() + 1);
2201+
let mut dir_iter = walked_dirs.iter().peekable();
21732202

21742203
let mut prev_dir: &'a str = "";
21752204
let mut prev_dir_valid = false;
@@ -2180,20 +2209,22 @@ fn populates_dirs_files_chunked_storage<'a>(
21802209
let dir_part: &'a str = &rel[..file.path.filename_offset as usize];
21812210

21822211
if !prev_dir_valid || prev_dir != dir_part {
2183-
let dir_string = chunk_storage.add_dir_immediate(dir_part);
2184-
2185-
// Compute last-segment offset: for "src/components/" -> 4 (points to "components/")
2186-
let last_seg = if dir_part.is_empty() {
2187-
0
2188-
} else {
2189-
let trimmed = dir_part.trim_end_matches(std::path::is_separator);
2190-
trimmed
2191-
.rfind(std::path::is_separator)
2192-
.map(|i| i + 1)
2193-
.unwrap_or(0) as u16
2194-
};
2212+
// Flush walked dirs up to and including this file's parent,
2213+
// keeping the table sorted for the find_dir_index binary search.
2214+
let mut matched = false;
2215+
while let Some(dir) = dir_iter.peek()
2216+
&& dir.as_str() <= dir_part
2217+
{
2218+
matched = dir.as_str() == dir_part;
2219+
push_dir_item(&mut dirs, chunk_storage, dir);
2220+
dir_iter.next();
2221+
}
21952222

2196-
dirs.push(DirItem::new(dir_string, last_seg));
2223+
// Root-level files ("" dir part) and parents the walker reported
2224+
// with a non-dir kind (e.g. followed symlinks) aren't in the list.
2225+
if !matched {
2226+
push_dir_item(&mut dirs, chunk_storage, dir_part);
2227+
}
21972228
current_dir_idx = (dirs.len() - 1) as u32;
21982229

21992230
prev_dir = dir_part;
@@ -2204,9 +2235,34 @@ fn populates_dirs_files_chunked_storage<'a>(
22042235
file.parent_dir_index = current_dir_idx;
22052236
}
22062237

2238+
for dir in dir_iter {
2239+
push_dir_item(&mut dirs, chunk_storage, dir);
2240+
}
2241+
22072242
dirs
22082243
}
22092244

2245+
fn push_dir_item(
2246+
dirs: &mut Vec<DirItem>,
2247+
chunk_storage: &mut crate::simd_path::ChunkedPathStoreBuilder,
2248+
dir_part: &str,
2249+
) {
2250+
let dir_string = chunk_storage.add_dir_immediate(dir_part);
2251+
2252+
// Compute last-segment offset: for "src/components/" -> 4 (points to "components/")
2253+
let last_seg = if dir_part.is_empty() {
2254+
0
2255+
} else {
2256+
let trimmed = dir_part.trim_end_matches(std::path::is_separator);
2257+
trimmed
2258+
.rfind(std::path::is_separator)
2259+
.map(|i| i + 1)
2260+
.unwrap_or(0) as u16
2261+
};
2262+
2263+
dirs.push(DirItem::new(dir_string, last_seg));
2264+
}
2265+
22102266
/// Fast extension-based binary detection. Avoids opening files during scan.
22112267
/// Covers the vast majority of binary files in typical repositories.
22122268
#[inline]
@@ -2341,13 +2397,9 @@ mod tests {
23412397
use super::*;
23422398

23432399
/// The watcher must watch every ancestor directory up to `base_path`,
2344-
/// not just the immediate parents of indexed files. Intermediate dirs
2345-
/// that contain only subdirectories (no direct files) are NOT in
2346-
/// `sync_data.dirs` — yet they must still appear in `extract_watch_dirs`
2347-
/// so Create events on new subdirectories below them fire.
2348-
///
2349-
/// Correctness regression guard for any refactor that replaces the
2350-
/// ancestor walk with a direct `sync_data.dirs` iteration.
2400+
/// not just the immediate parents of indexed files. The dir table is
2401+
/// built from the walker's visited dirs, so pure ancestors (dirs that
2402+
/// contain only subdirectories) must be present and emitted exactly once.
23512403
#[test]
23522404
fn extract_watch_dirs_includes_pure_ancestor_dirs() {
23532405
let dir = tempfile::tempdir().unwrap();
@@ -2361,17 +2413,6 @@ mod tests {
23612413
// base/src/components/button.txt (src/components has a file)
23622414
// base/src/routes/home.txt (src/routes has a file)
23632415
// base/lib/deep/nested/util.txt (lib and lib/deep have no files)
2364-
//
2365-
// `sync_data.dirs` will only contain:
2366-
// src/components/
2367-
// src/routes/
2368-
// lib/deep/nested/
2369-
//
2370-
// But the watcher also needs:
2371-
// src/ (pure ancestor — no direct files)
2372-
// lib/ (pure ancestor)
2373-
// lib/deep/ (pure ancestor)
2374-
// otherwise new siblings like `src/NewDir/x.txt` are missed.
23752416
for rel in [
23762417
"src/components/button.txt",
23772418
"src/routes/home.txt",
@@ -2429,6 +2470,97 @@ mod tests {
24292470
);
24302471
}
24312472

2473+
/// Regression guard for #725: dirs that are EMPTY at scan time are merged
2474+
/// into `sync_data.dirs` so they are searchable and get an inotify watch;
2475+
/// files created in them later must be detected.
2476+
#[test]
2477+
fn for_each_dir_includes_empty_directories() {
2478+
let dir = tempfile::tempdir().unwrap();
2479+
let base_buf = crate::path_utils::canonicalize(dir.path()).unwrap();
2480+
let base = base_buf.as_path();
2481+
2482+
// Tree:
2483+
// base/init.lua (file directly under base)
2484+
// base/commands/ (empty at scan — the #725 repro)
2485+
// base/src/main.rs (src is indexed)
2486+
// base/src/plugins/extra/ (empty chain under an indexed dir)
2487+
std::fs::create_dir_all(base.join("commands")).unwrap();
2488+
std::fs::create_dir_all(base.join("src/plugins/extra")).unwrap();
2489+
std::fs::write(base.join("init.lua"), b"x").unwrap();
2490+
std::fs::write(base.join("src/main.rs"), b"x").unwrap();
2491+
2492+
let mut picker = FilePicker::new(FilePickerOptions {
2493+
base_path: base.to_str().unwrap().into(),
2494+
watch: false,
2495+
..Default::default()
2496+
})
2497+
.unwrap();
2498+
picker.collect_files().unwrap();
2499+
2500+
let mut watch_dirs: Vec<PathBuf> = Vec::new();
2501+
picker.for_each_dir(|p| {
2502+
watch_dirs.push(p.to_path_buf());
2503+
std::ops::ControlFlow::Continue(())
2504+
});
2505+
let watch_set: std::collections::HashSet<PathBuf> = watch_dirs.iter().cloned().collect();
2506+
2507+
for rel in ["commands", "src/plugins", "src/plugins/extra", "src"] {
2508+
assert!(
2509+
watch_set.contains(&base.join(rel)),
2510+
"expected {rel} in watch dirs, got {watch_set:?}",
2511+
);
2512+
}
2513+
2514+
// Dirs covered by indexed files must not be duplicated.
2515+
assert_eq!(
2516+
watch_dirs.len(),
2517+
watch_set.len(),
2518+
"duplicate watch dir emitted: {watch_dirs:?}",
2519+
);
2520+
}
2521+
2522+
#[test]
2523+
fn dir_table_merges_walked_dirs_with_file_parents() {
2524+
let mut pairs: Vec<(FileItem, String)> = ["src/main.rs", "src/deep/lib.rs", "root.txt"]
2525+
.iter()
2526+
.map(|p| {
2527+
let (item, rel) = FileItem::new(PathBuf::from(p), Path::new(""), None);
2528+
(item, rel)
2529+
})
2530+
.collect();
2531+
pairs.sort_by(|(a, pa), (b, pb)| {
2532+
pa[..a.path.filename_offset as usize]
2533+
.cmp(&pb[..b.path.filename_offset as usize])
2534+
.then_with(|| pa.cmp(pb))
2535+
});
2536+
2537+
// Sorted '/'-terminated walker output: file parents + an empty dir +
2538+
// a sibling sharing a prefix with a file parent.
2539+
let walked: Vec<String> = ["empty/", "src/", "src/deep/", "src/deeper/"]
2540+
.iter()
2541+
.map(|s| s.to_string())
2542+
.collect();
2543+
2544+
let mut builder = crate::simd_path::ChunkedPathStoreBuilder::new(pairs.len());
2545+
let dirs = populates_dirs_files_chunked_storage(&mut pairs, &walked, &mut builder);
2546+
let store = builder.finish();
2547+
let arena = store.as_arena_ptr();
2548+
2549+
let table: Vec<String> = dirs.iter().map(|d| d.relative_path(arena)).collect();
2550+
// Sorted: "" (root files) first, all walked dirs present exactly once.
2551+
assert_eq!(table, ["", "empty/", "src/", "src/deep/", "src/deeper/"]);
2552+
2553+
// Every file's parent_dir_index points at its own dir entry.
2554+
for (file, _) in &pairs {
2555+
let dir = &dirs[file.parent_dir_index as usize];
2556+
let rel = file.relative_path(arena);
2557+
assert!(
2558+
rel.starts_with(&dir.relative_path(arena)),
2559+
"file {rel} must live under its parent dir",
2560+
);
2561+
}
2562+
}
2563+
24322564
#[test]
24332565
fn common_dir_prefix_len_cases() {
24342566
assert_eq!(common_dir_prefix_len("", ""), 0);

crates/fff-core/src/walk/mod.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ pub(crate) use ripgrep::walk_collect_files;
2020

2121
pub(crate) struct WalkOutput {
2222
pub(crate) pairs: Vec<(FileItem, String)>,
23+
/// Every non-ignored directory the walk visited, as '/'-canonical
24+
/// '/'-terminated paths relative to the base (base itself excluded).
25+
/// This is the source of the picker's dir table; unsorted (parallel walk).
26+
pub(crate) dirs: Vec<String>,
2327
pub(crate) ignore_rules: Option<WalkIgnoreRules>,
2428
}
2529

crates/fff-core/src/walk/ripgrep.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,10 @@ pub(crate) fn walk_collect_files(
3535
let walker = walk_builder.build_parallel();
3636

3737
let pairs = parking_lot::Mutex::new(Vec::<(FileItem, String)>::new());
38+
let dirs = parking_lot::Mutex::new(Vec::<String>::new());
3839
walker.run(|| {
3940
let pairs = &pairs;
41+
let dirs = &dirs;
4042
let counter = Arc::clone(synced_files_count);
4143
let base_path = base_path.to_path_buf();
4244

@@ -60,13 +62,24 @@ pub(crate) fn walk_collect_files(
6062

6163
pairs.lock().push((file_item, rel_path));
6264
counter.fetch_add(1, Ordering::Relaxed);
65+
} else if entry.depth() > 0 && entry.file_type().is_some_and(|ft| ft.is_dir()) {
66+
let path = entry.path();
67+
if !is_git_file(path)
68+
&& let Ok(rel) = path.strip_prefix(&base_path)
69+
{
70+
let mut rel = crate::path_utils::to_canonical_slashes(&rel.to_string_lossy())
71+
.into_owned();
72+
rel.push('/');
73+
dirs.lock().push(rel);
74+
}
6375
}
6476
ignore::WalkState::Continue
6577
})
6678
});
6779

6880
Ok(WalkOutput {
6981
pairs: pairs.into_inner(),
82+
dirs: dirs.into_inner(),
7083
ignore_rules: None,
7184
})
7285
}

crates/fff-core/src/walk/zlob.rs

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
1-
//! Filesystem traversal backed by zlob's native parallel walker.
2-
//! Active when the `zlob` feature is enabled (requires the Zig toolchain).
3-
41
use crate::file_picker::is_known_binary_extension_basename;
52
use crate::ignore::IGNORED_DIRS;
63
use crate::types::FileItem;
74
use crate::walk::{WalkIgnoreRules, WalkOutput};
5+
use parking_lot::Mutex;
86
use std::path::Path;
97
use std::sync::Arc;
108
use std::sync::atomic::{AtomicUsize, Ordering};
@@ -48,12 +46,25 @@ pub(crate) fn walk_collect_files(
4846
tracing::warn!(?e, "zlob extra_ignore rejected; walking without it");
4947
}
5048

51-
let pairs = parking_lot::Mutex::new(Vec::<(FileItem, String)>::new());
49+
let pairs = Mutex::new(Vec::new());
50+
let dirs = Mutex::new(Vec::new());
5251

5352
let outcome = match builder.run(|entry| {
5453
if !entry.is_file() {
54+
// `.git` and ignored dirs are pruned by zlob itself (GITIGNORE
55+
// flag); empty rel path is the walk root, covered separately.
56+
if entry.is_dir() {
57+
let rel_bytes = entry.relative_path_bytes();
58+
if !rel_bytes.is_empty() {
59+
let mut rel = String::from_utf8_lossy(rel_bytes).into_owned();
60+
rel.push('/');
61+
dirs.lock().push(rel);
62+
}
63+
}
64+
5565
return WalkState::Continue;
5666
}
67+
5768
let rel_bytes = entry.relative_path_bytes();
5869

5970
// `basename()` returns `&str` for files only.
@@ -106,6 +117,7 @@ pub(crate) fn walk_collect_files(
106117

107118
Ok(WalkOutput {
108119
pairs,
120+
dirs: dirs.into_inner(),
109121
ignore_rules,
110122
})
111123
}

0 commit comments

Comments
 (0)