-
-
Notifications
You must be signed in to change notification settings - Fork 962
Fix symlink behaviour on kqueue #524
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
Merged
Merged
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
|
Still got some build failures in CI. macOS 11 (after restart it did work): FreeBSD: NetBSD: |
76959bc to
3dabc49
Compare
fa77cc2 to
37e4300
Compare
There were two problems here: - When we see a link inside a directory the resolved version would get added to w.watches etc. but the link won't, resulting in many spurious create events for the link. This fixes #277; I'm surprised there aren't more reports for this). - filepath.EvalSymlinks() will resolve any symlinks in the entire path, rather than just the link itself. This would cause paths such as: /path/to/LINK/dir/dir/WATCH-THIS To have the wrong Event.Name; many of the test cases failed because of this because /tmp is a link to /private/tmp on macOS, but curiously no one reported it AFAIK (I guess many people don't use symlinks all that much). Example: % mkdir /tmp/xxx % touch /tmp/xxx/FILE % ln -s /tmp/xxx/LINK /tmp/xxx/FILE # 25 years of Unix experience, and still... ln: /tmp/xxx/FILE: File exists % ln -s /tmp/xxx/FILE /tmp/xxx/LINK Before it would do: % go run ./cmd/fsnotify watch /tmp/xxx & % touch /tmp/xxx/FILE 03:23:03.2731 1 CHMOD "/private/tmp/xxx/FILE" 03:23:03.2744 2 CHMOD "/tmp/xxx/FILE" ^C % fsnotify watch /tmp/xxx/LINK & % touch /tmp/xxx/LINK 03:26:37.3576 1 CHMOD "/private/tmp/xxx/FILE" And now it does: % go run ./cmd/fsnotify watch /tmp/xxx & 03:23:47.8911 1 CHMOD "/tmp/xxx/FILE" ^C % fsnotify watch /tmp/xxx/LINK & % touch /tmp/xxx/LINK 03:27:38.5227 1 CHMOD "/tmp/xxx/FILE"
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.
There were two problems here:
When we see a link inside a directory the resolved version would get added to w.watches etc. but the link won't, resulting in many spurious create events for the link. This fixes Spurious symlink CREATE events on macOS when watching a directory #277; I'm surprised there aren't more reports for this).
filepath.EvalSymlinks() will resolve any symlinks in the entire path, rather than just the link itself. This would cause paths such as:
To have the wrong Event.Name; many of the test cases failed because of this because /tmp is a link to /private/tmp on macOS, but curiously no one reported it AFAIK (I guess many people don't use symlinks all that much).
Example:
Before it would do:
And now it does: