fix: provider output handling and watch rebuild re-invocation#13732
fix: provider output handling and watch rebuild re-invocation#13732
Conversation
Provider info and error messages containing newlines broke the TTY progress display (timer drifting to a new line, broken cursor movement). Extract only the first line for progress events via firstLine(). Full messages remain available through the provider's own debug message type. Skip provider services during watch rebuild convergence by adding a SkipProviders flag to CreateOptions, set only by the watch rebuild path. This prevents unnecessary re-invocation of providers on every file change while preserving normal provider execution for all other commands (up, create, run, scale). Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR improves watch-mode stability and reduces unnecessary work by (1) sanitizing provider progress messages that contain newlines (which break TTY progress rendering) and (2) avoiding repeated provider re-invocation during watch-triggered rebuild convergence.
Changes:
- Truncate provider
info/errorprogress events to the first line viafirstLine()to keep TTY progress rendering stable. - Add
SkipProviderstoapi.CreateOptionsand use it from the watch rebuild path to prevent provider services from re-running on each file change. - Implement provider skipping in convergence when
SkipProvidersis enabled.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/compose/watch.go | Sets SkipProviders: true when recreating services during watch rebuilds. |
| pkg/compose/plugins.go | Uses firstLine() for provider progress events and adds the firstLine helper. |
| pkg/compose/convergence.go | Skips provider services during convergence when options.SkipProviders is set. |
| pkg/api/api.go | Extends CreateOptions with a SkipProviders flag. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| s = strings.TrimRight(s, "\n") | ||
| if i := strings.IndexByte(s, '\n'); i >= 0 { | ||
| return s[:i] |
There was a problem hiding this comment.
firstLine only trims \n, so CRLF messages ("\r\n") will leave a trailing \r in the returned first line. That can still interfere with TTY cursor control and makes the doc comment (“stripping any trailing newlines”) inaccurate on Windows. Consider trimming "\r\n" and/or explicitly stripping a trailing \r from the first line segment.
| s = strings.TrimRight(s, "\n") | |
| if i := strings.IndexByte(s, '\n'); i >= 0 { | |
| return s[:i] | |
| s = strings.TrimRight(s, "\r\n") | |
| if i := strings.IndexByte(s, '\n'); i >= 0 { | |
| return strings.TrimRight(s[:i], "\r") |
| // Skip provider services when the caller opted out (e.g. watch rebuild), | ||
| // since providers were already set up during initial "up". | ||
| if service.Provider != nil && options.SkipProviders { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
New SkipProviders behavior is introduced here, but there doesn’t appear to be any test exercising convergence when provider services are present and SkipProviders is true. Given how central convergence is, adding a unit test that asserts provider services are not invoked (and dependent services still converge) would help prevent regressions.
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [docker/compose](https://github.com/docker/compose) | patch | `v5.1.2` → `v5.1.3` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>docker/compose (docker/compose)</summary> ### [`v5.1.3`](https://github.com/docker/compose/releases/tag/v5.1.3) [Compare Source](docker/compose@v5.1.2...v5.1.3) #### What's Changed ##### 🐛 Fixes - fix: provider output handling and watch rebuild re-invocation by [@​glours](https://github.com/glours) in [#​13732](docker/compose#13732) ##### 🔧 Internal - Add Docker Desktop Logs view hints and navigation shortcut by [@​glours](https://github.com/glours) in [#​13721](docker/compose#13721) - Build and push Docker Desktop module image on release by [@​glours](https://github.com/glours) in [#​13726](docker/compose#13726) - Fix typo in SECURITY.md by [@​glours](https://github.com/glours) in [#​13730](docker/compose#13730) - Make hook hint deep links clickable using OSC 8 terminal hyperlinks by [@​glours](https://github.com/glours) in [#​13734](docker/compose#13734) - Remove 'provenance' attribute' by [@​glours](https://github.com/glours) in [#​13738](docker/compose#13738) ##### ⚙️ Dependencies - build(deps): bump github.com/containerd/containerd/v2 from 2.2.2 to 2.2.3 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​13737](docker/compose#13737) **Full Changelog**: <docker/compose@v5.1.2...v5.1.3> </details> --- ### Configuration 📅 **Schedule**: (UTC) - Branch creation - At any time (no schedule defined) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4xMjAuMSIsInVwZGF0ZWRJblZlciI6IjQzLjEyMC4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiLCJhdXRvbWF0aW9uOmJvdC1hdXRob3JlZCIsImRlcGVuZGVuY3ktdHlwZTo6cGF0Y2giXX0=-->
What I did
Provider info and error messages containing newlines broke the TTY progress display (timer drifting to a new line, broken cursor movement). Extract only the first line for progress events via firstLine(). Full messages remain available through the provider's own debug message type.
Skip provider services during watch rebuild convergence by adding a SkipProviders flag to CreateOptions, set only by the watch rebuild path. This prevents unnecessary re-invocation of providers on every file change while preserving normal provider execution for all other commands (up, create, run, scale).
Related issue
N/A
(not mandatory) A picture of a cute animal, if possible in relation to what you did
