From 250cd44d7075c30b1530c7253326b4a91397d618 Mon Sep 17 00:00:00 2001 From: Talon Bowler Date: Thu, 30 May 2024 13:41:04 -0700 Subject: [PATCH 1/4] Adds a --call flag as an alias to the --print flag and hides the later. Signed-off-by: Talon Bowler --- commands/build.go | 11 ++++++----- docs/reference/buildx_build.md | 2 +- docs/reference/buildx_debug_build.md | 2 +- util/buildflags/printfunc.go | 8 ++++++++ 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/commands/build.go b/commands/build.go index c228a40e..f42186d2 100644 --- a/commands/build.go +++ b/commands/build.go @@ -597,11 +597,6 @@ func buildCmd(dockerCli command.Cli, rootOpts *rootOptions, debugConfig *debug.D flags.StringArrayVar(&options.platforms, "platform", platformsDefault, "Set target platform for build") - if confutil.IsExperimental() { - flags.StringVar(&options.printFunc, "print", "", "Print result of information request (e.g., outline, targets)") - cobrautil.MarkFlagsExperimental(flags, "print") - } - flags.BoolVar(&options.exportPush, "push", false, `Shorthand for "--output=type=registry"`) flags.BoolVarP(&options.quiet, "quiet", "q", false, "Suppress the build output and print image ID on success") @@ -633,12 +628,18 @@ func buildCmd(dockerCli command.Cli, rootOpts *rootOptions, debugConfig *debug.D cobrautil.MarkFlagsExperimental(flags, "root", "detach", "server-config") } + flags.StringVar(&options.printFunc, "call", "", `Set method for evaluating build ("check", "outline", "targets")`) + // hidden flags var ignore string var ignoreSlice []string var ignoreBool bool var ignoreInt int64 + flags.StringVar(&options.printFunc, "print", "", "Print result of information request (e.g., outline, targets)") + cobrautil.MarkFlagsExperimental(flags, "print") + flags.MarkHidden("print") + flags.BoolVar(&ignoreBool, "compress", false, "Compress the build context using gzip") flags.MarkHidden("compress") diff --git a/docs/reference/buildx_build.md b/docs/reference/buildx_build.md index cf4afdc3..d3e3c253 100644 --- a/docs/reference/buildx_build.md +++ b/docs/reference/buildx_build.md @@ -24,6 +24,7 @@ Start a build | [`--builder`](#builder) | `string` | | Override the configured builder instance | | [`--cache-from`](#cache-from) | `stringArray` | | External cache sources (e.g., `user/app:cache`, `type=local,src=path/to/dir`) | | [`--cache-to`](#cache-to) | `stringArray` | | Cache export destinations (e.g., `user/app:cache`, `type=local,dest=path/to/dir`) | +| `--call` | `string` | | Set method for evaluating build (`check`, `outline`, `targets`) | | [`--cgroup-parent`](https://docs.docker.com/reference/cli/docker/image/build/#cgroup-parent) | `string` | | Set the parent cgroup for the `RUN` instructions during build | | `--detach` | | | Detach buildx server (supported only on linux) (EXPERIMENTAL) | | [`-f`](https://docs.docker.com/reference/cli/docker/image/build/#file), [`--file`](https://docs.docker.com/reference/cli/docker/image/build/#file) | `string` | | Name of the Dockerfile (default: `PATH/Dockerfile`) | @@ -36,7 +37,6 @@ Start a build | [`--no-cache-filter`](#no-cache-filter) | `stringArray` | | Do not cache specified stages | | [`-o`](#output), [`--output`](#output) | `stringArray` | | Output destination (format: `type=local,dest=path`) | | [`--platform`](#platform) | `stringArray` | | Set target platform for build | -| `--print` | `string` | | Print result of information request (e.g., outline, targets) (EXPERIMENTAL) | | [`--progress`](#progress) | `string` | `auto` | Set type of progress output (`auto`, `plain`, `tty`). Use plain to show container output | | [`--provenance`](#provenance) | `string` | | Shorthand for `--attest=type=provenance` | | `--pull` | | | Always attempt to pull all referenced images | diff --git a/docs/reference/buildx_debug_build.md b/docs/reference/buildx_debug_build.md index 5a62e3bc..6a904a4b 100644 --- a/docs/reference/buildx_debug_build.md +++ b/docs/reference/buildx_debug_build.md @@ -20,6 +20,7 @@ Start a build | `--builder` | `string` | | Override the configured builder instance | | `--cache-from` | `stringArray` | | External cache sources (e.g., `user/app:cache`, `type=local,src=path/to/dir`) | | `--cache-to` | `stringArray` | | Cache export destinations (e.g., `user/app:cache`, `type=local,dest=path/to/dir`) | +| `--call` | `string` | | Set method for evaluating build (`check`, `outline`, `targets`) | | [`--cgroup-parent`](https://docs.docker.com/reference/cli/docker/image/build/#cgroup-parent) | `string` | | Set the parent cgroup for the `RUN` instructions during build | | `--detach` | | | Detach buildx server (supported only on linux) (EXPERIMENTAL) | | [`-f`](https://docs.docker.com/reference/cli/docker/image/build/#file), [`--file`](https://docs.docker.com/reference/cli/docker/image/build/#file) | `string` | | Name of the Dockerfile (default: `PATH/Dockerfile`) | @@ -32,7 +33,6 @@ Start a build | `--no-cache-filter` | `stringArray` | | Do not cache specified stages | | `-o`, `--output` | `stringArray` | | Output destination (format: `type=local,dest=path`) | | `--platform` | `stringArray` | | Set target platform for build | -| `--print` | `string` | | Print result of information request (e.g., outline, targets) (EXPERIMENTAL) | | `--progress` | `string` | `auto` | Set type of progress output (`auto`, `plain`, `tty`). Use plain to show container output | | `--provenance` | `string` | | Shorthand for `--attest=type=provenance` | | `--pull` | | | Always attempt to pull all referenced images | diff --git a/util/buildflags/printfunc.go b/util/buildflags/printfunc.go index 63807e48..070971c3 100644 --- a/util/buildflags/printfunc.go +++ b/util/buildflags/printfunc.go @@ -13,6 +13,14 @@ func ParsePrintFunc(str string) (*controllerapi.PrintFunc, error) { if str == "" { return nil, nil } + + // "check" has been added as an alias for "lint", + // in order to maintain backwards compatibility + // we need to convert it. + if str == "check" { + str = "lint" + } + csvReader := csv.NewReader(strings.NewReader(str)) fields, err := csvReader.Read() if err != nil { From 89810dc998e88ef3edc2a30c4d8f40ecb858a080 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Mon, 3 Jun 2024 10:29:40 -0700 Subject: [PATCH 2/4] build: set default call method name to build Signed-off-by: Tonis Tiigi --- commands/build.go | 2 +- docs/reference/buildx_build.md | 2 +- docs/reference/buildx_debug_build.md | 2 +- util/buildflags/printfunc.go | 4 +++- 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/commands/build.go b/commands/build.go index f42186d2..7982ce32 100644 --- a/commands/build.go +++ b/commands/build.go @@ -628,7 +628,7 @@ func buildCmd(dockerCli command.Cli, rootOpts *rootOptions, debugConfig *debug.D cobrautil.MarkFlagsExperimental(flags, "root", "detach", "server-config") } - flags.StringVar(&options.printFunc, "call", "", `Set method for evaluating build ("check", "outline", "targets")`) + flags.StringVar(&options.printFunc, "call", "build", `Set method for evaluating build ("check", "outline", "targets")`) // hidden flags var ignore string diff --git a/docs/reference/buildx_build.md b/docs/reference/buildx_build.md index d3e3c253..6170fc3a 100644 --- a/docs/reference/buildx_build.md +++ b/docs/reference/buildx_build.md @@ -24,7 +24,7 @@ Start a build | [`--builder`](#builder) | `string` | | Override the configured builder instance | | [`--cache-from`](#cache-from) | `stringArray` | | External cache sources (e.g., `user/app:cache`, `type=local,src=path/to/dir`) | | [`--cache-to`](#cache-to) | `stringArray` | | Cache export destinations (e.g., `user/app:cache`, `type=local,dest=path/to/dir`) | -| `--call` | `string` | | Set method for evaluating build (`check`, `outline`, `targets`) | +| `--call` | `string` | `build` | Set method for evaluating build (`check`, `outline`, `targets`) | | [`--cgroup-parent`](https://docs.docker.com/reference/cli/docker/image/build/#cgroup-parent) | `string` | | Set the parent cgroup for the `RUN` instructions during build | | `--detach` | | | Detach buildx server (supported only on linux) (EXPERIMENTAL) | | [`-f`](https://docs.docker.com/reference/cli/docker/image/build/#file), [`--file`](https://docs.docker.com/reference/cli/docker/image/build/#file) | `string` | | Name of the Dockerfile (default: `PATH/Dockerfile`) | diff --git a/docs/reference/buildx_debug_build.md b/docs/reference/buildx_debug_build.md index 6a904a4b..af9e7606 100644 --- a/docs/reference/buildx_debug_build.md +++ b/docs/reference/buildx_debug_build.md @@ -20,7 +20,7 @@ Start a build | `--builder` | `string` | | Override the configured builder instance | | `--cache-from` | `stringArray` | | External cache sources (e.g., `user/app:cache`, `type=local,src=path/to/dir`) | | `--cache-to` | `stringArray` | | Cache export destinations (e.g., `user/app:cache`, `type=local,dest=path/to/dir`) | -| `--call` | `string` | | Set method for evaluating build (`check`, `outline`, `targets`) | +| `--call` | `string` | `build` | Set method for evaluating build (`check`, `outline`, `targets`) | | [`--cgroup-parent`](https://docs.docker.com/reference/cli/docker/image/build/#cgroup-parent) | `string` | | Set the parent cgroup for the `RUN` instructions during build | | `--detach` | | | Detach buildx server (supported only on linux) (EXPERIMENTAL) | | [`-f`](https://docs.docker.com/reference/cli/docker/image/build/#file), [`--file`](https://docs.docker.com/reference/cli/docker/image/build/#file) | `string` | | Name of the Dockerfile (default: `PATH/Dockerfile`) | diff --git a/util/buildflags/printfunc.go b/util/buildflags/printfunc.go index 070971c3..6a5a95c1 100644 --- a/util/buildflags/printfunc.go +++ b/util/buildflags/printfunc.go @@ -9,8 +9,10 @@ import ( "github.com/pkg/errors" ) +const defaultPrintFunc = "build" + func ParsePrintFunc(str string) (*controllerapi.PrintFunc, error) { - if str == "" { + if str == "" || str == defaultPrintFunc { return nil, nil } From ef4a165e48375c5ee6f1922b54d6dd87b72e4713 Mon Sep 17 00:00:00 2001 From: "Jonathan A. Sternberg" Date: Fri, 31 May 2024 17:02:53 -0500 Subject: [PATCH 3/4] commands: add an alias for --check to be the same as --call=check This adds an alias for `--check` that causes it to behave the same as `--call=check`. This is done using `BoolFunc` to call a function when the option is seen and to set it to the correct value. This should allow command line flags like `--check --call=targets` to work correctly (even though they conflict) by making it so the first invocation sets the print function to `check` and the second overwrites the first. This is the expected behavior for these types of boolean flags. `BoolFunc` itself is part of the standard library flags package, but never seems to have made it into pflag possibly because it was added in go 1.21. https://pkg.go.dev/flag#FlagSet.BoolFunc Signed-off-by: Jonathan A. Sternberg --- commands/build.go | 16 ++++++++++++++++ docs/reference/buildx_build.md | 1 + docs/reference/buildx_debug_build.md | 1 + util/cobrautil/boolfunc.go | 11 +++++++++++ 4 files changed, 29 insertions(+) create mode 100644 util/cobrautil/boolfunc.go diff --git a/commands/build.go b/commands/build.go index 7982ce32..9ae01139 100644 --- a/commands/build.go +++ b/commands/build.go @@ -629,6 +629,8 @@ func buildCmd(dockerCli command.Cli, rootOpts *rootOptions, debugConfig *debug.D } flags.StringVar(&options.printFunc, "call", "build", `Set method for evaluating build ("check", "outline", "targets")`) + flags.VarPF(callAlias(options, "check"), "check", "", `Shorthand for "--call=check"`) + flags.Lookup("check").NoOptDefVal = "true" // hidden flags var ignore string @@ -1003,6 +1005,20 @@ func maybeJSONArray(v string) []string { return []string{v} } +func callAlias(options *buildOptions, value string) cobrautil.BoolFuncValue { + return func(s string) error { + v, err := strconv.ParseBool(s) + if err != nil { + return err + } + + if v { + options.printFunc = value + } + return nil + } +} + // timeBuildCommand will start a timer for timing the build command. It records the time when the returned // function is invoked into a metric. func timeBuildCommand(mp metric.MeterProvider, attrs attribute.Set) func(err error) { diff --git a/docs/reference/buildx_build.md b/docs/reference/buildx_build.md index 6170fc3a..13513f27 100644 --- a/docs/reference/buildx_build.md +++ b/docs/reference/buildx_build.md @@ -26,6 +26,7 @@ Start a build | [`--cache-to`](#cache-to) | `stringArray` | | Cache export destinations (e.g., `user/app:cache`, `type=local,dest=path/to/dir`) | | `--call` | `string` | `build` | Set method for evaluating build (`check`, `outline`, `targets`) | | [`--cgroup-parent`](https://docs.docker.com/reference/cli/docker/image/build/#cgroup-parent) | `string` | | Set the parent cgroup for the `RUN` instructions during build | +| `--check` | | | Shorthand for `--call=check` | | `--detach` | | | Detach buildx server (supported only on linux) (EXPERIMENTAL) | | [`-f`](https://docs.docker.com/reference/cli/docker/image/build/#file), [`--file`](https://docs.docker.com/reference/cli/docker/image/build/#file) | `string` | | Name of the Dockerfile (default: `PATH/Dockerfile`) | | `--iidfile` | `string` | | Write the image ID to a file | diff --git a/docs/reference/buildx_debug_build.md b/docs/reference/buildx_debug_build.md index af9e7606..ca8b825f 100644 --- a/docs/reference/buildx_debug_build.md +++ b/docs/reference/buildx_debug_build.md @@ -22,6 +22,7 @@ Start a build | `--cache-to` | `stringArray` | | Cache export destinations (e.g., `user/app:cache`, `type=local,dest=path/to/dir`) | | `--call` | `string` | `build` | Set method for evaluating build (`check`, `outline`, `targets`) | | [`--cgroup-parent`](https://docs.docker.com/reference/cli/docker/image/build/#cgroup-parent) | `string` | | Set the parent cgroup for the `RUN` instructions during build | +| `--check` | | | Shorthand for `--call=check` | | `--detach` | | | Detach buildx server (supported only on linux) (EXPERIMENTAL) | | [`-f`](https://docs.docker.com/reference/cli/docker/image/build/#file), [`--file`](https://docs.docker.com/reference/cli/docker/image/build/#file) | `string` | | Name of the Dockerfile (default: `PATH/Dockerfile`) | | `--iidfile` | `string` | | Write the image ID to a file | diff --git a/util/cobrautil/boolfunc.go b/util/cobrautil/boolfunc.go new file mode 100644 index 00000000..53e9d368 --- /dev/null +++ b/util/cobrautil/boolfunc.go @@ -0,0 +1,11 @@ +package cobrautil + +type BoolFuncValue func(string) error + +func (f BoolFuncValue) Set(s string) error { return f(s) } + +func (f BoolFuncValue) String() string { return "" } + +func (f BoolFuncValue) Type() string { return "bool" } + +func (f BoolFuncValue) IsBoolFlag() bool { return true } From 0902294e1a5faa005cac35893f9624c075c0451e Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Mon, 3 Jun 2024 12:09:40 -0700 Subject: [PATCH 4/4] ensure call aliases also work formatting parameters Signed-off-by: Tonis Tiigi --- util/buildflags/printfunc.go | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/util/buildflags/printfunc.go b/util/buildflags/printfunc.go index 6a5a95c1..1019702f 100644 --- a/util/buildflags/printfunc.go +++ b/util/buildflags/printfunc.go @@ -12,17 +12,10 @@ import ( const defaultPrintFunc = "build" func ParsePrintFunc(str string) (*controllerapi.PrintFunc, error) { - if str == "" || str == defaultPrintFunc { + if str == "" { return nil, nil } - // "check" has been added as an alias for "lint", - // in order to maintain backwards compatibility - // we need to convert it. - if str == "check" { - str = "lint" - } - csvReader := csv.NewReader(strings.NewReader(str)) fields, err := csvReader.Read() if err != nil { @@ -51,5 +44,17 @@ func ParsePrintFunc(str string) (*controllerapi.PrintFunc, error) { f.Name = field } } + + // "check" has been added as an alias for "lint", + // in order to maintain backwards compatibility + // we need to convert it. + if f.Name == "check" { + f.Name = "lint" + } + + if f.Name == defaultPrintFunc { + return nil, nil + } + return f, nil }