Skip watch rebuilds for files whose content didn't change

changes/watch-repeated-events.fix.md

@@ -0,0 +1 @@
+- Watch mode no longer rebuilds, and prints the same errors again, when one file change is reported twice.

rewatch-ocaml/tests/check_command_validation.sh

@@ -1291,11 +1291,16 @@ if [ -s "$work/quiet-watch.out" ] || \
   cat "$work/quiet-watch.out" "$work/quiet-watch.err" >&2
   exit 1
 fi
-# macOS and Windows can report one write as several change events, and each
-# rebuild reports the parse error again. Compare the distinct reports only.
+# macOS and Windows can report one write as several change events. The
+# watcher skips a rebuild when the file is unchanged, but a rebuild that sees
+# the write half done would report the error again, so compare the distinct
+# reports only. Each report starts with a blank line; the split is before it
+# and must not cut a CRLF line ending.
 node -e '
   const fs = require("fs");
-  const blocks = fs.readFileSync(process.argv[1], "utf8").split(/^(?=  Syntax error!)/m);
+  const blocks = fs
+    .readFileSync(process.argv[1], "utf8")
+    .split(/(?<=\n)(?=\r?\n  Syntax error!\r?$)/m);
   const distinct = blocks.filter((block, i) => block !== blocks[i - 1]);
   fs.writeFileSync(process.argv[2], distinct.join(""));
 ' "$work/quiet-watch.err" "$work/quiet-watch.err.distinct"

rewatch-ocaml/watch_snapshot.ml

@@ -316,8 +316,11 @@ let changes_between before after =
     (fun (first : change) second -> String.compare first.path second.path)
     !changes
 
+type entries_update = Unchanged | Updated of entry list | Not_in_snapshot
+
 let update_entries digest_cache previous changes =
   let changed = Hashtbl.create (List.length changes) in
+  let any_changed = ref false in
   List.iter
     (fun (change : change) -> Hashtbl.replace changed change.path change.kind)
     changes;
@@ -329,13 +332,14 @@ let update_entries digest_cache previous changes =
         | Some Removed ->
           Hashtbl.remove changed entry.path;
           Hashtbl.remove digest_cache entry.path;
+          any_changed := true;
           None
         | Some (Added | Modified) -> (
           Hashtbl.remove changed entry.path;
           try
             let stat = Unix.stat entry.path in
             let digest = hash_and_cache digest_cache entry.path stat in
-            Some
+            let updated =
               {
                 path = entry.path;
                 state =
@@ -346,17 +350,24 @@ let update_entries digest_cache previous changes =
                       digest;
                     };
               }
+            in
+            if not (entry_equal updated entry) then any_changed := true;
+            Some updated
           with
           | Unix.Unix_error ((Unix.ENOENT | Unix.ENOTDIR), _, _) ->
             Hashtbl.remove digest_cache entry.path;
+            any_changed := true;
             None
           | Unix.Unix_error _ | Sys_error _ ->
             (* Preserve the old baseline so the caller still runs the build.
                Source diagnostics then remain inside the watcher's recoverable
                build boundary instead of terminating the watch loop here. *)
+            any_changed := true;
             Some entry))
   in
-  if Hashtbl.length changed = 0 then Some updated else None
+  if Hashtbl.length changed > 0 then Not_in_snapshot
+  else if !any_changed then Updated updated
+  else Unchanged
 
 let changes_are_incremental changes =
   changes <> []

rewatch-ocaml/watch_snapshot.mli

@@ -47,8 +47,14 @@ val create_with_symlink_paths : digest_cache -> Watch_scope.t -> registration
 
 val changes_between : entry list -> entry list -> change list
 
-val update_entries :
-  digest_cache -> entry list -> change list -> entry list option
+(* The result of re-reading the files a native watcher reported. A platform
+   can report one write more than once (macOS FSEvents may repeat it after
+   the drain window), so [Unchanged] means every file still matches its
+   entry and no build is needed. [Not_in_snapshot] means a reported path has
+   no entry and the snapshot must be reconciled. *)
+type entries_update = Unchanged | Updated of entry list | Not_in_snapshot
+
+val update_entries : digest_cache -> entry list -> change list -> entries_update
 
 val changes_are_incremental : change list -> bool
 

rewatch-ocaml/watcher.ml

@@ -223,12 +223,13 @@ let run_locked ~native_create ~report_native_fallback ~root ~prod ~features
       | Some [] -> native_loop watcher scope symlink_targets previous
       | Some changes -> (
         match Watch_snapshot.update_entries digest_cache previous changes with
-        | Some before_build ->
+        | Updated before_build ->
           Output.debug ~verbosity "doing Incremental";
           begin_rebuild Incremental;
           build ~poll ~changes:(Some changes) |> finish_rebuild;
           native_loop watcher scope symlink_targets before_build
-        | None -> native_reconcile watcher scope previous)
+        | Unchanged -> native_loop watcher scope symlink_targets previous
+        | Not_in_snapshot -> native_reconcile watcher scope previous)
       | None -> native_reconcile watcher scope previous)
   and native_reconcile watcher (scope : Watch_scope.t) previous =
     let current = Watch_snapshot.create digest_cache scope in

tests/rewatch_ounit_tests/watcher_tests.ml

@@ -172,26 +172,36 @@ let tests =
             };
           ]
       in
-      Test_support.write_file source "let value = 2\n";
+      let modified = [Watch_snapshot.{path = source; kind = Modified}] in
       check
-        (Option.is_some
-           (Watch_snapshot.update_entries digest_cache previous
-              [Watch_snapshot.{path = source; kind = Modified}]))
+        (Watch_snapshot.update_entries digest_cache previous modified
+        = Unchanged)
+        "a repeated event for unchanged content needs no rebuild";
+      Test_support.write_file source "let value = 2\n";
+      let updated =
+        match Watch_snapshot.update_entries digest_cache previous modified with
+        | Updated entries -> Some entries
+        | Unchanged | Not_in_snapshot -> None
+      in
+      check (Option.is_some updated)
         "known content events update the retained snapshot";
       check
-        (Option.is_none
-           (Watch_snapshot.update_entries digest_cache previous
-              [
-                Watch_snapshot.
-                  {path = Filename.concat root "B.res"; kind = Added};
-              ]))
+        (match updated with
+        | Some entries ->
+          Watch_snapshot.update_entries digest_cache entries modified
+          = Unchanged
+        | None -> false)
+        "a repeated event after the update needs no rebuild";
+      check
+        (Watch_snapshot.update_entries digest_cache previous
+           [Watch_snapshot.{path = Filename.concat root "B.res"; kind = Added}]
+        = Not_in_snapshot)
         "unknown content events request structural reconciliation";
       Sys.remove source;
       Unix.mkdir source 0o755;
       check
-        (Option.is_some
-           (Watch_snapshot.update_entries digest_cache previous
-              [Watch_snapshot.{path = source; kind = Modified}]))
+        (Watch_snapshot.update_entries digest_cache previous modified
+        = Updated previous)
         "a transient snapshot read failure preserves the rebuild baseline");
   Test_support.with_temp_dir "rewatch-snapshot-racy-" (fun root ->
       let root = Unix.realpath root in