From a50e89c38e6fa890731e672dcc61c57b39e1e475 Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Tue, 25 Oct 2022 10:55:36 +0100 Subject: [PATCH] progress: explicitly fail if tty requested but not available The NewPrinter function is mostly borrowed from buildkit. However, at some point, it seems that the implementations drifted. This patch updates buildx to be more similar in behavior to it's buildkit counterpart, specifically, it will explicitly fail if a TTY output is requested using "--progress=tty", but the output is not available. To gracefully fallback to plain progress in this scenario, "--progress=plain" is required. Signed-off-by: Justin Chadwell --- commands/bake.go | 5 ++++- commands/build.go | 7 +++++-- commands/imagetools/create.go | 5 ++++- commands/util.go | 7 +++++-- util/progress/printer.go | 26 ++++++++++++++++---------- 5 files changed, 34 insertions(+), 16 deletions(-) diff --git a/commands/bake.go b/commands/bake.go index 2266545d..2fbb264f 100644 --- a/commands/bake.go +++ b/commands/bake.go @@ -75,7 +75,10 @@ func runBake(dockerCli command.Cli, targets []string, in bakeOptions) (err error ctx2, cancel := context.WithCancel(context.TODO()) defer cancel() - printer := progress.NewPrinter(ctx2, os.Stderr, os.Stderr, in.progress) + printer, err := progress.NewPrinter(ctx2, os.Stderr, os.Stderr, in.progress) + if err != nil { + return err + } defer func() { if printer != nil { diff --git a/commands/build.go b/commands/build.go index 42be0690..ddcdbb09 100644 --- a/commands/build.go +++ b/commands/build.go @@ -109,7 +109,7 @@ func runBuild(dockerCli command.Cli, in buildOptions) (err error) { return errors.Errorf("--no-cache and --no-cache-filter cannot currently be used together") } - if in.quiet && in.progress != "auto" && in.progress != "quiet" { + if in.quiet && in.progress != progress.PrinterModeAuto && in.progress != progress.PrinterModeQuiet { return errors.Errorf("progress=%s and quiet cannot be used together", in.progress) } else if in.quiet { in.progress = "quiet" @@ -284,7 +284,10 @@ func buildTargets(ctx context.Context, dockerCli command.Cli, opts map[string]bu ctx2, cancel := context.WithCancel(context.TODO()) defer cancel() - printer := progress.NewPrinter(ctx2, os.Stderr, os.Stderr, progressMode) + printer, err := progress.NewPrinter(ctx2, os.Stderr, os.Stderr, progressMode) + if err != nil { + return "", nil, err + } var mu sync.Mutex var idx int diff --git a/commands/imagetools/create.go b/commands/imagetools/create.go index 1f67b974..2d86ffef 100644 --- a/commands/imagetools/create.go +++ b/commands/imagetools/create.go @@ -182,7 +182,10 @@ func runCreate(dockerCli command.Cli, in createOptions, args []string) error { ctx2, cancel := context.WithCancel(context.TODO()) defer cancel() - printer := progress.NewPrinter(ctx2, os.Stderr, os.Stderr, in.progress) + printer, err := progress.NewPrinter(ctx2, os.Stderr, os.Stderr, in.progress) + if err != nil { + return err + } eg, _ := errgroup.WithContext(ctx) pw := progress.WithPrefix(printer, "internal", true) diff --git a/commands/util.go b/commands/util.go index 38777420..7fe5a6eb 100644 --- a/commands/util.go +++ b/commands/util.go @@ -460,7 +460,10 @@ func boot(ctx context.Context, ngi *nginfo) (bool, error) { return false, nil } - printer := progress.NewPrinter(context.TODO(), os.Stderr, os.Stderr, "auto") + printer, err := progress.NewPrinter(context.TODO(), os.Stderr, os.Stderr, progress.PrinterModeAuto) + if err != nil { + return false, err + } baseCtx := ctx eg, _ := errgroup.WithContext(ctx) @@ -477,7 +480,7 @@ func boot(ctx context.Context, ngi *nginfo) (bool, error) { }(idx) } - err := eg.Wait() + err = eg.Wait() err1 := printer.Wait() if err == nil { err = err1 diff --git a/util/progress/printer.go b/util/progress/printer.go index e7dde141..1247a7aa 100644 --- a/util/progress/printer.go +++ b/util/progress/printer.go @@ -11,6 +11,7 @@ import ( "github.com/moby/buildkit/client" "github.com/moby/buildkit/util/progress/progressui" "github.com/opencontainers/go-digest" + "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -69,7 +70,7 @@ func (p *Printer) ClearLogSource(v interface{}) { } } -func NewPrinter(ctx context.Context, w io.Writer, out console.File, mode string) *Printer { +func NewPrinter(ctx context.Context, w io.Writer, out console.File, mode string) (*Printer, error) { statusCh := make(chan *client.SolveStatus) doneCh := make(chan struct{}) @@ -83,21 +84,26 @@ func NewPrinter(ctx context.Context, w io.Writer, out console.File, mode string) mode = v } - go func() { - var c console.Console - switch mode { - case PrinterModeQuiet: - w = io.Discard - case PrinterModeAuto, PrinterModeTty: - if cons, err := console.ConsoleFromFile(out); err == nil { - c = cons + var c console.Console + switch mode { + case PrinterModeQuiet: + w = io.Discard + case PrinterModeAuto, PrinterModeTty: + if cons, err := console.ConsoleFromFile(out); err == nil { + c = cons + } else { + if mode == PrinterModeTty { + return nil, errors.Wrap(err, "failed to get console") } } + } + + go func() { resumeLogs := logutil.Pause(logrus.StandardLogger()) // not using shared context to not disrupt display but let is finish reporting errors pw.warnings, pw.err = progressui.DisplaySolveStatus(ctx, "", c, w, statusCh) resumeLogs() close(doneCh) }() - return pw + return pw, nil }