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
50 changes: 38 additions & 12 deletions pkg/compose/watch.go
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,11 @@ func getWatchRules(config *types.DevelopConfig, service types.ServiceConfig) ([]
return nil, err
}

dockerFileIgnore, err := dockerFileIgnoreMatcher(service)
if err != nil {
return nil, err
}

for _, trigger := range config.Watch {
ignore, err := watch.NewDockerPatternMatcher(trigger.Path, trigger.Ignore)
if err != nil {
Expand All @@ -352,15 +357,22 @@ func getWatchRules(config *types.DevelopConfig, service types.ServiceConfig) ([]
}
}

ignores := []watch.PathMatcher{
dockerIgnores,
watch.EphemeralPathMatcher(),
dotGitIgnore,
ignore,
}
// Copy actions only: rebuild on the same tree must still fire.
switch trigger.Action {
case types.WatchActionSync, types.WatchActionSyncRestart, types.WatchActionSyncExec:
ignores = append(ignores, dockerFileIgnore)
}
Comment on lines 339 to +370

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

composeFileIgnore only exists in initialSync; getWatchRules never builds it, so compose files edited after watch starts are still synced, the bug this PR claims to fix.
Fix: inside the for _, trigger := range config.Watch loop, build composeFileIgnore anchored on trigger.Path (same as ignore/include just above, using cli.DefaultFileNames/DefaultOverrideFileNames), and append it alongside dockerFileIgnore for sync/sync-restart/sync-exec triggers. Anchor on trigger.Path, not **/+basename — this only excludes the real compose file at the watched root, not a same-named file nested elsewhere. Verified working in an isolated build/test.

Also folds in a nit: the switch trigger.Action { case Sync, SyncRestart, SyncExec: ... } (line 367-370) duplicates isSync() (line 382) instead of reusing it — isSync(trigger) || trigger.Action == types.WatchActionSyncExec covers the same three cases.


rules = append(rules, watchRule{
Trigger: trigger,
include: include,
ignore: watch.NewCompositeMatcher(
dockerIgnores,
watch.EphemeralPathMatcher(),
dotGitIgnore,
ignore,
),
ignore: watch.NewCompositeMatcher(ignores...),
service: service.Name,
})
}
Expand Down Expand Up @@ -760,6 +772,19 @@ func (s *composeService) pruneDanglingImagesOnRebuild(ctx context.Context, proje
}
}

// **/anchored so the matcher hits both initialSync's basenames and the
// watch loop's absolute host paths.
func dockerFileIgnoreMatcher(service types.ServiceConfig) (watch.PathMatcher, error) {
if service.Build == nil {
return watch.EmptyMatcher{}, nil
}
name := service.Build.Dockerfile
if name == "" {
name = "Dockerfile"
}
return watch.NewDockerPatternMatcher("/", []string{"**/" + filepath.Base(name)})
}
Comment on lines +775 to +786

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Basename-only match, needed for initialSync but now also applied to the continuous loop, which matches full paths and never needed it. Any unrelated file sharing the Dockerfile's basename anywhere in the tree silently stops syncing. Fix: give getWatchRules its own path-qualified matcher instead of reusing this one; not a same-line suggestion.


// Walks develop.watch.path and checks which files should be copied inside the container
// ignores develop.watch.ignore, Dockerfile, compose files, bind mounted paths and .git
func (s *composeService) initialSync(ctx context.Context, service types.ServiceConfig, trigger types.Trigger, syncer sync.Syncer) error {
Expand All @@ -778,14 +803,14 @@ func (s *composeService) initialSync(ctx context.Context, service types.ServiceC
return err
}

// also exclude override compose files and any custom-named Dockerfile
dockerFilePatterns := append([]string{"Dockerfile"}, cli.DefaultFileNames...)
dockerFilePatterns = append(dockerFilePatterns, cli.DefaultOverrideFileNames...)
if service.Build != nil && service.Build.Dockerfile != "" {
dockerFilePatterns = append(dockerFilePatterns, filepath.Base(service.Build.Dockerfile))
dockerFileIgnore, err := dockerFileIgnoreMatcher(service)
if err != nil {
return err
}

dockerFileIgnore, err := watch.NewDockerPatternMatcher("/", dockerFilePatterns)
composeFiles := append([]string{}, cli.DefaultFileNames...)
composeFiles = append(composeFiles, cli.DefaultOverrideFileNames...)
composeFileIgnore, err := watch.NewDockerPatternMatcher("/", composeFiles)
if err != nil {
return err
}
Expand All @@ -795,6 +820,7 @@ func (s *composeService) initialSync(ctx context.Context, service types.ServiceC
watch.EphemeralPathMatcher(),
dotGitIgnore,
dockerFileIgnore,
composeFileIgnore,
triggerIgnore)

pathsToCopy, err := s.initialSyncFiles(service, trigger, ignoreInitialSync)
Expand Down
64 changes: 64 additions & 0 deletions pkg/compose/watch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,70 @@ func TestInitialSync_ExcludesNestedCustomNamedDockerfile(t *testing.T) {
}})
}

// getWatchRules historically never excluded the service Dockerfile (see
// #14117). The continuous loop matches absolute host paths, so a
// basename-only matcher would miss them.
func TestGetWatchRules_ExcludesDockerfileFromSync(t *testing.T) {
rules, err := getWatchRules(&types.DevelopConfig{
Watch: []types.Trigger{{
Path: "/proj",
Action: types.WatchActionSync,
Target: "/app",
}},
}, types.ServiceConfig{
Name: "svc",
Build: &types.BuildConfig{Context: t.TempDir()},
})
assert.NilError(t, err)
assert.Equal(t, 1, len(rules))

assert.Assert(t, rules[0].Matches(watch.NewFileEvent("/proj/Dockerfile")) == nil)
assert.Assert(t, rules[0].Matches(watch.NewFileEvent("/proj/compose.yaml")) != nil)

got := rules[0].Matches(watch.NewFileEvent("/proj/app.go"))
assert.DeepEqual(t, got, &sync.PathMapping{
HostPath: "/proj/app.go",
ContainerPath: "/app/app.go",
})
}

func TestGetWatchRules_ExcludesCustomNamedDockerfileFromSync(t *testing.T) {
rules, err := getWatchRules(&types.DevelopConfig{
Watch: []types.Trigger{{
Path: "/proj",
Action: types.WatchActionSync,
Target: "/app",
}},
}, types.ServiceConfig{
Name: "svc",
Build: &types.BuildConfig{Context: t.TempDir(), Dockerfile: "docker/Dockerfile.prod"},
})
assert.NilError(t, err)
assert.Assert(t, rules[0].Matches(watch.NewFileEvent("/proj/docker/Dockerfile.prod")) == nil)
assert.Assert(t, rules[0].Matches(watch.NewFileEvent("/proj/Dockerfile")) != nil)
assert.Assert(t, rules[0].Matches(watch.NewFileEvent("/proj/app.go")) != nil)
}

func TestGetWatchRules_CopyActionsExcludeDockerfile(t *testing.T) {
rules, err := getWatchRules(&types.DevelopConfig{
Watch: []types.Trigger{
{Path: "/proj", Action: types.WatchActionSync, Target: "/app"},
{Path: "/proj", Action: types.WatchActionRebuild},
{Path: "/proj", Action: types.WatchActionSyncExec, Target: "/app"},
},
}, types.ServiceConfig{
Name: "svc",
Build: &types.BuildConfig{Context: t.TempDir()},
})
assert.NilError(t, err)
assert.Equal(t, 3, len(rules))

event := watch.NewFileEvent("/proj/Dockerfile")
assert.Assert(t, rules[0].Matches(event) == nil)
assert.Assert(t, rules[1].Matches(event) != nil)
assert.Assert(t, rules[2].Matches(event) == nil)
}

// TestPruneDanglingImagesOnRebuild verifies the post-rebuild prune only
// removes superseded dangling images: a dangling image whose ID matches one
// of the freshly built images must be spared. The lookup used to probe the
Expand Down