add support for default global args #44
Conversation
pkg/build/buildopts.go
Outdated
| bp := utils.BuildPlatforms() | ||
|
|
There was a problem hiding this comment.
Thanks for the fix @stevapple! I do think we can just use DefaultSpec here unless @wlan0 has any concerns with that.
There was a problem hiding this comment.
After investigating the implementation of utils.BuildPlatforms, it looks like container wanted to support a list of build platforms with the help of Rosetta. That means linux/amd64 will be an acceptable (but not default) build platform even when running on an Apple Silicon host. In practice, even BuildKit doesn't quite support such use case though.
In case of blocking future implementation of multi build platform support, I would suggest turning the property into a list, as in 3492640.
|
PR looks good to me overall, except a few nits. I would also like to see a small section added to the docs explaining how this PR enables new cross platform patterns. There needs to be unit tests for this PR. As of now, no unit tests exist for frontend, so this is a good opportunity to start adding tests. |
|
Thanks for adding tests. LGTM |
|
@wlan0 Could you tag a new version with this patch so that we can include it in the upcoming monthly release? |
|
Ping again @wlan0 , would you mind tagging a new version? |
|
@stevapple Made a new tag and opened a PR on container to take the new tag apple/container#605 |
…605) ## Type of Change - [x] Dependency update ## Motivation and Context A change was made in container-builder-shim to support BuildKit's default global args apple/container-builder-shim#44. A new tag of container-builder-shim was made with this change and this PR updates to that new tag for container-builder-shim. Signed-off-by: Kathryn Baldauf <k_baldauf@apple.com>
This PR:
FROMdirectives. This will allow us to use variables like$BUILDPLATFORMand will resolve [Bug]:containerdo support cross-platform build, but may fail with--platformspecified in Dockerfile container#372.BuildPlatforminBOptsand buildconvertOpt.BuildPlatformsagainst it. BuildKit only takes the first build platform so this change should be transparent.I do have a question on how
utils.BuildPlatforms()is implemented and why we didn't just useplatforms.DefaultSpec(). However, this can be left to future discussions and we can maintain the current behavior.