From 7660acf9c7c80e7dbc33a120c7b053e7759af368 Mon Sep 17 00:00:00 2001 From: "Jonathan A. Sternberg" Date: Mon, 16 Jun 2025 12:24:04 -0500 Subject: [PATCH] progress: ensure bake waits for progress to finish printing on error conditions Some minor fixes to the printer and how bake invokes it. Bake previously had a race condition that could result in the display not updating on an error condition, but it was much rarer because the channel communication was much closer. The refactor added a proxy for the status channel so there was more of an opportunity to surface the race condition. When bake exits with an error when reading the bakefiles, it doesn't wait for the printer to finish so it is possible for the printer to update the display after an error is printed. This adds an extra `Wait` in a defer to make sure the printer is finished. `Wait` has also been fixed to allow it to be called multiple times and have the same behavior. Previously, it only waited for the done channel once so only the first wait would block. The `onclose` method is now called every time the display is paused or stopped. That was the previous behavior and it's been restored here. The display only gets refreshed if we aren't exiting. There's no point in initializing another display if we're about to exit. The metric writer attached to the printer was erroneously removed. It is now assigned properly. Signed-off-by: Jonathan A. Sternberg --- commands/bake.go | 6 ++++++ util/progress/printer.go | 21 +++++++++++---------- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/commands/bake.go b/commands/bake.go index 571385058598..b325c0e16e3e 100644 --- a/commands/bake.go +++ b/commands/bake.go @@ -162,7 +162,13 @@ func runBake(ctx context.Context, dockerCli command.Cli, targets []string, in ba attributes := bakeMetricAttributes(dockerCli, driverType, url, cmdContext, targets, &in) progressMode := progressui.DisplayMode(cFlags.progress) + var printer *progress.Printer + defer func() { + if printer != nil { + printer.Wait() + } + }() makePrinter := func() error { var err error diff --git a/util/progress/printer.go b/util/progress/printer.go index 85208b7e200e..38b90c493f8f 100644 --- a/util/progress/printer.go +++ b/util/progress/printer.go @@ -51,8 +51,8 @@ type Printer struct { func (p *Printer) Wait() error { p.closeOnce.Do(func() { close(p.status) - <-p.done }) + <-p.done return p.err } @@ -144,6 +144,7 @@ func NewPrinter(ctx context.Context, out console.File, mode progressui.DisplayMo interrupt: make(chan interruptRequest), state: printerStateRunning, done: make(chan struct{}), + metrics: opt.mw, } go pw.run(ctx, d) @@ -155,21 +156,21 @@ func (p *Printer) run(ctx context.Context, d progressui.Display) { defer close(p.interrupt) var ss []*client.SolveStatus - for p.state != printerStateDone { + for { switch p.state { case printerStatePaused: ss, p.err = p.bufferDisplay(ctx, ss) case printerStateRunning: - var warnings []client.VertexWarning - warnings, ss, p.err = p.updateDisplay(ctx, d, ss) - p.warnings = append(p.warnings, warnings...) - - d, _ = p.newDisplay() + p.warnings, ss, p.err = p.updateDisplay(ctx, d, ss) + if p.opt.onclose != nil { + p.opt.onclose() + } } - } - if p.opt.onclose != nil { - p.opt.onclose() + if p.state == printerStateDone { + break + } + d, _ = p.newDisplay() } }