-
Notifications
You must be signed in to change notification settings - Fork 584
CLI: Add --platform flag #313
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Will this also work with |
|
@tamird In it's current state here no, but it's trivial to add support if we want. My team is currently debating whether we want this in general though, so we'll see 😬. If they do want this flag I'll add to build as well. |
|
Sounds good. More broadly, do you folks care about compatibility with docker's porcelain? |
|
@tamird It depends. We'd like to not be beholden to the UX of other tools, but I'd say we'd take any suggestions on a case by case basis if we think it's a good idea. Do you have any things you'd like to see besides --platform as an example? |
|
@dcantah nah, currently this is the main one I need. It's a bummer that |
jglogan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before we merge anything, let's discuss what needs to be in this PR if we're going to support os, arch, and platform.
|
Let's not take a scattershot approach to this. With the change this is where we are:
This PR should bring these commands (and any others that deal in platform specification) into harmony, and we should have sane input validation to ensure that the CLI rejects invalid or ambiguous combinations with clear error messages. I think that |
|
Ack, will amend this Thursday |
Fixes apple#231 --platform is useful as it's what most folks are used to from other container tools. We support two separate flags today that when combined (--os and --arch) do essentially the same thing, but this is a combined single string form.
|
Another thing to mention about the It would be great to have support for that here, too. |
|
@tamird Yep. I think it's worthwhile to have it on build/pull/create/run. |
|
@dcantah just to clarify further: in order to build multiarch images you pass a comma-separate list of platforms as the value of |
|
@tamird Yep. It's tricky as I don't believe the argument parser we use will give us that by default. I'll have to write some extra glue. |
|
I keep trying to find time to update this but to no avail. @jglogan may take a stab at it |
|
I'll go ahead and close this one since we have this addressed in #545. Thanks Danny. |
- Fixes #231. - Extends #313 (@dcantah) so that all of `container create`, `container run`, `container build`, `container image pull`, and `container image save` accept the three options. - `container build` now processes comma-separated lists for `--platform`, `--arch`, and `--os`. It first checks `--platform`, assembling the union of all platform values. If that set is non-empty, the builder builds the values in the set. Otherwise, the set consists of all combinations of the specified architecture and os values, finally defaulting to `linux` and the host architecture if no options are provided. - All other commands work accept a single platform, preferring the `--platform` option over `--arch` and `--os` when both are specified. `--os` defaults to `linux`, and `--arch` defaults to the host architecture. - Clarify help messages and present the args in consistent order, with platform first since it takes precedence if present. - Deduplicate redundant platform options for `container build`.
Fixes #231
--platform is useful as it's what most folks are used to from other container tools. We support two separate flags today that when combined (--os and --arch) do essentially the same thing, but this is a combined single string form.