-
Notifications
You must be signed in to change notification settings - Fork 786
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
Fix platform handling for empty os/arch values #3698
Conversation
Why are we passing an empty value for one (but not the other) to this API? |
|
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Luap99, rhatdan The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
It looks like I have a legitimate test failure:
@nalind Any idea how this patch could result in the image being pulled? |
The pull logic in common's libimage bases some of its behavior on the contents of the SystemContext that we pass to it, though that's changed a couple of times at this point. |
I guess at this point I will just fix the issue in podman to get things done but we should still consider fixing this in buildah, so either this approach or we make it error for an invalid platform string (e.g. |
Ahh I cannot really fix this in podman so I still need this. I looked at c/common and it looks like containers/common#880 fixes the test issue (at least locally). |
@Luap99 looks like you have file conflicts with this. Rebase fun I'm sure. |
If you run `buildah bud --platform windows/` the os is ignored at the moment and it uses the host os. We should still use the os from the platform in this case. This fixes an issue with podman-remote build where it is possible that only the os is set in the platform string. Signed-off-by: Paul Holzinger <[email protected]>
Is this a flake?
Would be great if someone could restart the test. |
Restarted. Yes, that's a flake :( |
The test failed again :/ |
Tests are finally green. |
LGTM |
/lgtm |
What type of PR is this?
/kind bug
What this PR does / why we need it:
If you run
buildah bud --platform windows/
the os is ignored at themoment and it uses the host os. We should still use the os from the
platform in this case. This fixes an issue with podman-remote build
where it is possible that only the os is set in the platform string.
How to verify it
Which issue(s) this PR fixes:
Special notes for your reviewer:
Does this PR introduce a user-facing change?