Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions pkg/compose/watch.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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
}
Expand Down
49 changes: 47 additions & 2 deletions pkg/watch/notify.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{}
127 changes: 123 additions & 4 deletions pkg/watch/notify_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}
Expand Down
47 changes: 47 additions & 0 deletions pkg/watch/paths.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import (
"fmt"
"os"
"path/filepath"

pathutil "github.com/docker/compose/v5/internal/paths"
)

func greatestExistingAncestor(path string) (string, error) {
Expand All @@ -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) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM] normalizeWatchRoots intersects child-trigger matchers into parent root — causes under-ignoring of paths

When building the normalized ignore matcher for a watch root, normalizeWatchRoots collects matchers from ALL related trigger paths — including those where the trigger is a child of the root:

if root == triggerPath || pathutil.IsChild(root, triggerPath) || pathutil.IsChild(triggerPath, root) {
    matchers = append(matchers, matcher)
}
normalizedIgnores[root] = NewIntersectMatcher(matchers...)

NewIntersectMatcher requires every matcher to agree before a path is ignored (logical AND). This means that if a child trigger path contributes its matcher to the parent root's intersection, a path must satisfy both the parent-root matcher and the unrelated child-root matcher to be ignored at the parent level.

Concrete example with a realistic multi-service compose setup:

  • Service A: path: /project, ignore: [vendor/]ignoresByWatchPath["/project"] = vendor_matcher
  • Service B: path: /project/pkg, ignore: [tmp/]ignoresByWatchPath["/project/pkg"] = tmp_matcher

For root /project, normalizeWatchRoots collects [vendor_matcher, tmp_matcher] (because /project/pkg is a child of /project).

normalizedIgnores["/project"] = intersect(vendor_matcher, tmp_matcher)

Now /project/vendor/x.go:

  • vendor_matcher.Matches(...)true
  • tmp_matcher.Matches(...)false (it only matches tmp/)
  • Result: not ignored

Expected: /project/vendor/x.go should be ignored per Service A's config, but it is not.

Impact: Paths that should be ignored per one service's configuration are not ignored when another service with overlapping (but different) watch roots exists. This means the watcher emits spurious events for these paths and could still encounter permission errors on paths that were supposed to be skipped.

Suggested fix: Use CompositePathMatcher (union/OR) when aggregating matchers from different trigger paths into a root's ignore matcher, rather than intersectPathMatcher. A path should be ignored if any trigger for that root scope ignores it:

normalizedIgnores[root] = CompositePathMatcher(matchers)

Reserve intersectPathMatcher for its intended purpose: combining matchers for the same trigger path (where a file should only be ignored if every watch rule for that exact path agrees).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initially I thought of using CompositeMatcher in normalizeWatchRoots. but in this case it would work incorrectly

Service A: path: /project, ignore: [repo/]
Service B: path: /project/repo/pkg, ignore: [tmp/]

In this example, the normalized root would be /project. If we use a CompositeMatcher, then /repo would be ignored, which means /project/repo/pkg would not be watched. That’s why I used IntersectMatcher instead.

matchers = append(matchers, matcher)
}
}
normalizedIgnores[root] = NewIntersectMatcher(matchers...)
}
return notifyList, normalizedIgnores, nil
}
Loading