diff --git a/CHANGELOG.md b/CHANGELOG.md index d79628cd..a173b55f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -124,6 +124,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **An idle window stops re-reading the repository on Linux** — with a + repository open, tty7 ran `git status -uall` about two and a half times a + second, forever, with nothing on screen changing and nobody touching the + machine. The filesystem watch was feeding itself: Linux reports opening a + file as a watch event, every read of `.git` is an open, and reading `.git` + is exactly how the watch's own events were answered — so each answer + scheduled the next question. Reads are no longer mistaken for changes; + a write finishing still is. macOS and Windows were never affected, which is + why this survived so long. + - **An alias put back in an `Include`d ssh config file is found again** — the check behind a parked remote workspace watched `~/.ssh/config` for changes and nothing else, but `Include` is common and editing an included file diff --git a/crates/tty7-core/src/host/conformance.rs b/crates/tty7-core/src/host/conformance.rs index 0328ae66..f165277b 100644 --- a/crates/tty7-core/src/host/conformance.rs +++ b/crates/tty7-core/src/host/conformance.rs @@ -66,6 +66,7 @@ macro_rules! for_each_host_case { search_respects_max_dirs, shells_are_named_and_have_a_default, watch_reports_create_and_delete, + watch_ignores_reads, watch_is_non_recursive, watch_set_dirs_adds_and_drops, watch_coalesces_within_window, @@ -946,6 +947,47 @@ pub fn watch_reports_create_and_delete(h: &dyn Host, sb: &dyn Sandbox) { ); } +/// Reading a watched file is not a change. +/// +/// The one case where a watch can feed itself: every consumer answers an event +/// by reading the directory it came from, so if a read is an event, the answer +/// asks the question again. `notify`'s inotify backend subscribes to `IN_OPEN`, +/// which makes this reachable on Linux and only there — an idle Source Control +/// panel ran `git status -uall` about 2.6 times a second before this was +/// filtered (issue #523). macOS passes this trivially, which is exactly why it +/// has to be a conformance case rather than a platform test. +pub fn watch_ignores_reads(h: &dyn Host, sb: &dyn Sandbox) { + let sandbox = sb.path(); + let f = h.join(sandbox, "read-me.txt"); + write(h, &f, "hello"); + + let sub = h.watch(&[sandbox.to_path_buf()]).unwrap(); + // The file this asserts about is created *before* the watch, so the drain + // has to come after the tail of that create can still arrive — FSEvents + // will hand one over a beat late, and a stale create is indistinguishable + // here from the reads below reporting. + std::thread::sleep(WATCH_QUIET); + drain(&sub); + + for _ in 0..5 { + h.read_file(&f, 1 << 20) + .expect("the watched file reads back"); + } + std::thread::sleep(WATCH_QUIET); + + let batches = collect_batches(&sub, Duration::from_millis(200)); + let leaked: Vec<&PathBuf> = batches + .iter() + .flatten() + .filter(|p| p.file_name().is_some_and(|n| n == "read-me.txt")) + .collect(); + assert!( + leaked.is_empty(), + "reading a watched file reported as a change, so a watch can feed \ + itself: {leaked:?}" + ); +} + pub fn watch_is_non_recursive(h: &dyn Host, sb: &dyn Sandbox) { let sandbox = sb.path(); let sub_dir = h.join(sandbox, "child"); diff --git a/crates/tty7-core/src/host/local.rs b/crates/tty7-core/src/host/local.rs index be391fa7..6288037e 100644 --- a/crates/tty7-core/src/host/local.rs +++ b/crates/tty7-core/src/host/local.rs @@ -381,6 +381,7 @@ fn local_watch(dirs: &[PathBuf], gitignore: Arc>) -> io::R let watcher = notify::recommended_watcher(move |res: notify::Result| { if let Ok(ev) = res && !ev.paths.is_empty() + && crate::host::is_content_change(&ev.kind) { let _ = raw_tx.send(ev.paths); } diff --git a/crates/tty7-core/src/host/mod.rs b/crates/tty7-core/src/host/mod.rs index 58552739..dcb0a724 100644 --- a/crates/tty7-core/src/host/mod.rs +++ b/crates/tty7-core/src/host/mod.rs @@ -262,6 +262,32 @@ pub trait Host: Send + Sync + 'static { } } +/// Did this watch event mean something *changed*, or only that something was +/// read? +/// +/// The distinction does not exist on macOS, and on Linux it decides whether a +/// watch can feed itself. `notify`'s inotify backend asks for `IN_OPEN`, so +/// every `open(2)` under a watched directory arrives as an event — and the one +/// thing every consumer of a watch does with an event is go and read the +/// directory it came from. Reading `.git/index` to answer "did the repository +/// change" is itself an event saying the repository may have changed, so the +/// answer schedules the next question, at whatever rate the debounce allows, +/// forever. Measured at ~2.6 `git status -uall` per second on an idle Source +/// Control panel before this filter existed (issue #523). +/// +/// `IN_CLOSE_WRITE` also arrives as `Access`, and that one is a real write +/// finishing, so it stays. Everything else under `Access` is a reader. +pub fn is_content_change(kind: ¬ify::EventKind) -> bool { + use notify::event::{AccessKind, AccessMode}; + + !matches!( + kind, + notify::EventKind::Access( + AccessKind::Open(_) | AccessKind::Read | AccessKind::Close(AccessMode::Read), + ) + ) +} + pub fn default_join(dir: &Path, name: &str, sep: char) -> PathBuf { let mut s = dir.to_string_lossy().into_owned(); if !s.is_empty() && !s.ends_with(sep) && !s.ends_with('/') { diff --git a/src/main.rs b/src/main.rs index df8e6e5d..2bc50a81 100644 --- a/src/main.rs +++ b/src/main.rs @@ -43,6 +43,11 @@ fn spawn_config_watcher(cx: &mut App) { let watched_file = config_file.clone(); let handler = move |res: notify::Result| { let Ok(event) = res else { return }; + // A reload re-reads the file, and on Linux reading it is itself an + // event — see `tty7_core::host::is_content_change`. + if !tty7_core::host::is_content_change(&event.kind) { + return; + } let hit = event .paths .iter()