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

Backport changes since v1.0 to Python 2.7 #756

Merged
merged 43 commits into from Feb 5, 2021

Conversation

CCP-Aporia
Copy link
Contributor

Should cover about everything, except for #703 because Python 2.7 doesn't provide required functionality.

A couple of notes:

  • GitHub Actions based CI is available in .github/workflows/testrunner.yaml - it should be trivial to port that file to master as well if desired.
  • The wheel builder might not be what we want? My experience is rather limited in this field.
  • Some of the tests appear to be flaky. Notably, pypy2 seems to have random issues with the C extension macOS. Additionally the polling observer sometimes complains about additional events every now and then. I couldn't figure out what's going on with either of these two.

SamSchott and others added 30 commits November 23, 2020 21:32
* drop python 2.7 support

* switch to os.fsencode / os.fsdecode on Unixes

* updated tests

* remove Python 2.7 from CI

* removed obsolete __future__ imports

* removed Py 2.7 references from docs

* removed Py 2.7 references from readme

* add test for path with invalid bytes

* add entries to changelog
To handle cases with lot of changes, this seems the highest safest value we can use.

Note: it will fail with `ERROR_INVALID_PARAMETER` when it is greater than 64 KB and
      the application is monitoring a directory over the network.
      This is due to a packet size limitation with the underlying file sharing protocols.
      https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-readdirectorychangesw#remarks

Also introduced `winapi.PATH_BUFFER_SIZE` (defaults to `2048`)
to keep the old behavior with path-realted functions.
They hang and timeout after 6 hours for some unknown reason
* Add compatibility with old macOS versions

* Define symbols only for older versions

And not for older or equal, since the equal version is when the symbol
was introduced.

* Ensure version macros are available

* Update changelog
* monitor our own root

* properly handle root change event

* test on all platforms that root deleted event is emitted ...

... and that emitter is stopped

* basic test for root moved

* winapi: emit DirDeletedEvent of removed root

* removed leftover commented-out code

* expect KeyError when unscheduling removed watch

* propagate root deleted event in inotify

* remove `test_move_self`

* catch OSError when stopping watch in fixture

* adapt test_inotify_c to stopped emitter after delete_self

* don't emit root deleted for child watches
* Actually use the input in the workflow

* Formatting

* Update description for input
…#746)

Since CPython 3.7 `PyEval_InitThreads()` does nothing and the function
has been deprecated since CPython 3.9.
* add: inotify file close events

* fix: NOWRITE_CLOSE doesn't modify dir

* update changelog & version

* add: test events

* Revert "update changelog & version"

This reverts commit 69d61d0.

* update changelog

* [gnu/linux] Fix ability to catch IN_CLOSE_WRITE, IN_CLOSE_NOWRITE events gorakhargosh#690

* [gnu/linux] Test IN_CLOSE_WRITE, IN_CLOSE_NOWRITE events

* Fix typos

* [gnu/linux] Fix test due to IN_CLOSE_NOWRITE event

* [gnu/linux] Revert IN_CLOSE_NOWRITE support gorakhargosh#747

* [gnu/linux] Extend test for IN_CLOSE_WRITE support gorakhargosh#747

* fix typos

* Update changelog gorakhargosh#747, gorakhargosh#690

Co-authored-by: Lukas Šupienis <l.supienis@ncs.lt>
Co-authored-by: Lukas Šupienis <lukas.supienis@gmail.com>
* Add `is_coalesced` property to `NativeEvent`

So that we can effectively decide if we need to perform additional
system calls to figure out what really happened.

* Replace `NativeEvent._event_type` with `repr()` support

It's more pythonic, and the `_event_type` implementation wasn't quite
usable anyway.

NB: the representation is not truly copy/paste python code if there is
a double quote inside event.path, but that should be a rare case so we
don't add the expensive special case handling there.

* Allow running tests with debugger attached

Some Python debuggers create additional threads, so we shouldn't assume that there is only one.

* Request notifications for watched root

* Expect events on macOS instead of using `time.sleep()`

It might be even better to check for the emitter class, as opposed to platform

* Add exception handling to FSEventsEmitter

Reduce the amount of 'silent breakage'

* Use sentinel event when setting up tests on macOS

So that we can avoid a race between test setup and fseventsd

* Improve handling of coalesced events

* Revert accidental platform check change

* Fix renaming_top_level_directory test on macOS

* Generate sub events for move operations

* Remove `filesystem_view` again

While the `filesystem_view` helps with filtering out additional
`FileCreatedEvent`+`DirModifiedEvent` pairs then it also introduces
a huge amount of edge cases for synthetic events caused by move and
rename operations. On top of that, in order to properly resolve
those edge cases we'd have to go back to a solution very similar to
the old directory snapshots, with all the performance penalties they
suffered from...

As such I think it's better to acknowledge the behaviour for coalesced
events instead, and thus remove the `filesystem_view` again.

* Update Changelog
SamSchott and others added 8 commits January 21, 2021 18:20
* drop support for macOS 10.12 and lower

* record inodes for all events

* use inode to match rename events

* add flush method

* update tests to new NativeEvent

* simplify handling of created and removed events

* added test for case-change

* clean up loop over native events

Co-authored-by: Mickaël Schoentgen <contact@tiger-222.fr>

* added changelog entries

* remove unneeded Py_INCREF calls

* Revert "simplify handling of created and removed events"

This reverts commit 75afbd2.

* allow inode to be None

this is the case for kFSEventStreamEventFlagRootChanged

* join event_dispatcher in `test_event_dispatcher`

* fix memory management for inode

* better logs for macOS tests

* show flags in log output

* fix returning inode attribute

* tweak error message for unsupported macOS

Co-authored-by: Mickaël Schoentgen <contact@tiger-222.fr>

* emit debug instead of info logs from `queue_events`

Co-authored-by: Mickaël Schoentgen <contact@tiger-222.fr>
Backports almost all changes from the current 1.0.x to python-2.7.

Notable exceptions are:
- Wheels are not built yet as the CI pipeline hasn't been adapted yet
- Non-file-system-encoded paths on UNIX might not be working as intended, need to test it.
We want to know about all failures.
This should get pretty close to what Python 3's `os.fsencode` / `os.fsdecode`
functions do.
This reverts commit b1c1b28.

The `surrogateescape` error handler doesn't exist until Python 3.
Unfortunately gorakhargosh#703 cannot easily be backported, so we won't have this
functionality in the Python 2.7 / 3.5 branch unless someone reports
this as an issue.
@BoboTiG
Copy link
Collaborator

BoboTiG commented Feb 3, 2021

Outch, this is a huge one. Part of those changes are not Python 2.7 compatible.
I prefer to open an issue and list/ask which commits you would like to see backported.

@BoboTiG BoboTiG linked an issue Feb 3, 2021 that may be closed by this pull request
@CCP-Aporia
Copy link
Contributor Author

CCP-Aporia commented Feb 3, 2021

As stated in the description, this back ports all changes except for the commits in #703 and anything that happened since this PR was opened. :-) The CI for it is (mostly) passing, too, with exception of what appears to be an ordering issue in the PollingObserver test and some unknown pypy-2.7 issues on macOS.

@BoboTiG
Copy link
Collaborator

BoboTiG commented Feb 3, 2021

OK, I'll review then :)

src/watchdog/observers/kqueue.py Outdated Show resolved Hide resolved
src/watchdog/observers/winapi.py Outdated Show resolved Hide resolved
src/watchdog/observers/winapi.py Outdated Show resolved Hide resolved
As pointed out in the PR review, this will be updated for Python 2.7 in the master branch
@CCP-Aporia
Copy link
Contributor Author

CCP-Aporia commented Feb 5, 2021

Thanks a lot for the review and spotting the few spots that I missed while back porting! :-)

The CI is all green now as well

As per code review feedback
Using a more descriptive name as suggested in the PR review.

Co-authored-by: Mickaël Schoentgen <contact@tiger-222.fr>
changelog.rst Outdated Show resolved Hide resolved
@BoboTiG BoboTiG merged commit e7f29d1 into gorakhargosh:python-2.7 Feb 5, 2021
@BoboTiG
Copy link
Collaborator

BoboTiG commented Feb 5, 2021

@CCP-Aporia I guess you tested the python-2.7 branch? Before doing a release, I would like you input :)

@CCP-Aporia
Copy link
Contributor Author

Thanks for merging! :-)
I've tested the branch to the extend of the test suite, and also integrated the current state of my branch in our product where it works just fine. However, we're really just relying on FileModifiedEvent there, so it doesn't really expand too much on the test suite other than running in a (very) complex environment on Windows and macOS.

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.

Python 2.7 backports for the next release
4 participants