-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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 podman build -o - #23897
base: main
Are you sure you want to change the base?
fix podman build -o - #23897
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Napuu The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Ephemeral COPR build failed. @containers/packit-build please check. |
b1f33de
to
9e5aaba
Compare
This looks like it should be handled in the build options CLI? https://github.com/containers/buildah/blob/main/pkg/cli/build.go#L267-L275 |
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.
Thank you for your PR! LGTM on first (quick) pass, but one blocker.
The failed packit jobs for epel-9 and fedora-39 will go away after a rebase on main. |
Ensure quiet flag is true when building directly to stdout. Fixes containers#23063 Signed-off-by: Santeri Kääriäinen <santeri.kaariainen@iki.fi>
9e5aaba
to
8cee797
Compare
Test LGTM. Did you look into Dan's question? I think we should try to understand why the Also, for when you repush, these might be two good additions to your test: Expect(output).ShouldNot(ContainSubstring("STEP "))
Expect(output).ShouldNot(ContainSubstring("COPY ")) |
The mentioned code in Buildah isn't having the desired effect because that function is not used by Podman. A somewhat similar functionality is implemented in the function where I made my changes. Seems like it's already acknowledged that it shouldn't really be on Podman's side: https://github.com/containers/podman/blob/main/cmd/podman/common/build.go#L270 I think that would need a bigger refactoring to get rid of that duplicated part altogether. |
Can you change your PR to use said code in Buildah? |
Did some initial digging, and it doesn't seem as trivial as my previous change, so not for now at least. I'll use |
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Ensure quiet flag is true when building directly to stdout.
Fixes #23063
Does this PR introduce a user-facing change?