-
Notifications
You must be signed in to change notification settings - Fork 2.1k
add top-level "docker bake" command as alias for "docker buildx bake" #5947
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
842d655 to
5c4d0bc
Compare
| | [`attach`](attach.md) | Attach local standard input, output, and error streams to a running container | | ||
| | [`bake`](bake.md) | Build from a file | | ||
| | [`build`](build.md) | Build an image from a Dockerfile | | ||
| | [`builder`](builder.md) | Manage builds | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably something we need to improve when generating these, and have an option to specify a custom target for these (so that bake can link to https://docs.docker.com/buildx/bake.md, and attach can link to docker container attach directly).
ISTR we had this for flags, but not sure now if we had it for commands.
| // alias for "docker buildx bake" if BuildKit is enabled (and the buildx plugin | ||
| // installed). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does it check if buildx plugin is installed? Through aliases annotation?
I just tested some permutations We can do some tweaking after this; currently it shows deprecation warnings buildx missing; BuildKit disabled: |
|
@vvoland @crazy-max let me know if that looks "good enough" for a start. I will try to make some tweaks after this to make the error-message more correct, but it should be more and more of a corner-case if buildx is missing. |
I think this should be a noop if buildx is missing or BuildKit disabled imo as there is no possibility to fallback to legacy builder with bake. Maybe we should error out in such case? |
Yeah, I could change the stub command to return an error; the tricky bit here is that all the warnings are printed in the complex logic we have to forward to buildx; we really need to look at that code again, as it's ... complex. Line 427 in c718d3f
Lines 45 to 134 in c718d3f
|
|
@crazy-max I pushed a temporary commit (to be squashed); to make it a hard failure; I think for this one we can ignore buildx missing; BuildKit disabled: Also added a check if buildx is installed to hide it from "docker --help", but maybe we shouldn't? |
| } | ||
| } | ||
|
|
||
| // FIXME(thaJeztah): need a better way for this; hiding the command here, so that it's present" by default for generating docs etc. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes this looks good for now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me squash the commits; it's not "perfect", but we can tweak more
c494926 to
ec664ee
Compare
The [`docker buildx bake`][1] command has reached GA; this patch adds
a top-level `docker bake` command as alias for `docker buildx bake` to
improve discoverability and make it more convenient to use.
With this patch:
docker --help
Usage: docker [OPTIONS] COMMAND
A self-sufficient runtime for containers
Common Commands:
run Create and run a new container from an image
exec Execute a command in a running container
ps List containers
build Build an image from a Dockerfile
bake Build from a file
pull Download an image from a registry
push Upload an image to a registry
images List images
...
The command is hidden if buildx is not installed;
docker --help
Usage: docker [OPTIONS] COMMAND
A self-sufficient runtime for containers
Common Commands:
run Create and run a new container from an image
exec Execute a command in a running container
ps List containers
build Build an image from a Dockerfile
pull Download an image from a registry
push Upload an image to a registry
images List images
...
We can do some tweaking after this; currently it show an error
in situations where buildx is missing. We don't account for
"DOCKER_BUILDKIT=0", because this is a new feature that requires
buildx, and cannot be "disabled";
buildx missing;
docker bake
ERROR: bake requires the buildx component but it is missing or broken.
Install the buildx component to use bake:
https://docs.docker.com/go/buildx/
BuildKit disabled:
DOCKER_BUILDKIT=0 docker bake
ERROR: bake requires the buildx component but it is missing or broken.
Install the buildx component to use bake:
https://docs.docker.com/go/buildx/
[1]: https://www.docker.com/blog/ga-launch-docker-bake/
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
| // This command is a placeholder / stub that is dynamically replaced by an | ||
| // alias for "docker buildx bake" if BuildKit is enabled (and the buildx plugin | ||
| // installed). | ||
| func NewBakeStubCommand(dockerCLI command.Streams) *cobra.Command { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LOL; thanks @robmry !
Changed this NewBakeStubComment -> NewBakeStubCommand (I blame autocorrect 😂)
The
docker buildx bakecommand has reached GA; this patch adds a top-leveldocker bakecommand as alias fordocker buildx baketo improve discoverability and make it more convenient to use.With this patch:
The command is hidden if buildx is not installed;
We can do some tweaking after this; currently it show an error in situations where buildx is missing. We don't account for "DOCKER_BUILDKIT=0", because this is a new feature that requires buildx, and cannot be "disabled";
buildx missing;
BuildKit disabled:
- Human readable description for the release notes
- A picture of a cute animal (not mandatory but encouraged)