diff --git a/pkg/compose/watch.go b/pkg/compose/watch.go index f63c51683a..1e22c9479a 100644 --- a/pkg/compose/watch.go +++ b/pkg/compose/watch.go @@ -198,8 +198,9 @@ 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 + ignoreMatchers []watch.PathMatcher ) for serviceName, service := range project.Services { config, err := loadDevelopmentConfig(service, project) @@ -254,6 +255,11 @@ func (s *composeService) watch(ctx context.Context, project *types.Project, opti } } } + ignore, err := watch.NewDockerPatternMatcher(trigger.Path, trigger.Ignore) + if err != nil { + return nil, err + } + ignoreMatchers = append(ignoreMatchers, ignore) paths = append(paths, trigger.Path) } @@ -268,7 +274,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, watch.NewCompositeMatcher(ignoreMatchers...)) if err != nil { return nil, err } diff --git a/pkg/watch/notify.go b/pkg/watch/notify.go index d63f5caf28..71dcaf1156 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 PathMatcher) (Notify, error) { + return newWatcher(paths, ignore) } const WindowsBufferSizeEnvVar = "COMPOSE_WATCH_WINDOWS_BUFFER_SIZE" diff --git a/pkg/watch/notify_test.go b/pkg/watch/notify_test.go index b1ec9b9644..0e9a7bc63b 100644 --- a/pkg/watch/notify_test.go +++ b/pkg/watch/notify_test.go @@ -526,7 +526,7 @@ func (f *notifyFixture) rebuildWatcher() { } // create a new watcher - notify, err := NewWatcher(f.paths) + notify, err := NewWatcher(f.paths, EmptyMatcher{}) if err != nil { f.T().Fatal(err) } diff --git a/pkg/watch/watcher_darwin.go b/pkg/watch/watcher_darwin.go index a63612aedf..e4b6552eba 100644 --- a/pkg/watch/watcher_darwin.go +++ b/pkg/watch/watcher_darwin.go @@ -115,7 +115,7 @@ func (d *fseventNotify) Errors() chan error { return d.errors } -func newWatcher(paths []string) (Notify, error) { +func newWatcher(paths []string, _ ...PathMatcher) (Notify, error) { dw := &fseventNotify{ stream: &fsevents.EventStream{ Latency: 50 * time.Millisecond, diff --git a/pkg/watch/watcher_naive.go b/pkg/watch/watcher_naive.go index 7798025e8b..0ccd1f33ce 100644 --- a/pkg/watch/watcher_naive.go +++ b/pkg/watch/watcher_naive.go @@ -45,6 +45,7 @@ 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 PathMatcher isWatcherRecursive bool watcher *fsnotify.Watcher @@ -113,6 +114,10 @@ func (d *naiveNotify) watchRecursively(dir string) error { return filepath.WalkDir(dir, func(path string, info fs.DirEntry, err error) error { if err != nil { + if os.IsPermission(err) && d.shouldIgnore(path) { + logrus.Debugf("Ignoring path: %s", path) + return filepath.SkipDir + } return err } @@ -180,6 +185,10 @@ func (d *naiveNotify) loop() { //nolint:gocyclo // TODO(dbentley): if there's a delete should we call d.watcher.Remove to prevent leaking? err := filepath.WalkDir(e.Name, func(path string, info fs.DirEntry, err error) error { if err != nil { + if os.IsPermission(err) && d.shouldIgnore(path) { + logrus.Debugf("Ignoring path: %s", path) + return filepath.SkipDir + } return err } @@ -252,12 +261,52 @@ func (d *naiveNotify) shouldSkipDir(path string) bool { // - 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). + // + // We prioritize "parent of watched path" checks before ignore checks so + // one trigger's ignore rules can't hide another trigger's nested watch root. + isChildOfWatchedDir := false 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 + } + } + + // skip the dir if ignore rules match this full subtree. + if d.shouldIgnoreEntireDir(path) { + return true + } + + return !isChildOfWatchedDir +} + +func (d *naiveNotify) shouldIgnoreEntireDir(path string) bool { + if d.ignore == nil { + return false } - return true + matches, err := d.ignore.MatchesEntireDir(path) + if err != nil { + logrus.Debugf("error checking ignored directory %q: %v", path, err) + return false + } + if matches { + return true + } + return false +} + +func (d *naiveNotify) shouldIgnore(path string) bool { + if d.ignore == nil { + return false + } + matches, err := d.ignore.Matches(path) + if err != nil { + logrus.Debugf("error checking ignored path %q: %v", path, err) + return false + } + return matches } func (d *naiveNotify) add(path string) error { @@ -270,7 +319,7 @@ func (d *naiveNotify) add(path string) error { return nil } -func newWatcher(paths []string) (Notify, error) { +func newWatcher(paths []string, ignore PathMatcher) (Notify, error) { fsw, err := fsnotify.NewWatcher() if err != nil { if strings.Contains(err.Error(), "too many open files") && runtime.GOOS == "linux" { @@ -297,9 +346,9 @@ func newWatcher(paths []string) (Notify, error) { } notifyList[path] = true } - wmw := &naiveNotify{ notifyList: notifyList, + ignore: ignore, watcher: fsw, events: fsw.Events, wrappedEvents: wrappedEvents, diff --git a/pkg/watch/watcher_naive_test.go b/pkg/watch/watcher_naive_test.go index b188de9390..ede4c0d7ab 100644 --- a/pkg/watch/watcher_naive_test.go +++ b/pkg/watch/watcher_naive_test.go @@ -157,3 +157,57 @@ 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: 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: ignore} + + bazelBin := filepath.Join(repoRoot, "bazel-bin") + assert.Assert(t, d.shouldIgnore(bazelBin), "expected bazel-bin path to match ignore pattern") +} + +func TestShouldSkipDirForIgnoredSubtreeWithoutException(t *testing.T) { + repoRoot := t.TempDir() + ignore, err := DockerIgnoreTesterFromContents(repoRoot, "bazel-bin/\n") + assert.NilError(t, err) + + d := &naiveNotify{ + ignore: 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: 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") +}