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] Emit FileModifiedEvent on permission change #815

Merged
merged 3 commits into from
Jul 13, 2021

Conversation

samschott
Copy link
Contributor

@samschott samschott commented Jul 13, 2021

This PR fixes an issue on macOS where permission changes to a file would not emit a FileModifiedEvent, unlike other metadata changes. It also moves the classification of metadata changes to a separate method FSEventsEmitter._is_meta_mod().

Edit: This brings FSEvents behaviour in line with Inotify and the winapi.

alongside other metadata modifications
@samschott samschott force-pushed the fix-missed-metadata-change branch from 8f2ba5c to c1e26ae Compare July 13, 2021 10:37
@samschott samschott force-pushed the fix-missed-metadata-change branch from 7bf2bb6 to 2b8091e Compare July 13, 2021 10:50
@BoboTiG
Copy link
Collaborator

BoboTiG commented Jul 13, 2021

Excellent, thank you @samschott 🍾
It is good on my side, let me know when the PR is ready to merge :)

@samschott
Copy link
Contributor Author

The changes really are minor so I think the PR is good to go. Two test failures which I have noticed seem unrelated:

  1. Flaky behaviour with test_fsevents.py::test_watcher_deletion_while_receiving_events_2 while running the tests. Not sure where this comes from.
  2. A crash of test_emitter.py::test_recursive_off with PyPy on Windows.

@BoboTiG
Copy link
Collaborator

BoboTiG commented Jul 13, 2021

Yes, I already saw them from time to time. I merge 💪

@BoboTiG BoboTiG merged commit 8407554 into gorakhargosh:master Jul 13, 2021
@samschott samschott deleted the fix-missed-metadata-change branch July 13, 2021 11:10
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.

2 participants