buildah --platform to take os/arch format and set default ARGS {BUILD|TARGET}{PLATFORM|OS|ARCH|VARIANT} #1903
Conversation
|
Can one of the admins verify this patch?
|
cmd/buildah/bud.go
Outdated
| } | ||
| args.setArch(p) | ||
| } else { | ||
| args.setArch(platforms.DefaultSpec()) |
There was a problem hiding this comment.
Why not us p? Remove the else {}
args.setArch(p)
| av := strings.SplitN(arg, "=", 2) | ||
| if len(av) > 1 { | ||
| args[av[0]] = av[1] | ||
| if av[0] == "TARGETPLATFORM" { |
There was a problem hiding this comment.
Is TARGETPLATFORM a constant defined somewhere?
There was a problem hiding this comment.
@rhatdan I'm thinking this PR is dependent on openshift/imagebuilder#142. @ grooverdan can you confirm? We may need to hold this PR until the imagebuilder gets merged/vendored.
There was a problem hiding this comment.
Defined in https://docs.docker.com/engine/reference/builder/#automatic-platform-args-in-the-global-scope.
Quite right, should wait for dependent PR openshift/imagebuilder#142. I even checked it works with the version there rather than the slightly different version in this PR.
I'll rebase/revendor/retest on the merge/amendment of openshift/imagebuilder#142.
|
bot, add author to whitelist |
grooverdan
force-pushed
the
args-target-platform
branch
2 times, most recently
from
09e58e4 to
67845fa
Compare
|
☔ The latest upstream changes (presumably #1933) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@grooverdan Needs a rebase. |
|
How about I do a rebase after openshift/imagebuilder#142 is merged and then I'll include its changes in vendor too? |
|
@grooverdan That would be some serious awesome-sauce. TYVM! |
grooverdan
force-pushed
the
args-target-platform
branch
from
67845fa to
e43fe2a
Compare
Signed-off-by: Daniel Black <[email protected]>
Signed-off-by: Daniel Black <[email protected]>
Add Variant to SystemContext, Builder, and V1Image Signed-off-by: Daniel Black <[email protected]>
Signed-off-by: Daniel Black <[email protected]>
Signed-off-by: Daniel Black <[email protected]>
Signed-off-by: Daniel Black <[email protected]>
Signed-off-by: Daniel Black <[email protected]>
Signed-off-by: Daniel Black <[email protected]>
Signed-off-by: Daniel Black <[email protected]>
grooverdan
force-pushed
the
args-target-platform
branch
from
e43fe2a to
d56331e
Compare
|
Re-based to master: re-vendored after containers/image#753 (probably incorrectly looking at vendor CI result). Apologies - I seem to forgot to upstream this bit containers/image#753 |
|
☔ The latest upstream changes (presumably 92ff215) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Any update on this? Would love to get closer toward #1590 . Thanks!! |
|
waiting on a variant implementation to land (containers/image#786) and then I can rebase to use it. |
…or armv7l
On armv7l, singularity was apparently downloading images for the v5
architecture variant instead of v7.
Currently, `skopeo copy` is used to copy the image into local containers
storage (/var/lib/containers/storage) for caching purposes, then it is
copied again to `{STAGING_DIR}/docker-oci`. This increases storage
requirements at build time and instead, the image should be copied
to `oci:{new elements cache directory}/oci:{REF}` and the temporary
definition file should have `From: {cache}/oci:{REF}`.
Migration to buildah now depends on
<containers/buildah#1903 (comment)>
(but preferably a dedicated --variant option).
|
@grooverdan This seems to have gotten lost. Containers/image has the feature you were waiting on, are you interested in picking this up? Should we add this functionality or should I just close it? |
|
@rhatdan, did your recent commit take care of all or at least some of this? |
|
Yes buildah currently handles this. Thanks @grooverdan |
5 participants
Closes: #1368
progress towards #1590