From c4d07f67e37a0a9e0dc48c6caf65d874af3b8862 Mon Sep 17 00:00:00 2001 From: Tibor Vass Date: Thu, 30 Apr 2020 12:25:30 -0700 Subject: [PATCH 1/4] commands: check if flag is set instead of using flagutil.Tristate Fixes --pull and --no-cache without argument Signed-off-by: Tibor Vass --- commands/bake.go | 8 ++++--- commands/build.go | 44 ++++++++++++++++++++++++++++++++------- util/flagutil/flagutil.go | 36 -------------------------------- 3 files changed, 41 insertions(+), 47 deletions(-) delete mode 100644 util/flagutil/flagutil.go diff --git a/commands/bake.go b/commands/bake.go index e25f088d..c64eefd6 100644 --- a/commands/bake.go +++ b/commands/bake.go @@ -106,13 +106,15 @@ func bakeCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command { Use: "bake [OPTIONS] [TARGET...]", Aliases: []string{"f"}, Short: "Build from a file", - RunE: func(cmd *cobra.Command, args []string) error { - return runBake(dockerCli, args, options) - }, } flags := cmd.Flags() + cmd.RunE = func(cmd *cobra.Command, args []string) error { + handleUnsetFlags(flags, &options) + return runBake(dockerCli, args, options) + } + flags.StringArrayVarP(&options.files, "file", "f", []string{}, "Build definition file") flags.BoolVar(&options.printOnly, "print", false, "Print the options without building") flags.StringArrayVar(&options.overrides, "set", nil, "Override target value (eg: targetpattern.key=value)") diff --git a/commands/build.go b/commands/build.go index ff9eb47d..5cdf251c 100644 --- a/commands/build.go +++ b/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" @@ -71,6 +70,18 @@ type commonOptions struct { exportLoad bool } +func (o *commonOptions) Unset(s string) error { + switch s { + case "pull": + o.noCache = nil + case "no-cache": + o.pull = nil + default: + return errors.Errorf("cannot unset flag %q", s) + } + return nil +} + func runBuild(dockerCli command.Cli, in buildOptions) error { if in.squash { return errors.Errorf("squash currently not implemented") @@ -209,6 +220,21 @@ func buildTargets(ctx context.Context, dockerCli command.Cli, opts map[string]bu return err } +type unsetter interface { + Unset(flagName string) error +} + +func handleUnsetFlags(flags *pflag.FlagSet, options unsetter) error { + for _, name := range []string{"pull", "no-cache"} { + if !flags.Lookup(name).Changed { + if err := options.Unset(name); err != nil { + return err + } + } + } + return nil +} + func buildCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command { var options buildOptions @@ -217,15 +243,17 @@ func buildCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command { Aliases: []string{"b"}, Short: "Start a build", Args: cli.ExactArgs(1), - RunE: func(cmd *cobra.Command, args []string) error { - options.contextPath = args[0] - options.builder = rootOpts.builder - return runBuild(dockerCli, options) - }, } flags := cmd.Flags() + cmd.RunE = func(cmd *cobra.Command, args []string) error { + options.contextPath = args[0] + options.builder = rootOpts.builder + handleUnsetFlags(flags, &options) + return runBuild(dockerCli, options) + } + flags.BoolVar(&options.exportPush, "push", false, "Shorthand for --output=type=registry") flags.BoolVar(&options.exportLoad, "load", false, "Shorthand for --output=type=docker") @@ -305,9 +333,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 { diff --git a/util/flagutil/flagutil.go b/util/flagutil/flagutil.go deleted file mode 100644 index f1f9c9b7..00000000 --- a/util/flagutil/flagutil.go +++ /dev/null @@ -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 -} From 18095ee87b563ab89c5cf05f31088f72b3a2967c Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Thu, 30 Apr 2020 13:01:45 -0700 Subject: [PATCH 2/4] bake: reset no-cache and pull if not set Signed-off-by: Tonis Tiigi --- commands/bake.go | 15 ++++++++++----- commands/build.go | 39 +++++---------------------------------- 2 files changed, 15 insertions(+), 39 deletions(-) diff --git a/commands/bake.go b/commands/bake.go index c64eefd6..7a876722 100644 --- a/commands/bake.go +++ b/commands/bake.go @@ -106,15 +106,20 @@ func bakeCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command { Use: "bake [OPTIONS] [TARGET...]", 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) + }, } flags := cmd.Flags() - cmd.RunE = func(cmd *cobra.Command, args []string) error { - handleUnsetFlags(flags, &options) - return runBake(dockerCli, args, options) - } - flags.StringArrayVarP(&options.files, "file", "f", []string{}, "Build definition file") flags.BoolVar(&options.printOnly, "print", false, "Print the options without building") flags.StringArrayVar(&options.overrides, "set", nil, "Override target value (eg: targetpattern.key=value)") diff --git a/commands/build.go b/commands/build.go index 5cdf251c..24b4702f 100644 --- a/commands/build.go +++ b/commands/build.go @@ -70,18 +70,6 @@ type commonOptions struct { exportLoad bool } -func (o *commonOptions) Unset(s string) error { - switch s { - case "pull": - o.noCache = nil - case "no-cache": - o.pull = nil - default: - return errors.Errorf("cannot unset flag %q", s) - } - return nil -} - func runBuild(dockerCli command.Cli, in buildOptions) error { if in.squash { return errors.Errorf("squash currently not implemented") @@ -220,21 +208,6 @@ func buildTargets(ctx context.Context, dockerCli command.Cli, opts map[string]bu return err } -type unsetter interface { - Unset(flagName string) error -} - -func handleUnsetFlags(flags *pflag.FlagSet, options unsetter) error { - for _, name := range []string{"pull", "no-cache"} { - if !flags.Lookup(name).Changed { - if err := options.Unset(name); err != nil { - return err - } - } - } - return nil -} - func buildCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command { var options buildOptions @@ -243,17 +216,15 @@ func buildCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command { Aliases: []string{"b"}, Short: "Start a build", Args: cli.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + options.contextPath = args[0] + options.builder = rootOpts.builder + return runBuild(dockerCli, options) + }, } flags := cmd.Flags() - cmd.RunE = func(cmd *cobra.Command, args []string) error { - options.contextPath = args[0] - options.builder = rootOpts.builder - handleUnsetFlags(flags, &options) - return runBuild(dockerCli, options) - } - flags.BoolVar(&options.exportPush, "push", false, "Shorthand for --output=type=registry") flags.BoolVar(&options.exportLoad, "load", false, "Shorthand for --output=type=docker") From c9676c79d17a1147950adea1173f231bf1ba60d0 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Thu, 30 Apr 2020 13:41:49 -0700 Subject: [PATCH 3/4] bake: fix hcl tags Signed-off-by: Tonis Tiigi --- bake/bake.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bake/bake.go b/bake/bake.go index 2f5479be..9608eed6 100644 --- a/bake/bake.go +++ b/bake/bake.go @@ -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"` - NoCache bool `json:"no-cache,omitempty": hcl:"no-cache,optional"` + Pull bool `json:"pull,omitempty" hcl:"pull,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. } From 77ddee9314c59de0a29ee4779d8a642776540adf Mon Sep 17 00:00:00 2001 From: Tibor Vass Date: Thu, 30 Apr 2020 13:44:32 -0700 Subject: [PATCH 4/4] bake: fix pull and no-cache overrides Signed-off-by: Tibor Vass --- bake/bake.go | 27 +++++++++++++++++++++------ bake/bake_test.go | 15 +++++++++++++++ 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/bake/bake.go b/bake/bake.go index 9608eed6..26c0e0f2 100644 --- a/bake/bake.go +++ b/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"` - NoCache bool `json:"no-cache,omitempty" hcl:"no-cache,optional"` + Pull *bool `json:"pull,omitempty" hcl:"pull,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, - Pull: t.Pull, + NoCache: noCache, + 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 } diff --git a/bake/bake_test.go b/bake/bake_test.go index 7c296756..49d64981 100644 --- a/bake/bake_test.go +++ b/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) {