fsevents: fix file event reporting

Commit 2d2af38 ("fsevents: really watch files with fsevents on macos 10.7+") from last November introduced a regression where events that were previously reported as UV_RENAME were now reported as UV_CHANGE. This commit rectifies that. Fixes: nodejs/node#27869 (cherry picked from commit d649d21)
vtjnash · May 28, 2019 · 45f0138 · 45f0138
1 parent 98ffc50
commit 45f0138
Show file tree

Hide file tree

Showing 3 changed files with 67 additions and 18 deletions.
diff --git a/src/unix/fsevents.c b/src/unix/fsevents.c
@@ -49,19 +49,9 @@ void uv__fsevents_loop_delete(uv_loop_t* loop) {
 #include <CoreFoundation/CFRunLoop.h>
 #include <CoreServices/CoreServices.h>
 
-/* These are macros to avoid "initializer element is not constant" errors
+/* Macro to avoid "initializer element is not constant" errors
  * with old versions of gcc.
  */
-#define kFSEventsModified (kFSEventStreamEventFlagItemFinderInfoMod |         \
-                           kFSEventStreamEventFlagItemModified |              \
-                           kFSEventStreamEventFlagItemInodeMetaMod |          \
-                           kFSEventStreamEventFlagItemChangeOwner |           \
-                           kFSEventStreamEventFlagItemXattrMod)
-
-#define kFSEventsRenamed  (kFSEventStreamEventFlagItemCreated |               \
-                           kFSEventStreamEventFlagItemRemoved |               \
-                           kFSEventStreamEventFlagItemRenamed)
-
 #define kFSEventsSystem   (kFSEventStreamEventFlagUserDropped |               \
                            kFSEventStreamEventFlagKernelDropped |             \
                            kFSEventStreamEventFlagEventIdsWrapped |           \
@@ -289,8 +279,6 @@ static void uv__fsevents_event_cb(ConstFSEventStreamRef streamRef,
             path--;
             len++;
           }
-          /* Created and Removed seem to be always set, but don't make sense */
-          flags &= ~kFSEventsRenamed;
         } else {
           /* Skip forward slash */
           path++;
@@ -311,12 +299,12 @@ static void uv__fsevents_event_cb(ConstFSEventStreamRef streamRef,
 
       memset(event, 0, sizeof(*event));
       memcpy(event->path, path, len + 1);
-      event->events = UV_RENAME;
+      event->events = UV_CHANGE;
 
-      if (0 == (flags & kFSEventsRenamed)) {
-        if (0 != (flags & kFSEventsModified) ||
-            0 == (flags & kFSEventStreamEventFlagItemIsDir))
-          event->events = UV_CHANGE;
+      if ((flags & kFSEventStreamEventFlagItemIsDir) ||
+          (flags & kFSEventStreamEventFlagItemRemoved) ||
+          (flags & kFSEventStreamEventFlagItemRenamed)) {
+        event->events = UV_RENAME;
       }
 
       QUEUE_INSERT_TAIL(&head, &event->member);

diff --git a/test/test-fs-event.c b/test/test-fs-event.c
@@ -619,6 +619,65 @@ TEST_IMPL(fs_event_watch_file_exact_path) {
   return 0;
 }
 
+static void file_remove_cb(uv_fs_event_t* handle,
+                           const char* path,
+                           int events,
+                           int status) {
+  fs_event_cb_called++;
+
+  ASSERT(0 == strcmp(path, "file1"));
+  /* TODO(bnoordhuis) Harmonize the behavior across platforms. Right now
+   * this test merely ensures the status quo doesn't regress.
+   */
+#ifdef __linux__
+  ASSERT(UV_CHANGE == events);
+#else
+  ASSERT(UV_RENAME == events);
+#endif
+  ASSERT(0 == status);
+
+  uv_close((uv_handle_t*) handle, NULL);
+}
+
+static void file_remove_next(uv_timer_t* handle) {
+  uv_close((uv_handle_t*) handle, NULL);
+  remove("watch_dir/file1");
+}
+
+static void file_remove_start(uv_timer_t* handle) {
+  uv_fs_event_t* watcher;
+  uv_loop_t* loop;
+
+  loop = handle->loop;
+  watcher = handle->data;
+
+  ASSERT(0 == uv_fs_event_init(loop, watcher));
+  ASSERT(0 == uv_fs_event_start(watcher, file_remove_cb, "watch_dir/file1", 0));
+  ASSERT(0 == uv_timer_start(handle, file_remove_next, 50, 0));
+}
+
+TEST_IMPL(fs_event_watch_file_remove) {
+  uv_fs_event_t watcher;
+  uv_timer_t timer;
+  uv_loop_t* loop;
+
+  remove("watch_dir/file1");
+  remove("watch_dir/");
+  create_dir("watch_dir");
+  create_file("watch_dir/file1");
+
+  loop = uv_default_loop();
+  timer.data = &watcher;
+
+  ASSERT(0 == uv_timer_init(loop, &timer));
+  ASSERT(0 == uv_timer_start(&timer, file_remove_start, 500, 0));
+  ASSERT(0 == uv_run(loop, UV_RUN_DEFAULT));
+  ASSERT(1 == fs_event_cb_called);
+
+  MAKE_VALGRIND_HAPPY();
+  return 0;
+}
+
 TEST_IMPL(fs_event_watch_file_twice) {
 #if defined(NO_FS_EVENTS)
   RETURN_SKIP(NO_FS_EVENTS);

diff --git a/test/test-list.h b/test/test-list.h
@@ -347,6 +347,7 @@ TEST_DECLARE   (fs_event_watch_dir_short_path)
 #endif
 TEST_DECLARE   (fs_event_watch_file)
 TEST_DECLARE   (fs_event_watch_file_exact_path)
+TEST_DECLARE   (fs_event_watch_file_remove)
 TEST_DECLARE   (fs_event_watch_file_twice)
 TEST_DECLARE   (fs_event_watch_file_current_dir)
 #ifdef _WIN32
@@ -932,6 +933,7 @@ TASK_LIST_START
 #endif
   TEST_ENTRY  (fs_event_watch_file)
   TEST_ENTRY  (fs_event_watch_file_exact_path)
+  TEST_ENTRY  (fs_event_watch_file_remove)
   TEST_ENTRY  (fs_event_watch_file_twice)
   TEST_ENTRY  (fs_event_watch_file_current_dir)
 #ifdef _WIN32