bakefile: try to keep defaults in the Dockerfile - #3898
Draft
thaJeztah wants to merge 1 commit into
Draft
Conversation
This is an attempt to keep defaults in the Dockerfile, without having to re-define them in the bakefile. Before this patch, build-args would always be set, but either with a default (specified in the bakefile) or an empty string. When setting a build-arg to an empty string, the default from the Dockerfile is ignored (and a literal empty string used instead). Ideally, bakefiles would have some option for this, but I don't think there currently is, but thinking something along the lines of; - a list of build-arg names to consider (taking them from the environment, if present) - possibly changing args to a list of strings instead of a map (so that `key` can be set without `=value`), although this would still require some mangling of the values (must be set without `=` to take environment variables). - maybe other options? Other things I looked at (but I'm very much not familiar with the HCL syntax); - custom conditions (this could reduce the boilerplating `somevar != ""` see https://developer.hashicorp.com/terraform/language/expressions/custom-conditions - local values; trying to use `local` (or `locals`) didn't work; trying to reference them through `local.variable_name` caused an error (but so did referencing variables through `var.variable_name`), perhaps it's just not supported by `bake` (again, I'm very unfamiliar with HCL so maybe I just did it wrong); https://developer.hashicorp.com/terraform/language/values/locals The HCL docs seem to mention them, so perhaps it's just not supported by bake; https://github.com/hashicorp/hcl/blob/27df1ec5fcbc64f6799707e98c7d0433b05ef140/guide/language_design.rst#blocks-vs-object-values I also looked at the current bakefile, and was a bit confused how merging of maps happens (looks like `inherits` does an implicit merge of maps). Either way, with this patch: The default is taken from the Dockerfile; ```bash docker buildx bake --set binary.platform=darwin/amd64,darwin/arm64 binary ... => [linux/arm64/v8 internal] load metadata for docker.io/library/golang:1.19.3-alpine 2.4s ... ``` But can still be overridden; ```bash GO_VERSION=1.19.2 docker buildx bake --set binary.platform=darwin/amd64,darwin/arm64 binary ... => [linux/arm64/v8 internal] load metadata for docker.io/library/golang:1.19.2-alpine 2.4s ... ``` Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #3898 +/- ##
=======================================
Coverage 59.22% 59.22%
=======================================
Files 287 287
Lines 24712 24712
=======================================
Hits 14635 14635
Misses 9192 9192
Partials 885 885 |
Member
Author
|
Github having issues? |
Member
|
docker/buildx#1449 should fix this issue so we avoid sending empty args |
Member
Author
Ah, nice! Does it allow distinguishing between |
Member
|
Yes if you put |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is an attempt to keep defaults in the Dockerfile, without having to re-define them in the bakefile. Before this patch, build-args would always be set, but either with a default (specified in the bakefile) or an empty string.
When setting a build-arg to an empty string, the default from the Dockerfile is ignored (and a literal empty string used instead).
Ideally, bakefiles would have some option for this, but I don't think there currently is, but thinking something along the lines of;
keycan be set without=value), although this would still require some mangling of the values (must be set without=to take environment variables).Other things I looked at (but I'm very much not familiar with the HCL syntax);
somevar != ""see https://developer.hashicorp.com/terraform/language/expressions/custom-conditionslocal(orlocals) didn't work; trying to reference them throughlocal.variable_namecaused an error (but so did referencing variables throughvar.variable_name), perhaps it's just not supported bybake(again, I'm very unfamiliar with HCL so maybe I just did it wrong); https://developer.hashicorp.com/terraform/language/values/locals The HCL docs seem to mention them, so perhaps it's just not supported by bake; https://github.com/hashicorp/hcl/blob/27df1ec5fcbc64f6799707e98c7d0433b05ef140/guide/language_design.rst#blocks-vs-object-valuesI also looked at the current bakefile, and was a bit confused how merging of maps happens (looks like
inheritsdoes an implicit merge of maps).Either way, with this patch:
The default is taken from the Dockerfile;
But can still be overridden;
- A picture of a cute animal (not mandatory but encouraged)