Repository: gorakhargosh/watchdog

Issue:
Ambiguity in setting FileClosedEvent in filter
If the filter is set to `FileClosedEvent`, the mask is set to `InotifyConstants.IN_CLOSE` which includes `IN_CLOSE_WRITE` and `IN_CLOSE_NOWRITE`, but only `IN_CLOSE_WRITE` will call `on_closed()`. I think line 236 should be modified to ` event_mask |= InotifyConstants.IN_CLOSE_WRITE` to improve performance and maintain logical consistency in the code.

https://github.com/gorakhargosh/watchdog/blob/9f23b599f369cdd873c0cf3dd9e69567234d78fb/src/watchdog/observers/inotify.py#L235-L236

https://github.com/gorakhargosh/watchdog/blob/9f23b599f369cdd873c0cf3dd9e69567234d78fb/src/watchdog/observers/inotify.py#L190-L199




Discussion:

Patch:
diff --git a/changelog.rst b/changelog.rst
index 7671ec0..1111f7d 100644
--- a/changelog.rst
+++ b/changelog.rst
@@ -15,6 +15,8 @@ Changelog
 - [core] Improve typing references for events (`#1040 <https://github.com/gorakhargosh/watchdog/issues/1040>`__)
 - [inotify] Renamed the ``inotify_event_struct`` class to ``InotifyEventStruct`` (`#1055 <https://github.com/gorakhargosh/watchdog/pull/1055>`__)
 - [inotify] Renamed the ``UnsupportedLibc`` exception to ``UnsupportedLibcError`` (`#1057 <https://github.com/gorakhargosh/watchdog/pull/1057>`__)
+- [inotify] Add support for ``IN_CLOSE_NOWRITE`` events. A ``FileClosedNoWriteEvent`` event will be fired, and its ``on_closed_no_write()`` dispatcher has been introduced (`#1046 <https://github.com/gorakhargosh/watchdog/pull/1046>`__)
+- [inotify] Removed the ``InotifyConstants.IN_CLOSE`` constant (`#1046 <https://github.com/gorakhargosh/watchdog/pull/1046>`__)
 - [watchmedo] Renamed the ``LogLevelException`` exception to ``LogLevelError`` (`#1057 <https://github.com/gorakhargosh/watchdog/pull/1057>`__)
 - [watchmedo] Renamed the ``WatchdogShutdown`` exception to ``WatchdogShutdownError`` (`#1057 <https://github.com/gorakhargosh/watchdog/pull/1057>`__)
 - [windows] Renamed the ``FILE_NOTIFY_INFORMATION`` class to ``FileNotifyInformation`` (`#1055 <https://github.com/gorakhargosh/watchdog/pull/1055>`__)
diff --git a/src/watchdog/events.py b/src/watchdog/events.py
index 2d6db21..82804e0 100644
--- a/src/watchdog/events.py
+++ b/src/watchdog/events.py
@@ -53,6 +53,10 @@
    :members:
    :show-inheritance:
 
+.. autoclass:: FileClosedNoWriteEvent
+   :members:
+   :show-inheritance:
+
 .. autoclass:: FileOpenedEvent
    :members:
    :show-inheritance:
@@ -96,7 +100,6 @@
 import os.path
 import re
 from dataclasses import dataclass, field
-from typing import ClassVar
 
 from watchdog.utils.patterns import match_any_paths
 
@@ -105,6 +108,7 @@
 EVENT_TYPE_CREATED = "created"
 EVENT_TYPE_MODIFIED = "modified"
 EVENT_TYPE_CLOSED = "closed"
+EVENT_TYPE_CLOSED_NO_WRITE = "closed_no_write"
 EVENT_TYPE_OPENED = "opened"
 
 
@@ -167,6 +171,12 @@ class FileClosedEvent(FileSystemEvent):
     event_type = EVENT_TYPE_CLOSED
 
 
+class FileClosedNoWriteEvent(FileSystemEvent):
+    """File system event representing an unmodified file close on the file system."""
+
+    event_type = EVENT_TYPE_CLOSED_NO_WRITE
+
+
 class FileOpenedEvent(FileSystemEvent):
     """File system event representing file close on the file system."""
 
@@ -206,15 +216,6 @@ class DirMovedEvent(FileSystemMovedEvent):
 class FileSystemEventHandler:
     """Base file system event handler that you can override methods from."""
 
-    dispatch_table: ClassVar = {
-        EVENT_TYPE_CREATED: "on_created",
-        EVENT_TYPE_DELETED: "on_deleted",
-        EVENT_TYPE_MODIFIED: "on_modified",
-        EVENT_TYPE_MOVED: "on_moved",
-        EVENT_TYPE_CLOSED: "on_closed",
-        EVENT_TYPE_OPENED: "on_opened",
-    }
-
     def dispatch(self, event: FileSystemEvent) -> None:
         """Dispatches events to the appropriate methods.
 
@@ -224,7 +225,7 @@ def dispatch(self, event: FileSystemEvent) -> None:
             :class:`FileSystemEvent`
         """
         self.on_any_event(event)
-        getattr(self, self.dispatch_table[event.event_type])(event)
+        getattr(self, f"on_{event.event_type}")(event)
 
     def on_any_event(self, event: FileSystemEvent) -> None:
         """Catch-all event handler.
@@ -280,6 +281,15 @@ def on_closed(self, event: FileClosedEvent) -> None:
             :class:`FileClosedEvent`
         """
 
+    def on_closed_no_write(self, event: FileClosedNoWriteEvent) -> None:
+        """Called when a file opened for reading is closed.
+
+        :param event:
+            Event representing file closing.
+        :type event:
+            :class:`FileClosedNoWriteEvent`
+        """
+
     def on_opened(self, event: FileOpenedEvent) -> None:
         """Called when a file is opened.
 
@@ -483,7 +493,12 @@ def on_modified(self, event: DirModifiedEvent | FileModifiedEvent) -> None:
     def on_closed(self, event: FileClosedEvent) -> None:
         super().on_closed(event)
 
-        self.logger.info("Closed file: %s", event.src_path)
+        self.logger.info("Closed modified file: %s", event.src_path)
+
+    def on_closed_no_write(self, event: FileClosedNoWriteEvent) -> None:
+        super().on_closed_no_write(event)
+
+        self.logger.info("Closed read file: %s", event.src_path)
 
     def on_opened(self, event: FileOpenedEvent) -> None:
         super().on_opened(event)
diff --git a/src/watchdog/observers/inotify.py b/src/watchdog/observers/inotify.py
index bd20762..c6cf338 100644
--- a/src/watchdog/observers/inotify.py
+++ b/src/watchdog/observers/inotify.py
@@ -75,6 +75,7 @@
     DirModifiedEvent,
     DirMovedEvent,
     FileClosedEvent,
+    FileClosedNoWriteEvent,
     FileCreatedEvent,
     FileDeletedEvent,
     FileModifiedEvent,
@@ -181,23 +182,25 @@ def queue_events(self, timeout, *, full_events=False):
                 cls = DirCreatedEvent if event.is_directory else FileCreatedEvent
                 self.queue_event(cls(src_path))
                 self.queue_event(DirModifiedEvent(os.path.dirname(src_path)))
-            elif event.is_close_write and not event.is_directory:
-                cls = FileClosedEvent
-                self.queue_event(cls(src_path))
-                self.queue_event(DirModifiedEvent(os.path.dirname(src_path)))
-            elif event.is_open and not event.is_directory:
-                cls = FileOpenedEvent
-                self.queue_event(cls(src_path))
             elif event.is_delete_self and src_path == self.watch.path:
                 cls = DirDeletedEvent if event.is_directory else FileDeletedEvent
                 self.queue_event(cls(src_path))
                 self.stop()
+            elif not event.is_directory:
+                if event.is_open:
+                    cls = FileOpenedEvent
+                    self.queue_event(cls(src_path))
+                elif event.is_close_write:
+                    cls = FileClosedEvent
+                    self.queue_event(cls(src_path))
+                    self.queue_event(DirModifiedEvent(os.path.dirname(src_path)))
+                elif event.is_close_nowrite:
+                    cls = FileClosedNoWriteEvent
+                    self.queue_event(cls(src_path))
 
     def _decode_path(self, path):
         """Decode path only if unicode string was passed to this emitter."""
-        if isinstance(self.watch.path, bytes):
-            return path
-        return os.fsdecode(path)
+        return path if isinstance(self.watch.path, bytes) else os.fsdecode(path)
 
     def get_event_mask_from_filter(self):
         """Optimization: Only include events we are filtering in inotify call"""
@@ -224,7 +227,9 @@ def get_event_mask_from_filter(self):
             elif cls in (DirDeletedEvent, FileDeletedEvent):
                 event_mask |= InotifyConstants.IN_DELETE
             elif cls is FileClosedEvent:
-                event_mask |= InotifyConstants.IN_CLOSE
+                event_mask |= InotifyConstants.IN_CLOSE_WRITE
+            elif cls is FileClosedNoWriteEvent:
+                event_mask |= InotifyConstants.IN_CLOSE_NOWRITE
             elif cls is FileOpenedEvent:
                 event_mask |= InotifyConstants.IN_OPEN
         return event_mask
@@ -233,27 +238,8 @@ def get_event_mask_from_filter(self):
 class InotifyFullEmitter(InotifyEmitter):
     """inotify(7)-based event emitter. By default this class produces move events even if they are not matched
     Such move events will have a ``None`` value for the unmatched part.
-
-    :param event_queue:
-        The event queue to fill with events.
-    :param watch:
-        A watch object representing the directory to monitor.
-    :type watch:
-        :class:`watchdog.observers.api.ObservedWatch`
-    :param timeout:
-        Read events blocking timeout (in seconds).
-    :type timeout:
-        ``float``
-    :param event_filter:
-        Collection of event types to emit, or None for no filtering (default).
-    :type event_filter:
-        Iterable[:class:`watchdog.events.FileSystemEvent`] | None
-
     """
 
-    def __init__(self, event_queue, watch, *, timeout=DEFAULT_EMITTER_TIMEOUT, event_filter=None):
-        super().__init__(event_queue, watch, timeout=timeout, event_filter=event_filter)
-
     def queue_events(self, timeout, *, events=True):
         InotifyEmitter.queue_events(self, timeout, full_events=events)
 
diff --git a/src/watchdog/observers/inotify_c.py b/src/watchdog/observers/inotify_c.py
index 88d8392..6f020a9 100644
--- a/src/watchdog/observers/inotify_c.py
+++ b/src/watchdog/observers/inotify_c.py
@@ -56,7 +56,6 @@ class InotifyConstants:
     IN_MOVE_SELF = 0x00000800  # Self was moved.
 
     # Helper user-space events.
-    IN_CLOSE = IN_CLOSE_WRITE | IN_CLOSE_NOWRITE  # Close.
     IN_MOVE = IN_MOVED_FROM | IN_MOVED_TO  # Moves.
 
     # Events sent by the kernel to a watch.
@@ -109,6 +108,7 @@ class InotifyConstants:
         InotifyConstants.IN_DELETE_SELF,
         InotifyConstants.IN_DONT_FOLLOW,
         InotifyConstants.IN_CLOSE_WRITE,
+        InotifyConstants.IN_CLOSE_NOWRITE,
         InotifyConstants.IN_OPEN,
     ],
 )
@@ -551,11 +551,7 @@ def __hash__(self):
     def _get_mask_string(mask):
         masks = []
         for c in dir(InotifyConstants):
-            if c.startswith("IN_") and c not in [
-                "IN_ALL_EVENTS",
-                "IN_CLOSE",
-                "IN_MOVE",
-            ]:
+            if c.startswith("IN_") and c not in {"IN_ALL_EVENTS", "IN_MOVE"}:
                 c_val = getattr(InotifyConstants, c)
                 if mask & c_val:
                     masks.append(c)
diff --git a/src/watchdog/tricks/__init__.py b/src/watchdog/tricks/__init__.py
index 5e977c3..cd0f2f2 100644
--- a/src/watchdog/tricks/__init__.py
+++ b/src/watchdog/tricks/__init__.py
@@ -49,7 +49,7 @@
 import threading
 import time
 
-from watchdog.events import EVENT_TYPE_OPENED, FileSystemEvent, PatternMatchingEventHandler
+from watchdog.events import EVENT_TYPE_CLOSED_NO_WRITE, EVENT_TYPE_OPENED, FileSystemEvent, PatternMatchingEventHandler
 from watchdog.utils import echo, platform
 from watchdog.utils.event_debouncer import EventDebouncer
 from watchdog.utils.process_watcher import ProcessWatcher
@@ -111,7 +111,7 @@ def __init__(
         self._process_watchers = set()
 
     def on_any_event(self, event: FileSystemEvent) -> None:
-        if event.event_type == EVENT_TYPE_OPENED:
+        if event.event_type in {EVENT_TYPE_OPENED, EVENT_TYPE_CLOSED_NO_WRITE}:
             # FIXME: see issue #949, and find a way to better handle that scenario
             return
 
@@ -277,7 +277,7 @@ def _stop_process(self) -> None:
 
     @echo_events
     def on_any_event(self, event: FileSystemEvent) -> None:
-        if event.event_type == EVENT_TYPE_OPENED:
+        if event.event_type in {EVENT_TYPE_OPENED, EVENT_TYPE_CLOSED_NO_WRITE}:
             # FIXME: see issue #949, and find a way to better handle that scenario
             return
 


Test patch:
diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
index 6afed28..df3f0ad 100644
--- a/.github/workflows/tests.yml
+++ b/.github/workflows/tests.yml
@@ -87,4 +87,4 @@ jobs:
         run: python -m pip install tox
 
       - name: Run ${{ matrix.tox.name }} in tox
-        run: python -m tox -e ${{ matrix.tox.environment }}
+        run: python -m tox -q -e ${{ matrix.tox.environment }}
diff --git a/tests/test_emitter.py b/tests/test_emitter.py
index 59707e5..85c31fa 100644
--- a/tests/test_emitter.py
+++ b/tests/test_emitter.py
@@ -28,6 +28,7 @@
     DirModifiedEvent,
     DirMovedEvent,
     FileClosedEvent,
+    FileClosedNoWriteEvent,
     FileCreatedEvent,
     FileDeletedEvent,
     FileModifiedEvent,
@@ -75,10 +76,9 @@ def test_create(p: P, event_queue: TestEventQueue, start_watching: StartWatching
         assert isinstance(event, FileClosedEvent)
 
 
-@pytest.mark.xfail(reason="known to be problematic")
-@pytest.mark.skipif(not platform.is_linux(), reason="FileCloseEvent only supported in GNU/Linux")
+@pytest.mark.skipif(not platform.is_linux(), reason="FileClosed*Event only supported in GNU/Linux")
 @pytest.mark.flaky(max_runs=5, min_passes=1, rerun_filter=rerun_filter)
-def test_close(p: P, event_queue: TestEventQueue, start_watching: StartWatching) -> None:
+def test_closed(p: P, event_queue: TestEventQueue, start_watching: StartWatching) -> None:
     with open(p("a"), "a"):
         start_watching()
 
@@ -91,9 +91,17 @@ def test_close(p: P, event_queue: TestEventQueue, start_watching: StartWatching)
     assert os.path.normpath(event.src_path) == os.path.normpath(p(""))
     assert isinstance(event, DirModifiedEvent)
 
-    # After read-only, only IN_CLOSE_NOWRITE is emitted but not caught for now #747
+    # After read-only, only IN_CLOSE_NOWRITE is emitted
     open(p("a")).close()
 
+    event = event_queue.get(timeout=5)[0]
+    assert event.src_path == p("a")
+    assert isinstance(event, FileOpenedEvent)
+
+    event = event_queue.get(timeout=5)[0]
+    assert event.src_path == p("a")
+    assert isinstance(event, FileClosedNoWriteEvent)
+
     assert event_queue.empty()
 
 
diff --git a/tests/test_events.py b/tests/test_events.py
index acb5f6f..811fe9b 100644
--- a/tests/test_events.py
+++ b/tests/test_events.py
@@ -17,6 +17,7 @@
 
 from watchdog.events import (
     EVENT_TYPE_CLOSED,
+    EVENT_TYPE_CLOSED_NO_WRITE,
     EVENT_TYPE_CREATED,
     EVENT_TYPE_DELETED,
     EVENT_TYPE_MODIFIED,
@@ -27,6 +28,7 @@
     DirModifiedEvent,
     DirMovedEvent,
     FileClosedEvent,
+    FileClosedNoWriteEvent,
     FileCreatedEvent,
     FileDeletedEvent,
     FileModifiedEvent,
@@ -94,6 +96,14 @@ def test_file_closed_event():
     assert not event.is_synthetic
 
 
+def test_file_closed_no_write_event():
+    event = FileClosedNoWriteEvent(path_1)
+    assert path_1 == event.src_path
+    assert event.event_type == EVENT_TYPE_CLOSED_NO_WRITE
+    assert not event.is_directory
+    assert not event.is_synthetic
+
+
 def test_file_opened_event():
     event = FileOpenedEvent(path_1)
     assert path_1 == event.src_path
@@ -132,6 +142,7 @@ def test_file_system_event_handler_dispatch():
     dir_cre_event = DirCreatedEvent("/path/blah.py")
     file_cre_event = FileCreatedEvent("/path/blah.txt")
     file_cls_event = FileClosedEvent("/path/blah.txt")
+    file_cls_nw_event = FileClosedNoWriteEvent("/path/blah.txt")
     file_opened_event = FileOpenedEvent("/path/blah.txt")
     dir_mod_event = DirModifiedEvent("/path/blah.py")
     file_mod_event = FileModifiedEvent("/path/blah.txt")
@@ -148,29 +159,50 @@ def test_file_system_event_handler_dispatch():
         file_cre_event,
         file_mov_event,
         file_cls_event,
+        file_cls_nw_event,
         file_opened_event,
     ]
 
+    checkpoint = 0
+
     class TestableEventHandler(FileSystemEventHandler):
         def on_any_event(self, event):
-            pass
+            nonlocal checkpoint
+            checkpoint += 1
 
         def on_modified(self, event):
+            nonlocal checkpoint
+            checkpoint += 1
             assert event.event_type == EVENT_TYPE_MODIFIED
 
         def on_deleted(self, event):
+            nonlocal checkpoint
+            checkpoint += 1
             assert event.event_type == EVENT_TYPE_DELETED
 
         def on_moved(self, event):
+            nonlocal checkpoint
+            checkpoint += 1
             assert event.event_type == EVENT_TYPE_MOVED
 
         def on_created(self, event):
+            nonlocal checkpoint
+            checkpoint += 1
             assert event.event_type == EVENT_TYPE_CREATED
 
         def on_closed(self, event):
+            nonlocal checkpoint
+            checkpoint += 1
             assert event.event_type == EVENT_TYPE_CLOSED
 
+        def on_closed_no_write(self, event):
+            nonlocal checkpoint
+            checkpoint += 1
+            assert event.event_type == EVENT_TYPE_CLOSED_NO_WRITE
+
         def on_opened(self, event):
+            nonlocal checkpoint
+            checkpoint += 1
             assert event.event_type == EVENT_TYPE_OPENED
 
     handler = TestableEventHandler()
@@ -179,6 +211,8 @@ def on_opened(self, event):
         assert not event.is_synthetic
         handler.dispatch(event)
 
+    assert checkpoint == len(all_events) * 2  # `on_any_event()` + specific `on_XXX()`
+
 
 def test_event_comparison():
     creation1 = FileCreatedEvent("foo")
diff --git a/tests/test_inotify_buffer.py b/tests/test_inotify_buffer.py
index 99285b7..9a8a650 100644
--- a/tests/test_inotify_buffer.py
+++ b/tests/test_inotify_buffer.py
@@ -94,13 +94,11 @@ def test_move_internal_batch(p):
         mv(p("dir1", f), p("dir2", f))
 
     # Check that all n events are paired
-    i = 0
-    while i < n:
+    for _ in range(n):
         frm, to = wait_for_move_event(inotify.read_event)
         assert os.path.dirname(frm.src_path).endswith(b"/dir1")
         assert os.path.dirname(to.src_path).endswith(b"/dir2")
         assert frm.name == to.name
-        i += 1
     inotify.close()
 
 
@@ -146,11 +144,8 @@ def delayed(*args, **kwargs):
 class InotifyBufferDelayedRead(InotifyBuffer):
     def run(self, *args, **kwargs):
         # Introduce a delay to trigger the race condition where the file descriptor is
-        # closed prior to a read being triggered.  Ignoring type concerns since we are
-        # intentionally doing something odd.
-        self._inotify.read_events = delay_call(  # type: ignore[method-assign]
-            function=self._inotify.read_events, seconds=1
-        )
+        # closed prior to a read being triggered.
+        self._inotify.read_events = delay_call(self._inotify.read_events, 1)
 
         return super().run(*args, **kwargs)
 
diff --git a/tests/test_logging_event_handler.py b/tests/test_logging_event_handler.py
index 9a66e94..81a8557 100644
--- a/tests/test_logging_event_handler.py
+++ b/tests/test_logging_event_handler.py
@@ -17,6 +17,7 @@
 
 from watchdog.events import (
     EVENT_TYPE_CLOSED,
+    EVENT_TYPE_CLOSED_NO_WRITE,
     EVENT_TYPE_CREATED,
     EVENT_TYPE_DELETED,
     EVENT_TYPE_MODIFIED,
@@ -27,6 +28,7 @@
     DirModifiedEvent,
     DirMovedEvent,
     FileClosedEvent,
+    FileClosedNoWriteEvent,
     FileCreatedEvent,
     FileDeletedEvent,
     FileModifiedEvent,
@@ -64,13 +66,16 @@ def on_closed(self, event):
         super().on_closed(event)
         assert event.event_type == EVENT_TYPE_CLOSED
 
+    def on_closed_no_write(self, event):
+        super().on_closed_no_write(event)
+        assert event.event_type == EVENT_TYPE_CLOSED_NO_WRITE
+
     def on_opened(self, event):
         super().on_opened(event)
         assert event.event_type == EVENT_TYPE_OPENED
 
 
 def test_logging_event_handler_dispatch():
-    # Utilities.
     dir_del_event = DirDeletedEvent("/path/blah.py")
     file_del_event = FileDeletedEvent("/path/blah.txt")
     dir_cre_event = DirCreatedEvent("/path/blah.py")
@@ -81,6 +86,7 @@ def test_logging_event_handler_dispatch():
     file_mov_event = FileMovedEvent("/path/blah.txt", "/path/blah")
     file_ope_event = FileOpenedEvent("/path/blah.txt")
     file_clo_event = FileClosedEvent("/path/blah.txt")
+    file_clo_nw_event = FileClosedNoWriteEvent("/path/blah.txt")
 
     all_events = [
         dir_mod_event,
@@ -93,6 +99,7 @@ def test_logging_event_handler_dispatch():
         file_mov_event,
         file_ope_event,
         file_clo_event,
+        file_clo_nw_event,
     ]
 
     handler = _TestableEventHandler()
diff --git a/tests/utils.py b/tests/utils.py
index 8b7b828..8b82f3a 100644
--- a/tests/utils.py
+++ b/tests/utils.py
@@ -62,13 +62,9 @@ def start_watching(
         # TODO: check if other platforms expect the trailing slash (e.g. `p('')`)
         path = self.tmp if path is None else path
 
-        emitter: EventEmitter
-        if platform.is_linux() and use_full_emitter:
-            emitter = InotifyFullEmitter(self.event_queue, ObservedWatch(path, recursive=recursive))
-        else:
-            emitter = Emitter(self.event_queue, ObservedWatch(path, recursive=recursive))
-
-        self.emitters.append(emitter)
+        watcher = ObservedWatch(path, recursive=recursive)
+        emitter_cls = InotifyFullEmitter if platform.is_linux() and use_full_emitter else Emitter
+        emitter = emitter_cls(self.event_queue, watcher)
 
         if platform.is_darwin():
             # TODO: I think this could be better...  .suppress_history should maybe
@@ -78,6 +74,7 @@ def start_watching(
             assert isinstance(emitter, FSEventsEmitter)
             emitter.suppress_history = True
 
+        self.emitters.append(emitter)
         emitter.start()
 
         return emitter
@@ -87,8 +84,7 @@ def expect_event(self, expected_event: FileSystemEvent, timeout: float = 2) -> N
 
         Provides some robustness for the otherwise flaky nature of asynchronous notifications.
         """
-        event = self.event_queue.get(timeout=timeout)[0]
-        assert event == expected_event
+        assert self.event_queue.get(timeout=timeout)[0] == expected_event
 
     def close(self) -> None:
         for emitter in self.emitters: