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

Fix watching non-recursive files in a symlinked directory (821) #822

Merged
merged 1 commit into from Jul 28, 2021
Merged

Fix watching non-recursive files in a symlinked directory (821) #822

merged 1 commit into from Jul 28, 2021

Conversation

NiklasRosenstein
Copy link
Contributor

@NiklasRosenstein NiklasRosenstein commented Jul 21, 2021

See #821

It appears that in fsevents, the events will contain the "real" file paths. This will break if fsevents end up generating events with the not-"real" path instead, for which I don't know if it could happen or not. The way around this would be to also use os.path.realpath() for the src_path in _is_recursive_event().

@NiklasRosenstein
Copy link
Contributor Author

@gorakhargosh I can't reproduce the test_watcher_deletion_while_receiving_events_2 test failing on my machine in Python 3.7. I've looked a bit at the test case and it seems it is looking to produce a failure that occurs randomly. Sounds to me like there could be a chance that this test just failed out of pure chance and not because the PR breaks something.

I don't have permission to re-run the tests. Could you kick it off once more? (Or provide some input on what may be going on here?)

@BoboTiG
Copy link
Collaborator

BoboTiG commented Jul 28, 2021

It seems good for me. How do you feel it @CCP-Aporia?

@CCP-Aporia
Copy link
Contributor

The change itself looks good to me. Thanks for fixing this @NiklasRosenstein !

The test failure should be unrelated to this change as well, though I am surprised to see that one being back. I thought we fixed all those issues around the watcher being None previously. 🤔

@BoboTiG
Copy link
Collaborator

BoboTiG commented Jul 28, 2021

Thanks for the confirmation @CCP-Aporia and thanks @NiklasRosenstein for the fix 👍

About the failure, it is quite random, let's not focus on it right now :)

@BoboTiG BoboTiG merged commit 426e29d into gorakhargosh:master Jul 28, 2021
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.

None yet

3 participants