diff --git a/pkg/compose/watch.go b/pkg/compose/watch.go index f63c51683a..cc9ca37ace 100644 --- a/pkg/compose/watch.go +++ b/pkg/compose/watch.go @@ -198,9 +198,12 @@ func (s *composeService) watch(ctx context.Context, project *types.Project, opti eg, ctx := errgroup.WithContext(ctx) var ( - rules []watchRule - paths []string + rules []watchRule + paths []string + ignoresByWatchPath map[string]watch.PathMatcher ) + ignoresByWatchPath = make(map[string]watch.PathMatcher) + for serviceName, service := range project.Services { config, err := loadDevelopmentConfig(service, project) if err != nil { @@ -254,9 +257,18 @@ func (s *composeService) watch(ctx context.Context, project *types.Project, opti } } } + var ignore watch.PathMatcher + ignore, err = watch.NewDockerPatternMatcher(trigger.Path, trigger.Ignore) + if err != nil { + return nil, err + } + + if existingMatcher, exists := ignoresByWatchPath[trigger.Path]; exists { + ignore = watch.NewIntersectMatcher(existingMatcher, ignore) + } + ignoresByWatchPath[trigger.Path] = ignore paths = append(paths, trigger.Path) } - serviceWatchRules, err := getWatchRules(config, service) if err != nil { return nil, err @@ -268,7 +280,7 @@ func (s *composeService) watch(ctx context.Context, project *types.Project, opti return nil, fmt.Errorf("none of the selected services is configured for watch, consider setting a 'develop' section") } - watcher, err := watch.NewWatcher(paths) + watcher, err := watch.NewWatcher(paths, ignoresByWatchPath) if err != nil { return nil, err } diff --git a/pkg/watch/notify.go b/pkg/watch/notify.go index d63f5caf28..de91721a3b 100644 --- a/pkg/watch/notify.go +++ b/pkg/watch/notify.go @@ -80,8 +80,8 @@ func (EmptyMatcher) MatchesEntireDir(f string) (bool, error) { return false, nil var _ PathMatcher = EmptyMatcher{} -func NewWatcher(paths []string) (Notify, error) { - return newWatcher(paths) +func NewWatcher(paths []string, ignore map[string]PathMatcher) (Notify, error) { + return newWatcher(paths, ignore) } const WindowsBufferSizeEnvVar = "COMPOSE_WATCH_WINDOWS_BUFFER_SIZE" @@ -134,3 +134,48 @@ func (c CompositePathMatcher) MatchesEntireDir(f string) (bool, error) { } var _ PathMatcher = CompositePathMatcher{} + +// intersectPathMatcher matches iff every matcher matches. With several develop.watch +// triggers on one watch root, skip/filter a path only when every trigger's ignores agree. +type intersectPathMatcher struct { + Matchers []PathMatcher +} + +// NewIntersectMatcher returns a PathMatcher that matches iff every matcher matches. +func NewIntersectMatcher(matchers ...PathMatcher) PathMatcher { + if len(matchers) == 0 { + return EmptyMatcher{} + } + if len(matchers) == 1 { + return matchers[0] + } + return intersectPathMatcher{Matchers: matchers} +} + +func (i intersectPathMatcher) Matches(f string) (bool, error) { + for _, t := range i.Matchers { + ret, err := t.Matches(f) + if err != nil { + return false, err + } + if !ret { + return false, nil + } + } + return true, nil +} + +func (i intersectPathMatcher) MatchesEntireDir(f string) (bool, error) { + for _, t := range i.Matchers { + ret, err := t.MatchesEntireDir(f) + if err != nil { + return false, err + } + if !ret { + return false, nil + } + } + return true, nil +} + +var _ PathMatcher = intersectPathMatcher{} diff --git a/pkg/watch/notify_test.go b/pkg/watch/notify_test.go index b1ec9b9644..1ecf22e2d0 100644 --- a/pkg/watch/notify_test.go +++ b/pkg/watch/notify_test.go @@ -50,6 +50,124 @@ func TestWindowsBufferSize(t *testing.T) { }) } +func TestNewIntersectMatcher(t *testing.T) { + root := t.TempDir() + + vendorOnly, err := DockerIgnoreTesterFromContents(root, "vendor/\n") + assert.NilError(t, err) + tmpOnly, err := DockerIgnoreTesterFromContents(root, "tmp/\n") + assert.NilError(t, err) + + inter := NewIntersectMatcher(vendorOnly, tmpOnly) + + vendorFile := filepath.Join(root, "vendor", "a.go") + matches, err := inter.Matches(vendorFile) + assert.NilError(t, err) + assert.Assert(t, !matches, "only one trigger ignores vendor; intersection must not treat path as ignored") + + bothIgnoreBuild1, err := DockerIgnoreTesterFromContents(root, "build/\n") + assert.NilError(t, err) + bothIgnoreBuild2, err := DockerIgnoreTesterFromContents(root, "build/\n") + assert.NilError(t, err) + interBuild := NewIntersectMatcher(bothIgnoreBuild1, bothIgnoreBuild2) + buildFile := filepath.Join(root, "build", "out") + matches, err = interBuild.Matches(buildFile) + assert.NilError(t, err) + assert.Assert(t, matches) + + dirEntire1, err := DockerIgnoreTesterFromContents(root, "cache/\n") + assert.NilError(t, err) + dirEntire2, err := DockerIgnoreTesterFromContents(root, "cache/\n") + assert.NilError(t, err) + interDir := NewIntersectMatcher(dirEntire1, dirEntire2) + entire, err := interDir.MatchesEntireDir(filepath.Join(root, "cache")) + assert.NilError(t, err) + assert.Assert(t, entire) + + partialEntire := NewIntersectMatcher(vendorOnly, tmpOnly) + entire, err = partialEntire.MatchesEntireDir(filepath.Join(root, "vendor")) + assert.NilError(t, err) + assert.Assert(t, !entire, "must not skip whole dir unless every matcher agrees it is entirely ignorable") +} + +func TestWatchRespectsDockerignore(t *testing.T) { + f := newNotifyFixture(t) + + root := f.TempDir("root") + ignore, err := DockerIgnoreTesterFromContents(root, "vendor/\n") + assert.NilError(t, err) + + f.ignores = map[string]PathMatcher{root: ignore} + f.watch(root) + f.fsync() + f.events = nil + + kept := filepath.Join(root, "src", "main.go") + f.WriteFile(kept, "package main\n") + f.assertEvents(filepath.Join(root, "src"), kept) + f.events = nil + + ignored := filepath.Join(root, "vendor", "mod", "x.go") + f.WriteFile(ignored, "module x\n") + f.assertEvents() +} + +func TestWatchPerRootIgnoresDoNotLeak(t *testing.T) { + f := newNotifyFixture(t) + + rootA := f.TempDir("root-a") + rootB := f.TempDir("root-b") + ignoreA, err := DockerIgnoreTesterFromContents(rootA, "vendor/\n") + assert.NilError(t, err) + + f.ignores = map[string]PathMatcher{rootA: ignoreA} + f.watch(rootA) + f.watch(rootB) + f.fsync() + f.events = nil + + ignoredUnderA := filepath.Join(rootA, "vendor", "x.go") + f.WriteFile(ignoredUnderA, "ignored\n") + f.assertEvents() + + keptUnderB := filepath.Join(rootB, "vendor", "x.go") + f.WriteFile(keptUnderB, "kept\n") + f.assertEvents(filepath.Join(rootB, "vendor"), keptUnderB) +} + +func TestWatchIntersectMatcherRequiresAllTriggers(t *testing.T) { + f := newNotifyFixture(t) + + root := f.TempDir("root") + ignoreVendor, err := DockerIgnoreTesterFromContents(root, "vendor/\n") + assert.NilError(t, err) + ignoreTmp, err := DockerIgnoreTesterFromContents(root, "tmp/\n") + assert.NilError(t, err) + + f.ignores = map[string]PathMatcher{root: NewIntersectMatcher(ignoreVendor, ignoreTmp)} + f.watch(root) + f.fsync() + f.events = nil + + vendorFile := filepath.Join(root, "vendor", "x", "go.mod") + f.WriteFile(vendorFile, "module x\n") + f.assertEvents(filepath.Join(root, "vendor"), filepath.Join(root, "vendor", "x"), vendorFile) + + ignoreBuild1, err := DockerIgnoreTesterFromContents(root, "build/\n") + assert.NilError(t, err) + ignoreBuild2, err := DockerIgnoreTesterFromContents(root, "build/\n") + assert.NilError(t, err) + + f.ignores = map[string]PathMatcher{root: NewIntersectMatcher(ignoreBuild1, ignoreBuild2)} + f.rebuildWatcher() + f.fsync() + f.events = nil + + buildFile := filepath.Join(root, "build", "out", "a") + f.WriteFile(buildFile, "artifact\n") + f.assertEvents() +} + func TestNoEvents(t *testing.T) { f := newNotifyFixture(t) f.assertEvents() @@ -493,9 +611,10 @@ type notifyFixture struct { cancel func() out *bytes.Buffer *TempDirFixture - notify Notify - paths []string - events []FileEvent + notify Notify + paths []string + ignores map[string]PathMatcher + events []FileEvent } func newNotifyFixture(t *testing.T) *notifyFixture { @@ -526,7 +645,7 @@ func (f *notifyFixture) rebuildWatcher() { } // create a new watcher - notify, err := NewWatcher(f.paths) + notify, err := NewWatcher(f.paths, f.ignores) if err != nil { f.T().Fatal(err) } diff --git a/pkg/watch/paths.go b/pkg/watch/paths.go index c0c893cd99..c4213a24b1 100644 --- a/pkg/watch/paths.go +++ b/pkg/watch/paths.go @@ -20,6 +20,8 @@ import ( "fmt" "os" "path/filepath" + + pathutil "github.com/docker/compose/v5/internal/paths" ) func greatestExistingAncestor(path string) (string, error) { @@ -39,3 +41,48 @@ func greatestExistingAncestor(path string) (string, error) { return path, nil } + +func greatestExistingAncestors(paths []string, ignoreList map[string]PathMatcher) ([]string, error) { + result := []string{} + for _, path := range paths { + newP, err := greatestExistingAncestor(path) + if err != nil { + return nil, fmt.Errorf("finding ancestor of %s: %w", path, err) + } + result = append(result, newP) + if path != newP { + ignore := ignoreList[path] + if oldMatcher, exists := ignoreList[newP]; exists { + ignore = NewIntersectMatcher(oldMatcher, ignore) + } + ignoreList[newP] = ignore + delete(ignoreList, path) + } + } + return result, nil +} + +func normalizeWatchRoots(paths []string, ignore map[string]PathMatcher) (map[string]bool, map[string]PathMatcher, error) { + notifyList := make(map[string]bool, len(paths)) + normalizedIgnores := make(map[string]PathMatcher, len(paths)) + + for _, root := range paths { + root, err := filepath.Abs(root) + if err != nil { + return nil, nil, err + } + notifyList[root] = true + + matchers := make([]PathMatcher, 0, len(ignore)) + for triggerPath, matcher := range ignore { + if matcher == nil { + continue + } + if root == triggerPath || pathutil.IsChild(root, triggerPath) || pathutil.IsChild(triggerPath, root) { + matchers = append(matchers, matcher) + } + } + normalizedIgnores[root] = NewIntersectMatcher(matchers...) + } + return notifyList, normalizedIgnores, nil +} diff --git a/pkg/watch/paths_test.go b/pkg/watch/paths_test.go index 60b4a15e7b..9bc35dd6ed 100644 --- a/pkg/watch/paths_test.go +++ b/pkg/watch/paths_test.go @@ -17,6 +17,7 @@ package watch import ( + "path/filepath" "runtime" "testing" @@ -41,3 +42,154 @@ func TestGreatestExistingAncestor(t *testing.T) { _, err = greatestExistingAncestor(missingTopLevel) assert.ErrorContains(t, err, "cannot watch root directory") } + +func TestGreatestExistingAncestorsMovesIgnoreToAncestor(t *testing.T) { + f := NewTempDirFixture(t) + + missing := f.JoinPath("missing", "child", "file.txt") + ignore, err := DockerIgnoreTesterFromContents(f.Path(), "vendor/\n") + assert.NilError(t, err) + + ignoreList := map[string]PathMatcher{missing: ignore} + paths, err := greatestExistingAncestors([]string{missing}, ignoreList) + assert.NilError(t, err) + assert.Equal(t, 1, len(paths)) + assert.Equal(t, f.Path(), paths[0]) + assert.Assert(t, ignoreList[f.Path()] != nil) + _, exists := ignoreList[missing] + assert.Assert(t, !exists) +} + +func TestGreatestExistingAncestorsIntersectsIgnoreOnAncestor(t *testing.T) { + f := NewTempDirFixture(t) + + missing := f.JoinPath("missing", "child", "file.txt") + vendorIgnore, err := DockerIgnoreTesterFromContents(f.Path(), "vendor/\n") + assert.NilError(t, err) + tmpIgnore, err := DockerIgnoreTesterFromContents(f.Path(), "tmp/\n") + assert.NilError(t, err) + + ignoreList := map[string]PathMatcher{ + f.Path(): vendorIgnore, + missing: tmpIgnore, + } + paths, err := greatestExistingAncestors([]string{missing}, ignoreList) + assert.NilError(t, err) + assert.Equal(t, 1, len(paths)) + assert.Equal(t, f.Path(), paths[0]) + + inter, ok := ignoreList[f.Path()].(intersectPathMatcher) + assert.Assert(t, ok) + assert.Equal(t, 2, len(inter.Matchers)) +} + +func TestNormalizeWatchRootsAbsolutizesPaths(t *testing.T) { + rel := "." + abs, err := filepath.Abs(rel) + assert.NilError(t, err) + + notifyList, _, err := normalizeWatchRoots([]string{rel}, nil) + assert.NilError(t, err) + assert.Assert(t, notifyList[abs]) +} + +func TestNormalizeWatchRootsAssignsRelatedIgnores(t *testing.T) { + f := NewTempDirFixture(t) + + root := f.Path() + child := f.JoinPath("child") + vendorIgnore, err := DockerIgnoreTesterFromContents(root, "vendor/\n") + assert.NilError(t, err) + unrelatedIgnore, err := DockerIgnoreTesterFromContents(root, "build/\n") + assert.NilError(t, err) + + ignores := map[string]PathMatcher{ + root: vendorIgnore, + child: vendorIgnore, + "/other": unrelatedIgnore, + } + notifyList, normalizedIgnores, err := normalizeWatchRoots([]string{root, child}, ignores) + assert.NilError(t, err) + assert.Assert(t, notifyList[root]) + assert.Assert(t, notifyList[child]) + + vendorFile := filepath.Join(root, "vendor", "mod.go") + matches, err := normalizedIgnores[root].Matches(vendorFile) + assert.NilError(t, err) + assert.Assert(t, matches) + + matches, err = normalizedIgnores[child].Matches(vendorFile) + assert.NilError(t, err) + assert.Assert(t, matches) + + buildFile := filepath.Join(root, "build", "out") + matches, err = normalizedIgnores[root].Matches(buildFile) + assert.NilError(t, err) + assert.Assert(t, !matches) +} + +func TestNormalizeWatchRootsSkipsNilMatchers(t *testing.T) { + f := NewTempDirFixture(t) + + root := f.Path() + notifyList, normalizedIgnores, err := normalizeWatchRoots([]string{root}, map[string]PathMatcher{root: nil}) + assert.NilError(t, err) + assert.Assert(t, notifyList[root]) + _, ok := normalizedIgnores[root].(EmptyMatcher) + assert.Assert(t, ok) +} + +func TestNormalizeWatchRootsUsesEmptyMatcherWithoutIgnores(t *testing.T) { + f := NewTempDirFixture(t) + + root := f.Path() + _, normalizedIgnores, err := normalizeWatchRoots([]string{root}, nil) + assert.NilError(t, err) + _, ok := normalizedIgnores[root].(EmptyMatcher) + assert.Assert(t, ok) +} + +func TestNormalizeWatchRootsInheritsParentIgnoreForChild(t *testing.T) { + f := NewTempDirFixture(t) + + root := f.Path() + child := f.JoinPath("pkg") + vendorIgnore, err := DockerIgnoreTesterFromContents(root, "pkg/vendor/\n") + assert.NilError(t, err) + + _, normalizedIgnores, err := normalizeWatchRoots([]string{child}, map[string]PathMatcher{root: vendorIgnore}) + assert.NilError(t, err) + + vendorFile := filepath.Join(child, "vendor", "x.go") + matches, err := normalizedIgnores[child].Matches(vendorFile) + assert.NilError(t, err) + assert.Assert(t, matches) +} + +func TestNormalizeWatchRootsIntersectsNestedIgnores(t *testing.T) { + f := NewTempDirFixture(t) + + root := f.Path() + child := f.JoinPath("pkg") + vendorIgnore, err := DockerIgnoreTesterFromContents(root, "vendor/\n") + assert.NilError(t, err) + tmpIgnore, err := DockerIgnoreTesterFromContents(root, "pkg/tmp/\n") + assert.NilError(t, err) + + ignores := map[string]PathMatcher{ + root: vendorIgnore, + child: tmpIgnore, + } + _, normalizedIgnores, err := normalizeWatchRoots([]string{root, child}, ignores) + assert.NilError(t, err) + + vendorFile := filepath.Join(root, "vendor", "x.go") + matches, err := normalizedIgnores[root].Matches(vendorFile) + assert.NilError(t, err) + assert.Assert(t, !matches, "nested ignores must all match for parent root") + + tmpUnderChild := filepath.Join(child, "tmp", "a") + matches, err = normalizedIgnores[child].Matches(tmpUnderChild) + assert.NilError(t, err) + assert.Assert(t, !matches, "nested ignores must all match for child root") +} diff --git a/pkg/watch/watcher_darwin.go b/pkg/watch/watcher_darwin.go index a63612aedf..d7fd71ac3e 100644 --- a/pkg/watch/watcher_darwin.go +++ b/pkg/watch/watcher_darwin.go @@ -26,6 +26,7 @@ import ( "time" "github.com/fsnotify/fsevents" + "github.com/sirupsen/logrus" pathutil "github.com/docker/compose/v5/internal/paths" ) @@ -39,6 +40,8 @@ type fseventNotify struct { stop chan struct{} pathsWereWatching map[string]any + // ignore maps each pathsWereWatching root to the merged PathMatcher for paths under it. + ignore map[string]PathMatcher closeOnce sync.Once } @@ -62,32 +65,51 @@ func (d *fseventNotify) loop() { continue } + if !d.shouldNotify(e.Path) { + continue + } + d.events <- NewFileEvent(e.Path) } } } } -// Add a path to be watched. Should only be called during initialization. -func (d *fseventNotify) initAdd(name string) { +// addStreamPath registers an existing ancestor path with the FSEvents stream. +func (d *fseventNotify) addStreamPath(name string) { d.stream.Paths = append(d.stream.Paths, name) - - if d.pathsWereWatching == nil { - d.pathsWereWatching = make(map[string]any) - } - d.pathsWereWatching[name] = struct{}{} } func (d *fseventNotify) Start() error { - if len(d.stream.Paths) == 0 { + notifyRoots := make([]string, 0, len(d.pathsWereWatching)) + for path := range d.pathsWereWatching { + notifyRoots = append(notifyRoots, path) + } + if len(notifyRoots) == 0 { return nil } + pathsToWatch, err := greatestExistingAncestors(notifyRoots, d.ignore) + if err != nil { + return err + } + pathsToWatch = pathutil.EncompassingPaths(pathsToWatch) + + _, normalizedIgnores, err := normalizeWatchRoots(notifyRoots, d.ignore) + if err != nil { + return err + } + d.ignore = normalizedIgnores + d.stream.Paths = nil + for _, path := range pathsToWatch { + d.addStreamPath(path) + } + d.closeOnce = sync.Once{} numberOfWatches.Add(int64(len(d.stream.Paths))) - err := d.stream.Start() + err = d.stream.Start() if err != nil { return err } @@ -115,7 +137,42 @@ func (d *fseventNotify) Errors() chan error { return d.errors } -func newWatcher(paths []string) (Notify, error) { +func (d *fseventNotify) shouldNotify(path string) bool { + + if _, ok := d.pathsWereWatching[path]; ok { + stat, err := os.Lstat(path) + isDir := err == nil && stat.IsDir() + return !isDir + } + + for root := range d.pathsWereWatching { + if pathutil.IsChild(root, path) { + if d.shouldIgnore(root, path) { + return false + } + return true + } + } + return false +} + +func (d *fseventNotify) shouldIgnore(dir, path string) bool { + if len(d.ignore) == 0 { + return false + } + + if ignore, exists := d.ignore[dir]; exists && ignore != nil { + matches, err := ignore.Matches(path) + if err != nil { + logrus.Debugf("error checking ignored path %q: %v", path, err) + return false + } + return matches + } + return false +} + +func newWatcher(paths []string, ignore map[string]PathMatcher) (Notify, error) { dw := &fseventNotify{ stream: &fsevents.EventStream{ Latency: 50 * time.Millisecond, @@ -129,13 +186,15 @@ func newWatcher(paths []string) (Notify, error) { stop: make(chan struct{}), } - paths = pathutil.EncompassingPaths(paths) - for _, path := range paths { - path, err := filepath.Abs(path) - if err != nil { - return nil, fmt.Errorf("newWatcher: %w", err) - } - dw.initAdd(path) + watchRoots := pathutil.EncompassingPaths(paths) + notifyList, normalizedIgnores, err := normalizeWatchRoots(watchRoots, ignore) + if err != nil { + return nil, fmt.Errorf("newWatcher: %w", err) + } + dw.ignore = normalizedIgnores + dw.pathsWereWatching = make(map[string]any, len(notifyList)) + for path := range notifyList { + dw.pathsWereWatching[path] = struct{}{} } return dw, nil diff --git a/pkg/watch/watcher_darwin_test.go b/pkg/watch/watcher_darwin_test.go index 50eb442b06..281e2b3c18 100644 --- a/pkg/watch/watcher_darwin_test.go +++ b/pkg/watch/watcher_darwin_test.go @@ -19,15 +19,24 @@ package watch import ( + "os" + "path/filepath" "testing" "gotest.tools/v3/assert" ) +func newFseventNotifyFixture(repo string, ignore map[string]PathMatcher) *fseventNotify { + return &fseventNotify{ + pathsWereWatching: map[string]any{repo: struct{}{}}, + ignore: ignore, + } +} + func TestFseventNotifyCloseIdempotent(t *testing.T) { // Create a watcher with a temporary directory tmpDir := t.TempDir() - watcher, err := newWatcher([]string{tmpDir}) + watcher, err := newWatcher([]string{tmpDir}, nil) assert.NilError(t, err) // Start the watcher @@ -46,3 +55,114 @@ func TestFseventNotifyCloseIdempotent(t *testing.T) { err = watcher.Close() assert.NilError(t, err) } + +func TestFseventNotifyShouldNotifyNilIgnore(t *testing.T) { + repo := t.TempDir() + child := filepath.Join(repo, "child.txt") + assert.NilError(t, os.WriteFile(child, []byte("x"), 0o644)) + + d := newFseventNotifyFixture(repo, nil) + assert.Assert(t, d.shouldNotify(child), "expected file under watched root to notify") + assert.Assert(t, !d.shouldNotify(repo), "expected directory event at watched root to be skipped") +} + +func TestFseventNotifyShouldNotifyWatchedFileRoot(t *testing.T) { + repo := t.TempDir() + fileRoot := filepath.Join(repo, "watched.go") + assert.NilError(t, os.WriteFile(fileRoot, []byte("package main\n"), 0o644)) + + d := newFseventNotifyFixture(fileRoot, nil) + assert.Assert(t, d.shouldNotify(fileRoot), "expected file that is the watch root to notify") +} + +func TestFseventNotifyShouldNotifyOutsideWatchedTree(t *testing.T) { + repo := t.TempDir() + other := t.TempDir() + + d := newFseventNotifyFixture(repo, nil) + outPath := filepath.Join(other, "outside.txt") + assert.NilError(t, os.WriteFile(outPath, []byte("x"), 0o644)) + assert.Assert(t, !d.shouldNotify(outPath), "expected path outside watch roots not to notify") +} + +func TestFseventNotifyShouldNotifyRespectsDockerignore(t *testing.T) { + repo := t.TempDir() + ignore, err := DockerIgnoreTesterFromContents(repo, "vendor/\n") + assert.NilError(t, err) + + d := newFseventNotifyFixture(repo, map[string]PathMatcher{repo: ignore}) + kept := filepath.Join(repo, "src", "main.go") + assert.NilError(t, os.MkdirAll(filepath.Dir(kept), 0o755)) + assert.NilError(t, os.WriteFile(kept, []byte("x"), 0o644)) + assert.Assert(t, d.shouldNotify(kept), "expected non-ignored path to notify") + + ignored := filepath.Join(repo, "vendor", "mod", "x.go") + assert.NilError(t, os.MkdirAll(filepath.Dir(ignored), 0o755)) + assert.NilError(t, os.WriteFile(ignored, []byte("x"), 0o644)) + assert.Assert(t, !d.shouldNotify(ignored), "expected dockerignored path not to notify") +} + +func TestFseventNotifyShouldNotifyDockerignoreNegation(t *testing.T) { + repo := t.TempDir() + ignore, err := DockerIgnoreTesterFromContents(repo, "bazel-bin/\n!bazel-bin/app-binary\n") + assert.NilError(t, err) + + d := newFseventNotifyFixture(repo, map[string]PathMatcher{repo: ignore}) + + ignoredChild := filepath.Join(repo, "bazel-bin", "cache", "out") + assert.NilError(t, os.MkdirAll(filepath.Dir(ignoredChild), 0o755)) + assert.NilError(t, os.WriteFile(ignoredChild, []byte("x"), 0o644)) + assert.Assert(t, !d.shouldNotify(ignoredChild), "expected ignored subtree under bazel-bin not to notify") + + excepted := filepath.Join(repo, "bazel-bin", "app-binary", "binary") + assert.NilError(t, os.MkdirAll(filepath.Dir(excepted), 0o755)) + assert.NilError(t, os.WriteFile(excepted, []byte("x"), 0o644)) + assert.Assert(t, d.shouldNotify(excepted), "expected negated dockerignore path to notify") +} + +func TestFseventNotifyShouldNotifyIntersectMatcher(t *testing.T) { + repo := t.TempDir() + ignoreVendor, err := DockerIgnoreTesterFromContents(repo, "vendor/\n") + assert.NilError(t, err) + ignoreTmp, err := DockerIgnoreTesterFromContents(repo, "tmp/\n") + assert.NilError(t, err) + + d := newFseventNotifyFixture(repo, map[string]PathMatcher{repo: NewIntersectMatcher(ignoreVendor, ignoreTmp)}) + vendorFile := filepath.Join(repo, "vendor", "x", "go.mod") + assert.NilError(t, os.MkdirAll(filepath.Dir(vendorFile), 0o755)) + assert.NilError(t, os.WriteFile(vendorFile, []byte("module x\n"), 0o644)) + assert.Assert(t, d.shouldNotify(vendorFile), "vendor must notify when only one intersect matcher ignores it") + + ignoreBuild1, err := DockerIgnoreTesterFromContents(repo, "build/\n") + assert.NilError(t, err) + ignoreBuild2, err := DockerIgnoreTesterFromContents(repo, "build/\n") + assert.NilError(t, err) + d2 := newFseventNotifyFixture(repo, map[string]PathMatcher{repo: NewIntersectMatcher(ignoreBuild1, ignoreBuild2)}) + buildFile := filepath.Join(repo, "build", "out", "a") + assert.NilError(t, os.MkdirAll(filepath.Dir(buildFile), 0o755)) + assert.NilError(t, os.WriteFile(buildFile, []byte("x"), 0o644)) + assert.Assert(t, !d2.shouldNotify(buildFile), "expected path ignored by every intersect matcher not to notify") +} + +func TestFseventNotifyShouldIgnoreDockerignoreDirectory(t *testing.T) { + repo := t.TempDir() + ignore, err := DockerIgnoreTesterFromContents(repo, "bazel-bin/\n!bazel-bin/app-binary\n") + assert.NilError(t, err) + + d := newFseventNotifyFixture(repo, map[string]PathMatcher{repo: ignore}) + bazelBin := filepath.Join(repo, "bazel-bin") + assert.NilError(t, os.MkdirAll(bazelBin, 0o755)) + assert.Assert(t, d.shouldIgnore(repo, bazelBin), "expected directory path to match dockerignore") +} + +func TestFseventNotifyShouldIgnoreLooksUpMatcherByWatchRoot(t *testing.T) { + repo := t.TempDir() + other := t.TempDir() + ignore, err := DockerIgnoreTesterFromContents(repo, "vendor/\n") + assert.NilError(t, err) + + d := newFseventNotifyFixture(repo, map[string]PathMatcher{repo: ignore}) + vendorFile := filepath.Join(repo, "vendor", "x.go") + assert.Assert(t, d.shouldIgnore(repo, vendorFile), "expected matcher keyed to watched root to apply") + assert.Assert(t, !d.shouldIgnore(other, vendorFile), "expected unrelated watch root not to apply matcher") +} diff --git a/pkg/watch/watcher_naive.go b/pkg/watch/watcher_naive.go index 7798025e8b..cb121bbada 100644 --- a/pkg/watch/watcher_naive.go +++ b/pkg/watch/watcher_naive.go @@ -45,6 +45,8 @@ type naiveNotify struct { // the notify list. It might be better to store this in a tree // structure, so we can filter the list quickly. notifyList map[string]bool + // ignore maps each notifyList root to the merged PathMatcher for paths under it. + ignore map[string]PathMatcher isWatcherRecursive bool watcher *fsnotify.Watcher @@ -59,12 +61,13 @@ func (d *naiveNotify) Start() error { return nil } - pathsToWatch := []string{} + notifyRoots := make([]string, 0, len(d.notifyList)) + for path := range d.notifyList { - pathsToWatch = append(pathsToWatch, path) + notifyRoots = append(notifyRoots, path) } - pathsToWatch, err := greatestExistingAncestors(pathsToWatch) + pathsToWatch, err := greatestExistingAncestors(notifyRoots, d.ignore) if err != nil { return err } @@ -72,6 +75,11 @@ func (d *naiveNotify) Start() error { pathsToWatch = pathutil.EncompassingPaths(pathsToWatch) } + _, d.ignore, err = normalizeWatchRoots(notifyRoots, d.ignore) + if err != nil { + return err + } + for _, name := range pathsToWatch { fi, err := os.Stat(name) if err != nil && !os.IsNotExist(err) { @@ -228,7 +236,7 @@ func (d *naiveNotify) shouldNotify(path string) bool { for root := range d.notifyList { if pathutil.IsChild(root, path) { - return true + return !d.shouldIgnore(root, path) } } return false @@ -240,24 +248,64 @@ func (d *naiveNotify) shouldSkipDir(path string) bool { return false } - // Suppose we're watching - // /src/.tiltignore - // but the .tiltignore file doesn't exist. - // - // Our watcher will create an inotify watch on /src/. - // - // But then we want to make sure we don't recurse from /src/ down to /src/node_modules. - // - // To handle this case, we only want to traverse dirs that are: - // - A child of a directory that's in our notify list, or - // - A parent of a directory that's in our notify list - // (i.e., to cover the "path doesn't exist" case). + // Only walk directories under a notifyList path or under an ancestor of one + // (Start() may watch a parent when the target is missing or is a file). + // Decide ancestor/descendant versus notifyList before applying ignores so one + // root's patterns cannot block reaching another root. + // When walking beneath a watched ancestor, prune subtrees only with that root's + // matcher from d.ignore. + isChildOfWatchedDir := false + var dir string for root := range d.notifyList { - if pathutil.IsChild(root, path) || pathutil.IsChild(path, root) { + if pathutil.IsChild(path, root) { + return false + } + if pathutil.IsChild(root, path) { + isChildOfWatchedDir = true + dir = root + } + } + + if isChildOfWatchedDir && d.shouldIgnoreEntireDir(dir, path) { + return true + } + + return !isChildOfWatchedDir +} + +func (d *naiveNotify) shouldIgnoreEntireDir(dir, path string) bool { + if len(d.ignore) == 0 { + return false + } + + if ignore, exists := d.ignore[dir]; exists && ignore != nil { + matches, err := ignore.MatchesEntireDir(path) + if err != nil { + logrus.Debugf("error checking ignored directory %q: %v", path, err) return false } + if matches { + return true + } + return matches } - return true + return false +} + +func (d *naiveNotify) shouldIgnore(dir, path string) bool { + if len(d.ignore) == 0 { + return false + } + + if ignore, exists := d.ignore[dir]; exists && ignore != nil { + matches, err := ignore.Matches(path) + if err != nil { + logrus.Debugf("error checking ignored path %q: %v", path, err) + return false + } + return matches + } + return false } func (d *naiveNotify) add(path string) error { @@ -270,7 +318,7 @@ func (d *naiveNotify) add(path string) error { return nil } -func newWatcher(paths []string) (Notify, error) { +func newWatcher(paths []string, ignore map[string]PathMatcher) (Notify, error) { fsw, err := fsnotify.NewWatcher() if err != nil { if strings.Contains(err.Error(), "too many open files") && runtime.GOOS == "linux" { @@ -286,20 +334,20 @@ func newWatcher(paths []string) (Notify, error) { isWatcherRecursive := err == nil wrappedEvents := make(chan FileEvent) - notifyList := make(map[string]bool, len(paths)) + + watchRoots := paths if isWatcherRecursive { - paths = pathutil.EncompassingPaths(paths) + watchRoots = pathutil.EncompassingPaths(paths) } - for _, path := range paths { - path, err := filepath.Abs(path) - if err != nil { - return nil, fmt.Errorf("newWatcher: %w", err) - } - notifyList[path] = true + + notifyList, normalizedIgnores, err := normalizeWatchRoots(watchRoots, ignore) + if err != nil { + return nil, fmt.Errorf("newWatcher: %w", err) } wmw := &naiveNotify{ notifyList: notifyList, + ignore: normalizedIgnores, watcher: fsw, events: fsw.Events, wrappedEvents: wrappedEvents, @@ -311,15 +359,3 @@ func newWatcher(paths []string) (Notify, error) { } var _ Notify = &naiveNotify{} - -func greatestExistingAncestors(paths []string) ([]string, error) { - result := []string{} - for _, p := range paths { - newP, err := greatestExistingAncestor(p) - if err != nil { - return nil, fmt.Errorf("finding ancestor of %s: %w", p, err) - } - result = append(result, newP) - } - return result, nil -} diff --git a/pkg/watch/watcher_naive_test.go b/pkg/watch/watcher_naive_test.go index b188de9390..ee73ea3033 100644 --- a/pkg/watch/watcher_naive_test.go +++ b/pkg/watch/watcher_naive_test.go @@ -157,3 +157,111 @@ func TestDontRecurseWhenWatchingParentsOfNonExistentFiles(t *testing.T) { t.Fatalf("watching more than 5 files: %d", n) } } + +func TestShouldSkipDirWithNegatedChildException(t *testing.T) { + repoRoot := t.TempDir() + ignore, err := DockerIgnoreTesterFromContents(repoRoot, "bazel-bin/\n!bazel-bin/app-binary\n") + assert.NilError(t, err) + + d := &naiveNotify{ + ignore: map[string]PathMatcher{repoRoot: ignore}, + notifyList: map[string]bool{repoRoot: true}, + } + + bazelBin := filepath.Join(repoRoot, "bazel-bin") + assert.Assert(t, !d.shouldSkipDir(bazelBin), "expected bazel-bin to remain traversable for negated child patterns") +} + +func TestShouldIgnorePathStillMatchesDirectoryPattern(t *testing.T) { + repoRoot := t.TempDir() + ignore, err := DockerIgnoreTesterFromContents(repoRoot, "bazel-bin/\n!bazel-bin/app-binary\n") + assert.NilError(t, err) + + d := &naiveNotify{ignore: map[string]PathMatcher{repoRoot: ignore}} + + bazelBin := filepath.Join(repoRoot, "bazel-bin") + assert.Assert(t, d.shouldIgnore(repoRoot, bazelBin), "expected bazel-bin path to match ignore pattern") +} + +func TestShouldIgnoreLooksUpMatcherByWatchRoot(t *testing.T) { + repoRoot := t.TempDir() + otherRoot := t.TempDir() + ignore, err := DockerIgnoreTesterFromContents(repoRoot, "vendor/\n") + assert.NilError(t, err) + + d := &naiveNotify{ignore: map[string]PathMatcher{repoRoot: ignore}} + + vendorFile := filepath.Join(repoRoot, "vendor", "x.go") + assert.Assert(t, d.shouldIgnore(repoRoot, vendorFile), "expected matcher keyed to watched root to apply") + assert.Assert(t, !d.shouldIgnore(otherRoot, vendorFile), "expected unrelated watch root not to apply matcher") +} + +func TestShouldIgnoreEntireDirLooksUpMatcherByWatchRoot(t *testing.T) { + repoRoot := t.TempDir() + ignore, err := DockerIgnoreTesterFromContents(repoRoot, "vendor/\n") + assert.NilError(t, err) + + d := &naiveNotify{ + ignore: map[string]PathMatcher{repoRoot: ignore}, + notifyList: map[string]bool{repoRoot: true}, + } + + vendorDir := filepath.Join(repoRoot, "vendor") + assert.Assert(t, d.shouldIgnoreEntireDir(repoRoot, vendorDir), "expected directory matcher keyed to watched root to apply") +} + +func TestShouldSkipDirForIgnoredSubtreeWithoutException(t *testing.T) { + repoRoot := t.TempDir() + ignore, err := DockerIgnoreTesterFromContents(repoRoot, "bazel-bin/\n") + assert.NilError(t, err) + + d := &naiveNotify{ + ignore: map[string]PathMatcher{repoRoot: ignore}, + notifyList: map[string]bool{repoRoot: true}, + } + + bazelBin := filepath.Join(repoRoot, "bazel-bin") + assert.Assert(t, d.shouldSkipDir(bazelBin), "expected fully ignored directory subtree to be skipped") +} + +func TestShouldSkipDirDoesNotSkipAncestorOfWatchedPath(t *testing.T) { + repoRoot := t.TempDir() + ignore, err := DockerIgnoreTesterFromContents(repoRoot, "parent/\n") + assert.NilError(t, err) + + watchedPath := filepath.Join(repoRoot, "parent", "child", "non-existent") + d := &naiveNotify{ + ignore: map[string]PathMatcher{watchedPath: ignore}, + notifyList: map[string]bool{watchedPath: true}, + } + + parent := filepath.Join(repoRoot, "parent") + assert.Assert(t, !d.shouldSkipDir(parent), "expected parent directory to remain traversable when it contains a watched path") +} + +func TestShouldSkipDirIntersectRequiresAllTriggersToAgree(t *testing.T) { + repoRoot := t.TempDir() + ignoreVendor, err := DockerIgnoreTesterFromContents(repoRoot, "vendor/\n") + assert.NilError(t, err) + ignoreTmp, err := DockerIgnoreTesterFromContents(repoRoot, "tmp/\n") + assert.NilError(t, err) + + d := &naiveNotify{ + ignore: map[string]PathMatcher{repoRoot: NewIntersectMatcher(ignoreVendor, ignoreTmp)}, + notifyList: map[string]bool{repoRoot: true}, + } + + vendorDir := filepath.Join(repoRoot, "vendor") + assert.Assert(t, !d.shouldSkipDir(vendorDir), "vendor must remain watched when another trigger does not ignore it") + + ignoreBuild1, err := DockerIgnoreTesterFromContents(repoRoot, "build/\n") + assert.NilError(t, err) + ignoreBuild2, err := DockerIgnoreTesterFromContents(repoRoot, "build/\n") + assert.NilError(t, err) + d2 := &naiveNotify{ + ignore: map[string]PathMatcher{repoRoot: NewIntersectMatcher(ignoreBuild1, ignoreBuild2)}, + notifyList: map[string]bool{repoRoot: true}, + } + buildDir := filepath.Join(repoRoot, "build") + assert.Assert(t, d2.shouldSkipDir(buildDir), "when every trigger ignores the same subtree, watcher may skip it") +}