Conversation
initialSync already skipped these after docker#14117, but getWatchRules never did. A later edit was still copied into the container. Anchor the shared ignore with **/ so it matches the watch loop's absolute paths, and apply it only to copy actions so rebuild still fires. Signed-off-by: orivital <orivital2924@gmail.com>
| // **/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) { | ||
| names := append([]string{"Dockerfile"}, cli.DefaultFileNames...) |
There was a problem hiding this comment.
this should only select service.Build.Dockerfile.
Default compose filenames and an unused Dockerfile stay eligible for sync. initialSync still excludes compose files. Signed-off-by: orivital <orivital2924@gmail.com>
8940757 to
fe986b5
Compare
glours
left a comment
There was a problem hiding this comment.
Compose-file exclusion isn't wired into getWatchRules (comment 1, isSync nit folded in) — fix described inline and verified by build+test, not posted as a suggestion since it touches too many lines cleanly. Basename matching over-applies to the continuous loop (comment 2, needs a new helper). Tests/gofmt clean otherwise.
| // **/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)}) | ||
| } |
There was a problem hiding this comment.
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.
| @@ -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) | |||
| } | |||
There was a problem hiding this comment.
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.
What I did
#14117 restored the
initialSyncexclusion sodocker compose watchdoes not copy the Dockerfile or compose files into the container on first sync. That PR called out that the continuous loop (getWatchRules) never had the same ignore, and that it would need a differently-anchored matcher because it matches absolute host paths rather than basenames.Without that, a
develop.watchsync on the project root still copiesDockerfile/compose.yaml(and a custombuild.dockerfile) into the container when those files change after watch has started.Shared the ignore as
dockerFileIgnoreMatcherwith**/-anchored patterns so it works for bothinitialSync's basenames and the watch loop's full paths. Applied it only to copy actions (sync,sync+restart,sync+exec) so a rebuild trigger on the same tree still fires when those files change.Test plan
go test ./pkg/compose/ -run 'TestGetWatchRules|TestInitialSync'docker compose watchon a rootsyncto/work: editingDockerfileafter start copies it into the container/workstays emptyTestWatch*e2e cases do not cover this path; the full e2e suite was not runRelated issue
N/A