From faeb8bb571dcc99bf877e098a611cf435f6854b6 Mon Sep 17 00:00:00 2001
From: Tibor Vass <tibor@docker.com>
Date: Mon, 6 Aug 2018 22:44:28 +0000
Subject: [PATCH] build: change --console=[auto,false,true] to
 --progress=[auto,plain,tty]

This changes the experimental --console flag to --progress following
feedback indicating avoidable confusion.

In addition to naming changes, the help output now has an additional
clarification, specifically: container output during builds are only
shown when progress output is set to plain. Not mentioning this was also
a big cause of confusion.

Signed-off-by: Tibor Vass <tibor@docker.com>
---
 cli/command/image/build.go          |  8 ++---
 cli/command/image/build_buildkit.go | 10 ++++--
 opts/opts.go                        | 47 +++++++----------------------
 3 files changed, 22 insertions(+), 43 deletions(-)

diff --git a/cli/command/image/build.go b/cli/command/image/build.go
index 1a94e52b9c..ace5b28744 100644
--- a/cli/command/image/build.go
+++ b/cli/command/image/build.go
@@ -58,7 +58,7 @@ type buildOptions struct {
 	isolation      string
 	quiet          bool
 	noCache        bool
-	console        opts.NullableBool
+	progress       string
 	rm             bool
 	forceRm        bool
 	pull           bool
@@ -153,9 +153,9 @@ func NewBuildCommand(dockerCli command.Cli) *cobra.Command {
 	flags.SetAnnotation("stream", "experimental", nil)
 	flags.SetAnnotation("stream", "version", []string{"1.31"})
 
-	flags.Var(&options.console, "console", "Show console output (with buildkit only) (true, false, auto)")
-	flags.SetAnnotation("console", "experimental", nil)
-	flags.SetAnnotation("console", "version", []string{"1.38"})
+	flags.StringVar(&options.progress, "progress", "auto", "Set type of progress output (only if BuildKit enabled) (auto, plain, tty). Use plain to show container output")
+	flags.SetAnnotation("progress", "experimental", nil)
+	flags.SetAnnotation("progress", "version", []string{"1.38"})
 	return cmd
 }
 
diff --git a/cli/command/image/build_buildkit.go b/cli/command/image/build_buildkit.go
index 4427bb182a..a7469a15c8 100644
--- a/cli/command/image/build_buildkit.go
+++ b/cli/command/image/build_buildkit.go
@@ -15,6 +15,7 @@ import (
 	"github.com/docker/cli/cli"
 	"github.com/docker/cli/cli/command"
 	"github.com/docker/cli/cli/command/image/build"
+	"github.com/docker/cli/opts"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/pkg/jsonmessage"
 	"github.com/docker/docker/pkg/stringid"
@@ -191,11 +192,14 @@ func doBuild(ctx context.Context, eg *errgroup.Group, dockerCli command.Cli, opt
 	t := newTracer()
 	ssArr := []*client.SolveStatus{}
 
+	if err := opts.ValidateProgressOutput(options.progress); err != nil {
+		return err
+	}
+
 	displayStatus := func(out *os.File, displayCh chan *client.SolveStatus) {
 		var c console.Console
-		// TODO: Handle interactive output in non-interactive environment.
-		consoleOpt := options.console.Value()
-		if cons, err := console.ConsoleFromFile(out); err == nil && (consoleOpt == nil || *consoleOpt) {
+		// TODO: Handle tty output in non-tty environment.
+		if cons, err := console.ConsoleFromFile(out); err == nil && (options.progress == "auto" || options.progress == "tty") {
 			c = cons
 		}
 		// not using shared context to not disrupt display but let is finish reporting errors
diff --git a/opts/opts.go b/opts/opts.go
index a246a4b259..6485e309e5 100644
--- a/opts/opts.go
+++ b/opts/opts.go
@@ -6,7 +6,6 @@ import (
 	"net"
 	"path"
 	"regexp"
-	"strconv"
 	"strings"
 
 	"github.com/docker/docker/api/types/filters"
@@ -308,6 +307,17 @@ func ValidateSysctl(val string) (string, error) {
 	return "", fmt.Errorf("sysctl '%s' is not whitelisted", val)
 }
 
+// ValidateProgressOutput errors out if an invalid value is passed to --progress
+func ValidateProgressOutput(val string) error {
+	valid := []string{"auto", "plain", "tty"}
+	for _, s := range valid {
+		if s == val {
+			return nil
+		}
+	}
+	return fmt.Errorf("invalid value %q passed to --progress, valid values are: %s", val, strings.Join(valid, ", "))
+}
+
 // FilterOpt is a flag type for validating filters
 type FilterOpt struct {
 	filter filters.Args
@@ -497,38 +507,3 @@ func (m *MemSwapBytes) UnmarshalJSON(s []byte) error {
 	b := MemBytes(*m)
 	return b.UnmarshalJSON(s)
 }
-
-// NullableBool is a type for tri-state boolean options
-type NullableBool struct {
-	b *bool
-}
-
-// Type returns the type
-func (n *NullableBool) Type() string {
-	return ""
-}
-
-// Value returns the value in *bool
-func (n *NullableBool) Value() *bool {
-	return n.b
-}
-
-// Set sets the value. If value is empty string or "auto", nil is set.
-// Otherwise true or false are set based on flag.Bool behavior.
-func (n *NullableBool) Set(value string) error {
-	if value != "auto" && value != "" {
-		b, err := strconv.ParseBool(value)
-		if err != nil {
-			return err
-		}
-		n.b = &b
-	}
-	return nil
-}
-
-func (n *NullableBool) String() string {
-	if n.b == nil {
-		return "auto"
-	}
-	return strconv.FormatBool(*n.b)
-}