Merge pull request #268 from tiborvass/fix-tristate

Fix --pull and --no-cache behavior

bake/bake.go

@@ -227,13 +227,13 @@ func (c Config) newOverrides(v []string) (map[string]*Target, error) {
 				if err != nil {
 					return nil, errors.Errorf("invalid value %s for boolean key no-cache", parts[1])
 				}
-				t.NoCache = noCache
+				t.NoCache = &noCache
 			case "pull":
 				pull, err := strconv.ParseBool(parts[1])
 				if err != nil {
 					return nil, errors.Errorf("invalid value %s for boolean key pull", parts[1])
 				}
-				t.Pull = pull
+				t.Pull = &pull
 			default:
 				return nil, errors.Errorf("unknown key: %s", keys[1])
 			}
@@ -348,8 +348,8 @@ type Target struct {
 	SSH        []string          `json:"ssh,omitempty" hcl:"ssh,optional"`
 	Platforms  []string          `json:"platforms,omitempty" hcl:"platforms,optional"`
 	Outputs    []string          `json:"output,omitempty" hcl:"output,optional"`
-	Pull       bool              `json:"pull,omitempty": hcl:"pull,optional"`
+	Pull       *bool             `json:"pull,omitempty" hcl:"pull,optional"`
-	NoCache    bool              `json:"no-cache,omitempty": hcl:"no-cache,optional"`
+	NoCache    *bool             `json:"no-cache,omitempty" hcl:"no-cache,optional"`
 	// IMPORTANT: if you add more fields here, do not forget to update newOverrides and README.
 }
 
@@ -396,6 +396,15 @@ func toBuildOpt(t *Target) (*build.Options, error) {
 		dockerfilePath = path.Join(contextPath, dockerfilePath)
 	}
 
+	noCache := false
+	if t.NoCache != nil {
+		noCache = *t.NoCache
+	}
+	pull := false
+	if t.Pull != nil {
+		pull = *t.Pull
+	}
+
 	bo := &build.Options{
 		Inputs: build.Inputs{
 			ContextPath:    contextPath,
@@ -404,8 +413,8 @@ func toBuildOpt(t *Target) (*build.Options, error) {
 		Tags:      t.Tags,
 		BuildArgs: t.Args,
 		Labels:    t.Labels,
-		NoCache:   t.NoCache,
+		NoCache:   noCache,
-		Pull:      t.Pull,
+		Pull:      pull,
 	}
 
 	platforms, err := platformutil.Parse(t.Platforms)
@@ -500,6 +509,12 @@ func merge(t1, t2 *Target) *Target {
 	if t2.Outputs != nil { // no merge
 		t1.Outputs = t2.Outputs
 	}
+	if t2.Pull != nil {
+		t1.Pull = t2.Pull
+	}
+	if t2.NoCache != nil {
+		t1.NoCache = t2.NoCache
+	}
 	t1.Inherits = append(t1.Inherits, t2.Inherits...)
 	return t1
 }

bake/bake_test.go

@@ -23,6 +23,7 @@ target "webDEP" {
 		VAR_INHERITED = "webDEP"
 		VAR_BOTH = "webDEP"
 	}
+	no-cache = true
 }
 
 target "webapp" {
@@ -44,6 +45,8 @@ target "webapp" {
 		require.Equal(t, "Dockerfile.webapp", *m["webapp"].Dockerfile)
 		require.Equal(t, ".", *m["webapp"].Context)
 		require.Equal(t, "webDEP", m["webapp"].Args["VAR_INHERITED"])
+		require.Equal(t, true, *m["webapp"].NoCache)
+		require.Nil(t, m["webapp"].Pull)
 	})
 
 	t.Run("InvalidTargetOverrides", func(t *testing.T) {
@@ -106,6 +109,18 @@ target "webapp" {
 		require.Equal(t, "foo", *m["webapp"].Context)
 	})
 
+	t.Run("NoCacheOverride", func(t *testing.T) {
+		m, err := ReadTargets(ctx, []string{fp}, []string{"webapp"}, []string{"webapp.no-cache=false"})
+		require.NoError(t, err)
+		require.Equal(t, false, *m["webapp"].NoCache)
+	})
+
+	t.Run("PullOverride", func(t *testing.T) {
+		m, err := ReadTargets(ctx, []string{fp}, []string{"webapp"}, []string{"webapp.pull=false"})
+		require.NoError(t, err)
+		require.Equal(t, false, *m["webapp"].Pull)
+	})
+
 	t.Run("PatternOverride", func(t *testing.T) {
 		// same check for two cases
 		multiTargetCheck := func(t *testing.T, m map[string]*Target, err error) {

commands/bake.go

@@ -107,6 +107,13 @@ func bakeCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command {
 		Aliases: []string{"f"},
 		Short:   "Build from a file",
 		RunE: func(cmd *cobra.Command, args []string) error {
+			// reset to nil to avoid override is unset
+			if !cmd.Flags().Lookup("no-cache").Changed {
+				options.noCache = nil
+			}
+			if !cmd.Flags().Lookup("pull").Changed {
+				options.pull = nil
+			}
 			return runBake(dockerCli, args, options)
 		},
 	}

commands/build.go

@@ -7,7 +7,6 @@ import (
 	"strings"
 
 	"github.com/docker/buildx/build"
-	"github.com/docker/buildx/util/flagutil"
 	"github.com/docker/buildx/util/platformutil"
 	"github.com/docker/buildx/util/progress"
 	"github.com/docker/cli/cli"
@@ -305,9 +304,9 @@ func buildCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command {
 }
 
 func commonBuildFlags(options *commonOptions, flags *pflag.FlagSet) {
-	flags.Var(flagutil.Tristate(options.noCache), "no-cache", "Do not use cache when building the image")
+	options.noCache = flags.Bool("no-cache", false, "Do not use cache when building the image")
 	flags.StringVar(&options.progress, "progress", "auto", "Set type of progress output (auto, plain, tty). Use plain to show container output")
-	flags.Var(flagutil.Tristate(options.pull), "pull", "Always attempt to pull a newer version of the image")
+	options.pull = flags.Bool("pull", false, "Always attempt to pull a newer version of the image")
 }
 
 func listToMap(values []string, defaultEnv bool) map[string]string {

util/flagutil/flagutil.go

@@ -1,36 +0,0 @@
-package flagutil
-
-import "strconv"
-
-type tristate struct {
-	opt *bool
-}
-
-// Tristate is a tri-state boolean flag type.
-// It can be set, but not unset.
-func Tristate(opt *bool) tristate {
-	return tristate{opt}
-}
-
-func (t tristate) Type() string {
-	return "tristate"
-}
-
-func (t tristate) String() string {
-	if t.opt == nil {
-		return "(unset)"
-	}
-	if *t.opt {
-		return "true"
-	}
-	return "false"
-}
-
-func (t tristate) Set(s string) error {
-	b, err := strconv.ParseBool(s)
-	if err != nil {
-		return err
-	}
-	t.opt = &b
-	return nil
-}