From 3d1b8301be7e37e825b381d4761e73632d61d176 Mon Sep 17 00:00:00 2001 From: Martin Tournoij Date: Wed, 16 Nov 2022 02:10:05 +0100 Subject: [PATCH] Add recursive watcher for Windows backend Recursive watches can be added by using a "/..." parameter, similar to the Go command: w.Add("dir") // Behaves as before. w.Add("dir/...") // Watch dir and all paths underneath it. w.Remove("dir") // Remove the watch for dir and, if // recursive, all paths underneath it too w.Remove("dir/...") // Behaves like just "dir" if the path was // recursive, error otherwise (probably // want to add recursive remove too at some // point). The advantage of using "/..." vs. an option is that it can be easily specified in configuration files and the like; for example from a TOML file: [watches] dirs = ["/tmp/one", "/tmp/two/..."] Options for this were previously discussed at: https://github.com/fsnotify/fsnotify/pull/339#discussion_r788246013 This should be expanded to other backends too; I started with Windows because the implementation is the both the easiest and has the least amount of control (just setting a boolean parameter), and we can focus mostly on writing tests and documentation and the for it, and we can then match the inotify and kqueue behaviour to the Windows one. Fixes #21 --- CHANGELOG.md | 5 +- backend_fen.go | 18 ++++-- backend_inotify.go | 18 ++++-- backend_inotify_test.go | 1 + backend_kqueue.go | 18 ++++-- backend_other.go | 18 ++++-- backend_windows.go | 51 +++++++++++------ fsnotify.go | 10 ++++ fsnotify_test.go | 119 +++++++++++++++++++++++++++++++++++++++- helpers_test.go | 35 +++++++----- mkdoc.zsh | 18 ++++-- 11 files changed, 254 insertions(+), 57 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 31fa1a47..5e08ba52 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ Unreleased - all: add `AddWith()`, which is identical to `Add()` but allows passing options. ([#521]) +- all: support recursively watching paths with `Add("path/...")`. ([#540]) + - windows: allow setting the 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. ([#521]) @@ -46,7 +48,7 @@ Unreleased - 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 work there. + won't compile there. [#371]: https://github.com/fsnotify/fsnotify/pull/371 @@ -58,6 +60,7 @@ Unreleased [#526]: https://github.com/fsnotify/fsnotify/pull/526 [#528]: https://github.com/fsnotify/fsnotify/pull/528 [#537]: https://github.com/fsnotify/fsnotify/pull/537 +[#540]: https://github.com/fsnotify/fsnotify/pull/540 1.6.0 - 2022-10-13 ------------------- diff --git a/backend_fen.go b/backend_fen.go index 255e5334..d2b985b7 100644 --- a/backend_fen.go +++ b/backend_fen.go @@ -184,7 +184,7 @@ 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. +// watched. // // 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 @@ -200,8 +200,9 @@ func (w *Watcher) Close() error { // # Watching directories // // All files in a directory are monitored, including new files that are created -// after the watcher is started. Subdirectories are not watched (i.e. it's -// non-recursive). +// after the watcher is started. By default subdirectories are not watched (i.e. +// it's non-recursive), but if the path ends with "/..." all files and +// subdirectories are watched too. // // # Watching files // @@ -266,8 +267,15 @@ func (w *Watcher) AddWith(name string, opts ...addOpt) error { // Remove stops monitoring the path for changes. // -// Directories are always removed non-recursively. For example, if you added -// /tmp/dir and /tmp/dir/subdir then you will need to remove both. +// If the path was added as a recursive watch (e.g. as "/tmp/dir/...") then the +// entire recusrice watch will be removed. You can use both "/tmp/dir" and +// "/tmp/dir/..." (they behave identical). +// +// You cannot remove individual files or subdirectories from recursive watches; +// e.g. Add("/tmp/path/...") and then Remove("/tmp/path/sub") will fail. +// +// For other watches directories are removed non-recursively. For example, if +// you added "/tmp/dir" and "/tmp/dir/subdir" then you will need to remove both. // // Removing a path that has not yet been added returns [ErrNonExistentWatch]. // diff --git a/backend_inotify.go b/backend_inotify.go index 2f47f4da..47fc9920 100644 --- a/backend_inotify.go +++ b/backend_inotify.go @@ -206,7 +206,7 @@ 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. +// watched. // // 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 @@ -222,8 +222,9 @@ func (w *Watcher) Close() error { // # Watching directories // // All files in a directory are monitored, including new files that are created -// after the watcher is started. Subdirectories are not watched (i.e. it's -// non-recursive). +// after the watcher is started. By default subdirectories are not watched (i.e. +// it's non-recursive), but if the path ends with "/..." all files and +// subdirectories are watched too. // // # Watching files // @@ -281,8 +282,15 @@ func (w *Watcher) AddWith(name string, opts ...addOpt) error { // Remove stops monitoring the path for changes. // -// Directories are always removed non-recursively. For example, if you added -// /tmp/dir and /tmp/dir/subdir then you will need to remove both. +// If the path was added as a recursive watch (e.g. as "/tmp/dir/...") then the +// entire recusrice watch will be removed. You can use both "/tmp/dir" and +// "/tmp/dir/..." (they behave identical). +// +// You cannot remove individual files or subdirectories from recursive watches; +// e.g. Add("/tmp/path/...") and then Remove("/tmp/path/sub") will fail. +// +// For other watches directories are removed non-recursively. For example, if +// you added "/tmp/dir" and "/tmp/dir/subdir" then you will need to remove both. // // Removing a path that has not yet been added returns [ErrNonExistentWatch]. // diff --git a/backend_inotify_test.go b/backend_inotify_test.go index c857291a..6965ee19 100644 --- a/backend_inotify_test.go +++ b/backend_inotify_test.go @@ -93,6 +93,7 @@ func TestInotifyDeleteOpenFile(t *testing.T) { w.collect(t) rm(t, file) + eventSeparator() e := w.events(t) cmpEvents(t, tmp, e, newEvents(t, `chmod /file`)) diff --git a/backend_kqueue.go b/backend_kqueue.go index b83e7798..ddb08a25 100644 --- a/backend_kqueue.go +++ b/backend_kqueue.go @@ -237,7 +237,7 @@ 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. +// watched. // // 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 @@ -253,8 +253,9 @@ func (w *Watcher) Close() error { // # Watching directories // // All files in a directory are monitored, including new files that are created -// after the watcher is started. Subdirectories are not watched (i.e. it's -// non-recursive). +// after the watcher is started. By default subdirectories are not watched (i.e. +// it's non-recursive), but if the path ends with "/..." all files and +// subdirectories are watched too. // // # Watching files // @@ -288,8 +289,15 @@ func (w *Watcher) AddWith(name string, opts ...addOpt) error { // Remove stops monitoring the path for changes. // -// Directories are always removed non-recursively. For example, if you added -// /tmp/dir and /tmp/dir/subdir then you will need to remove both. +// If the path was added as a recursive watch (e.g. as "/tmp/dir/...") then the +// entire recusrice watch will be removed. You can use both "/tmp/dir" and +// "/tmp/dir/..." (they behave identical). +// +// You cannot remove individual files or subdirectories from recursive watches; +// e.g. Add("/tmp/path/...") and then Remove("/tmp/path/sub") will fail. +// +// For other watches directories are removed non-recursively. For example, if +// you added "/tmp/dir" and "/tmp/dir/subdir" then you will need to remove both. // // Removing a path that has not yet been added returns [ErrNonExistentWatch]. // diff --git a/backend_other.go b/backend_other.go index bbf85a2f..ba7b51ea 100644 --- a/backend_other.go +++ b/backend_other.go @@ -119,7 +119,7 @@ func (w *Watcher) WatchList() []string { return nil } // // 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. +// watched. // // 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 @@ -135,8 +135,9 @@ func (w *Watcher) WatchList() []string { return nil } // # Watching directories // // All files in a directory are monitored, including new files that are created -// after the watcher is started. Subdirectories are not watched (i.e. it's -// non-recursive). +// after the watcher is started. By default subdirectories are not watched (i.e. +// it's non-recursive), but if the path ends with "/..." all files and +// subdirectories are watched too. // // # Watching files // @@ -162,8 +163,15 @@ func (w *Watcher) AddWith(name string, opts ...addOpt) error { return nil } // Remove stops monitoring the path for changes. // -// Directories are always removed non-recursively. For example, if you added -// /tmp/dir and /tmp/dir/subdir then you will need to remove both. +// If the path was added as a recursive watch (e.g. as "/tmp/dir/...") then the +// entire recusrice watch will be removed. You can use both "/tmp/dir" and +// "/tmp/dir/..." (they behave identical). +// +// You cannot remove individual files or subdirectories from recursive watches; +// e.g. Add("/tmp/path/...") and then Remove("/tmp/path/sub") will fail. +// +// For other watches directories are removed non-recursively. For example, if +// you added "/tmp/dir" and "/tmp/dir/subdir" then you will need to remove both. // // Removing a path that has not yet been added returns [ErrNonExistentWatch]. // diff --git a/backend_windows.go b/backend_windows.go index 2dd9aff6..1d7365a0 100644 --- a/backend_windows.go +++ b/backend_windows.go @@ -193,7 +193,7 @@ 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. +// watched. // // 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 @@ -209,8 +209,9 @@ func (w *Watcher) Close() error { // # Watching directories // // All files in a directory are monitored, including new files that are created -// after the watcher is started. Subdirectories are not watched (i.e. it's -// non-recursive). +// after the watcher is started. By default subdirectories are not watched (i.e. +// it's non-recursive), but if the path ends with "/..." all files and +// subdirectories are watched too. // // # Watching files // @@ -258,8 +259,15 @@ func (w *Watcher) AddWith(name string, opts ...addOpt) error { // Remove stops monitoring the path for changes. // -// Directories are always removed non-recursively. For example, if you added -// /tmp/dir and /tmp/dir/subdir then you will need to remove both. +// If the path was added as a recursive watch (e.g. as "/tmp/dir/...") then the +// entire recusrice watch will be removed. You can use both "/tmp/dir" and +// "/tmp/dir/..." (they behave identical). +// +// You cannot remove individual files or subdirectories from recursive watches; +// e.g. Add("/tmp/path/...") and then Remove("/tmp/path/sub") will fail. +// +// For other watches directories are removed non-recursively. For example, if +// you added "/tmp/dir" and "/tmp/dir/subdir" then you will need to remove both. // // Removing a path that has not yet been added returns [ErrNonExistentWatch]. // @@ -362,13 +370,14 @@ type inode struct { } type watch struct { - ov windows.Overlapped - ino *inode // i-number - path string // Directory path - mask uint64 // Directory itself is being watched with these notify flags - names map[string]uint64 // Map of names being watched and their notify flags - rename string // Remembers the old name while renaming a file - buf []byte // buffer, allocated later + ov windows.Overlapped + ino *inode // i-number + recurse bool // Recursive watch? + path string // Directory path + mask uint64 // Directory itself is being watched with these notify flags + names map[string]uint64 // Map of names being watched and their notify flags + rename string // Remembers the old name while renaming a file + buf []byte // buffer, allocated later } type ( @@ -442,6 +451,7 @@ func (m watchMap) set(ino *inode, watch *watch) { // Must run within the I/O thread. func (w *Watcher) addWatch(pathname string, flags uint64, bufsize int) error { + pathname, recurse := recursivePath(pathname) dir, err := w.getDir(pathname) if err != nil { return err @@ -461,10 +471,11 @@ func (w *Watcher) addWatch(pathname string, flags uint64, bufsize int) error { return os.NewSyscallError("CreateIoCompletionPort", err) } watchEntry = &watch{ - ino: ino, - path: dir, - names: make(map[string]uint64), - buf: make([]byte, bufsize), + ino: ino, + path: dir, + names: make(map[string]uint64), + recurse: recurse, + buf: make([]byte, bufsize), } w.mu.Lock() w.watches.set(ino, watchEntry) @@ -494,6 +505,8 @@ func (w *Watcher) addWatch(pathname string, flags uint64, bufsize int) error { // Must run within the I/O thread. func (w *Watcher) remWatch(pathname string) error { + pathname, recurse := recursivePath(pathname) + dir, err := w.getDir(pathname) if err != nil { return err @@ -507,6 +520,10 @@ func (w *Watcher) remWatch(pathname string) error { watch := w.watches.get(ino) w.mu.Unlock() + if recurse && !watch.recurse { + return fmt.Errorf("can't use /... with non-recursive watch %q", pathname) + } + err = windows.CloseHandle(ino.handle) if err != nil { w.sendError(os.NewSyscallError("CloseHandle", err)) @@ -568,7 +585,7 @@ func (w *Watcher) startRead(watch *watch) error { hdr := (*reflect.SliceHeader)(unsafe.Pointer(&watch.buf)) rdErr := windows.ReadDirectoryChanges(watch.ino.handle, (*byte)(unsafe.Pointer(hdr.Data)), uint32(hdr.Len), - false, mask, nil, &watch.ov, 0) + watch.recurse, mask, nil, &watch.ov, 0) if rdErr != nil { err := os.NewSyscallError("ReadDirectoryChanges", rdErr) if rdErr == windows.ERROR_ACCESS_DENIED && watch.mask&provisional == 0 { diff --git a/fsnotify.go b/fsnotify.go index 142169da..cf7caab5 100644 --- a/fsnotify.go +++ b/fsnotify.go @@ -12,6 +12,7 @@ package fsnotify import ( "errors" "fmt" + "path/filepath" "strings" ) @@ -131,3 +132,12 @@ func getOptions(opts ...addOpt) withOpts { func WithBufferSize(bytes int) addOpt { return func(opt *withOpts) { opt.bufsize = bytes } } + +// Check if this path is recursive (ends with "/..."), and return the path with +// the /... stripped. +func recursivePath(path string) (string, bool) { + if filepath.Base(path) == "..." { + return filepath.Dir(path), true + } + return path, false +} diff --git a/fsnotify_test.go b/fsnotify_test.go index 12a284be..5a206b8d 100644 --- a/fsnotify_test.go +++ b/fsnotify_test.go @@ -656,7 +656,7 @@ func TestWatchAttrib(t *testing.T) { } } -func TestWatchRm(t *testing.T) { +func TestWatchRemove(t *testing.T) { tests := []testCase{ {"remove watched file", func(t *testing.T, w *Watcher, tmp string) { file := join(tmp, "file") @@ -759,6 +759,106 @@ func TestWatchRm(t *testing.T) { WRITE "/j" WRITE "/j" `}, + + {"remove recursive", func(t *testing.T, w *Watcher, tmp string) { + recurseOnly(t) + + mkdirAll(t, tmp, "dir1", "subdir") + mkdirAll(t, tmp, "dir2", "subdir") + touch(t, tmp, "dir1", "subdir", "file") + touch(t, tmp, "dir2", "subdir", "file") + + addWatch(t, w, tmp, "dir1", "...") + addWatch(t, w, tmp, "dir2", "...") + cat(t, "dir1", "subdir", "file") + cat(t, "dir2", "subdir", "file") + + if err := w.Remove(join(tmp, "dir1")); err != nil { + t.Fatal(err) + } + if err := w.Remove(join(tmp, "dir2", "...")); err != nil { + t.Fatal(err) + } + + if w := w.WatchList(); len(w) != 0 { + t.Errorf("WatchList not empty: %s", w) + } + + cat(t, "dir1", "subdir", "file") + cat(t, "dir2", "subdir", "file") + }, ` + write /dir1/subdir/file + write /dir2/subdir/file + `}, + } + + for _, tt := range tests { + tt := tt + tt.run(t) + } +} + +func TestWatchRecursive(t *testing.T) { + recurseOnly(t) + + tests := []testCase{ + // Make a nested directory tree, then write some files there. + {"basic", func(t *testing.T, w *Watcher, tmp string) { + mkdirAll(t, tmp, "/one/two/three/four") + addWatch(t, w, tmp, "/...") + + cat(t, "asd", tmp, "/file.txt") + cat(t, "asd", tmp, "/one/two/three/file.txt") + }, ` + create /file.txt + write /file.txt + create /one/two/three/file.txt + write /one/two/three/file.txt + `}, + + // Create a new directory tree and then some files under that. + {"add directory", func(t *testing.T, w *Watcher, tmp string) { + mkdirAll(t, tmp, "/one/two/three/four") + addWatch(t, w, tmp, "/...") + + mkdirAll(t, tmp, "/one/two/new/dir") + touch(t, tmp, "/one/two/new/file") + touch(t, tmp, "/one/two/new/dir/file") + }, ` + create /one/two/new + create /one/two/new/file + create /one/two/new/dir/file + `}, + + // Remove nested directory + {"remove directory", func(t *testing.T, w *Watcher, tmp string) { + mkdirAll(t, tmp, "/one/two/three/four") + addWatch(t, w, tmp, "...") + + cat(t, "asd", tmp, "one/two/three/file.txt") + rmAll(t, tmp, "one/two") + }, ` + create "/one/two/three/file.txt" + remove "/one/two/three/four" + remove "/one/two/three" + remove "/one/two" + `}, + + // Rename nested directory + {"rename directory", func(t *testing.T, w *Watcher, tmp string) { + mkdirAll(t, tmp, "/one/two/three/four") + addWatch(t, w, tmp, "...") + + mv(t, filepath.Join(tmp, "one"), tmp, "one-rename") + touch(t, tmp, "one-rename/file") + touch(t, tmp, "one-rename/two/three/file") + }, ` + rename /one + create /one-rename + rename /one-rename + create /one-rename/file + create /one-rename/two/three/file + `}, } for _, tt := range tests { @@ -1063,6 +1163,23 @@ func TestRemove(t *testing.T) { w.Close() } }) + + t.Run("remove with ... when non-recursive", func(t *testing.T) { + recurseOnly(t) + t.Parallel() + + tmp := t.TempDir() + w := newWatcher(t) + addWatch(t, w, tmp) + + if err := w.Remove(join(tmp, "...")); err == nil { + t.Fatal("err was nil") + } + if err := w.Remove(tmp); err != nil { + t.Fatal(err) + } + }) + } func TestEventString(t *testing.T) { diff --git a/helpers_test.go b/helpers_test.go index 85c1fc2b..24f00299 100644 --- a/helpers_test.go +++ b/helpers_test.go @@ -138,19 +138,19 @@ func mkdir(t *testing.T, path ...string) { } // mkdir -p -// func mkdirAll(t *testing.T, path ...string) { -// t.Helper() -// if len(path) < 1 { -// t.Fatalf("mkdirAll: path must have at least one element: %s", path) -// } -// err := os.MkdirAll(join(path...), 0o0755) -// if err != nil { -// t.Fatalf("mkdirAll(%q): %s", join(path...), err) -// } -// if shouldWait(path...) { -// eventSeparator() -// } -// } +func mkdirAll(t *testing.T, path ...string) { + t.Helper() + if len(path) < 1 { + t.Fatalf("mkdirAll: path must have at least one element: %s", path) + } + err := os.MkdirAll(join(path...), 0o0755) + if err != nil { + t.Fatalf("mkdirAll(%q): %s", join(path...), err) + } + if shouldWait(path...) { + eventSeparator() + } +} // ln -s func symlink(t *testing.T, target string, link ...string) { @@ -576,3 +576,12 @@ func isSolaris() bool { } return false } + +func recurseOnly(t *testing.T) { + switch runtime.GOOS { + case "windows": + // Run test. + default: + t.Skip("recursion not yet supported on " + runtime.GOOS) + } +} diff --git a/mkdoc.zsh b/mkdoc.zsh index 228061e1..95f4a8fd 100755 --- a/mkdoc.zsh +++ b/mkdoc.zsh @@ -69,7 +69,7 @@ add=$(<