diff --git a/Cargo.lock b/Cargo.lock index 575f327..e6ef78a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1046,7 +1046,7 @@ checksum = "8f50febec83f5ee1df3015341d8bd429f2d1cc62bcba7ea2076759d315084683" [[package]] name = "tgrep-cli" -version = "1.0.2" +version = "1.0.3" dependencies = [ "anyhow", "assert_cmd", @@ -1072,7 +1072,7 @@ dependencies = [ [[package]] name = "tgrep-core" -version = "1.0.2" +version = "1.0.3" dependencies = [ "anyhow", "criterion", diff --git a/Cargo.toml b/Cargo.toml index 8e27680..426b619 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,7 +3,7 @@ members = ["tgrep-core", "tgrep-cli"] resolver = "2" [workspace.package] -version = "1.0.2" +version = "1.0.3" edition = "2024" license = "MIT" repository = "https://github.com/microsoft/tgrep" diff --git a/tgrep-cli/src/serve.rs b/tgrep-cli/src/serve.rs index 9ab5549..d201982 100644 --- a/tgrep-cli/src/serve.rs +++ b/tgrep-cli/src/serve.rs @@ -296,6 +296,12 @@ struct ServerState { index_total: std::sync::atomic::AtomicU64, /// True when file watching is enabled for this server. watch_enabled: bool, + /// The live watcher and the directories it is subscribed to. + /// + /// Held here rather than by `run` because the subscription set is not + /// fixed: publishing a new ignore matcher renarrows it, and a directory + /// created after startup has to be subscribed as it appears. + watch_registry: Mutex>, /// Directories to exclude from indexing. exclude_dirs: Vec, /// Disable all source-control ignore files for every server discovery path. @@ -570,6 +576,7 @@ pub fn run(root: &Path, index_path: Option<&Path>, options: ServeOptions<'_>) -> index_progress: std::sync::atomic::AtomicU64::new(0), index_total: std::sync::atomic::AtomicU64::new(0), watch_enabled: !no_watch, + watch_registry: Mutex::new(None), exclude_dirs: exclude_dirs.to_vec(), no_ignore, no_require_git, @@ -656,14 +663,13 @@ pub fn run(root: &Path, index_path: Option<&Path>, options: ServeOptions<'_>) -> } // Start file watcher (unless --no-watch) - let _watcher = if no_watch { + if no_watch { eprintln!("[trace] file watcher disabled (--no-watch)"); - None } else { let watcher_state = Arc::clone(&state); let watcher_root = root.clone(); - start_file_watcher(watcher_state, &watcher_root, watcher_queue_cap) - }; + start_file_watcher(watcher_state, &watcher_root, watcher_queue_cap); + } // Set up graceful shutdown let shutdown_index_dir = index_dir.clone(); @@ -744,14 +750,30 @@ fn build_stale_matcher( matcher } -/// Commit matcher and index semantics together while the stale refresh holds -/// `snapshot_gate`. `None` is a legitimate matcher when no rules exist. -fn commit_stale_matcher( +/// Publish a new ignore matcher and bring everything that depends on it up to +/// date. `None` is a legitimate matcher when no rules exist. +/// +/// Callers on the stale path hold `snapshot_gate` for write, which is what +/// makes the matcher swap and the index decisions around it atomic from the +/// watcher's point of view. +/// +/// Returns the directories that were newly subscribed to as a result. Those +/// were unwatched while the caller's walk ran, so anything written to them in +/// that window produced no event and appears in no walk result. Callers pass +/// the list to [`reindex_files_in`] once `state.file_stamps` describes the +/// index they just published. +#[must_use = "newly watched directories need a recovery scan or writes race the subscription"] +fn publish_ignore_matcher( state: &ServerState, + root: &Path, matcher: Option, -) { +) -> Vec { *state.gitignore.write().unwrap() = matcher; state.gitignore_pending.store(false, Ordering::SeqCst); + // New rules mean a different set of directories worth hearing about: + // a tightened rule releases the subscriptions under it, and a relaxed + // one takes subscriptions for the tree it used to hide. + sync_watch_registrations(state, root) } fn handle_connection(stream: TcpStream, state: &ServerState) -> Result<()> { @@ -1482,11 +1504,7 @@ fn handle_reload(id: Option, state: &ServerState) -> String { } } -fn start_file_watcher( - state: Arc, - root: &Path, - queue_cap: usize, -) -> Option { +fn start_file_watcher(state: Arc, root: &Path, queue_cap: usize) -> bool { use std::sync::mpsc::{RecvTimeoutError, TrySendError}; let root_path = root.to_path_buf(); @@ -1525,19 +1543,52 @@ fn start_file_watcher( Ok(w) => w, Err(e) => { eprintln!("[trace] warning: failed to start file watcher: {e}"); - return None; + return false; } }; - if let Err(e) = watcher.watch(root, RecursiveMode::Recursive) { + // The root is always subscribed. On a whole-subtree backend that single + // recursive subscription is the entire watch set; on a per-directory + // backend it is the anchor, and `sync_watch_registrations` below adds the + // descendants the ignore rules allow. + let root_mode = if PER_DIRECTORY_WATCHES { + RecursiveMode::NonRecursive + } else { + RecursiveMode::Recursive + }; + if let Err(e) = watcher.watch(root, root_mode) { eprintln!("[trace] warning: failed to watch directory: {e}"); - return None; + return false; + } + + *state.watch_registry.lock().unwrap() = Some(WatchRegistry { + watcher, + watched: std::iter::once(root.to_path_buf()).collect(), + }); + + // Subscribing to the descendants needs the ignore matcher, and on a warm + // start it is still being built on another thread. Skipping the sync here + // costs nothing: events are dropped while `gitignore_pending` is set, and + // the publish that clears it runs this same sync. Subscribing first and + // narrowing afterwards would mean briefly holding exactly the watches this + // is meant to avoid — on a repo big enough to exhaust the inotify budget, + // long enough to fail. + if state.gitignore_pending.load(Ordering::SeqCst) { + eprintln!("[trace] watcher subscriptions deferred until the ignore matcher is ready"); + } else { + // The newly watched directories are deliberately not rechecked here. + // This is every directory in the repository, the index was built or + // opened moments ago, and the stale check that follows startup + // reconciles the same drift while holding `snapshot_gate` — which this + // path does not hold and must not take, since the watcher has to be + // receiving events before that check runs. + let _ = sync_watch_registrations(&state, root); } let worker_state = Arc::clone(&state); let worker_root = root_path; let worker_index_dir = state.index_dir.clone(); - std::thread::Builder::new() + if std::thread::Builder::new() .name("tgrep-watcher".into()) .spawn(move || { loop { @@ -1579,14 +1630,19 @@ fn start_file_watcher( } } }) - .ok()?; + .is_err() + { + eprintln!("[trace] warning: failed to start the watcher worker thread"); + *state.watch_registry.lock().unwrap() = None; + return false; + } state .watcher_active .store(true, std::sync::atomic::Ordering::Relaxed); eprintln!("[trace] file watcher started"); - Some(watcher) + true } /// Decide whether the file watcher should skip a path entirely. @@ -1617,6 +1673,30 @@ fn should_skip_watcher_path( rel_path: &str, exclude_dirs: &[String], gitignore: Option<&tgrep_core::gitignore::IgnoreMatcher>, +) -> bool { + should_skip_watcher_entry(rel_path, exclude_dirs, gitignore, false) +} + +/// [`should_skip_watcher_path`] for a path known to be a directory. +/// +/// Two rules read differently for a directory. `--exclude` names apply to the +/// final segment as well, because the walker drops the whole subtree when the +/// entry it is looking at *is* the excluded directory. And the gitignore +/// matcher is told it is matching a directory, so a directory-only rule like +/// `build/` matches — as a file path, `build` does not. +fn should_skip_watcher_dir( + rel_path: &str, + exclude_dirs: &[String], + gitignore: Option<&tgrep_core::gitignore::IgnoreMatcher>, +) -> bool { + should_skip_watcher_entry(rel_path, exclude_dirs, gitignore, true) +} + +fn should_skip_watcher_entry( + rel_path: &str, + exclude_dirs: &[String], + gitignore: Option<&tgrep_core::gitignore::IgnoreMatcher>, + is_dir: bool, ) -> bool { // Single streaming pass over path components — no Vec allocation // on the hot watcher path. The hidden-component check applies to @@ -1632,23 +1712,21 @@ fn should_skip_watcher_path( if seg.starts_with('.') { return true; } - // Ancestor (i.e. not the last segment) — apply exclude_dirs. - if segments.peek().is_some() - && !exclude_dirs.is_empty() - && exclude_dirs.iter().any(|d| d == seg) - { + // An ancestor is always a directory; the final segment is one only + // when the caller says so. + let segment_is_dir = segments.peek().is_some() || is_dir; + if segment_is_dir && !exclude_dirs.is_empty() && exclude_dirs.iter().any(|d| d == seg) { return true; } } // Gitignore check (if a matcher is available). if let Some(gi) = gitignore { - // We don't know whether the path is a dir or a file here — for - // the watcher's purposes we treat all events as "file" matches. - // Notify usually fires per-file events anyway, and gitignore - // rules that target dirs would have already skipped the dir's - // contents via `matched_path_or_any_parents`. - if gi.is_ignored(Path::new(rel_path), false) { + // For a file event we don't know whether the path is a dir, so we + // treat it as a file. Notify usually fires per-file events anyway, + // and gitignore rules that target dirs would have already skipped + // the dir's contents via `matched_path_or_any_parents`. + if gi.is_ignored(Path::new(rel_path), is_dir) { return true; } } @@ -1656,9 +1734,453 @@ fn should_skip_watcher_path( false } +/// Whether a changed path is an ignore-rules source, i.e. one whose contents +/// feed the matcher published in `ServerState::gitignore`. +/// +/// `.gitignore` and `.ignore` are matched by file name at any depth, because +/// [`tgrep_core::gitignore::matcher_from_ignore_paths`] anchors nested files of +/// both kinds. `.ignore` must be included even though it is git-agnostic — +/// leaving it out meant a `.ignore` written while the server was live never +/// refreshed the matcher, so the watcher kept indexing files the rule excluded. +/// +/// `p4ignore.ini` stays root-scoped, mirroring the walker, which only reads the +/// root-level file. fn is_ignore_rules_file(root: &Path, path: &Path) -> bool { - path.file_name().and_then(|name| name.to_str()) == Some(".gitignore") - || path == root.join(tgrep_core::gitignore::P4IGNORE_FILENAME) + let name = path.file_name().and_then(|name| name.to_str()); + matches!( + name, + Some(tgrep_core::gitignore::GITIGNORE_FILENAME) + | Some(tgrep_core::gitignore::DOT_IGNORE_FILENAME) + ) || path == root.join(tgrep_core::gitignore::P4IGNORE_FILENAME) +} + +/// Whether this platform's `notify` backend takes one OS subscription per +/// directory rather than a single recursive one for the whole tree. +/// +/// inotify has no recursive mode. `RecursiveMode::Recursive` makes notify walk +/// the tree itself and spend one watch descriptor per directory, so every +/// ignored directory costs a descriptor from the per-user +/// `fs.inotify.max_user_watches` budget purely to deliver events we then throw +/// away. Worse, notify's registration loop propagates the first failure, so a +/// repo whose ignored build output exhausts the budget makes `watch()` return +/// an error and the server loses its watcher entirely. +/// +/// ReadDirectoryChangesW (Windows) and FSEvents (macOS) subscribe once for the +/// whole subtree, so there is no per-directory registration to withhold. On +/// those platforms filtering on delivery is the only lever available, and +/// [`should_skip_watcher_path`] remains it. +/// +/// Deliberately limited to the backends we can exercise in CI. kqueue and +/// `PollWatcher` are per-path too, but nothing here builds or tests them. +const PER_DIRECTORY_WATCHES: bool = cfg!(any(target_os = "linux", target_os = "android")); + +/// Whether `path` is a directory in its own right rather than a symlink to one. +/// +/// [`Path::is_dir`] follows links, so it answers "does this lead to a +/// directory", which is the wrong question here: the walker does not follow +/// symlinks, so a symlinked directory is not part of the indexed tree. Treating +/// one as a directory would subscribe to and index its target — possibly a tree +/// outside `root` entirely, and possibly a cycle. +fn is_real_dir(path: &Path) -> bool { + std::fs::symlink_metadata(path).is_ok_and(|meta| meta.file_type().is_dir()) +} + +/// The watcher plus the set of directories it is currently subscribed to. +/// +/// Only meaningful when [`PER_DIRECTORY_WATCHES`] is true; elsewhere `watched` +/// holds just the root, which is subscribed recursively. +struct WatchRegistry { + watcher: RecommendedWatcher, + watched: std::collections::HashSet, +} + +impl WatchRegistry { + /// Subscribe to every directory in `desired` that is not already + /// subscribed, leaving existing subscriptions alone. + /// + /// Returns the directories that were newly subscribed to. A single + /// directory that cannot be subscribed is reported and skipped rather than + /// failing the whole call: the watcher is still useful for everything + /// else, and giving up on the entire tree is exactly the failure mode this + /// registration exists to avoid. + fn add_all<'a>(&mut self, desired: impl IntoIterator) -> Vec { + self.subscribe(desired, false) + } + + /// Subscribe to every directory in `dirs`, re-issuing the subscription even + /// for ones already recorded as watched. + /// + /// For directories that have just appeared, where membership in `watched` + /// proves nothing. The kernel drops an inotify watch by itself when its + /// directory is deleted or moved away, and nothing tells us the descriptor + /// is gone — so a path recreated at the same location would look + /// subscribed while receiving no events at all. [`Self::add_all`] would + /// skip it, and so would every later [`Self::sync`], since the path is in + /// `desired` *and* in `watched`: the entry stays poisoned until the server + /// restarts. Re-adding is cheap and idempotent (`inotify_add_watch` + /// returns the existing descriptor), so the doubt is worth paying for. + /// + /// Returns only the directories that were not previously recorded, so a + /// caller's notion of "newly watched" keeps its meaning. + fn resubscribe_all<'a>(&mut self, dirs: impl IntoIterator) -> Vec { + self.subscribe(dirs, true) + } + + fn subscribe<'a>( + &mut self, + desired: impl IntoIterator, + force: bool, + ) -> Vec { + let mut added = Vec::new(); + let mut failures = 0; + // Iterating `desired` and testing membership is deliberate: a + // `difference` would be proportional to the whole watched set, and + // this runs per newly created directory on repositories where that set + // is tens of thousands of entries. + for dir in desired { + let known = self.watched.contains(dir); + if known && !force { + continue; + } + match self.watcher.watch(dir, RecursiveMode::NonRecursive) { + Ok(()) => { + if !known { + self.watched.insert(dir.clone()); + added.push(dir.clone()); + } + } + Err(e) => { + // A forced re-add that fails means the directory is gone + // again; drop the entry so a later attempt can retry it + // rather than trusting a descriptor that does not exist. + if known { + self.watched.remove(dir); + } + // One line per call, not per directory: exhausting the + // inotify budget fails thousands of these at once. + if failures == 0 { + eprintln!( + "[trace] warning: could not watch {}: {e} \ + (continuing with the directories that succeeded)", + dir.display() + ); + } + failures += 1; + } + } + } + if failures > 1 { + eprintln!("[trace] warning: {failures} directories could not be watched"); + } + added + } + + /// Drop a path that no longer exists from the subscription set. + /// + /// The kernel has already released the descriptor if the directory was + /// deleted; the `unwatch` is for the moved-away case, where it is still + /// live and now pointing outside the tree. What matters either way is + /// clearing `watched`, so that if the path comes back, it is treated as + /// the new directory it is instead of an already-subscribed one. + /// + /// Cheap by design: one hash lookup per removal event, because deleting a + /// tree delivers one event per directory in it and anything proportional + /// to the whole watched set would turn that into quadratic work. + /// Descendants left behind by a move are pruned by the next + /// [`Self::sync`], which no longer finds them under the root. + fn forget(&mut self, path: &Path) { + let _ = path; + } + + /// Bring the subscription set in line with `desired`, subscribing to + /// directories that are newly relevant and dropping ones that are not. + /// + /// Returns `(added, removed)`. Only for a set that describes the whole + /// tree — anything absent from `desired` is unsubscribed. To subscribe to + /// a subtree without disturbing the rest, use [`Self::add_all`]. + fn sync(&mut self, desired: &std::collections::HashSet) -> (Vec, usize) { + let stale: Vec = self.watched.difference(desired).cloned().collect(); + let mut removed = 0; + for dir in stale { + // Best effort. inotify drops a descriptor by itself when the + // directory is deleted, so "not found" is an expected outcome + // here, not an error worth reporting. + let _ = self.watcher.unwatch(&dir); + self.watched.remove(&dir); + removed += 1; + } + + (self.add_all(desired), removed) + } +} + +/// Every directory at or below `start` whose contents the watcher needs to +/// hear about. +/// +/// A per-directory subscription reports the files directly inside it, so the +/// set is "`start`, plus every descendant directory the indexer would walk +/// into". Ignored directories are pruned along with their subtrees, which is +/// the whole point: the tree under `target/` or `node_modules/` is usually +/// most of the directories in a repo. +/// +/// `root` is the repository root and is only used to build the relative paths +/// the ignore rules are written against; `start` is where the walk begins. +/// They differ when a subtree that appeared at runtime is being subscribed, +/// and conflating them would match every rule at the wrong anchor. +/// +/// `start` itself is always included — callers are responsible for not asking +/// about a directory that is ignored. +/// +/// Symlinked directories are not descended into, matching the walker (the +/// `ignore` crate does not follow links by default). That also keeps a +/// symlink cycle from turning this into an infinite walk. +fn watchable_dirs( + root: &Path, + start: &Path, + exclude_dirs: &[String], + gitignore: Option<&tgrep_core::gitignore::IgnoreMatcher>, +) -> std::collections::HashSet { + let mut found = std::collections::HashSet::new(); + found.insert(start.to_path_buf()); + + let mut stack = vec![start.to_path_buf()]; + while let Some(dir) = stack.pop() { + let Ok(entries) = std::fs::read_dir(&dir) else { + // An unreadable directory is not a reason to abandon the rest of + // the tree; the periodic reconcile is what catches what we miss. + continue; + }; + for entry in entries.flatten() { + if !entry.file_type().is_ok_and(|t| t.is_dir()) { + continue; + } + let path = entry.path(); + let Ok(rel) = path.strip_prefix(root) else { + continue; + }; + let rel = rel.to_string_lossy().replace('\\', "/"); + if should_skip_watcher_dir(&rel, exclude_dirs, gitignore) { + continue; + } + stack.push(path.clone()); + found.insert(path); + } + } + found +} + +/// Recompute the watcher's subscriptions against the ignore rules in force. +/// +/// Called when the watcher starts and every time the ignore matcher is +/// published, so relaxing a rule subscribes to the tree it used to hide and +/// tightening one drops it. +/// +/// Returns the directories that were newly subscribed to. Until a directory is +/// subscribed it cannot report anything, so a file written to one of these +/// between the caller's walk and this call is in neither the walk's results nor +/// any event. The caller is expected to hand the list to +/// [`reindex_files_in`] once its own bookkeeping is settled. +fn sync_watch_registrations(state: &ServerState, root: &Path) -> Vec { + if !PER_DIRECTORY_WATCHES { + return Vec::new(); + } + let mut registry = state.watch_registry.lock().unwrap(); + let Some(registry) = registry.as_mut() else { + // The watcher has not started yet. It syncs once as it comes up, so + // there is nothing to do and nothing to remember. + return Vec::new(); + }; + + let start = Instant::now(); + let desired = { + let gitignore = state.gitignore.read().unwrap(); + watchable_dirs(root, root, &state.exclude_dirs, gitignore.as_ref()) + }; + let total = desired.len(); + let (added, removed) = registry.sync(&desired); + if !added.is_empty() || removed > 0 { + eprintln!( + "[trace] watcher subscriptions: {total} directories \ + (+{}, -{removed}) in {:.1}ms", + added.len(), + start.elapsed().as_secs_f64() * 1000.0 + ); + } + added +} + +/// Re-check the files directly inside `dirs`, indexing the ones that changed. +/// +/// Used to close the gap between a walk and the subscriptions that follow it: +/// [`reindex_file`] compares stamps first, so for a tree that did not change +/// under us this costs one `metadata` call per file and indexes nothing. +/// +/// Callers must already hold `snapshot_gate`, and `state.file_stamps` must +/// already describe the index as published — a merge that replaces the stamps +/// afterwards would both discard what this records and make every file here +/// look changed. +fn reindex_files_in(state: &ServerState, root: &Path, dirs: &[PathBuf]) { + if dirs.is_empty() { + return; + } + let start = Instant::now(); + for dir in dirs { + let Ok(entries) = std::fs::read_dir(dir) else { + continue; + }; + for entry in entries.flatten() { + if !entry.file_type().is_ok_and(|t| t.is_file()) { + continue; + } + let path = entry.path(); + let Ok(rel) = path.strip_prefix(root) else { + continue; + }; + let rel = rel.to_string_lossy().replace('\\', "/"); + let skip = { + let gitignore = state.gitignore.read().unwrap(); + should_skip_watcher_path(&rel, &state.exclude_dirs, gitignore.as_ref()) + }; + if !skip { + reindex_file(state, &path, &rel); + } + } + } + eprintln!( + "[trace] watcher: rechecked {} newly watched directories in {:.1}ms", + dirs.len(), + start.elapsed().as_secs_f64() * 1000.0 + ); +} + +/// Subscribe to a directory that has just appeared, and to anything already +/// inside it. +/// +/// Non-recursive subscriptions are not extended by notify — it only auto-adds +/// watches beneath a watch that was registered as recursive — so a new +/// directory has to be picked up here or its contents are invisible. +/// +/// Files that landed between the directory's creation and its subscription +/// would be missed by definition, so the same pass indexes what it finds. +/// The caller must already hold `snapshot_gate`. +/// +/// The descent subscribes to each level *before* reading it. Reading first +/// leaves a window in which a child created in between is in neither place: +/// not in what this pass enumerates, and not yet able to report itself. That +/// window is small but it is exactly the one a checkout or a build fills, and +/// anything lost in it stays invisible until the hourly reconcile. +fn watch_new_subtree(state: &Arc, root: &Path, dir: &Path) { + // `is_dir` follows symlinks; the walker does not. Refuse a symlinked + // directory here so we never subscribe to, or index, a tree the indexer + // would not have walked into. + if !is_real_dir(dir) { + return; + } + let Ok(rel_dir) = dir.strip_prefix(root) else { + return; + }; + let rel_dir = rel_dir.to_string_lossy().replace('\\', "/"); + // The event that brought us here was filtered with file semantics, so a + // `build/`-style rule that only ever matches directories has not been + // applied to this path yet. Re-check before subscribing to it. + if !rel_dir.is_empty() { + let gitignore = state.gitignore.read().unwrap(); + if should_skip_watcher_dir(&rel_dir, &state.exclude_dirs, gitignore.as_ref()) { + return; + } + } + + let mut level = vec![dir.to_path_buf()]; + let mut seen: std::collections::HashSet = std::collections::HashSet::new(); + let mut files: Vec<(PathBuf, String)> = Vec::new(); + let mut found_ignore_rules = false; + + while !level.is_empty() { + { + let mut registry = state.watch_registry.lock().unwrap(); + let Some(registry) = registry.as_mut() else { + return; + }; + // Additive, not a sync: this covers only the new subtree, and + // `sync` would read everything outside it as stale and unsubscribe + // from the entire rest of the repository. Staying additive also + // matters at scale — a monorepo can hold tens of thousands of + // watched directories, and materialising a union for every newly + // created directory would be quadratic over a checkout. + // + // Forced, because these directories have just appeared: a path + // recreated where a watched one used to be is still recorded as + // watched, but the kernel dropped its descriptor when the original + // went away. + registry.add_all(level.iter()); + } + + // The registry lock is released before any `read_dir`, so a large + // subtree does not hold it across the I/O for a whole level. + let mut next = Vec::new(); + for subdir in level.drain(..) { + if !seen.insert(subdir.clone()) { + continue; + } + let Ok(entries) = std::fs::read_dir(&subdir) else { + continue; + }; + for entry in entries.flatten() { + let Ok(file_type) = entry.file_type() else { + continue; + }; + let path = entry.path(); + let Ok(rel) = path.strip_prefix(root) else { + continue; + }; + let rel = rel.to_string_lossy().replace('\\', "/"); + // `DirEntry::file_type` does not follow symlinks, so a + // symlinked directory is neither descended into nor indexed. + if file_type.is_dir() { + let skip = { + let gitignore = state.gitignore.read().unwrap(); + should_skip_watcher_dir(&rel, &state.exclude_dirs, gitignore.as_ref()) + }; + if !skip { + next.push(path); + } + } else if file_type.is_file() { + // A subtree that arrives whole — a clone, a `mv`, a branch + // switch — can carry its own ignore rules. Those files are + // dot-prefixed, so the scan below would silently drop them + // and index the rest of the subtree against rules that do + // not know about them. + if !state.no_ignore && is_ignore_rules_file(root, &path) { + found_ignore_rules = true; + continue; + } + let skip = { + let gitignore = state.gitignore.read().unwrap(); + should_skip_watcher_path(&rel, &state.exclude_dirs, gitignore.as_ref()) + }; + if !skip { + files.push((path, rel)); + } + } + } + } + level = next; + } + + if found_ignore_rules { + // Indexing now would apply the wrong rules to the whole subtree, and + // anything wrongly indexed would stay until something touched it + // again. The refresh rewalks and republishes, which covers these files + // correctly; it runs on its own thread and takes `snapshot_gate` + // there, so scheduling it while we hold the gate is safe. + state.ignore_rules_dirty.store(true, Ordering::SeqCst); + schedule_ignore_rules_refresh(Arc::clone(state), root.to_path_buf()); + return; + } + + for (path, rel) in &files { + reindex_file(state, path, rel); + } } fn schedule_ignore_rules_refresh(state: Arc, root: PathBuf) { @@ -1698,8 +2220,6 @@ fn schedule_ignore_rules_refresh(state: Arc, root: PathBuf) { } fn handle_fs_event(state: &Arc, root: &Path, event: &Event) { - use tgrep_core::meta::FileStamp; - let dominated_kinds = matches!( event.kind, EventKind::Create(_) | EventKind::Modify(_) | EventKind::Remove(_) @@ -1774,6 +2294,18 @@ fn handle_fs_event(state: &Arc, root: &Path, event: &Event) { let is_remove = matches!(event.kind, EventKind::Remove(_)) || !path.exists(); if is_remove { + // A watched directory that disappears takes its descriptor with + // it, but not its entry in the registry. Clearing that entry is + // what lets the path be subscribed again if it comes back — and + // keeps `watched` from accumulating dead paths between syncs. + // Done for every removed path rather than only known directories, + // since by now there is nothing left to ask what it was; a path + // that was never watched is a single failed hash lookup. + if PER_DIRECTORY_WATCHES + && let Some(registry) = state.watch_registry.lock().unwrap().as_mut() + { + registry.forget(path); + } // notify can deliver Remove events for transient/unknown paths // (e.g. a build tool's temp file). Suppress the noisy log line // for those, but still apply the delete unconditionally — if @@ -1795,66 +2327,87 @@ fn handle_fs_event(state: &Arc, root: &Path, event: &Event) { } if !path.is_file() { + // `is_real_dir` rather than `is_dir`: the latter follows symlinks, + // and a link to a directory is not something the walker descends + // into, so subscribing to and indexing its target would pull in a + // tree the index never contained — possibly outside `root`. + if PER_DIRECTORY_WATCHES && is_real_dir(path) { + // With non-recursive subscriptions notify will not extend the + // watch set for us, so a directory that just appeared — and + // anything already inside it — has to be picked up here. + watch_new_subtree(state, root, path); + } continue; } - // Compute the file's current stamp and skip if it matches what we - // last indexed. notify on Windows in particular fires Modify events - // for atime/attribute updates, opens, etc. — re-indexing on those - // would re-read large files, churn the live overlay, and produce a - // misleading "modified" trace for files that didn't actually change. - let current = match std::fs::metadata(path) { - Ok(m) => FileStamp { - mtime: m - .modified() - .ok() - .and_then(|t| t.duration_since(std::time::SystemTime::UNIX_EPOCH).ok()) - .map(|d| d.as_secs()) - .unwrap_or(0), - size: m.len(), - }, - Err(_) => continue, - }; - if state.file_stamps.read().unwrap().get(&rel_path) == Some(¤t) { - continue; - } + reindex_file(state, path, &rel_path); + } +} - // Read contents and extract trigrams OUTSIDE the index write lock - // so a concurrent search (which needs a read lock) is not blocked - // on our file I/O and trigram parsing. Windows' SRWLock is - // writer-preferring: a single waiting writer here would otherwise - // stall every subsequent search request. - let data = match std::fs::read(path) { - Ok(d) => d, - Err(_) => continue, - }; - let text = tgrep_core::encoding::decode_for_index(&data); - let is_binary = tgrep_core::trigram::is_binary(&text); - let per_tri = if is_binary { - None - } else { - Some(tgrep_core::live::LiveIndex::compute_trigram_masks(&text)) - }; +/// Read a file and merge it into the live index, unless its stamp says the +/// content we already indexed is current. +/// +/// The caller must hold `snapshot_gate`: the read, the commit, and the stamp +/// update have to be atomic with respect to a flush or auto-save. +fn reindex_file(state: &ServerState, path: &Path, rel_path: &str) { + use tgrep_core::meta::FileStamp; - eprintln!("[trace] reindex: modified {rel_path}"); - // gate acquired at the function level — the commit + stamp - // update is processed atomically with respect to flush/auto-save. - { - let mut index = state.index.write().unwrap(); - match per_tri { - Some(per_tri) => index.live.commit_upsert(&rel_path, per_tri), - None => index.live.delete_file(&rel_path), - } - } - state - .file_stamps - .write() - .unwrap() - .insert(rel_path.clone(), current); - if let Ok(mut cache) = state.cache.write() { - cache.pop(&rel_path); + // Compute the file's current stamp and skip if it matches what we + // last indexed. notify on Windows in particular fires Modify events + // for atime/attribute updates, opens, etc. — re-indexing on those + // would re-read large files, churn the live overlay, and produce a + // misleading "modified" trace for files that didn't actually change. + let current = match std::fs::metadata(path) { + Ok(m) => FileStamp { + mtime: m + .modified() + .ok() + .and_then(|t| t.duration_since(std::time::SystemTime::UNIX_EPOCH).ok()) + .map(|d| d.as_secs()) + .unwrap_or(0), + size: m.len(), + }, + Err(_) => return, + }; + if state.file_stamps.read().unwrap().get(rel_path) == Some(¤t) { + return; + } + + // Read contents and extract trigrams OUTSIDE the index write lock + // so a concurrent search (which needs a read lock) is not blocked + // on our file I/O and trigram parsing. Windows' SRWLock is + // writer-preferring: a single waiting writer here would otherwise + // stall every subsequent search request. + let data = match std::fs::read(path) { + Ok(d) => d, + Err(_) => return, + }; + let text = tgrep_core::encoding::decode_for_index(&data); + let is_binary = tgrep_core::trigram::is_binary(&text); + let per_tri = if is_binary { + None + } else { + Some(tgrep_core::live::LiveIndex::compute_trigram_masks(&text)) + }; + + eprintln!("[trace] reindex: modified {rel_path}"); + // Gate held by the caller — the commit + stamp update is processed + // atomically with respect to flush/auto-save. + { + let mut index = state.index.write().unwrap(); + match per_tri { + Some(per_tri) => index.live.commit_upsert(rel_path, per_tri), + None => index.live.delete_file(rel_path), } } + state + .file_stamps + .write() + .unwrap() + .insert(rel_path.to_string(), current); + if let Ok(mut cache) = state.cache.write() { + cache.pop(rel_path); + } } /// Whether a scheduled reconcile should run now. @@ -2311,19 +2864,57 @@ fn stamps_for_indexed( .collect() } +/// Rebuild the ignore matcher and reconcile the index against the filesystem. +/// +/// Returns whether the index can be trusted afterwards. A `false` means the +/// walk or the merge failed and the stamps are not describing the index. fn background_refresh_stale( state: &Arc, root: &Path, index_dir: &Path, compare_index_membership: bool, ) -> bool { - use tgrep_core::meta; - use tgrep_core::walker; - let _refresh = state.stale_refresh_lock.lock().unwrap(); // Keep watcher/auto-save mutations out for the complete walk → matcher → - // merge cycle. Search queries do not take this gate and remain available. + // merge → recovery cycle. Search queries do not take this gate and remain + // available. Held here rather than inside so the recovery scan below is + // still covered by it. let _gate = state.snapshot_gate.write().unwrap(); + + let mut newly_watched = Vec::new(); + let ok = refresh_stale_locked( + state, + root, + index_dir, + compare_index_membership, + &mut newly_watched, + ); + + // Directories that were not subscribed while the walk ran could not report + // a write, and the walk may have passed them before it happened, so a file + // created in that window is in neither place. Recheck them now that the + // subscriptions exist. + // + // Only on success, and only here at the end: a failed walk or merge leaves + // `file_stamps` describing something other than the published index, and + // `stream_merge_stale_changes` replaces the stamps wholesale, so scanning + // any earlier would both be discarded and re-read every changed file. + if ok { + reindex_files_in(state, root, &newly_watched); + } + ok +} + +fn refresh_stale_locked( + state: &Arc, + root: &Path, + index_dir: &Path, + compare_index_membership: bool, + newly_watched: &mut Vec, +) -> bool { + use tgrep_core::meta; + use tgrep_core::walker; + let start = Instant::now(); eprintln!("[trace] stale check: comparing index against filesystem..."); @@ -2351,10 +2942,10 @@ fn background_refresh_stale( // with a delete, would be enough. // // Committing here rather than at each exit is invisible to the watcher: - // this function holds `snapshot_gate` for write across its whole body, and + // the caller holds `snapshot_gate` for write across this whole body, and // the only reader of `state.gitignore` takes the read side first, so no // event can observe the matcher before this function returns either way. - commit_stale_matcher(state, build_stale_matcher(state, root, &walk)); + *newly_watched = publish_ignore_matcher(state, root, build_stale_matcher(state, root, &walk)); if walk.skipped_error > 0 { eprintln!( @@ -2592,8 +3183,13 @@ fn bootstrap_index_build(state: &Arc, root: &Path, index_dir: &Path state.no_require_git, ); let found = matcher.is_some(); - *state.gitignore.write().unwrap() = matcher; - state.gitignore_pending.store(false, Ordering::SeqCst); + // These subscriptions are the repository's first, so "newly watched" + // is every directory in it. No recovery scan: the startup stale check + // runs right after this and does a full walk-versus-index diff, which + // is a strict superset of what a recovery scan would find — and doing + // it here would stat the entire tree while holding the gate, on the + // path a warm start exists to keep fast. + let _ = publish_ignore_matcher(state, root, matcher); eprintln!( "[trace] gitignore matcher built from {} file(s) in {:.1}ms{}", outcome.gitignore_files.len(), @@ -2686,8 +3282,11 @@ fn background_index_build(state: &Arc, root: &Path, index_dir: &Pat state.no_require_git, ); let has_matcher = matcher.is_some(); - *state.gitignore.write().unwrap() = matcher; - state.gitignore_pending.store(false, Ordering::SeqCst); + // Same reasoning as the bootstrap publish: this is the cold-start + // build, so "newly watched" is the whole tree, `indexing` keeps the + // watcher off the index for the rest of the build, and the stale check + // that follows reconciles anything written during it. + let _ = publish_ignore_matcher(state, root, matcher); eprintln!( "[trace] gitignore matcher built from index walk in {:.1}ms \ ({} .gitignore + {} .ignore files{})", @@ -3440,6 +4039,7 @@ mod tests { index_progress: std::sync::atomic::AtomicU64::new(0), index_total: std::sync::atomic::AtomicU64::new(0), watch_enabled: true, + watch_registry: Mutex::new(None), exclude_dirs: Vec::new(), no_ignore: false, no_require_git: false, @@ -3661,6 +4261,276 @@ mod tests { )); } + #[test] + fn watch_registry_add_all_is_additive_but_sync_prunes() { + // These two must not be confused. `watch_new_subtree` learns only + // about the subtree that just appeared, so if it went through `sync` + // every directory outside that subtree would look stale and the server + // would unsubscribe from the entire rest of the repository — turning a + // new folder into a silent, total loss of file watching. + let tmp = TempDir::new().unwrap(); + let a = tmp.path().join("a"); + let b = tmp.path().join("b"); + let c = tmp.path().join("c"); + for dir in [&a, &b, &c] { + std::fs::create_dir(dir).unwrap(); + } + + let watcher = notify::recommended_watcher(|_: notify::Result| {}).unwrap(); + let mut registry = WatchRegistry { + watcher, + watched: std::collections::HashSet::new(), + }; + + let added = registry.add_all(&[a.clone(), b.clone()]); + assert_eq!(added.len(), 2); + + // Adding a subtree leaves existing subscriptions untouched, and + // re-adding one already present is a no-op rather than a duplicate. + let added = registry.add_all(&[b.clone(), c.clone()]); + assert_eq!( + added, + vec![c.clone()], + "b was already watched and must not be re-added" + ); + assert_eq!( + registry.watched, + [a.clone(), b.clone(), c.clone()].into_iter().collect(), + "add_all dropped a subscription outside the set it was given" + ); + + // `sync`, by contrast, is authoritative over the whole tree. + let (added, removed) = registry.sync(&[c.clone()].into_iter().collect()); + assert_eq!((added.len(), removed), (0, 2)); + assert_eq!(registry.watched, [c].into_iter().collect()); + } + + #[test] + #[ignore] + fn a_recreated_directory_is_subscribed_again_rather_than_assumed_watched() { + // The kernel releases an inotify watch by itself when its directory is + // deleted or moved away, and says nothing about it. A path recreated at + // the same location therefore *looks* subscribed while receiving no + // events — and because it is in `desired` as well as in `watched`, no + // later `sync` can tell the difference either. The entry stays poisoned + // for the life of the process, so the directory silently stops being + // watched forever. + let tmp = TempDir::new().unwrap(); + let a = tmp.path().join("a"); + std::fs::create_dir(&a).unwrap(); + + let watcher = notify::recommended_watcher(|_: notify::Result| {}).unwrap(); + let mut registry = WatchRegistry { + watcher, + watched: std::collections::HashSet::new(), + }; + assert_eq!(registry.add_all(std::slice::from_ref(&a)).len(), 1); + + // What the removal event does. Without it the entry below survives. + registry.forget(&a); + assert!( + !registry.watched.contains(&a), + "a removed directory must not be left recorded as watched" + ); + assert_eq!( + registry.add_all(std::slice::from_ref(&a)).len(), + 1, + "a directory recreated after removal must be subscribed again" + ); + + // And the belt-and-braces half: even with the entry still present — + // a move away delivers no event for the descendants it takes with it — + // a directory that has just appeared gets its subscription re-issued. + // Already-known paths are not reported as new, so the recovery scan + // does not treat the whole subtree as freshly watched. + assert!( + registry + .resubscribe_all(std::slice::from_ref(&a)) + .is_empty(), + "re-issuing a subscription must not report an existing path as new" + ); + assert!(registry.watched.contains(&a)); + } + + #[cfg(unix)] + #[test] + fn is_real_dir_rejects_a_symlink_to_a_directory() { + // `Path::is_dir` follows links, so it would report a symlinked + // directory as a directory and the watcher would subscribe to and + // index the link's target — a tree the walker never descends into, and + // one that can sit entirely outside the repository root. + let tmp = TempDir::new().unwrap(); + let real = tmp.path().join("real"); + std::fs::create_dir(&real).unwrap(); + let link = tmp.path().join("link"); + std::os::unix::fs::symlink(&real, &link).unwrap(); + + assert!(is_real_dir(&real)); + assert!( + link.is_dir(), + "precondition: is_dir follows the link, which is the trap" + ); + assert!(!is_real_dir(&link)); + assert!(!is_real_dir(&tmp.path().join("missing"))); + let file = tmp.path().join("file.txt"); + std::fs::write(&file, "x").unwrap(); + assert!(!is_real_dir(&file)); + } + + #[test] + fn watchable_dirs_prunes_ignored_and_hidden_subtrees() { + // The point of the subscription set: an ignored directory costs one + // inotify watch descriptor per directory inside it, so pruning has to + // happen before the subtree is walked, not after its events arrive. + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + std::fs::create_dir(root.join(".git")).unwrap(); + std::fs::write(root.join(".gitignore"), "build/\n").unwrap(); + + for dir in [ + "src", + "src/nested", + "build", + "build/a", + "build/a/deep", + ".git/objects", + "vendor", + "vendor/pkg", + ] { + std::fs::create_dir_all(root.join(dir)).unwrap(); + } + + let gi = tgrep_core::gitignore::build_matcher(root).expect("matcher should build"); + let exclude = vec!["vendor".to_string()]; + let dirs = watchable_dirs(root, root, &exclude, Some(&gi)); + + let rel: std::collections::HashSet = dirs + .iter() + .map(|p| { + p.strip_prefix(root) + .unwrap() + .to_string_lossy() + .replace('\\', "/") + }) + .collect(); + + // The root itself is always watched, plus the directories the indexer + // would descend into. + assert!(rel.contains(""), "root must always be watched: {rel:?}"); + assert!(rel.contains("src")); + assert!(rel.contains("src/nested")); + + // A gitignored directory and everything beneath it. + assert!( + !rel.contains("build"), + "gitignored dir was watched: {rel:?}" + ); + assert!(!rel.contains("build/a")); + assert!(!rel.contains("build/a/deep")); + + // Hidden directories, which the walker skips too. + assert!(!rel.contains(".git")); + assert!(!rel.contains(".git/objects")); + + // `--exclude` names prune the directory itself, not just its children. + assert!(!rel.contains("vendor"), "excluded dir was watched: {rel:?}"); + assert!(!rel.contains("vendor/pkg")); + } + + #[test] + fn watchable_dirs_without_a_matcher_keeps_everything_visible() { + // `--no-ignore` publishes no matcher. The subscription set must then + // be the whole tree minus hidden paths, matching what the walk indexes; + // silently narrowing it would drop events for files that ARE indexed. + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + std::fs::create_dir_all(root.join("build/a")).unwrap(); + std::fs::create_dir_all(root.join(".hidden")).unwrap(); + + let dirs = watchable_dirs(root, root, &[], None); + let rel: std::collections::HashSet = dirs + .iter() + .map(|p| { + p.strip_prefix(root) + .unwrap() + .to_string_lossy() + .replace('\\', "/") + }) + .collect(); + + assert!(rel.contains("build")); + assert!(rel.contains("build/a")); + assert!(!rel.contains(".hidden")); + } + + #[test] + fn watchable_dirs_anchors_rules_at_the_root_not_the_start_directory() { + // A subtree that appears at runtime is walked from itself, but the + // ignore rules are written against paths relative to the repository + // root. Walking with the subtree as the anchor would test "nested" + // against a rule meant for "src/nested" and prune the wrong things. + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + std::fs::create_dir(root.join(".git")).unwrap(); + std::fs::write(root.join(".gitignore"), "src/fresh/skipped/\n/keep/\n").unwrap(); + + for dir in ["src/fresh/skipped", "src/fresh/keep", "src/fresh/kept"] { + std::fs::create_dir_all(root.join(dir)).unwrap(); + } + + let gi = tgrep_core::gitignore::build_matcher(root).expect("matcher should build"); + let dirs = watchable_dirs(root, &root.join("src/fresh"), &[], Some(&gi)); + let rel: std::collections::HashSet = dirs + .iter() + .map(|p| { + p.strip_prefix(root) + .unwrap() + .to_string_lossy() + .replace('\\', "/") + }) + .collect(); + + assert!( + rel.contains("src/fresh"), + "start dir must be watched: {rel:?}" + ); + assert!(rel.contains("src/fresh/kept")); + assert!( + !rel.contains("src/fresh/skipped"), + "a root-anchored rule was not applied: {rel:?}" + ); + // `keep/` is anchored at the root, so it must NOT prune + // `src/fresh/keep` just because the walk started at `src/fresh`. + assert!( + rel.contains("src/fresh/keep"), + "a root-anchored rule was applied at the wrong depth: {rel:?}" + ); + } + + #[test] + fn skip_watcher_dir_applies_directory_semantics() { + // A directory-only gitignore rule (`build/`) does not match the path + // `build` when it is tested as a file, which is why the subscription + // set needs its own dir-aware entry point. + let tmp = TempDir::new().unwrap(); + std::fs::create_dir(tmp.path().join(".git")).unwrap(); + std::fs::write(tmp.path().join(".gitignore"), "build/\n").unwrap(); + let gi = tgrep_core::gitignore::build_matcher(tmp.path()).expect("matcher should build"); + + assert!(should_skip_watcher_dir("build", &[], Some(&gi))); + assert!(!should_skip_watcher_path("build", &[], Some(&gi))); + + // `--exclude` prunes the named directory itself... + let exclude = vec!["target".to_string()]; + assert!(should_skip_watcher_dir("target", &exclude, None)); + assert!(should_skip_watcher_dir("src/target", &exclude, None)); + // ...but a *file* of that name is still indexed, so it must not be + // skipped. This is the invariant `should_skip_watcher_path` already + // held, and sharing an implementation must not have changed it. + assert!(!should_skip_watcher_path("target", &exclude, None)); + assert!(!should_skip_watcher_path("src/target", &exclude, None)); + } + #[test] fn identifies_live_ignore_rule_changes() { let root = Path::new("workspace"); @@ -3672,6 +4542,14 @@ mod tests { root, Path::new("workspace/p4ignore.ini") )); + // `.ignore` is a first-class ignore source (and, unlike `.gitignore`, + // applies outside a git repo), so live edits to it must refresh the + // matcher too — at the root and nested. + assert!(is_ignore_rules_file(root, Path::new("workspace/.ignore"))); + assert!(is_ignore_rules_file( + root, + Path::new("workspace/nested/.ignore") + )); assert!(!is_ignore_rules_file( root, Path::new("workspace/nested/p4ignore.ini") diff --git a/tgrep-cli/tests/watcher_dot_ignore.rs b/tgrep-cli/tests/watcher_dot_ignore.rs index 652141c..914dc5d 100644 --- a/tgrep-cli/tests/watcher_dot_ignore.rs +++ b/tgrep-cli/tests/watcher_dot_ignore.rs @@ -101,22 +101,21 @@ fn wait_for_match(port: u16, pattern: &str, timeout: Duration) -> bool { } } -#[test] -fn watcher_honors_dot_ignore_and_still_indexes_new_files() { - let dir = TempDir::new().unwrap(); - let root = dir.path(); - let index_dir = root.join(".tgrep_test_index"); - - // No `.git` here: `.ignore` must apply on its own. - fs::write(root.join(".ignore"), "secret/\n").unwrap(); - fs::create_dir_all(root.join("secret")).unwrap(); - fs::create_dir_all(root.join("src")).unwrap(); - fs::write( - root.join("src").join("lib.rs"), - "fn seeded() { let normal_source_marker = 1; }\n", - ) - .unwrap(); +/// Poll until `pattern` stops being searchable, returning whether it went away. +fn wait_for_no_match(port: u16, pattern: &str, timeout: Duration) -> bool { + let start = Instant::now(); + loop { + if search_matches(port, pattern) == 0 { + return true; + } + if start.elapsed() > timeout { + return false; + } + thread::sleep(Duration::from_millis(100)); + } +} +fn build_index(root: &Path, index_dir: &Path) { let status = Command::new(tgrep_bin()) .args([ "index", @@ -127,7 +126,9 @@ fn watcher_honors_dot_ignore_and_still_indexes_new_files() { .status() .expect("failed to run tgrep index"); assert!(status.success(), "initial index build failed"); +} +fn spawn_server(root: &Path, index_dir: &Path) -> ServerGuard { let child = Command::new(tgrep_bin()) .args([ "serve", @@ -139,7 +140,27 @@ fn watcher_honors_dot_ignore_and_still_indexes_new_files() { .stdout(std::process::Stdio::null()) .spawn() .expect("failed to start tgrep serve"); - let _server = ServerGuard { child }; + ServerGuard { child } +} + +#[test] +fn watcher_honors_dot_ignore_and_still_indexes_new_files() { + let dir = TempDir::new().unwrap(); + let root = dir.path(); + let index_dir = root.join(".tgrep_test_index"); + + // No `.git` here: `.ignore` must apply on its own. + fs::write(root.join(".ignore"), "secret/\n").unwrap(); + fs::create_dir_all(root.join("secret")).unwrap(); + fs::create_dir_all(root.join("src")).unwrap(); + fs::write( + root.join("src").join("lib.rs"), + "fn seeded() { let normal_source_marker = 1; }\n", + ) + .unwrap(); + + build_index(root, &index_dir); + let _server = spawn_server(root, &index_dir); let port = wait_for_port(&index_dir); @@ -176,3 +197,71 @@ fn watcher_honors_dot_ignore_and_still_indexes_new_files() { "watcher indexed a file under a directory excluded by .ignore" ); } + +/// A `.ignore` written *after* the server is live must refresh the ignore +/// rules, exactly as a `.gitignore` write does. +/// +/// The startup matcher is built from the walk that the initial index used, so +/// a `.ignore` that already exists is honored for free — which is what the +/// test above covers. Rules that appear later only take effect if the watcher +/// recognizes the `.ignore` write as an ignore-rules change and schedules the +/// refresh; when it does not, the stale matcher stays published and the +/// already-indexed content under the newly excluded directory stays +/// searchable indefinitely (the periodic reconcile is on an hourly timer). +/// +/// The fixture seeds the ignored file *before* indexing, so the assertion is a +/// transition — searchable, then not — rather than a fixed sleep racing the +/// refresh. +#[test] +fn late_dot_ignore_refreshes_the_watchers_ignore_rules() { + let dir = TempDir::new().unwrap(); + let root = dir.path(); + let index_dir = root.join(".tgrep_test_index"); + + // No `.git` here either: `.ignore` is not git-gated, so this pins the + // refresh path for the one ignore source that works outside a repo. + fs::create_dir_all(root.join("secret")).unwrap(); + fs::create_dir_all(root.join("src")).unwrap(); + fs::write( + root.join("secret").join("creds.txt"), + "late_ignored_leak_marker\n", + ) + .unwrap(); + fs::write( + root.join("src").join("lib.rs"), + "fn seeded() { let normal_source_marker = 1; }\n", + ) + .unwrap(); + + build_index(root, &index_dir); + let _server = spawn_server(root, &index_dir); + + let port = wait_for_port(&index_dir); + + assert!( + wait_for_match(port, "normal_source_marker", Duration::from_secs(30)), + "expected the seeded source file to be searchable" + ); + // Positive control: with no `.ignore` yet, the seeded file under `secret/` + // is legitimately indexed. Without this the assertion below could pass + // simply because the file was never indexed in the first place. + assert!( + wait_for_match(port, "late_ignored_leak_marker", Duration::from_secs(30)), + "expected the file under secret/ to be indexed before any .ignore exists" + ); + thread::sleep(Duration::from_secs(2)); + + fs::write(root.join(".ignore"), "secret/\n").unwrap(); + + assert!( + wait_for_no_match(port, "late_ignored_leak_marker", Duration::from_secs(60)), + "a .ignore written while the server was live never refreshed the ignore \ + rules; content under the newly excluded directory is still searchable" + ); + + // The refresh must not take the rest of the index with it. + assert!( + search_matches(port, "normal_source_marker") > 0, + "the ignore-rules refresh dropped a file that is not ignored" + ); +} diff --git a/tgrep-cli/tests/watcher_watch_registration.rs b/tgrep-cli/tests/watcher_watch_registration.rs new file mode 100644 index 0000000..87d370d --- /dev/null +++ b/tgrep-cli/tests/watcher_watch_registration.rs @@ -0,0 +1,472 @@ +//! The watcher must not take OS subscriptions for trees it is going to ignore. +//! +//! On Linux `notify` has no recursive inotify mode: `RecursiveMode::Recursive` +//! walks the tree and spends one watch descriptor per directory. Ignored build +//! output is usually most of the directories in a repository, so subscribing to +//! it burns the per-user `fs.inotify.max_user_watches` budget on events that +//! are then discarded — and because notify propagates the first registration +//! failure, a repo large enough to exhaust that budget loses its watcher +//! entirely. +//! +//! This is observable: the kernel reports a process's inotify watches in +//! `/proc//fdinfo/`, one `inotify wd:` line per watch. The test below +//! counts them for a live server and pins that the ignored subtree is absent, +//! while asserting the watcher still works — a watcher that registered nothing +//! at all would trivially satisfy the count. +//! +//! Windows (ReadDirectoryChangesW) and macOS (FSEvents) subscribe once for the +//! whole subtree, so there is no per-directory registration to withhold and +//! nothing here applies; the count assertion is Linux-only for that reason. + +use std::fs; +use std::io::{BufRead, BufReader, Write}; +use std::net::TcpStream; +use std::path::Path; +use std::process::{Child, Command}; +use std::thread; +use std::time::{Duration, Instant}; + +use tempfile::TempDir; + +/// Directories inside the ignored tree. Large enough that a recursive +/// subscription is unmistakable in the watch count, small enough to create +/// quickly. +#[cfg(target_os = "linux")] +const IGNORED_DIRS: usize = 60; +/// Directories inside the indexed tree. +#[cfg(target_os = "linux")] +const SOURCE_DIRS: usize = 4; + +fn tgrep_bin() -> std::path::PathBuf { + assert_cmd::cargo::cargo_bin("tgrep") +} + +struct ServerGuard { + child: Child, +} + +impl Drop for ServerGuard { + fn drop(&mut self) { + let _ = self.child.kill(); + let _ = self.child.wait(); + } +} + +fn send_request(port: u16, request: &str) -> std::io::Result { + let mut stream = TcpStream::connect(format!("127.0.0.1:{port}"))?; + stream.set_read_timeout(Some(Duration::from_secs(30)))?; + writeln!(stream, "{request}")?; + stream.flush()?; + let mut reader = BufReader::new(stream); + let mut response = String::new(); + reader.read_line(&mut response)?; + Ok(response) +} + +fn search_matches(port: u16, pattern: &str) -> u64 { + let request = serde_json::json!({ + "jsonrpc": "2.0", + "method": "search", + "id": 1, + "params": { "pattern": pattern } + }) + .to_string(); + let response = send_request(port, &request).expect("search request failed"); + let value: serde_json::Value = serde_json::from_str(&response).expect("invalid JSON response"); + value + .pointer("/result/num_matches") + .and_then(|v| v.as_u64()) + .unwrap_or_else(|| panic!("missing num_matches in response: {response}")) +} + +fn wait_for_match(port: u16, pattern: &str, timeout: Duration) -> bool { + let start = Instant::now(); + loop { + if search_matches(port, pattern) > 0 { + return true; + } + if start.elapsed() > timeout { + return false; + } + thread::sleep(Duration::from_millis(100)); + } +} + +/// Read `(pid, port)` once the server is accepting connections. +fn wait_for_server(index_dir: &Path) -> (u32, u16) { + let serve_json = index_dir.join("serve.json"); + let start = Instant::now(); + loop { + assert!( + start.elapsed() <= Duration::from_secs(60), + "tgrep serve did not start within 60 seconds" + ); + if let Ok(data) = fs::read_to_string(&serve_json) + && let Ok(info) = serde_json::from_str::(&data) + && let Some(p) = info.get("port").and_then(|v| v.as_u64()) + && let Some(pid) = info.get("pid").and_then(|v| v.as_u64()) + && TcpStream::connect(format!("127.0.0.1:{p}")).is_ok() + { + return (pid as u32, p as u16); + } + thread::sleep(Duration::from_millis(20)); + } +} + +/// Total inotify watch descriptors held by `pid`. +/// +/// Each inotify file descriptor's `fdinfo` lists one `inotify wd:` line per +/// watch, so summing them across the process's descriptors gives the number of +/// directories it is subscribed to. +#[cfg(target_os = "linux")] +fn inotify_watch_count(pid: u32) -> usize { + let dir = format!("/proc/{pid}/fdinfo"); + let Ok(entries) = fs::read_dir(&dir) else { + panic!("could not read {dir}; /proc must be mounted for this test"); + }; + let mut total = 0; + for entry in entries.flatten() { + // Descriptors come and go while we read; a vanished one is not a + // failure, it simply holds no watches we can count. + if let Ok(contents) = fs::read_to_string(entry.path()) { + total += contents + .lines() + .filter(|line| line.starts_with("inotify wd:")) + .count(); + } + } + total +} + +#[cfg(target_os = "linux")] +#[test] +fn watcher_does_not_subscribe_to_gitignored_directories() { + let dir = TempDir::new().unwrap(); + let root = dir.path(); + let index_dir = root.join(".tgrep_test_index"); + + // A real (if empty) `.git` entry: `.gitignore` rules only apply inside a + // repository, so without it the fixture would measure the wrong thing. + fs::create_dir_all(root.join(".git")).unwrap(); + fs::write(root.join(".gitignore"), "build/\n").unwrap(); + + for i in 0..IGNORED_DIRS { + let sub = root.join("build").join(format!("out{i}")); + fs::create_dir_all(&sub).unwrap(); + fs::write(sub.join("artifact.txt"), "ignored_build_output\n").unwrap(); + } + for i in 0..SOURCE_DIRS { + let sub = root.join("src").join(format!("pkg{i}")); + fs::create_dir_all(&sub).unwrap(); + fs::write( + sub.join("lib.rs"), + format!("fn seeded{i}() {{ let normal_source_marker = {i}; }}\n"), + ) + .unwrap(); + } + + let status = Command::new(tgrep_bin()) + .args([ + "index", + root.to_str().unwrap(), + "--index-path", + index_dir.to_str().unwrap(), + ]) + .status() + .expect("failed to run tgrep index"); + assert!(status.success(), "initial index build failed"); + + let child = Command::new(tgrep_bin()) + .args([ + "serve", + "--index-path", + index_dir.to_str().unwrap(), + root.to_str().unwrap(), + ]) + .stderr(std::process::Stdio::null()) + .stdout(std::process::Stdio::null()) + .spawn() + .expect("failed to start tgrep serve"); + let _server = ServerGuard { child }; + + let (pid, port) = wait_for_server(&index_dir); + + // Positive control, and a wait for the watcher to be live: subscriptions + // are deferred until the ignore matcher is published, so counting before + // that would pass for the wrong reason. + assert!( + wait_for_match(port, "normal_source_marker", Duration::from_secs(30)), + "expected the seeded source files to be searchable" + ); + fs::write( + root.join("src").join("added.rs"), + "fn added() { let watcher_added_marker = 1; }\n", + ) + .unwrap(); + assert!( + wait_for_match(port, "watcher_added_marker", Duration::from_secs(30)), + "watcher never indexed a newly created ordinary file, so the watch \ + count below would be meaningless" + ); + + let watches = inotify_watch_count(pid); + + // The tree the watcher legitimately needs is the root, `src`, `src/pkg*`, + // and `.git` is hidden so it is not watched either. Allow generous slack + // for anything else in the process holding an inotify fd, but stay far + // below the ~66 a recursive subscription over this fixture would take. + let allowed = SOURCE_DIRS + 8; + assert!( + watches <= allowed, + "watcher holds {watches} inotify watches for a tree whose only \ + non-ignored directories are the root plus {SOURCE_DIRS} under src/; \ + it is subscribing to the {IGNORED_DIRS} gitignored directories under \ + build/ (expected at most {allowed})" + ); +} + +/// New directories still have to be picked up. +/// +/// Non-recursive subscriptions are not extended by notify, so a directory +/// created after startup is invisible unless the watcher subscribes to it as +/// it appears. This runs everywhere: on a whole-subtree backend it simply +/// re-confirms the existing behaviour, which is the point — the two paths must +/// agree. +#[test] +fn watcher_indexes_files_in_directories_created_after_startup() { + let dir = TempDir::new().unwrap(); + let root = dir.path(); + let index_dir = root.join(".tgrep_test_index"); + + fs::create_dir_all(root.join(".git")).unwrap(); + fs::write(root.join(".gitignore"), "build/\n").unwrap(); + fs::create_dir_all(root.join("src")).unwrap(); + fs::write( + root.join("src").join("lib.rs"), + "fn seeded() { let normal_source_marker = 1; }\n", + ) + .unwrap(); + + let status = Command::new(tgrep_bin()) + .args([ + "index", + root.to_str().unwrap(), + "--index-path", + index_dir.to_str().unwrap(), + ]) + .status() + .expect("failed to run tgrep index"); + assert!(status.success(), "initial index build failed"); + + let child = Command::new(tgrep_bin()) + .args([ + "serve", + "--index-path", + index_dir.to_str().unwrap(), + root.to_str().unwrap(), + ]) + .stderr(std::process::Stdio::null()) + .stdout(std::process::Stdio::null()) + .spawn() + .expect("failed to start tgrep serve"); + let _server = ServerGuard { child }; + + let (_pid, port) = wait_for_server(&index_dir); + assert!( + wait_for_match(port, "normal_source_marker", Duration::from_secs(30)), + "expected the seeded source file to be searchable" + ); + thread::sleep(Duration::from_secs(2)); + + // A whole new subtree, several levels deep, written in one go. The files + // land immediately after their directories, which is exactly the race the + // subscription pass has to close. + let nested = root.join("src").join("fresh").join("deeper"); + fs::create_dir_all(&nested).unwrap(); + fs::write( + nested.join("new.rs"), + "fn fresh() { let nested_new_dir_marker = 2; }\n", + ) + .unwrap(); + + assert!( + wait_for_match(port, "nested_new_dir_marker", Duration::from_secs(30)), + "watcher never indexed a file created in a directory that did not \ + exist when the server started" + ); + + // ...and a directory created inside the ignored tree stays ignored. + let ignored = root.join("build").join("fresh"); + fs::create_dir_all(&ignored).unwrap(); + fs::write(ignored.join("out.txt"), "new_ignored_dir_marker\n").unwrap(); + thread::sleep(Duration::from_secs(3)); + assert_eq!( + search_matches(port, "new_ignored_dir_marker"), + 0, + "watcher indexed a file in a directory created under a gitignored path" + ); +} + +/// A subtree that arrives already populated can carry its own ignore rules. +/// +/// A clone, a `git mv`, a branch switch or an unpacked archive all land this +/// way: the directory and everything under it appear in one step, so there is +/// no moment at which the `.gitignore` inside it is seen on its own. The +/// recovery pass skips dot-prefixed files, so without explicitly looking for +/// ignore rules it would index the subtree against rules that never mentioned +/// it — and the wrongly indexed files would stay until something touched them +/// again or the hourly reconcile came round. +#[test] +fn watcher_honors_ignore_rules_inside_a_subtree_that_arrives_whole() { + let dir = TempDir::new().unwrap(); + let root = dir.path(); + let index_dir = root.join(".tgrep_test_index"); + + fs::create_dir_all(root.join(".git")).unwrap(); + fs::write(root.join(".gitignore"), "build/\n").unwrap(); + fs::create_dir_all(root.join("src")).unwrap(); + fs::write( + root.join("src").join("lib.rs"), + "fn seeded() { let normal_source_marker = 1; }\n", + ) + .unwrap(); + + let status = Command::new(tgrep_bin()) + .args([ + "index", + root.to_str().unwrap(), + "--index-path", + index_dir.to_str().unwrap(), + ]) + .status() + .expect("failed to run tgrep index"); + assert!(status.success(), "initial index build failed"); + + let child = Command::new(tgrep_bin()) + .args([ + "serve", + "--index-path", + index_dir.to_str().unwrap(), + root.to_str().unwrap(), + ]) + .stderr(std::process::Stdio::null()) + .stdout(std::process::Stdio::null()) + .spawn() + .expect("failed to start tgrep serve"); + let _server = ServerGuard { child }; + + let (_pid, port) = wait_for_server(&index_dir); + assert!( + wait_for_match(port, "normal_source_marker", Duration::from_secs(30)), + "expected the seeded source file to be searchable" + ); + thread::sleep(Duration::from_secs(2)); + + // Staged under a dot-prefixed name so the watcher ignores it while it is + // being built, and on the same filesystem so the move below is atomic. + let staging = root.join(".staging"); + fs::create_dir_all(&staging).unwrap(); + fs::write(staging.join(".gitignore"), "secret.txt\n").unwrap(); + fs::write(staging.join("secret.txt"), "moved_subtree_secret_marker\n").unwrap(); + fs::write( + staging.join("keep.rs"), + "fn keep() { let moved_subtree_keep_marker = 3; }\n", + ) + .unwrap(); + + fs::rename(&staging, root.join("vendor")).unwrap(); + + assert!( + wait_for_match(port, "moved_subtree_keep_marker", Duration::from_secs(60)), + "watcher never indexed the non-ignored file in a subtree that arrived whole" + ); + assert_eq!( + search_matches(port, "moved_subtree_secret_marker"), + 0, + "watcher indexed a file excluded by a .gitignore that arrived inside \ + the same subtree, so the subtree was indexed against stale rules" + ); +} + +/// A directory removed and recreated at the same path must still be watched. +/// +/// The kernel drops an inotify watch when its directory goes away and does not +/// say so, leaving the path recorded as watched with no descriptor behind it. +/// Nothing later can tell that entry from a live one — it is in the desired set +/// *and* in the watched set — so without explicitly clearing it on removal the +/// directory stops being watched for the life of the process. +/// +/// `rm -rf build && mkdir build`, a branch switch, and a `git clean` all do +/// exactly this. +#[test] +fn watcher_rewatches_a_directory_that_is_removed_and_recreated() { + let dir = TempDir::new().unwrap(); + let root = dir.path(); + let index_dir = root.join(".tgrep_test_index"); + + fs::create_dir_all(root.join(".git")).unwrap(); + fs::write(root.join(".gitignore"), "build/\n").unwrap(); + fs::create_dir_all(root.join("src").join("gen")).unwrap(); + fs::write( + root.join("src").join("lib.rs"), + "fn seeded() { let normal_source_marker = 1; }\n", + ) + .unwrap(); + fs::write( + root.join("src").join("gen").join("old.rs"), + "fn old() { let pre_delete_marker = 2; }\n", + ) + .unwrap(); + + let status = Command::new(tgrep_bin()) + .args([ + "index", + root.to_str().unwrap(), + "--index-path", + index_dir.to_str().unwrap(), + ]) + .status() + .expect("failed to run tgrep index"); + assert!(status.success(), "initial index build failed"); + + let child = Command::new(tgrep_bin()) + .args([ + "serve", + "--index-path", + index_dir.to_str().unwrap(), + root.to_str().unwrap(), + ]) + .stderr(std::process::Stdio::null()) + .stdout(std::process::Stdio::null()) + .spawn() + .expect("failed to start tgrep serve"); + let _server = ServerGuard { child }; + + let (_pid, port) = wait_for_server(&index_dir); + assert!( + wait_for_match(port, "pre_delete_marker", Duration::from_secs(30)), + "expected the seeded file in src/gen to be searchable" + ); + thread::sleep(Duration::from_secs(2)); + + let gen_dir = root.join("src").join("gen"); + fs::remove_dir_all(&gen_dir).unwrap(); + thread::sleep(Duration::from_secs(2)); + fs::create_dir_all(&gen_dir).unwrap(); + + // Deliberately after the recreation has been processed. A file written in + // the same breath would be picked up by the subscription pass's own scan + // and would say nothing about whether the watch itself was re-established. + thread::sleep(Duration::from_secs(3)); + fs::write( + gen_dir.join("new.rs"), + "fn regenerated() { let post_recreate_marker = 3; }\n", + ) + .unwrap(); + + assert!( + wait_for_match(port, "post_recreate_marker", Duration::from_secs(30)), + "watcher never saw a file written to a directory that was removed and \ + recreated, so its subscription was not re-established" + ); +}