frontends: add dockerui package to make frontend parameters reusable #3606
Conversation
|
What is "ui"? (Does not seem to be "user interface"?) |
|
There is a slight logic change on reading |
|
@AkihiroSuda I'm open to other names. It is supposed to mean that this is a mapping to the flags/args of Docker build command. If we add a new flag there then we would update this package with the handler code. |
tonistiigi
force-pushed
the
dockerui
branch
2 times, most recently
from
f7bc61e
to
28b46d3
Compare
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
| keyTarget = "target" | ||
| keyCgroupParent = "cgroup-parent" | ||
| keyForceNetwork = "force-network-mode" | ||
| keyGlobalAddHosts = "add-hosts" | ||
| keyHostname = "hostname" | ||
| keyImageResolveMode = "image-resolve-mode" | ||
| keyMultiPlatform = "multi-platform" | ||
| keyNoCache = "no-cache" | ||
| keyShmSize = "shm-size" | ||
| keyTargetPlatform = "platform" | ||
| keyUlimit = "ulimit" | ||
| keyCacheFrom = "cache-from" // for registry only. deprecated in favor of keyCacheImports | ||
| keyCacheImports = "cache-imports" // JSON representation of []CacheOptionsEntry |
There was a problem hiding this comment.
Could we take the opportunity to expose some keys for clients? Could be handy for the toSolveOpt func in buildx for example: https://github.com/docker/buildx/blob/78058ce5f301fb5f7acb31eaee16fc85e37911dc/build/build.go#L363
Maybe move them in another sub package as discussed to avoid unneeded imports.
There was a problem hiding this comment.
Yeah, it can be a subpackage. This can be a follow-up. Maybe it makes sense to put the parsers for these values also to subpackage(would need to see how it would be used on the client side to decide that).
| for i, tp := range targets { | ||
| i, tp := i, tp | ||
| eg.Go(func() error { | ||
| ref, img, err := fn(ctx, tp, i) |
There was a problem hiding this comment.
As far as I can tell, i is only used to unset ConvertOpt.Warn to nil - is there a way we can remove it from here? I think all we should need here is the context and the platform.
There was a problem hiding this comment.
What do you suggest for the solution for the Warn issue then?
There was a problem hiding this comment.
Yeah, not sure 🤔 I tried playing around with maybe doing platform matching here, but it's way more hacky than just having the index parameter.
I think it's fine as is, but we should watch this interface to avoid changing it that much in the future, if we want frontend authors to use it and be able to easily upgrade between buildkit versions.
Can any GUI or webUI be used? |
|
The package name This is not related to anything web UI or GUI focused. |
BuildKit frontend protocol does not set restrictions on what parameters can be passed to frontends, what local sources they are expected to read, that files to open to read build definition etc. Usually however when writing a new frontend the authors expect it to work when called by Docker. Do do that, they need to figure out how the Docker flags map to frontend attributes and use the same names. Usually, it means that they just need to copy a significant amount of code from the Dockerfile frontend.
This PR splits up the handling of parameters and contexts that Docker clients pass into a separate reusable package that any frontend can use to significantly simplify their code. We can also use this now to make the gateway frontend understand the named context without duplicating any code. The Dockerfile parts are still separated and other frontends do not need to import any actual Dockerfile logic.