From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Mon, 27 May 2019 14:31:53 +0200 Subject: 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: https://github.com/nodejs/node/issues/27869 diff --git a/deps/uv/src/unix/fsevents.c b/deps/uv/src/unix/fsevents.c index ddacda31fef87eee131fc2ee2ff46cc88be429d9..911023a6562319af5e9e3412ddbfd2ca14f77b05 100644 --- a/deps/uv/src/unix/fsevents.c +++ b/deps/uv/src/unix/fsevents.c @@ -49,19 +49,9 @@ void uv__fsevents_loop_delete(uv_loop_t* loop) { #include #include -/* 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/deps/uv/test/test-fs-event.c b/deps/uv/test/test-fs-event.c index ea34bd63a70625c3e2c60d5a1bbb087c5f0bbb2e..38d722a27ef1a78717726272d54578c734280242 100644 --- a/deps/uv/test/test-fs-event.c +++ b/deps/uv/test/test-fs-event.c @@ -62,6 +62,15 @@ static char fs_event_filename[1024]; static int timer_cb_touch_called; static int timer_cb_exact_called; +static void expect_filename(const char* path, const char* expected) { +#if defined(__APPLE__) || defined(_WIN32) || defined(__linux__) + ASSERT(0 == strcmp(path, expected)); +#else + if (path != NULL) + ASSERT(0 == strcmp(path, expected)); +#endif +} + static void fs_event_fail(uv_fs_event_t* handle, const char* filename, int events, @@ -130,11 +139,7 @@ static void fs_event_cb_dir(uv_fs_event_t* handle, const char* filename, ASSERT(handle == &fs_event); ASSERT(status == 0); ASSERT(events == UV_CHANGE); - #if defined(__APPLE__) || defined(_WIN32) || defined(__linux__) - ASSERT(strcmp(filename, "file1") == 0); - #else - ASSERT(filename == NULL || strcmp(filename, "file1") == 0); - #endif + expect_filename(filename, "file1"); ASSERT(0 == uv_fs_event_stop(handle)); uv_close((uv_handle_t*)handle, close_cb); } @@ -312,11 +317,7 @@ static void fs_event_cb_file(uv_fs_event_t* handle, const char* filename, ASSERT(handle == &fs_event); ASSERT(status == 0); ASSERT(events == UV_CHANGE); - #if defined(__APPLE__) || defined(_WIN32) || defined(__linux__) - ASSERT(strcmp(filename, "file2") == 0); - #else - ASSERT(filename == NULL || strcmp(filename, "file2") == 0); - #endif + expect_filename(filename, "file2"); ASSERT(0 == uv_fs_event_stop(handle)); uv_close((uv_handle_t*)handle, close_cb); } @@ -339,11 +340,7 @@ static void fs_event_cb_file_current_dir(uv_fs_event_t* handle, ASSERT(handle == &fs_event); ASSERT(status == 0); ASSERT(events == UV_CHANGE); - #if defined(__APPLE__) || defined(_WIN32) || defined(__linux__) - ASSERT(strcmp(filename, "watch_file") == 0); - #else - ASSERT(filename == NULL || strcmp(filename, "watch_file") == 0); - #endif + expect_filename(filename, "watch_file"); /* Regression test for SunOS: touch should generate just one event. */ { @@ -619,6 +616,69 @@ 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++; + + expect_filename(path, "file1"); + /* TODO(bnoordhuis) Harmonize the behavior across platforms. Right now + * this test merely ensures the status quo doesn't regress. + */ +#if defined(_AIX) || defined(__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; + +#if defined(__MVS__) + RETURN_SKIP("test does not work on this OS"); +#endif + + 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/deps/uv/test/test-list.h b/deps/uv/test/test-list.h index a48f6f3806806af6bf844558d08fa9679e268112..f9020fc6c1ec8d076c470fa5042d062b74226ae4 100644 --- a/deps/uv/test/test-list.h +++ b/deps/uv/test/test-list.h @@ -335,6 +335,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 @@ -924,6 +925,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