diff --git a/CHANGELOG.md b/CHANGELOG.md index ea60130d..f9bf9ec9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,10 +13,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changes and fixes +- inotify: remove watcher if a watched path is renamed ([#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. + - all: return ErrClosed on Add() when the watcher is closed ([#516]) [#371]: https://github.com/fsnotify/fsnotify/pull/371 [#516]: https://github.com/fsnotify/fsnotify/pull/516 +[#518]: https://github.com/fsnotify/fsnotify/pull/518 ## [1.6.0] - 2022-10-13 diff --git a/backend_fen.go b/backend_fen.go index 47372e5a..8db297ad 100644 --- a/backend_fen.go +++ b/backend_fen.go @@ -187,11 +187,11 @@ func (w *Watcher) Close() error { // // A path can only be watched once; attempting to watch it more than once will // return an error. Paths that do not yet exist on the filesystem cannot be -// added. A watch will be automatically removed if the path is deleted. +// added. // -// A path will remain watched if it gets renamed to somewhere else on the same -// filesystem, but the monitor will get removed if the path gets deleted and -// re-created, or if it's moved to a different filesystem. +// A watch will be automatically removed if the watched path is deleted or +// renamed. The exception is the Windows backend, which doesn't remove the +// watcher on renames. // // Notifications on network filesystems (NFS, SMB, FUSE, etc.) or special // filesystems (/proc, /sys, etc.) generally don't work. diff --git a/backend_inotify.go b/backend_inotify.go index 75736ce7..355729a0 100644 --- a/backend_inotify.go +++ b/backend_inotify.go @@ -120,8 +120,8 @@ type Watcher struct { fd int mu sync.Mutex // Map access inotifyFile *os.File - watches map[string]*watch // Map of inotify watches (key: path) - paths map[int]string // Map of watched paths (key: watch descriptor) + watches map[string]*watch // Map of inotify watches (path → watch) + paths map[int]string // Map of watched paths (watch descriptor → path) done chan struct{} // Channel for sending a "quit message" to the reader goroutine doneResp chan struct{} // Channel to respond to Close } @@ -209,11 +209,11 @@ func (w *Watcher) Close() error { // // A path can only be watched once; attempting to watch it more than once will // return an error. Paths that do not yet exist on the filesystem cannot be -// added. A watch will be automatically removed if the path is deleted. +// added. // -// A path will remain watched if it gets renamed to somewhere else on the same -// filesystem, but the monitor will get removed if the path gets deleted and -// re-created, or if it's moved to a different filesystem. +// A watch will be automatically removed if the watched path is deleted or +// renamed. The exception is the Windows backend, which doesn't remove the +// watcher on renames. // // Notifications on network filesystems (NFS, SMB, FUSE, etc.) or special // filesystems (/proc, /sys, etc.) generally don't work. @@ -285,25 +285,20 @@ func (w *Watcher) Remove(name string) error { // Fetch the watch. w.mu.Lock() defer w.mu.Unlock() - watch, ok := w.watches[name] - // Remove it from inotify. + watch, ok := w.watches[name] if !ok { return fmt.Errorf("%w: %s", ErrNonExistentWatch, name) } - // We successfully removed the watch if InotifyRmWatch doesn't return an - // error, we need to clean up our internal state to ensure it matches - // inotify's kernel state. + return w.remove(name, watch) +} + +// Unlocked! +func (w *Watcher) remove(name string, watch *watch) error { delete(w.paths, int(watch.wd)) delete(w.watches, name) - // inotify_rm_watch will return EINVAL if the file has been deleted; - // the inotify will already have been removed. - // watches and pathes are deleted in ignoreLinux() implicitly and asynchronously - // by calling inotify_rm_watch() below. e.g. readEvents() goroutine receives IN_IGNORE - // so that EINVAL means that the wd is being rm_watch()ed or its file removed - // by another thread and we have not received IN_IGNORE event. success, errno := unix.InotifyRmWatch(w.fd, watch.wd) if success == -1 { // TODO: Perhaps it's not helpful to return an error here in every case; @@ -318,7 +313,6 @@ func (w *Watcher) Remove(name string) error { // are watching is deleted. return errno } - return nil } @@ -417,14 +411,23 @@ func (w *Watcher) readEvents() { // the "paths" map. w.mu.Lock() name, ok := w.paths[int(raw.Wd)] - // IN_DELETE_SELF occurs when the file/directory being watched is removed. - // This is a sign to clean up the maps, otherwise we are no longer in sync - // with the inotify kernel state which has already deleted the watch - // automatically. + // inotify will automatically remove the watch on deletes; just need + // to clean our state here. if ok && mask&unix.IN_DELETE_SELF == unix.IN_DELETE_SELF { delete(w.paths, int(raw.Wd)) delete(w.watches, name) } + // We can't really update the state when a watched path is moved; + // only IN_MOVE_SELF is sent and not IN_MOVED_{FROM,TO}. So remove + // the watch. + if ok && mask&unix.IN_MOVE_SELF == unix.IN_MOVE_SELF { + err := w.remove(name, w.watches[name]) + if err != nil { + if !w.sendError(err) { + return + } + } + } w.mu.Unlock() if nameLen > 0 { diff --git a/backend_kqueue.go b/backend_kqueue.go index 141ca835..348ef638 100644 --- a/backend_kqueue.go +++ b/backend_kqueue.go @@ -241,11 +241,11 @@ func (w *Watcher) Close() error { // // A path can only be watched once; attempting to watch it more than once will // return an error. Paths that do not yet exist on the filesystem cannot be -// added. A watch will be automatically removed if the path is deleted. +// added. // -// A path will remain watched if it gets renamed to somewhere else on the same -// filesystem, but the monitor will get removed if the path gets deleted and -// re-created, or if it's moved to a different filesystem. +// A watch will be automatically removed if the watched path is deleted or +// renamed. The exception is the Windows backend, which doesn't remove the +// watcher on renames. // // Notifications on network filesystems (NFS, SMB, FUSE, etc.) or special // filesystems (/proc, /sys, etc.) generally don't work. @@ -676,8 +676,8 @@ func (w *Watcher) sendFileCreatedEventIfNew(filePath string, fileInfo os.FileInf func (w *Watcher) internalWatch(name string, fileInfo os.FileInfo) (string, error) { if fileInfo.IsDir() { - // mimic Linux providing delete events for subdirectories - // but preserve the flags used if currently watching subdirectory + // mimic Linux providing delete events for subdirectories, but preserve + // the flags used if currently watching subdirectory w.mu.Lock() flags := w.dirFlags[name] w.mu.Unlock() diff --git a/backend_other.go b/backend_other.go index 9e2d082a..ae409b51 100644 --- a/backend_other.go +++ b/backend_other.go @@ -25,11 +25,11 @@ func (w *Watcher) Close() error { // // A path can only be watched once; attempting to watch it more than once will // return an error. Paths that do not yet exist on the filesystem cannot be -// added. A watch will be automatically removed if the path is deleted. +// added. // -// A path will remain watched if it gets renamed to somewhere else on the same -// filesystem, but the monitor will get removed if the path gets deleted and -// re-created, or if it's moved to a different filesystem. +// A watch will be automatically removed if the watched path is deleted or +// renamed. The exception is the Windows backend, which doesn't remove the +// watcher on renames. // // Notifications on network filesystems (NFS, SMB, FUSE, etc.) or special // filesystems (/proc, /sys, etc.) generally don't work. diff --git a/backend_windows.go b/backend_windows.go index fe09be87..5e62b4b4 100644 --- a/backend_windows.go +++ b/backend_windows.go @@ -196,11 +196,11 @@ func (w *Watcher) Close() error { // // A path can only be watched once; attempting to watch it more than once will // return an error. Paths that do not yet exist on the filesystem cannot be -// added. A watch will be automatically removed if the path is deleted. +// added. // -// A path will remain watched if it gets renamed to somewhere else on the same -// filesystem, but the monitor will get removed if the path gets deleted and -// re-created, or if it's moved to a different filesystem. +// A watch will be automatically removed if the watched path is deleted or +// renamed. The exception is the Windows backend, which doesn't remove the +// watcher on renames. // // Notifications on network filesystems (NFS, SMB, FUSE, etc.) or special // filesystems (/proc, /sys, etc.) generally don't work. diff --git a/fsnotify_test.go b/fsnotify_test.go index 7d3d92fb..0a3f7789 100644 --- a/fsnotify_test.go +++ b/fsnotify_test.go @@ -444,8 +444,6 @@ func TestWatchRename(t *testing.T) { `}, {"rename watched directory", func(t *testing.T, w *Watcher, tmp string) { - addWatch(t, w, tmp) - dir := filepath.Join(tmp, "dir") mkdir(t, dir) addWatch(t, w, dir) @@ -453,29 +451,14 @@ func TestWatchRename(t *testing.T) { mv(t, dir, tmp, "dir-renamed") touch(t, tmp, "dir-renamed/file") }, ` - CREATE "/dir" # mkdir - RENAME "/dir" # mv - CREATE "/dir-renamed" - RENAME "/dir" - CREATE "/dir/file" # touch - - windows: - CREATE "/dir" # mkdir - RENAME "/dir" # mv - CREATE "/dir-renamed" - CREATE "/dir-renamed/file" # touch + rename /dir - # TODO: no results for the touch; this is probably a bug; windows - # was fixed in #370. kqueue: - CREATE "/dir" # mkdir - CREATE "/dir-renamed" # mv - REMOVE|RENAME "/dir" - fen: - CREATE "/dir" # mkdir - RENAME "/dir" # mv - CREATE "/dir-renamed" - WRITE "/dir-renamed" # touch + remove|rename /dir + + # TODO(v2): Windows should behave the same by default. See #518 + windows: + create /dir/file `}, {"rename watched file", func(t *testing.T, w *Watcher, tmp string) { @@ -488,19 +471,12 @@ func TestWatchRename(t *testing.T) { mv(t, file, rename) mv(t, rename, tmp, "rename-two") }, ` - # TODO: this should update the path. And even then, not clear what - # go renamed to what. - rename /file # mv file rename - rename /file # mv rename rename-two - - # TODO: seems to lose the watch? - kqueue, fen: - rename /file + rename /file - # It's actually more correct on Windows. + # TODO(v2): Windows should behave the same by default. See #518 windows: - rename /file - rename /rename-one + rename /file + rename /rename-one `}, {"re-add renamed file", func(t *testing.T, w *Watcher, tmp string) { @@ -517,19 +493,14 @@ func TestWatchRename(t *testing.T) { cat(t, "hello", file) }, ` rename /file # mv file rename - write /rename # cat hello >rename + # Watcher gets removed on rename, so no write for /rename write /file # cat hello >file - # TODO: wrong. - linux: - RENAME "/file" - WRITE "/file" - WRITE "" - - # TODO: wrong. - kqueue, fen: - RENAME "/file" - WRITE "/file" + # TODO(v2): Windows should behave the same by default. See #518 + windows: + rename /file + write /rename + write /file `}, } diff --git a/mkdoc.zsh b/mkdoc.zsh index 664ec549..a0659b0c 100755 --- a/mkdoc.zsh +++ b/mkdoc.zsh @@ -78,11 +78,11 @@ add=$(<