Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,12 @@ jobs:
# fff-python requires full python o3 machinery which is very slow
run: cargo test --no-default-features --features zlob --workspace --exclude fff-nvim --exclude fff-python

- name: Test hidden paths with the ripgrep watcher
if: runner.os == 'Linux'
env:
RUSTC_WRAPPER: sccache
run: cargo test -p fff-search --test new_directory_watcher_test -- non_git_watcher_skips_hidden_paths --exact

stress-test:
name: Fuzz Tests
runs-on: ${{ matrix.os }}
Expand Down
8 changes: 4 additions & 4 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ git2 = { version = "0.21.0", default-features = false, features = [
glidesort = "0.1"
globset = "0.4"
heed = "0.22.0"
ignore = "0.4.22"
ignore = "0.4.33"
memmap2 = "0.9"
mimalloc = "0.1.47"
signal-hook-registry = "1.4"
Expand Down
3 changes: 1 addition & 2 deletions crates/fff-core/src/file_picker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,7 @@ pub(crate) struct FileSync {
/// Chunk-level deduped path store. Arc so post-scan snapshots can hold
/// the arena alive while iterating file paths.
chunked_paths: Option<Arc<crate::simd_path::ChunkedPathStore>>,
/// Ignore rules the walker assembled (zlob backend only). Shared with the
/// background watcher so filesystem events can be filtered without libgit2.
// The watcher reuses these rules to filter filesystem events.
pub(crate) ignore_rules: Option<Arc<crate::walk::WalkIgnoreRules>>,
}

Expand Down
58 changes: 28 additions & 30 deletions crates/fff-core/src/walk/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,21 @@ pub(crate) struct WalkOutput {
pub(crate) ignore_rules: Option<WalkIgnoreRules>,
}

#[cfg_attr(not(feature = "zlob"), derive(Clone))]
pub(crate) struct WalkIgnoreRules {
#[cfg(feature = "zlob")]
inner: ::zlob::walk::WalkerOutcomeRules,
#[cfg(feature = "zlob")]
skip_hidden: bool,
#[cfg(not(feature = "zlob"))]
_never: std::convert::Infallible,
inner: ::ignore::IncrementalIgnore,
}

// SAFETY: the underlying storage is immutable, heap-owned, and thread-safe to
// read from concurrently (mirrors zlob's `IgnoreRules: Send + Sync`).
#[cfg(feature = "zlob")]
unsafe impl Send for WalkIgnoreRules {}
#[cfg(feature = "zlob")]
unsafe impl Sync for WalkIgnoreRules {}

impl std::fmt::Debug for WalkIgnoreRules {
Expand All @@ -43,30 +48,25 @@ impl std::fmt::Debug for WalkIgnoreRules {
}
}

// In ripgrep builds `WalkIgnoreRules` is never constructed (the `_never`
// field is uninhabited), so its methods are legitimately dead there.
#[cfg_attr(not(feature = "zlob"), allow(dead_code))]
impl WalkIgnoreRules {
/// Returns `true` if the provided path is ignored by the collected rule set
///
/// `relative_path` has to be relative to the walker's provided base path
pub(crate) fn is_ignored(&self, relative_path: &Path) -> bool {
#[cfg(feature = "zlob")]
{
self.inner
.rules()
.is_some_and(|rules| rules.is_ignored(relative_path))
}
#[cfg(not(feature = "zlob"))]
#[cfg(feature = "zlob")]
pub(crate) fn is_ignored(&self, relative_path: &Path, _is_dir: bool) -> bool {
if self.skip_hidden
&& relative_path
.iter()
.any(|name| name.as_encoded_bytes().starts_with(b"."))
{
let _ = relative_path;
match self._never {}
return true;
}
self.inner
.rules()
.is_some_and(|rules| rules.is_ignored(relative_path))
}

// The old `is_ignored_untrusted` variant was folded away when zlob's
// ignore matcher moved to full ancestor enumeration — trailing-slash
// sniffing on the input is now sufficient for external queries.
#[cfg(not(feature = "zlob"))]
pub(crate) fn is_ignored(&mut self, relative_path: &Path, is_dir: bool) -> bool {
self.inner.matched(relative_path, is_dir).is_ignore()
}
}

#[cfg(test)]
Expand Down Expand Up @@ -225,8 +225,6 @@ mod tests {
assert_eq!(names, ["projects.toml"]);
}

// Only the zlob backend surfaces reusable ignore rules; they must match
// the same tree the walk respected.
#[cfg(feature = "zlob")]
#[test]
fn surfaces_reusable_ignore_rules() {
Expand All @@ -243,9 +241,9 @@ mod tests {
walk_collect_files(root, true, false, 1, &counter, &Default::default(), None).unwrap();

let rules = out.ignore_rules.expect("zlob surfaces ignore rules");
assert!(rules.is_ignored(Path::new("target/")));
assert!(rules.is_ignored(Path::new("debug.log")));
assert!(!rules.is_ignored(Path::new("Cargo.toml")));
assert!(rules.is_ignored(Path::new("target/"), true));
assert!(rules.is_ignored(Path::new("debug.log"), false));
assert!(!rules.is_ignored(Path::new("Cargo.toml"), false));
}

// Both backends must honor `info/exclude`, which linked worktrees read from
Expand Down Expand Up @@ -304,9 +302,9 @@ mod tests {
#[cfg(feature = "zlob")]
{
let rules = out.ignore_rules.expect("zlob surfaces ignore rules");
assert!(rules.is_ignored(Path::new("excluded/later/new.txt")));
assert!(rules.is_ignored(Path::new("debug.log")));
assert!(!rules.is_ignored(Path::new("src/a.txt")));
assert!(rules.is_ignored(Path::new("excluded/later/new.txt"), false));
assert!(rules.is_ignored(Path::new("debug.log"), false));
assert!(!rules.is_ignored(Path::new("src/a.txt"), false));
}
}
}
Expand Down Expand Up @@ -353,8 +351,8 @@ mod tests {

// The watcher filters incremental updates with these rules.
let rules = out.ignore_rules.expect("zlob surfaces ignore rules");
assert!(rules.is_ignored(std::path::Path::new(".worktrees/wt1/new.ts")));
assert!(!rules.is_ignored(std::path::Path::new("src/hit.ts")));
assert!(rules.is_ignored(std::path::Path::new(".worktrees/wt1/new.ts"), false));
assert!(!rules.is_ignored(std::path::Path::new("src/hit.ts"), false));
}

// disable_git must also turn off the git exclude files, not only .gitignore.
Expand Down
7 changes: 5 additions & 2 deletions crates/fff-core/src/walk/ripgrep.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::file_picker::FilePickerIgnoreOptions;
use crate::ignore::non_git_repo_overrides;
use crate::types::FileItem;
use crate::walk::WalkOutput;
use crate::walk::{WalkIgnoreRules, WalkOutput};
use crate::watch::is_git_file;
use ignore::WalkBuilder;
use std::path::Path;
Expand Down Expand Up @@ -85,6 +85,9 @@ pub(crate) fn walk_collect_files(
Ok(WalkOutput {
pairs,
dirs,
ignore_rules: None,
// Ripgrep lets .ignore re-include hidden entries in a non-Git root.
ignore_rules: (!is_git_repo).then(|| WalkIgnoreRules {
inner: walk_builder.build_matchers().pop().expect("one walk root"),
}),
})
}
11 changes: 5 additions & 6 deletions crates/fff-core/src/walk/zlob.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,12 +191,11 @@ pub(crate) fn walk_collect_files(
// Always report the exact final total regardless of the last step.
synced_files_count.store(pairs.len(), Ordering::Relaxed);

// Retain the ignore rules only when the walk actually gathered some
// (git roots with .gitignore/.ignore). Otherwise callers fall back.
let ignore_rules = outcome
.rules()
.is_some()
.then(|| WalkIgnoreRules { inner: outcome });
let ignore_rules =
(use_non_git_defaults || outcome.rules().is_some()).then_some(WalkIgnoreRules {
inner: outcome,
skip_hidden: use_non_git_defaults,
});

Ok(WalkOutput {
pairs,
Expand Down
34 changes: 17 additions & 17 deletions crates/fff-core/src/watcher/background_watcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ pub(crate) fn handle_debounced_events(
.read()
.ok()
.and_then(|g| g.as_ref().and_then(|p| p.ignore_rules()));
let filter = IgnoreFilter::new(
let mut filter = IgnoreFilter::new(
base_path,
walker_rules,
repo.as_ref(),
Expand Down Expand Up @@ -442,7 +442,7 @@ pub(crate) fn handle_debounced_events(
.paths
.iter()
// but we are smart enough and not falling into the paths
.all(|p| !p.is_dir() && !filter.is_ignored(p));
.all(|p| !p.is_dir() && !filter.is_ignored(p, false));

if !small_and_known && try_trigger_full_rescan(RescanReason::KernelEventLoss) {
return Vec::new();
Expand Down Expand Up @@ -471,7 +471,7 @@ pub(crate) fn handle_debounced_events(
if ignore_file_changed {
if path
.parent()
.is_some_and(|parent| filter.is_ignored(parent))
.is_some_and(|parent| filter.is_ignored(parent, true))
{
continue;
}
Expand Down Expand Up @@ -529,7 +529,8 @@ pub(crate) fn handle_debounced_events(
let (is_dir, is_ignored) = if is_removed {
(false, true)
} else {
(path.is_dir(), filter.is_ignored(path))
let is_dir = path.is_dir();
(is_dir, filter.is_ignored(path, is_dir))
};

if is_folder_removal {
Expand Down Expand Up @@ -889,27 +890,27 @@ fn index_new_directory(
};

// Subtree-rooted walks still need to check the original root's ignore rules.
let filter = IgnoreFilter::new(
let mut filter = IgnoreFilter::new(
base_path,
walker_rules,
repo.as_ref(),
ignore_options.disable_git,
);
let join_unless_ignored = |relative_path: &str| -> Option<PathBuf> {
let mut join_unless_ignored = |relative_path: &str, is_dir: bool| -> Option<PathBuf> {
let path = walk_base.join(relative_path);
(scope.is_some() || !filter.is_ignored(&path)).then_some(path)
(scope.is_some() || !filter.is_ignored(&path, is_dir)).then_some(path)
};

let files_to_add: Vec<PathBuf> = walk
.pairs
.iter()
.filter_map(|(_, path)| join_unless_ignored(path))
.filter_map(|(_, path)| join_unless_ignored(path, false))
.collect();

let subdirs: Vec<PathBuf> = walk
.dirs
.iter()
.filter_map(|path| join_unless_ignored(path.trim_end_matches('/')))
.filter_map(|path| join_unless_ignored(path.trim_end_matches('/'), true))
.collect();

if files_to_add.is_empty() {
Expand Down Expand Up @@ -1007,7 +1008,7 @@ fn watch_dirs_nonrecursive<'a>(

struct IgnoreFilter<'a> {
base_path: &'a Path,
/// Reusable ignore rules from the last walk (zlob backend only).
// Ignore rules from the last walk.
rules: Option<Arc<crate::walk::WalkIgnoreRules>>,
/// libgit2 repo, consulted only when `rules` is `None`. Borrowed from the
/// caller's repo (also used for git-status queries) to avoid re-opening.
Expand All @@ -1030,16 +1031,15 @@ impl<'a> IgnoreFilter<'a> {
}
}

/// Whether `path` (absolute) is ignored.
fn is_ignored(&self, path: &Path) -> bool {
if let Some(rules) = self.rules.as_ref() {
fn is_ignored(&mut self, path: &Path, is_dir: bool) -> bool {
if let Some(rules) = self.rules.as_mut() {
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(relative);
// Ripgrep caches directory rules. Start each batch with the unloaded matcher.
#[cfg(not(feature = "zlob"))]
let rules = Arc::make_mut(rules);
return rules.is_ignored(relative, is_dir);
}
match self.repo {
Some(repo) => repo.is_path_ignored(path) == Ok(true),
Expand Down
37 changes: 37 additions & 0 deletions crates/fff-core/tests/new_directory_watcher_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -694,3 +694,40 @@ fn gitignored_new_directory_excluded() {
assert_eq!(grep_count, 0, "Gitignored content should NOT be grepable");
}
}

#[test]
fn non_git_watcher_skips_hidden_paths() {
let tmp = TempDir::new().unwrap();
let base = tmp.path().canonicalize().unwrap();
fs::write(base.join("seed.txt"), "seed").unwrap();
let (shared_picker, _frecency) = make_watched_picker(&base);
wait_ready(&shared_picker);

fs::create_dir(base.join(".hidden")).unwrap();
let hidden = base.join(".hidden/output.txt");
fs::write(&hidden, "build output").unwrap();
let dotfile = base.join(".marker");
fs::write(&dotfile, "hidden").unwrap();

let visible = base.join("visible");
fs::create_dir(&visible).unwrap();
// The second write reaches the watcher after it has processed the directory creation.
for name in ["first.txt", "second.txt"] {
let path = visible.join(name);
fs::write(&path, "visible").unwrap();
poll_until(&shared_picker, WATCHER_TIMEOUT, name, |picker| {
picker.get_file_by_path(&path).is_some()
});
}

let guard = shared_picker.read().unwrap();
let picker = guard.as_ref().unwrap();
assert!(
picker.get_file_by_path(&hidden).is_none(),
"indexed {hidden:?}"
);
assert!(
picker.get_file_by_path(&dotfile).is_none(),
"indexed {dotfile:?}"
);
}
Loading