Rename --print to --call and make previous name hidden #2487
Conversation
daghack
force-pushed
the
call-lint
branch
2 times, most recently
from
59ea5ec
to
ffd2154
Compare
|
Ah, yes! I think I misunderstood the goal slightly! Updated! |
commands/build.go
Outdated
| @@ -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", "", "Call a specific frontend method") | |||
There was a problem hiding this comment.
Needs to also align tests like changing testBuildPrint to testBuildCall in
Line 800 in 7427adb
There was a problem hiding this comment.
I'm concerned about the name of this flag (and the description of it). I don't expect most users to know what a frontend method is. Buildx should have an opinionated UX, but this seems like the opposite, where it's just a very shallow layer on top of buildkit's frontends.
There was a problem hiding this comment.
@colinhemmings - can we get a better description for call? plz
There was a problem hiding this comment.
I agree that the user won't know what frontend is. This flag is for setting the build method use. So I would recommend the following help Set method for evaluating build ("check", "outline", "targets").
@dvdksn thoughts?
There was a problem hiding this comment.
Updated with the suggested description @colinhemmings 🫡
There was a problem hiding this comment.
The description is better. I still have reservations about the flag name. To me, a "call" flag doesn't really imply that this changes the build request type from the default "build an artifact from source" to something else. Neither did the --print flag really (maybe if you squinted hard enough), but that one was experimental. Another flag name that's more descriptive might be:
--mode string Set method for evaluating build (default "build")
Even with a different flag name, having this switch on the build command is a bit misleading because the user is no longer building anything. Subrequests (as far as my understanding goes) is meant for other tasks, like analyzing source files, "introspection", or some custom output generation.
tonistiigi
changed the title
commands/build.go
Outdated
| @@ -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")`) | |||
There was a problem hiding this comment.
nit: we could use subrequests names from BuildKit instead of hardcoding like: https://github.com/moby/buildkit/blob/03d2f9fc4074ea8bf36cea2db8d0829f5442864b/frontend/subrequests/lint/lint.go#L19
Would probably need to move these consts to another package in https://github.com/moby/buildkit/tree/03d2f9fc4074ea8bf36cea2db8d0829f5442864b/frontend/subrequests
Signed-off-by: Talon Bowler <talon.bowler@docker.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
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 <jonathan.sternberg@docker.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
This adds the
--callflag, which allows for calling out to specific frontend method. This is effectively just an alias for the--printexperimental flag that both brings the feature out of experimental and renames it, while maintaining backwards compatibility with the already existing flag.