From 21e5d564af58b67413b8803d474011da8e97993c Mon Sep 17 00:00:00 2001 From: Nick Santos Date: Sat, 13 Jul 2019 14:13:24 -0400 Subject: [PATCH] watch: FileEvents must always be absolute (#1841) there were a lot of confused tests that were using relative paths, then trying to workaround this --- pkg/watch/notify.go | 15 ++++++++++++++- pkg/watch/notify_test.go | 6 +++--- pkg/watch/watcher_darwin.go | 10 +++++++--- pkg/watch/watcher_naive.go | 4 ++++ 4 files changed, 28 insertions(+), 7 deletions(-) diff --git a/pkg/watch/notify.go b/pkg/watch/notify.go index a0e170dfa..5e2a6766c 100644 --- a/pkg/watch/notify.go +++ b/pkg/watch/notify.go @@ -2,6 +2,8 @@ package watch import ( "expvar" + "fmt" + "path/filepath" "github.com/windmilleng/tilt/internal/logger" ) @@ -11,7 +13,18 @@ var ( ) type FileEvent struct { - Path string + path string +} + +func NewFileEvent(p string) FileEvent { + if !filepath.IsAbs(p) { + panic(fmt.Sprintf("NewFileEvent only accepts absolute paths. Actual: %s", p)) + } + return FileEvent{path: p} +} + +func (e FileEvent) Path() string { + return e.path } type Notify interface { diff --git a/pkg/watch/notify_test.go b/pkg/watch/notify_test.go index b172accec..69ff0787a 100644 --- a/pkg/watch/notify_test.go +++ b/pkg/watch/notify_test.go @@ -600,16 +600,16 @@ F: f.T().Fatal(err) case event := <-f.notify.Events(): - if strings.Contains(event.Path, syncPath) { + if strings.Contains(event.Path(), syncPath) { break F } - if strings.Contains(event.Path, anySyncPath) { + if strings.Contains(event.Path(), anySyncPath) { continue } // Don't bother tracking duplicate changes to the same path // for testing. - if len(f.events) > 0 && f.events[len(f.events)-1].Path == event.Path { + if len(f.events) > 0 && f.events[len(f.events)-1].Path() == event.Path() { continue } diff --git a/pkg/watch/watcher_darwin.go b/pkg/watch/watcher_darwin.go index 17e476e21..ae757f1a4 100644 --- a/pkg/watch/watcher_darwin.go +++ b/pkg/watch/watcher_darwin.go @@ -4,6 +4,8 @@ import ( "path/filepath" "time" + "github.com/pkg/errors" + "github.com/windmilleng/tilt/internal/logger" "github.com/windmilleng/tilt/internal/ospath" @@ -62,9 +64,7 @@ func (d *darwinNotify) loop() { continue } - d.events <- FileEvent{ - Path: e.Path, - } + d.events <- NewFileEvent(e.Path) } } } @@ -133,6 +133,10 @@ func newWatcher(paths []string, ignore PathMatcher, l logger.Logger) (*darwinNot } for _, path := range paths { + path, err := filepath.Abs(path) + if err != nil { + return nil, errors.Wrap(err, "newWatcher") + } dw.initAdd(path) } diff --git a/pkg/watch/watcher_naive.go b/pkg/watch/watcher_naive.go index 8fd2975f9..b543a140a 100644 --- a/pkg/watch/watcher_naive.go +++ b/pkg/watch/watcher_naive.go @@ -207,6 +207,10 @@ func newWatcher(paths []string, ignore PathMatcher, l logger.Logger) (*naiveNoti wrappedEvents := make(chan FileEvent) notifyList := make(map[string]bool, len(paths)) for _, path := range paths { + path, err := filepath.Abs(path) + if err != nil { + return nil, errors.Wrap(err, "newWatcher") + } notifyList[path] = true }