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 <[email protected]> ps-id: 204d7599-9ffb-4e95-8cff-871f39b4dc68
ocaml · Oct 14, 2022 · e11e256 · e11e256
1 parent 12354bb
commit e11e256
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 23 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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);