Skip to content

Commit

Permalink
watch: tweak dir change events
Browse files Browse the repository at this point in the history
fixes #6485

Signed-off-by: Nick Santos <[email protected]>
  • Loading branch information
nicks committed Jan 16, 2025
1 parent 5acd4ad commit 33925ac
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 6 deletions.
9 changes: 9 additions & 0 deletions internal/ospath/ospath.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,15 @@ func IsDir(path string) bool {
return f.Mode().IsDir()
}

func IsDirLstat(path string) bool {
f, err := os.Lstat(path)
if err != nil {
return false
}

return f.Mode().IsDir()
}

func IsBrokenSymlink(path string) (bool, error) {
// Stat resolves symlinks, lstat does not.
// So if Stat reports IsNotExist, but Lstat does not,
Expand Down
17 changes: 17 additions & 0 deletions internal/watch/notify_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,23 @@ func TestWatchDirectoryAndTouchIt(t *testing.T) {
f.assertEvents()
}

func TestWatchDirectoryAndTouchSubdir(t *testing.T) {
f := newNotifyFixture(t)

cTime := time.Now()
root := f.TempDir("root")
path := filepath.Join(root, "change")
a := filepath.Join(path, "a.txt")
f.WriteFile(a, "a")

f.watch(root)
f.fsync()

err := os.Chtimes(path, cTime, time.Now())
assert.NoError(t, err)
f.assertEvents()
}

func TestWatchNonExistentPathDoesNotFireSiblingEvent(t *testing.T) {
f := newNotifyFixture(t)

Expand Down
27 changes: 21 additions & 6 deletions internal/watch/watcher_naive.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,16 +151,32 @@ func (d *naiveNotify) loop() {
}

if e.Op&fsnotify.Create != fsnotify.Create {
if d.shouldNotify(e.Name) {
d.wrappedEvents <- FileEvent{e.Name}
if !d.shouldNotify(e.Name) {
continue
}

// Don't send events for directories when the modtime is being changed.
//
// This is a bit of a hack because every OS represents modtime updates
// a bit differently and they don't map well to fsnotify events.
//
// On Windows, updating the modtime of a directory is a fsnotify.Write.
// On Linux, it's a fsnotify.Chmod.
isDirUpdateOnly := (e.Op == fsnotify.Write || e.Op == fsnotify.Chmod) &&
ospath.IsDir(e.Name)
if isDirUpdateOnly {
continue
}

d.wrappedEvents <- FileEvent{e.Name}
continue
}

if d.isWatcherRecursive {
if d.shouldNotify(e.Name) {
d.wrappedEvents <- FileEvent{e.Name}
if !d.shouldNotify(e.Name) {
continue
}
d.wrappedEvents <- FileEvent{e.Name}
continue
}

Expand Down Expand Up @@ -223,8 +239,7 @@ func (d *naiveNotify) shouldNotify(path string) bool {

if _, ok := d.notifyList[path]; ok {
// We generally don't care when directories change at the root of an ADD
stat, err := os.Lstat(path)
isDir := err == nil && stat.IsDir()
isDir := ospath.IsDirLstat(path)
if isDir {
return false
}
Expand Down

0 comments on commit 33925ac

Please sign in to comment.