Repository: gorakhargosh/watchdog

Issue:
Modifed event triggered twice
if I copy and paste a file on OSX lion
it triggered created event once and modified event twice
should it be just trigger once modified event?

`
INFO:root:Created file: /Users/ouyangjichao/BoxSync/rename/testwatchdog.py
INFO:root:Modified file: /Users/ouyangjichao/BoxSync/rename/testwatchdog.py
INFO:root:Modified file: /Users/ouyangjichao/BoxSync/rename/testwatchdog.py
`


Discussion:
BoboTiG: Great!

Could you rebase your branch to fix conflicts?

I dis not look, but is ot necessary for IN_CLOSE_NOWRITE to generate an event? If ghe file was not modified, then the event should not be sent. WDYT?
ysard: It's now synced with master. I also fixed a test that failed in Travis.

About the other point, it's a good question...

- For the sake of completeness and if we stick to the strict definition of the event: these are the 2 masks that must be considered.
Moreover this is already provided here (in helper user-space events):
https://github.com/gorakhargosh/watchdog/blob/847a059ae717424baea81a949c7726fc3520bb8a/src/watchdog/observers/inotify_c.py#L93


- It's hard to know if people also expect notification when a file closes after read-only;  from what I read in the issues and on stackoverflow it's `IN_CLOSE_WRITE` that's currently the most lacking.

In practice, the use of IN_CLOSE_NOWRITE is probably uncommon and "can" generate a higher processing cost because the slightest file traversal by a program emits `IN_CLOSE_NOWRITE | IN_ISDIR`.

You will always find someone who needs an unimplemented event for a project...


The safest thing to do would be to let the user choose the mask he is interested in; however the Inotify class (and the keyword argument `event_mask` of its constructor) is not accessible from the API.

---

Proof of the overhead with `IN_CLOSE_WRITE | IN_CLOSE_NOWRITE`:

```
fd = open("./dir/hello", "w")
<InotifyEvent: src_path=b'./dir/hello', wd=8, mask=IN_CREATE, cookie=0, name=hello>
<InotifyEvent: src_path=b'./dir', wd=1, mask=IN_CLOSE_NOWRITE|IN_ISDIR, cookie=0, name=dir>
<InotifyEvent: src_path=b'./dir', wd=8, mask=IN_CLOSE_NOWRITE|IN_ISDIR, cookie=0, name=>
<InotifyEvent: src_path=b'./dir/hello', wd=8, mask=IN_CLOSE_NOWRITE, cookie=0, name=hello>

fd.write("a")
fd.flush()
<InotifyEvent: src_path=b'./dir/hello', wd=8, mask=IN_MODIFY, cookie=0, name=hello>
<InotifyEvent: src_path=b'./dir/hello', wd=8, mask=IN_CLOSE_NOWRITE, cookie=0, name=hello>

fd.close()
<InotifyEvent: src_path=b'./dir/hello', wd=8, mask=IN_CLOSE_WRITE, cookie=0, name=hello>


os.remove("./dir/hello")

<InotifyEvent: src_path=b'./dir/hello', wd=8, mask=IN_DELETE, cookie=0, name=hello>
<InotifyEvent: src_path=b'./dir', wd=1, mask=IN_CLOSE_NOWRITE|IN_ISDIR, cookie=0, name=dir>
<InotifyEvent: src_path=b'./dir', wd=8, mask=IN_CLOSE_NOWRITE|IN_ISDIR, cookie=0, name=>
```

With only IN_CLOSE_WRITE:
```
<InotifyEvent: src_path=b'./dir/hello', wd=8, mask=IN_CREATE, cookie=0, name=hello>
<InotifyEvent: src_path=b'./dir/hello', wd=8, mask=IN_MODIFY, cookie=0, name=hello>
<InotifyEvent: src_path=b'./dir/hello', wd=8, mask=IN_CLOSE_WRITE, cookie=0, name=hello>
```

---

Other note: As indicated in the doc, the observers are specific to each OS (https://pythonhosted.org/watchdog/api.html#watchdog.observers.Observer).

However, to give access to the event we are interested in here, it was necessary to modify the class `watchdog.events.FileSystemEventHandler` by adding the `on_closed` method.

This class is meant to be universal and this modification is not portable. As the issue #217 reminds us, we must warn the user that this method is only available under GNU/Linux.

=> Should I add a note in the docstring?

The current issue is a long-standing problem (#245, #184, #313, #280, even #119 for the IN_ACCESS event).
It is still possible for the user to achieve this by adding the desired methods if the lib does not offer them via monkey patching, but this is not ideal.
BoboTiG: IMO the `IN_CLOSE_NOWRITE` event will add too much noise. And the benefits of having the `IN_CLOSE` event is to be able to know when a file modification has been done for real.

Could you do that?
You could comment the code you added to support `IN_CLOSE_NOWRITE` to keep the track for later. And duplicate and adapt https://github.com/gorakhargosh/watchdog/blob/847a059ae717424baea81a949c7726fc3520bb8a/src/watchdog/observers/inotify_c.py#L93 to remove `IN_CLOSE_NOWRITE`.

BoboTiG: Could you add a line in the logs (+ your GitHub nickename and @lukassup to our beloved contributors):
```rst
- [inotify] Add support for ``IN_CLOSE_WRITE`` events. A ``FileCloseEvent`` event will be fired. Note that ``IN_CLOSE_NOWRITE`` is not handled to prevent much noise. (`#184 <https://github.com/gorakhargosh/watchdog/pull/184>`_, `#245 <https://github.com/gorakhargosh/watchdog/pull/245>`_, `#280 <https://github.com/gorakhargosh/watchdog/pull/280>`_, `#313 <https://github.com/gorakhargosh/watchdog/pull/313>`_, `#690 <https://github.com/gorakhargosh/watchdog/pull/690>`_)
```
ysard: It's done now ;)

Patch:
diff --git a/changelog.rst b/changelog.rst
index 167f5cb..52943e5 100644
--- a/changelog.rst
+++ b/changelog.rst
@@ -9,7 +9,8 @@ Changelog
 202x-xx-xx • `full history <https://github.com/gorakhargosh/watchdog/compare/v1.0.2...master>`__
 
 - Avoid deprecated ``PyEval_InitThreads`` on Python 3.7+ (`#746 <https://github.com/gorakhargosh/watchdog/pull/746>`_)
-- Thanks to our beloved contributors: @bstaletic
+- [inotify] Add support for ``IN_CLOSE_WRITE`` events. A ``FileCloseEvent`` event will be fired. Note that ``IN_CLOSE_NOWRITE`` events are not handled to prevent much noise. (`#184 <https://github.com/gorakhargosh/watchdog/pull/184>`_, `#245 <https://github.com/gorakhargosh/watchdog/pull/245>`_, `#280 <https://github.com/gorakhargosh/watchdog/pull/280>`_, `#313 <https://github.com/gorakhargosh/watchdog/pull/313>`_, `#690 <https://github.com/gorakhargosh/watchdog/pull/690>`_)
+- Thanks to our beloved contributors: @bstaletic, @lukassup, @ysard
 
 
 1.0.2
diff --git a/src/watchdog/events.py b/src/watchdog/events.py
index 73e82e8..8b9bca8 100755
--- a/src/watchdog/events.py
+++ b/src/watchdog/events.py
@@ -52,6 +52,10 @@
    :members:
    :show-inheritance:
 
+.. autoclass:: FileClosedEvent
+   :members:
+   :show-inheritance:
+
 .. autoclass:: DirCreatedEvent
    :members:
    :show-inheritance:
@@ -95,6 +99,7 @@
 EVENT_TYPE_DELETED = 'deleted'
 EVENT_TYPE_CREATED = 'created'
 EVENT_TYPE_MODIFIED = 'modified'
+EVENT_TYPE_CLOSED = 'closed'
 
 
 class FileSystemEvent:
@@ -212,6 +217,12 @@ class FileMovedEvent(FileSystemMovedEvent):
     """File system event representing file movement on the file system."""
 
 
+class FileClosedEvent(FileSystemEvent):
+    """File system event representing file close on the file system."""
+
+    event_type = EVENT_TYPE_CLOSED
+
+
 # Directory events.
 
 
@@ -263,6 +274,7 @@ def dispatch(self, event):
             EVENT_TYPE_DELETED: self.on_deleted,
             EVENT_TYPE_MODIFIED: self.on_modified,
             EVENT_TYPE_MOVED: self.on_moved,
+            EVENT_TYPE_CLOSED: self.on_closed,
         }[event.event_type](event)
 
     def on_any_event(self, event):
@@ -310,6 +322,15 @@ def on_modified(self, event):
             :class:`DirModifiedEvent` or :class:`FileModifiedEvent`
         """
 
+    def on_closed(self, event):
+        """Called when a file opened for writing is closed.
+
+        :param event:
+            Event representing file closing.
+        :type event:
+            :class:`FileClosedEvent`
+        """
+
 
 class PatternMatchingEventHandler(FileSystemEventHandler):
     """
diff --git a/src/watchdog/observers/inotify.py b/src/watchdog/observers/inotify.py
index 5cb8c5d..7c5d9c6 100644
--- a/src/watchdog/observers/inotify.py
+++ b/src/watchdog/observers/inotify.py
@@ -86,6 +86,7 @@
     FileModifiedEvent,
     FileMovedEvent,
     FileCreatedEvent,
+    FileClosedEvent,
     generate_sub_moved_events,
     generate_sub_created_events,
 )
@@ -170,6 +171,13 @@ 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_close_nowrite and not event.is_directory:
+            #     cls = FileClosedEvent
+            #     self.queue_event(cls(src_path))
             elif event.is_delete_self and src_path == self.watch.path:
                 self.queue_event(DirDeletedEvent(src_path))
                 self.stop()
diff --git a/src/watchdog/observers/inotify_c.py b/src/watchdog/observers/inotify_c.py
index 61452dd..8925147 100644
--- a/src/watchdog/observers/inotify_c.py
+++ b/src/watchdog/observers/inotify_c.py
@@ -139,6 +139,7 @@ class InotifyConstants:
         InotifyConstants.IN_DELETE,
         InotifyConstants.IN_DELETE_SELF,
         InotifyConstants.IN_DONT_FOLLOW,
+        InotifyConstants.IN_CLOSE_WRITE,
     ])
 
 


Test patch:
diff --git a/tests/test_emitter.py b/tests/test_emitter.py
index 6df98ae..4fd0506 100644
--- a/tests/test_emitter.py
+++ b/tests/test_emitter.py
@@ -31,7 +31,8 @@
     DirDeletedEvent,
     DirModifiedEvent,
     DirCreatedEvent,
-    DirMovedEvent
+    DirMovedEvent,
+    FileClosedEvent,
 )
 from watchdog.observers.api import ObservedWatch
 
@@ -107,6 +108,33 @@ def test_create():
         assert os.path.normpath(event.src_path) == os.path.normpath(p(''))
         assert isinstance(event, DirModifiedEvent)
 
+    if platform.is_linux():
+        event = event_queue.get(timeout=5)[0]
+        assert event.src_path == p('a')
+        assert isinstance(event, FileClosedEvent)
+
+
+@pytest.mark.skipif(not platform.is_linux(), reason="FileCloseEvent only supported in GNU/Linux")
+@pytest.mark.flaky(max_runs=5, min_passes=1, rerun_filter=rerun_filter)
+def test_close():
+    f_d = open(p('a'), 'a')
+    start_watching()
+    f_d.close()
+
+    # After file creation/open in append mode
+    event = event_queue.get(timeout=5)[0]
+    assert event.src_path == p('a')
+    assert isinstance(event, FileClosedEvent)
+
+    event = event_queue.get(timeout=5)[0]
+    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 catched for now #747
+    open(p('a'), 'r').close()
+
+    assert event_queue.empty()
+
 
 @pytest.mark.flaky(max_runs=5, min_passes=1, rerun_filter=rerun_filter)
 @pytest.mark.skipif(
@@ -153,6 +181,11 @@ def test_modify():
     assert event.src_path == p('a')
     assert isinstance(event, FileModifiedEvent)
 
+    if platform.is_linux():
+        event = event_queue.get(timeout=5)[0]
+        assert event.src_path == p('a')
+        assert isinstance(event, FileClosedEvent)
+
 
 @pytest.mark.flaky(max_runs=5, min_passes=1, rerun_filter=rerun_filter)
 def test_move():
@@ -423,7 +456,7 @@ def test_renaming_top_level_directory():
         if event_queue.empty():
             break
 
-    assert all([isinstance(e, (FileCreatedEvent, FileMovedEvent, DirModifiedEvent)) for e in events])
+    assert all([isinstance(e, (FileCreatedEvent, FileMovedEvent, DirModifiedEvent, FileClosedEvent)) for e in events])
 
     for event in events:
         if isinstance(event, FileCreatedEvent):
diff --git a/tests/test_events.py b/tests/test_events.py
index f41a3f8..476f6ef 100644
--- a/tests/test_events.py
+++ b/tests/test_events.py
@@ -19,6 +19,7 @@
     FileDeletedEvent,
     FileModifiedEvent,
     FileCreatedEvent,
+    FileClosedEvent,
     DirDeletedEvent,
     DirModifiedEvent,
     DirCreatedEvent,
@@ -29,6 +30,7 @@
     EVENT_TYPE_CREATED,
     EVENT_TYPE_DELETED,
     EVENT_TYPE_MOVED,
+    EVENT_TYPE_CLOSED,
 )
 
 path_1 = '/path/xyz'
@@ -82,6 +84,14 @@ def test_file_moved_event():
     assert not event.is_synthetic
 
 
+def test_file_closed_event():
+    event = FileClosedEvent(path_1)
+    assert path_1 == event.src_path
+    assert EVENT_TYPE_CLOSED == event.event_type
+    assert not event.is_directory
+    assert not event.is_synthetic
+
+
 def test_dir_deleted_event():
     event = DirDeletedEvent(path_1)
     assert path_1 == event.src_path
@@ -111,6 +121,7 @@ def test_file_system_event_handler_dispatch():
     file_del_event = FileDeletedEvent('/path/blah.txt')
     dir_cre_event = DirCreatedEvent('/path/blah.py')
     file_cre_event = FileCreatedEvent('/path/blah.txt')
+    file_cls_event = FileClosedEvent('/path/blah.txt')
     dir_mod_event = DirModifiedEvent('/path/blah.py')
     file_mod_event = FileModifiedEvent('/path/blah.txt')
     dir_mov_event = DirMovedEvent('/path/blah.py', '/path/blah')
@@ -125,6 +136,7 @@ def test_file_system_event_handler_dispatch():
         file_del_event,
         file_cre_event,
         file_mov_event,
+        file_cls_event,
     ]
 
     class TestableEventHandler(FileSystemEventHandler):
@@ -144,6 +156,9 @@ def on_moved(self, event):
         def on_created(self, event):
             assert event.event_type == EVENT_TYPE_CREATED
 
+        def on_closed(self, event):
+            assert event.event_type == EVENT_TYPE_CLOSED
+
     handler = TestableEventHandler()
 
     for event in all_events: