Skip to content

Conversation

@iTrooz
Copy link

@iTrooz iTrooz commented Nov 4, 2025

Checklist

Ensure you have completed the following checklist for your pull request to be reviewed:

  • Certify you wrote the patch or otherwise have the right to pass it on as an open-source patch by signing all
    commits. (git commit -s). (If needed, use git commit -s --amend). The author email must match
    the sign-off email address. See CONTRIBUTING.md
    for more information.
  • Referenced issues using Fixes: #00000 in commit message (if applicable)
  • Tests have been added/updated (or no tests are needed)
  • Documentation has been updated (or no documentation changes are needed)
  • All commits pass make validatepr (format/lint checks)
  • Release note entered in the section below (or None if no user-facing changes)

Does this PR introduce a user-facing change?

make build flag `--output=tar,dest=-` have correct behaviour (streaming to stdout)
print error on build flag `--output=type=INVALID` instead of outputting image to folder `type=INVALID`

Note

I don't believe documentation changes are needed. I may be wrong ?

This PR lays the groundwork for introducing type=registry, which I will work on when this PR is merged

Signed-off-by: iTrooz <hey@itrooz.fr>
Signed-off-by: iTrooz <hey@itrooz.fr>
Signed-off-by: iTrooz <hey@itrooz.fr>
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 4, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: iTrooz
Once this PR has been reviewed and has the lgtm label, please assign luap99 for approval. For more information see the Code Review Process.

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@iTrooz iTrooz force-pushed the build_output_flag branch from b8090f8 to 25f902d Compare November 4, 2025 14:50
@iTrooz
Copy link
Author

iTrooz commented Nov 4, 2025

I assume this change should be submitted against https://github.com/containers/buildah instead. But where am I supposed to put tests ? Should they stay in this repository ?

@Luap99
Copy link
Member

Luap99 commented Nov 4, 2025

I assume this change should be submitted against https://github.com/containers/buildah instead. But where am I supposed to put tests ? Should they stay in this repository ?

https://github.com/containers/buildah/blob/main/tests/bud.bats for example. Givent he same build command is eposed in buildah you should be able to to just add tests there. Once that is merged in buildah and we updated buildah here we can still have like one or two cases to validate that podman produces the same expected errors.

@mheon
Copy link
Member

mheon commented Nov 4, 2025

For when you port this over to buildah: I wonder if a check that /dev/stdout is not a TTY is appropriate? Streaming a tar file to the terminal isn't very useful, so we want to make sure the user is redirecting to a file or piping to another process.

@iTrooz
Copy link
Author

iTrooz commented Nov 5, 2025

Done ! containers/buildah#6476

I wonder if a check that /dev/stdout is not a TTY is appropriate?

Probably, especially since docker does it. I could submit this in a separate PR, but I'd like to keep this one focused on this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants