diff --git a/pkg/watch/notify_test.go b/pkg/watch/notify_test.go index 7acb5d5c1..ed75e444f 100644 --- a/pkg/watch/notify_test.go +++ b/pkg/watch/notify_test.go @@ -5,7 +5,6 @@ import ( "io/ioutil" "os" "path/filepath" - "runtime" "strings" "testing" "time" @@ -128,6 +127,26 @@ func TestWatchNonExistentPath(t *testing.T) { f.assertEvents(path) } +func TestWatchNonExistentPathDoesNotFireSiblingEvent(t *testing.T) { + f := newNotifyFixture(t) + defer f.tearDown() + + root := f.TempDir("root") + watchedFile := filepath.Join(root, "a.txt") + unwatchedSibling := filepath.Join(root, "b.txt") + + err := f.notify.Add(watchedFile) + if err != nil { + t.Fatal(err) + } + + f.fsync() + + d1 := "hello\ngo\n" + f.WriteFile(unwatchedSibling, d1) + f.assertEvents() +} + func TestRemove(t *testing.T) { f := newNotifyFixture(t) defer f.tearDown() @@ -319,18 +338,13 @@ func TestWatchNonexistentDirectory(t *testing.T) { if err != nil { t.Fatal(err) } - parent := f.JoinPath("root", "parent") file := f.JoinPath("root", "parent", "a") f.watch(file) f.fsync() f.events = nil f.WriteFile(file, "hello") - if runtime.GOOS == "darwin" { - f.assertEvents(file) - } else { - f.assertEvents(parent, file) - } + f.assertEvents(file) } type notifyFixture struct { diff --git a/pkg/watch/watcher_naive.go b/pkg/watch/watcher_naive.go index 83c0275c2..564595191 100644 --- a/pkg/watch/watcher_naive.go +++ b/pkg/watch/watcher_naive.go @@ -21,7 +21,11 @@ type naiveNotify struct { events chan fsnotify.Event wrappedEvents chan FileEvent errors chan error - watchList map[string]bool + + // Paths that we're watching that should be passed up to the caller. + // Note that we may have to watch ancestors of these paths + // in order to fulfill the API promise. + notifyList map[string]bool } func (d *naiveNotify) Add(name string) error { @@ -32,24 +36,22 @@ func (d *naiveNotify) Add(name string) error { // if it's a file that doesn't exist, watch its parent if os.IsNotExist(err) { - err, fileWatched := d.watchUpRecursively(name) + err = d.watchAncestorOfMissingPath(name) if err != nil { - return errors.Wrapf(err, "watchUpRecursively(%q)", name) + return errors.Wrapf(err, "watchAncestorOfMissingPath(%q)", name) } - d.watchList[fileWatched] = true } else if fi.IsDir() { err = d.watchRecursively(name) if err != nil { return errors.Wrapf(err, "notify.Add(%q)", name) } - d.watchList[name] = true } else { err = d.watcher.Add(name) if err != nil { return errors.Wrapf(err, "notify.Add(%q)", name) } - d.watchList[name] = true } + d.notifyList[name] = true return nil } @@ -71,22 +73,22 @@ func (d *naiveNotify) watchRecursively(dir string) error { }) } -func (d *naiveNotify) watchUpRecursively(path string) (error, string) { +func (d *naiveNotify) watchAncestorOfMissingPath(path string) error { if path == string(filepath.Separator) { - return fmt.Errorf("cannot watch root directory"), "" + return fmt.Errorf("cannot watch root directory") } _, err := os.Stat(path) if err != nil && !os.IsNotExist(err) { - return errors.Wrapf(err, "os.Stat(%q)", path), "" + return errors.Wrapf(err, "os.Stat(%q)", path) } if os.IsNotExist(err) { parent := filepath.Dir(path) - return d.watchUpRecursively(parent) + return d.watchAncestorOfMissingPath(parent) } - return d.watcher.Add(path), path + return d.watcher.Add(path) } func (d *naiveNotify) Close() error { @@ -122,35 +124,39 @@ func (d *naiveNotify) loop() { Op: fsnotify.Create, Name: path, } - d.sendEventIfWatched(newE) - // TODO(dmiller): symlinks 😭 - err = d.Add(path) - if err != nil { - log.Printf("Error watching path %s: %s", e.Name, err) + + if d.shouldNotify(newE) { + d.wrappedEvents <- FileEvent{newE.Name} + + // TODO(dmiller): symlinks 😭 + err = d.Add(path) + if err != nil { + log.Printf("Error watching path %s: %s", e.Name, err) + } } return nil }) if err != nil { log.Printf("Error walking directory %s: %s", e.Name, err) } - } else { - d.sendEventIfWatched(e) + } else if d.shouldNotify(e) { + d.wrappedEvents <- FileEvent{e.Name} } } } -func (d *naiveNotify) sendEventIfWatched(e fsnotify.Event) { - if _, ok := d.watchList[e.Name]; ok { - d.wrappedEvents <- FileEvent{e.Name} +func (d *naiveNotify) shouldNotify(e fsnotify.Event) bool { + if _, ok := d.notifyList[e.Name]; ok { + return true } else { // TODO(dmiller): maybe use a prefix tree here? - for path := range d.watchList { + for path := range d.notifyList { if pathIsChildOf(e.Name, path) { - d.wrappedEvents <- FileEvent{e.Name} - break + return true } } } + return false } func NewWatcher() (*naiveNotify, error) { @@ -166,7 +172,7 @@ func NewWatcher() (*naiveNotify, error) { events: fsw.Events, wrappedEvents: wrappedEvents, errors: fsw.Errors, - watchList: map[string]bool{}, + notifyList: map[string]bool{}, } go wmw.loop()