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

[macOS] Events without "renamed" or "modified" flags are dropped #702

Closed
samschott opened this issue Nov 21, 2020 · 16 comments
Closed

[macOS] Events without "renamed" or "modified" flags are dropped #702

samschott opened this issue Nov 21, 2020 · 16 comments

Comments

@samschott
Copy link
Contributor

samschott commented Nov 21, 2020

On macOS, all events which don't have the native kFSEventStreamEventFlagItemModified or kFSEventStreamEventFlagItemRenamed flags set are dropped and then crash the emitter thread.

I believe the offending lines are:

elif event.is_modified or event.is_inode_meta_mod or event.is_xattr_mod :
cls = DirModifiedEvent if event.is_directory else FileModifiedEvent
self.queue_event(cls(event.path))

If event.is_renamed and event.is_modified both are False, the next attributes checked are event.is_inode_meta_mod and event.is_xattr_mod. However, those attributes don't exist / are never created in the watchdog_fsevents.c extension module.

Edit: This was introduced with the changes to fsevents in #680.

@samschott samschott changed the title [macOS] All events don't have "renamed" or "modified" flags are dropped [macOS] Events without "renamed" or "modified" flags are dropped Nov 21, 2020
@BoboTiG
Copy link
Collaborator

BoboTiG commented Nov 21, 2020

@CCP-Aporia do you mind having a look?

@samschott
Copy link
Contributor Author

Looking at the conversions in the PR, it appears that @ceelian encountered a similar issue.

Also, I was surprised that this did not show up as a test failure. But then I realised that all the test_emitter tests are skipped on macOS due to #546. However, #546 does not explain why those tests are broken.

@BoboTiG
Copy link
Collaborator

BoboTiG commented Nov 22, 2020

Out of curiosity, does the issue happen when running disabled tests?

@samschott
Copy link
Contributor Author

None of the emitter tests currently pass on macOS, for apparently two reasons:

  1. The code to flush old events does not work properly:

    if platform.is_darwin():
    # FSEvents will report old events (like create for mkdtemp in test
    # setup. Waiting for a considerable time seems to 'flush' the events.
    time.sleep(10)

  2. The created event from the tmp dir crashes the emitter before the tests can start.

The tests with fsevents2 fail as well. Again for two reasons:

  1. Flushing previous events does not work.
  2. Paths returned by fsvents2 are always in bytes.

@samschott
Copy link
Contributor Author

With the patches here to fsvents2 and test_emitters, I can get most tests to pass:

tests/test_emitter.py::test_create PASSED                                                                                                                              [  5%]
tests/test_emitter.py::test_delete PASSED                                                                                                                              [ 10%]
tests/test_emitter.py::test_modify FAILED                                                                                                                              [ 15%]
tests/test_emitter.py::test_move PASSED                                                                                                                                [ 21%]
tests/test_emitter.py::test_move_to PASSED                                                                                                                             [ 26%]
tests/test_emitter.py::test_move_to_full SKIPPED                                                                                                                       [ 31%]
tests/test_emitter.py::test_move_from PASSED                                                                                                                           [ 36%]
tests/test_emitter.py::test_move_from_full SKIPPED                                                                                                                     [ 42%]
tests/test_emitter.py::test_separate_consecutive_moves PASSED                                                                                                          [ 47%]
tests/test_emitter.py::test_delete_self FAILED                                                                                                                         [ 52%]
tests/test_emitter.py::test_fast_subdirectory_creation_deletion FAILED                                                                                                 [ 57%]
tests/test_emitter.py::test_passing_unicode_should_give_unicode PASSED                                                                                                 [ 63%]
tests/test_emitter.py::test_passing_bytes_should_give_bytes PASSED                                                                                                     [ 68%]
tests/test_emitter.py::test_recursive_on PASSED                                                                                                                        [ 73%]
tests/test_emitter.py::test_recursive_off FAILED                                                                                                                       [ 78%]
tests/test_emitter.py::test_renaming_top_level_directory FAILED                                                                                                        [ 84%]
tests/test_emitter.py::test_renaming_top_level_directory_on_windows SKIPPED                                                                                            [ 89%]
tests/test_emitter.py::test_move_nested_subdirectories FAILED                                                                                                          [ 94%]
tests/test_emitter.py::test_move_nested_subdirectories_on_windows SKIPPED                                                                                              [100%]

It's mostly the nested tests which fail because some events for children of directories are not generated. This may be related to events being coalesced hierarchically, as described in the developer docs. Seeing that the new fsevents module is based on fsevents2, this may need some more work in the module itself. A "quick fix" may not be possible.

Finally, here is the full test output: test_resutls.txt

@CCP-Aporia
Copy link
Contributor

Thanks for the detailed report! I'll make some time this week to look into a fix.

@CCP-Aporia
Copy link
Contributor

To give a small update on this: I have made necessary changes to expose the missing event flags, event.is_inode_meta_mod and event.is_xattr_mod now work as expected (including all of the other so far missing event flags like event.is_mount or event.is_cloned).
I'm now working through the skipped emitter tests to make sure these work as well with the fsevents module.

@BoboTiG
Copy link
Collaborator

BoboTiG commented Nov 27, 2020

It would be awesome to have regression test working, thanks for your work 💪

@BoboTiG
Copy link
Collaborator

BoboTiG commented Dec 1, 2020

@CCP-Aporia I am coming for news :)

@CCP-Aporia
Copy link
Contributor

CCP-Aporia commented Dec 1, 2020

I'm still working on the tests (there's a "fun" memory corruption crash with Python 2.7 on Big Sur). Also I'm travelling for the next few days with limited internet access so won't be able to post an update, but will have time to hopefully figure this out.

@BoboTiG
Copy link
Collaborator

BoboTiG commented Dec 1, 2020

Rebase your branch on master, Python 2.7 and 3.4 supports have been dropped ;)

@CCP-Aporia
Copy link
Contributor

Oh I didn't realize that. It is a little bit unfortunate because we are still on Python 2.7 (and will be for the foreseeable future), so I kind of need to get that working. :-)

@BoboTiG
Copy link
Collaborator

BoboTiG commented Dec 1, 2020

I am OK to keep a Python 2 branch for some time ;) I will create it and you will be able to open a PR against it (+ backport to master). I will keep releasing watchdog for Python 2 and Python 3 then.

@BoboTiG
Copy link
Collaborator

BoboTiG commented Dec 1, 2020

About Python 2.7 and Big Sur, keep in mind that Big Sur will be officially supported on 3.8+. So if a test is not passing, let's skip it if it makes your like easier ;)

@BoboTiG
Copy link
Collaborator

BoboTiG commented Dec 1, 2020

Here it is: https://github.com/gorakhargosh/watchdog/tree/python-2.7

@CCP-Aporia
Copy link
Contributor

Thanks a lot! Will report back on Monday.

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

No branches or pull requests

3 participants