From e11e256d602aceaade556ca711a7478f58044ed1 Mon Sep 17 00:00:00 2001
From: Rudi Grinberg <me@rgrinberg.com>
Date: Tue, 11 Oct 2022 11:07:12 -0600
Subject: [PATCH] fix(fsevents): correctly handle unknown events

We should apply our action mask before converting it into an OCaml
variant. Otherwise, we'll get events that we don't understand.

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

ps-id: 204d7599-9ffb-4e95-8cff-871f39b4dc68
---
 src/dune_file_watcher/dune_file_watcher.ml |  7 +++----
 src/fsevents/fsevents.ml                   |  6 ++++--
 src/fsevents/fsevents.mli                  |  8 ++++----
 src/fsevents/fsevents_stubs.c              | 18 +++++-------------
 4 files changed, 16 insertions(+), 23 deletions(-)

diff --git a/src/dune_file_watcher/dune_file_watcher.ml b/src/dune_file_watcher/dune_file_watcher.ml
index f6aa006523bd..194a846b019c 100644
--- a/src/dune_file_watcher/dune_file_watcher.ml
+++ b/src/dune_file_watcher/dune_file_watcher.ml
@@ -514,10 +514,9 @@ let fsevents ?exclusion_paths ~latency ~paths scheduler f =
   fsevents
 
 let fsevents_standard_event event path =
-  let action = Fsevents.Event.action event in
   let kind =
-    match action with
-    | Unknown -> Fs_memo_event.Unknown
+    match Fsevents.Event.action event with
+    | Rename | Unknown -> Fs_memo_event.Unknown
     | Create -> Created
     | Remove -> Deleted
     | Modify ->
@@ -540,7 +539,7 @@ let create_fsevents ?(latency = 0.2) ~(scheduler : Scheduler.t) () =
         else
           match Fsevents.Event.action event with
           | Remove -> None
-          | Unknown | Create | Modify ->
+          | Rename | Unknown | Create | Modify ->
             Option.map (Fs_sync.consume_event sync_table path) ~f:(fun id ->
                 Event.Sync id))
   in
diff --git a/src/fsevents/fsevents.ml b/src/fsevents/fsevents.ml
index 6619b1711f7b..2ad3d11bad7f 100644
--- a/src/fsevents/fsevents.ml
+++ b/src/fsevents/fsevents.ml
@@ -194,10 +194,11 @@ module Event = struct
   let kind t = kind t.flags
 
   type action =
-    | Unknown
     | Create
     | Remove
     | Modify
+    | Rename
+    | Unknown
 
   external action : Int32.t -> action = "dune_fsevents_action"
 
@@ -209,7 +210,8 @@ module Event = struct
       | Create -> "Create"
       | Remove -> "Remove"
       | Modify -> "Modify"
-      | Unknown -> "Unknown")
+      | Unknown -> "Unknown"
+      | Rename -> "Rename")
 
   let to_dyn t =
     let open Dyn in
diff --git a/src/fsevents/fsevents.mli b/src/fsevents/fsevents.mli
index 4c1c07b97c01..85e8611be666 100644
--- a/src/fsevents/fsevents.mli
+++ b/src/fsevents/fsevents.mli
@@ -44,13 +44,13 @@ module Event : sig
   val kind : t -> kind
 
   type action =
+    | Create (* [path t] guaranteed to exist *)
+    | Remove (* [path t] guaranteed to be absent *)
+    | Modify (* [path t] guaranteed to exist *)
+    | Rename
     | Unknown
         (** multiple actions merged into one by debouncing or an uninformative
             "rename". inspect the FS to see what happened *)
-    | Create (* [path t] guaranteed to exist *)
-    | Remove (* [path t] guaranteed to be absent *)
-    | Modify
-  (* [path t] guaranteed to exist *)
 
   val dyn_of_action : action -> Dyn.t
 
diff --git a/src/fsevents/fsevents_stubs.c b/src/fsevents/fsevents_stubs.c
index 4014a4374184..7a6c27817baf 100644
--- a/src/fsevents/fsevents_stubs.c
+++ b/src/fsevents/fsevents_stubs.c
@@ -270,24 +270,16 @@ CAMLprim value dune_fsevents_action(value v_flags) {
   CAMLlocal1(v_action);
 
   uint32_t flags = Int32_val(v_flags) & action_mask;
-  int count = __builtin_popcount(flags);
-
-  flags = Int32_val(v_flags);
-  if (count >= 2 || flags & kFSEventStreamEventFlagItemRenamed) {
-    // we don't bother tracking renamed acurately for now. macos makes it
-    // tricky by not telling is which path is created and which one is deleted.
-    // it is possible to reverse engineer this from the chain of inodes in the
-    // events, but it's also error prone as inodes can be reused. so for now, we
-    // avoid this is and treat renamed as unknown
+  if (flags & kFSEventStreamEventFlagItemCreated) {
     v_action = Val_int(0);
-  } else if (flags & kFSEventStreamEventFlagItemCreated) {
-    v_action = Val_int(1);
   } else if (flags & kFSEventStreamEventFlagItemRemoved) {
-    v_action = Val_int(2);
+    v_action = Val_int(1);
   } else if (flags & kFSEventStreamEventFlagItemModified) {
+    v_action = Val_int(2);
+  } else if (flags & kFSEventStreamEventFlagItemRenamed) {
     v_action = Val_int(3);
   } else {
-    caml_failwith("fsevents: unexpected event action");
+    v_action = Val_int(4);
   }
 
   CAMLreturn(v_action);