Commit `951aa728e2`

951aa728e21b6f09a832c049e74a7b3181482404

parent: 1429684118

Unsigned ci/pages: success ci/test: success

cmc <hello@cleberg.net> · 2026-10-18 20:22 UTC

watch: ignore filesystem access events (!9)

inotify's watch mask includes `IN_OPEN`, so reading a file raises an event on it — and a build reads every source file. Each rebuild triggered the next, as fast as the machine could build; under `orgo serve` the browser reloads every few milliseconds. macOS and Windows report no access events, so only Linux was affected.

`is_change` drops `EventKind::Access(_)` except `Close(Write)`, which follows a write rather than a read. Non-access kinds are kept, including the backends' catch-all `Any` — FSEvents and the Windows backend report unclassified changes that way.

The end-to-end test now counts rebuilds through `run_with` instead of watching `syntax.css`'s mtime, so "a quiet tree builds nothing" covers the build's own reads as well as its writes.

Verified on the Linux runner: build 43 (the test alone, no fix) fails with 12 rebuilds in 1.5s of an idle tree; build 45 (test + fix) passes. Build 46, triggered on main for a control, passes with the old mtime assertion — that check was blind to the loop, which is why CI stayed green.

Reported by a user against `orgo serve`.

Layout: unified · split

src/watch.rs +29 −3

 //! compare mtimes. Native events cost nothing while nothing happens, and arrive in
 //! milliseconds when something does.
 //!
-//! Two things matter more than the watching itself:
+//! Three things matter more than the watching itself:
 //!
 //! 1. **Not watching our own output.** `orgo watch . -o _site` puts the output inside
 //!    the source. Rebuilding writes files, writing files raises events, and events
 //!    trigger a rebuild — a loop that never stops and never idles. [`ChangeFilter`] is
 //!    what prevents it, and it is a pure function precisely so it can be tested without
 //!    a filesystem.
-//! 2. **Debouncing.** Saving a file in an editor is rarely one event: editors write a
+//! 2. **Not treating a read as a change.** A build reads every source file, and inotify
+//!    reports opens. [`is_change`] is what keeps that from closing the same loop by a
+//!    different route.
+//! 3. **Debouncing.** Saving a file in an editor is rarely one event: editors write a
 //!    temp file, rename it over the original, and touch the directory. Rebuilding per
 //!    event would rebuild several times per save.
 use anyhow::{Context, Result};
 use camino::{Utf8Path, Utf8PathBuf};
-use notify::{Config as NotifyConfig, RecursiveMode, Watcher};
+use notify::event::{AccessKind, AccessMode};
+use notify::{Config as NotifyConfig, EventKind, RecursiveMode, Watcher};
 use crate::site::{build_site, BuildOptions};
     }
 }
+/// Does this event report a change to the tree, rather than someone reading it?
+///
+/// inotify's watch mask includes `IN_OPEN`, so reading a file raises an event on it — and
+/// a build reads every source file. Forwarding those makes each rebuild the trigger for
+/// the next, at whatever rate the machine can build. macOS and Windows report no access
+/// events, which is why this only showed up on Linux.
+///
+/// `Access(Close(Write))` is the exception: it follows a write, not a read, and the only
+/// files a build writes are its own output, which [`ChangeFilter`] already drops.
+/// Everything that is not an access is kept, the backends' catch-all `Any` included —
+/// FSEvents and the Windows backend both report changes they cannot classify that way, so
+/// an allow-list of `Create`/`Modify`/`Remove` would drop real edits.
+pub fn is_change(kind: &EventKind) -> bool {
+    match kind {
+        EventKind::Access(access) => matches!(access, AccessKind::Close(AccessMode::Write)),
+        _ => true,
+    }
+}
 /// The platform's native watcher, falling back to polling where that is unavailable —
 /// some network and container filesystems have no event API, and `watch` failing outright
 /// there would be worse than being slow.
 fn make_watcher(tx: mpsc::Sender<Vec<Utf8PathBuf>>) -> Result<Box<dyn Watcher>> {
     let handler = move |result: notify::Result<notify::Event>| {
         if let Ok(event) = result {
+            if !is_change(&event.kind) {
+                return;
+            }
             let paths: Vec<Utf8PathBuf> = event
                 .paths
                 .into_iter()

tests/watch.rs +72 −30

 //! guarantee testable without waiting on a filesystem.
 use std::sync::atomic::{AtomicU32, Ordering};
+use std::sync::Arc;
 use std::time::{Duration, Instant};
 use camino::{Utf8Path, Utf8PathBuf};
     );
 }
+// ---------------------------------------------------------------------------
+// Event kinds
+// ---------------------------------------------------------------------------
+/// A read is not a change. inotify reports opens, and a build reads every source file, so
+/// an access forwarded as a change makes each rebuild trigger the next.
+#[test]
+fn access_events_do_not_count_as_changes() {
+    use notify::event::{AccessKind, AccessMode, CreateKind, DataChange, ModifyKind, RemoveKind};
+    use notify::EventKind;
+    use orgo::watch::is_change;
+    for read in [
+        AccessKind::Any,
+        AccessKind::Read,
+        AccessKind::Open(AccessMode::Any),
+        AccessKind::Open(AccessMode::Read),
+        AccessKind::Close(AccessMode::Read),
+    ] {
+        assert!(
+            !is_change(&EventKind::Access(read)),
+            "{read:?} is a read, not a change"
+        );
+    }
+    assert!(
+        is_change(&EventKind::Access(AccessKind::Close(AccessMode::Write))),
+        "closing a file that was written to is a change"
+    );
+    assert!(is_change(&EventKind::Create(CreateKind::File)));
+    assert!(is_change(&EventKind::Modify(ModifyKind::Data(DataChange::Any))));
+    assert!(is_change(&EventKind::Remove(RemoveKind::File)));
+    assert!(
+        is_change(&EventKind::Any),
+        "FSEvents and the Windows backend report unclassified changes this way"
+    );
+}
 // ---------------------------------------------------------------------------
 // End to end
 // ---------------------------------------------------------------------------
-/// Drive the real watcher against a real edit. Timing-dependent by nature, so it polls
+/// Drive the real watcher against a real edit, then hold still. Timing-dependent by
-/// for the expected result with a generous ceiling rather than sleeping a fixed amount.
+/// nature, so it polls for the expected result with a generous ceiling rather than
+/// sleeping a fixed amount.
 #[test]
 fn watching_rebuilds_the_site_when_a_source_file_changes() {
     let root = tmpdir("watchrun");
         .unwrap()
         .contains("First version."));
-    let (src_t, out_t) = (src.clone(), out.clone());
+    // Count rebuilds rather than reading an output file's mtime: a quiet watcher is
+    // "no more rebuilds", and that is what the counter says directly.
+    let rebuilds = Arc::new(AtomicU32::new(0));
+    let (src_t, out_t, counter) = (src.clone(), out.clone(), Arc::clone(&rebuilds));
     let handle = std::thread::spawn(move || {
-        let _ = orgo::watch::run(&src_t, &out_t, &BuildOptions::default());
+        let _ = orgo::watch::run_with(&src_t, &out_t, &BuildOptions::default(), |_| {
+            counter.fetch_add(1, Ordering::Relaxed);
+        });
     });
     // The edit is repeated rather than made once after a fixed head start. The watcher
     // defect worth failing a build over. (Found by the first CI run on Linux, where a
     // 300ms head start was not enough and macOS had never noticed.)
     let deadline = Instant::now() + Duration::from_secs(30);
-    let mut rebuilt = false;
     let mut wrote_at: Option<Instant> = None;
-    while Instant::now() < deadline {
+    while rebuilds.load(Ordering::Relaxed) == 0 && Instant::now() < deadline {
         if wrote_at.is_none_or(|t| t.elapsed() >= Duration::from_millis(400)) {
             std::fs::write(src.join("index.org"), "#+TITLE: Home\n\nSecond version.\n").unwrap();
             wrote_at = Some(Instant::now());
         }
-        if std::fs::read_to_string(out.join("index.html"))
-            .map(|h| h.contains("Second version."))
-            .unwrap_or(false)
-        {
-            rebuilt = true;
-            break;
-        }
         std::thread::sleep(Duration::from_millis(50));
     }
-    assert!(rebuilt, "an edit should trigger a rebuild within 30s");
+    assert!(
+        rebuilds.load(Ordering::Relaxed) > 0,
+        "an edit should trigger a rebuild within 30s"
+    );
+    assert!(std::fs::read_to_string(out.join("index.html"))
+        .unwrap()
+        .contains("Second version."));
-    // The output lives inside the source, so the rebuild's own writes raised events. If
+    // Nothing is editing the tree now, so nothing more should be built. Two of the
-    // those are not filtered out, watch spins forever.
+    // rebuild's own side effects would otherwise feed back in as changes:
     //
-    // The file to watch for that is `syntax.css`, not `index.html`. The incremental
+    // 1. Its writes. The output lives inside the source, so they raise events —
-    // build leaves an unchanged page alone, so `index.html` holds still even *during* a
+    //    [`ChangeFilter`] drops them.
-    // runaway loop — an assertion on it passes whether or not the filter works, which is
+    // 2. Its reads. A build reads every source file, and inotify reports opens, so a
-    // exactly what it did before this comment existed. `syntax.css` is rewritten on
+    //    watcher that treats an access as a change rebuilds at full speed. Reported
-    // every build, so its mtime is a direct record of how many builds have run.
+    //    against `orgo serve`, where every rebuild reloads the browser.
-    // Long enough for the rebuild from the last repeated write to have landed before the
+    //
-    // first reading, or this measures that instead of a feedback loop.
+    // The first sleep lets the rebuild from the last repeated write land, or this
-    let stylesheet = out.join("syntax.css");
+    // measures that instead of a feedback loop.
+    std::thread::sleep(Duration::from_millis(1500));
+    let settled = rebuilds.load(Ordering::Relaxed);
     std::thread::sleep(Duration::from_millis(1500));
-    let first = std::fs::metadata(&stylesheet).unwrap().modified().unwrap();
-    std::thread::sleep(Duration::from_millis(1200));
-    let second = std::fs::metadata(&stylesheet).unwrap().modified().unwrap();
     assert_eq!(
-        first, second,
+        settled,
-        "the build's own writes must not feed back in as changes — watch is rebuilding \
+        rebuilds.load(Ordering::Relaxed),
-         in a loop"
+        "a quiet tree must produce no rebuilds — watch is rebuilding in a loop"
     );
     drop(handle); // the watcher thread ends with the process

Commit 951aa728e2

Commit `951aa728e2`