Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Force FSEvents to use recursive mode to ensure that events are delivered #1013

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Rosuav
Copy link

@Rosuav Rosuav commented Oct 9, 2023

Code that works fine on Linux can fail on a Mac, due to inotify working fine with non-recursive but FSEvents failing. The solution is to ensure recursive mode.

Patch mandates recursive mode when using FSEvents.

Fixes #918.

@Rosuav
Copy link
Author

Rosuav commented Oct 12, 2023

Does anyone who has a Mac want to help out with this? Otherwise, I'm going to have to keep on triggering CI failures just to see how it runs.

@BoboTiG
Copy link
Collaborator

BoboTiG commented Oct 12, 2023

I could give a hand later maybe.
In the mean time, do you mind rebasing on master, and fix that one 🙏🏻:

    def test_add_watch_twice(observer: BaseObserver, p: P) -> None:
        """Adding the same watch twice used to result in a null pointer return without an exception.
    
        See https://github.com/gorakhargosh/watchdog/issues/765
        """
    
        a = p("a")
        mkdir(a)
        h = FileSystemEventHandler()
>       w = ObservedWatch(a)
E       TypeError: ObservedWatch.__init__() missing 1 required positional argument: 'recursive'

@Rosuav Rosuav force-pushed the fsevents-always-recursive branch from 296570e to b59d7b4 Compare October 12, 2023 08:33
@Rosuav
Copy link
Author

Rosuav commented Oct 12, 2023

Done and done, though honestly, I don't expect it to work this time either.

@mikejgray
Copy link

@Rosuav What needed to be done on this one? I'm running into the same issue and would be happy to help, whether it's contributing code or just testing on my Macs

@Rosuav
Copy link
Author

Rosuav commented Apr 20, 2024

@Rosuav What needed to be done on this one?

Testing. The existing code works just fine on Linux, so all I can really do is make sure that my attempts don't actually break things. I have no way to see whether it's any improvement.

So if you can grab the code, try it out, and see whether it works, that would be immensely helpful. You can quickly iterate on it, make small changes and test, without waiting for the whole "commit, push, wait for CI, read the log" thing. As you can see, I tend to wander off and do other things (you know how it is, always myriad other things to look into).

@mikejgray
Copy link

@Rosuav What needed to be done on this one?

Testing. The existing code works just fine on Linux, so all I can really do is make sure that my attempts don't actually break things. I have no way to see whether it's any improvement.

So if you can grab the code, try it out, and see whether it works, that would be immensely helpful. You can quickly iterate on it, make small changes and test, without waiting for the whole "commit, push, wait for CI, read the log" thing. As you can see, I tend to wander off and do other things (you know how it is, always myriad other things to look into).

As much as I'd love to be helpful in the testing process, it seems (at least on Apple Silicon) that there is nothing specifically to test:

Success: no issues found in 48 source files
  py312: OK (122.61=setup[30.82]+cmd[91.79] seconds)
  py311: OK (118.76=setup[21.11]+cmd[97.65] seconds)
  py310: OK (114.51=setup[22.61]+cmd[91.90] seconds)
  py39: OK (119.55=setup[24.20]+cmd[95.35] seconds)
  py38: OK (117.34=setup[20.43]+cmd[96.91] seconds)
  pypy3: SKIP (0.00 seconds)
  docs: OK (20.40=setup[18.41]+cmd[1.99] seconds)
  mypy: OK (22.84=setup[13.36]+cmd[9.48] seconds)

However, when I tried a practical local test, it didn't seem like it was working. It's entirely possible I'm misunderstanding how FileSystemEventHandler.on_any_event works though.

import os
from unittest.mock import Mock
from src.watchdog.events import FileSystemEventHandler
from src.watchdog.observers import Observer


change_comes_from_within = Mock()


class FileEventHandler(FileSystemEventHandler):
    def on_any_event(self, event):
        change_comes_from_within()


observer = Observer()
event_handler = FileEventHandler()
w = observer.schedule(event_handler, ".")
observer.start()
with open("test.txt", "w", encoding="utf-8") as f:
    f.write("hello")
os.remove("test.txt")
change_comes_from_within.assert_any_call()

Output

❯ /Users/Mike/Documents/coding/watchdog/.venv/bin/python /Users/Mike/Documents/coding/watchdog/mac-test.py
Traceback (most recent call last):
  File "/Users/Mike/Documents/coding/watchdog/mac-test.py", line 26, in <module>
    change_comes_from_within.assert_any_call()
  File "/Users/Mike/.pyenv/versions/3.12.0/lib/python3.12/unittest/mock.py", line 1015, in assert_any_call
    raise AssertionError(
AssertionError: mock() call not found

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No file system events observed when not using recursive mode
3 participants