inotify: remove watch when renaming a watched path

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
 

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.

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 {

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.
@@ -677,8 +677,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()

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.

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.

fsnotify_test.go

@@ -444,38 +444,21 @@ 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)
 
 			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
 		`},
 	}
 

mkdoc.zsh

@@ -78,11 +78,11 @@ add=$(<<EOF
 //
 // 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.