-
Notifications
You must be signed in to change notification settings - Fork 5.8k
fix(watch): exclude Dockerfile and compose files from the sync loop #14217
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 { | ||
|
|
@@ -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) | ||
| } | ||
|
|
||
| rules = append(rules, watchRule{ | ||
| Trigger: trigger, | ||
| include: include, | ||
| ignore: watch.NewCompositeMatcher( | ||
| dockerIgnores, | ||
| watch.EphemeralPathMatcher(), | ||
| dotGitIgnore, | ||
| ignore, | ||
| ), | ||
| ignore: watch.NewCompositeMatcher(ignores...), | ||
| service: service.Name, | ||
| }) | ||
| } | ||
|
|
@@ -760,6 +772,21 @@ 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) { | ||
| names := append([]string{"Dockerfile"}, cli.DefaultFileNames...) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this should only select service.Build.Dockerfile.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| names = append(names, cli.DefaultOverrideFileNames...) | ||
| if service.Build != nil && service.Build.Dockerfile != "" { | ||
| names = append(names, filepath.Base(service.Build.Dockerfile)) | ||
| } | ||
| patterns := make([]string, len(names)) | ||
| for i, name := range names { | ||
| patterns[i] = "**/" + name | ||
| } | ||
| return watch.NewDockerPatternMatcher("/", patterns) | ||
| } | ||
|
Comment on lines
+775
to
+786
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Basename-only match, needed for |
||
|
|
||
| // 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 { | ||
|
|
@@ -778,14 +805,7 @@ 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 := watch.NewDockerPatternMatcher("/", dockerFilePatterns) | ||
| dockerFileIgnore, err := dockerFileIgnoreMatcher(service) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
composeFileIgnoreonly exists ininitialSync;getWatchRulesnever builds it, so compose files edited afterwatchstarts are still synced, the bug this PR claims to fix.Fix: inside the
for _, trigger := range config.Watchloop, buildcomposeFileIgnoreanchored ontrigger.Path(same asignore/includejust above, usingcli.DefaultFileNames/DefaultOverrideFileNames), and append it alongsidedockerFileIgnorefor sync/sync-restart/sync-exec triggers. Anchor ontrigger.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) duplicatesisSync()(line 382) instead of reusing it —isSync(trigger) || trigger.Action == types.WatchActionSyncExeccovers the same three cases.