-
-
Notifications
You must be signed in to change notification settings - Fork 962
Return ErrClosed on Add() when the watcher is closed #516
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
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
Author
|
CI failures are unrelated to this PR. |
nshalman
approved these changes
Oct 13, 2022
Contributor
nshalman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some nits, but this seems reasonable to me.
Thanks for fixing the illumos CI!
Also consistently return nil for Remove() and WatchList(). For Remove() we could return an error, but I don't really see the point: if the watcher is closed it's basically a no-op so returning nil is fine. For WatchList() I'd like to return an error, but there is no error return, so explicitly returning nil is the best we can do. Fixes #511
nshalman
approved these changes
Oct 13, 2022
25 tasks
belimawr
added a commit
to belimawr/fsnotify
that referenced
this pull request
Sep 25, 2024
Release 1.7.0 This version of fsnotify needs Go 1.17. Additions: - illumos: add FEN backend to support illumos and Solaris. ([fsnotify#371]) - all: add `NewBufferedWatcher()` to use a buffered channel, which can be useful in cases where you can't control the kernel buffer and receive a large number of events in bursts. ([fsnotify#550], [fsnotify#572]) - all: add `AddWith()`, which is identical to `Add()` but allows passing options. ([fsnotify#521]) - windows: allow setting the ReadDirectoryChangesW() buffer size with `fsnotify.WithBufferSize()`; the default of 64K is the highest value that works on all platforms and is enough for most purposes, but in some cases a highest buffer is needed. ([fsnotify#521]) Changes and fixes: - inotify: remove watcher if a watched path is renamed ([fsnotify#518]) After a rename the reported name wasn't updated, or even an empty string. Inotify doesn't provide any good facilities to update it, so just remove the watcher. This is already how it worked on kqueue and FEN. On Windows this does work, and remains working. - windows: don't listen for file attribute changes ([fsnotify#520]) File attribute changes are sent as `FILE_ACTION_MODIFIED` by the Windows API, with no way to see if they're a file write or attribute change, so would show up as a fsnotify.Write event. This is never useful, and could result in many spurious Write events. - windows: return `ErrEventOverflow` if the buffer is full ([fsnotify#525]) Before it would merely return "short read", making it hard to detect this error. - kqueue: make sure events for all files are delivered properly when removing a watched directory ([fsnotify#526]) Previously they would get sent with `""` (empty string) or `"."` as the path name. - kqueue: don't emit spurious Create events for symbolic links ([fsnotify#524]) The link would get resolved but kqueue would "forget" it already saw the link itself, resulting on a Create for every Write event for the directory. - all: return `ErrClosed` on `Add()` when the watcher is closed ([fsnotify#516]) - other: add `Watcher.Errors` and `Watcher.Events` to the no-op `Watcher` in `backend_other.go`, making it easier to use on unsupported platforms such as WASM, AIX, etc. ([fsnotify#528]) - other: use the `backend_other.go` no-op if the `appengine` build tag is set; Google AppEngine forbids usage of the unsafe package so the inotify backend won't compile there. [fsnotify#371]: fsnotify#371 [fsnotify#516]: fsnotify#516 [fsnotify#518]: fsnotify#518 [fsnotify#520]: fsnotify#520 [fsnotify#521]: fsnotify#521 [fsnotify#524]: fsnotify#524 [fsnotify#525]: fsnotify#525 [fsnotify#526]: fsnotify#526 [fsnotify#528]: fsnotify#528 [fsnotify#537]: fsnotify#537 [fsnotify#550]: fsnotify#550 [fsnotify#572]: fsnotify#572
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Also consistently return nil for Remove() and WatchList(). For Remove() we could return an error, but I don't really see the point: if the watcher is closed it's basically a no-op so returning nil is fine.
For WatchList() I'd like to return an error, but there is no error return, so explicitly returning nil is the best we can do.
Fixes #511