Skip to content

Conversation

@thaJeztah
Copy link
Member

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);

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;

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;

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
...

- A picture of a cute animal (not mandatory but encouraged)

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 <[email protected]>
@codecov-commenter
Copy link

Codecov Report

Merging #3898 (2f6e3c9) into master (64c8976) will not change coverage.
The diff coverage is n/a.

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           

@thaJeztah
Copy link
Member Author

Github having issues?

#19 https://github.com/theupdateframework/notary/releases/download/v0.6.1/notary-Linux-amd64
#19 ERROR: invalid response status 503

#20 [build-base-alpine 2/4] COPY --from=xx / /
#20 CANCELED
------
 > https://github.com/theupdateframework/notary/releases/download/v0.6.1/notary-Linux-amd64:
------
ERROR: failed to solve: failed to load cache key: invalid response status 503
make: *** [docker.Makefile:124: build-e2e-image] Error 1
Error: Process completed with exit code 2.

@crazy-max
Copy link
Member

crazy-max commented Dec 4, 2022

docker/buildx#1449 should fix this issue so we avoid sending empty args

@thaJeztah
Copy link
Member Author

docker/buildx#1449 should fix this issue so we avoid sending empty args

Ah, nice! Does it allow distinguishing between VAR="" / VAR= (override with empty string) and VAR ?

@crazy-max
Copy link
Member

Yes if you put VAR="", it will override with an empty string but not VAR=null.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants