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

Return byte paths if a byte path was given on macOS #726

Merged
merged 1 commit into from Dec 11, 2020

Conversation

samschott
Copy link
Contributor

This fixes an issue on macOS where file system events would always be generated with unicode paths, even if a byte path was passed to the observer.

@samschott samschott changed the title Return byte paths if a byte path was given Return byte paths if a byte path was given on macOS Dec 10, 2020
@CCP-Aporia
Copy link
Contributor

Would you be open to do this type conversion in the C extension directly? It is possible to keep track there if it was a string (= unicode) or bytes object that was passed in. And it would mean that everyone who uses NativeEvent directly gets the same behaviour.

@samschott
Copy link
Contributor Author

Sure. I have pretty much zero experience with C or Python's C API. So maybe you are better suited to make those changes?

@CCP-Aporia
Copy link
Contributor

Absolutely no problem, I'll take a stab at this over the weekend. Until then I think it's fine to have this solution as well.

@BoboTiG BoboTiG merged commit 44b9618 into gorakhargosh:master Dec 11, 2020
@BoboTiG
Copy link
Collaborator

BoboTiG commented Dec 11, 2020

Thanks :)

@samschott samschott deleted the preserve-byte-paths-macos branch January 5, 2021 13:50
BoboTiG added a commit that referenced this pull request Feb 5, 2021
* Fix installation of Python 3.7.9 on Windows (#705)

* [windows] winapi.BUFFER_SIZE now defaults to 64000 (instead of 2048)

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.

* [mac] Regression fixes for native fsevents (#717)

* [mac] Fix missing event_id attribute in fsevents (#722)

* [mac] Return byte paths if a byte path was given in fsevents (#726)

* Pin py.test to a version that supports 2.7

* Disable pypy2 tests on Windows

They hang and timeout after 6 hours for some unknown reason

* [mac] Add compatibility with old macOS versions (#733)

* Ensure version macros are available

* Uniformize event for deletion of watched dir (#727)

* [inotify] Add support for the IN_CLOSE_WRITE event (#747)

* [mac] Support coalesced fsevents (#734)

* 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.

* [mac] Improve handling of rename events (#750)

* drop support for macOS 10.12 and lower

Co-authored-by: SamSchott <ss2151@cam.ac.uk>
Co-authored-by: Boris Staletic <boris.staletic@gmail.com>
Co-authored-by: Mickaël Schoentgen <contact@tiger-222.fr>
Co-authored-by: Dustin Ingram <di@users.noreply.github.com>
Co-authored-by: ysard <ysard@users.noreply.github.com>
Co-authored-by: Lukas Šupienis <l.supienis@ncs.lt>
Co-authored-by: Lukas Šupienis <lukas.supienis@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants