Skip to content

Conversation

@inknos
Copy link
Collaborator

@inknos inknos commented Oct 22, 2025

Fixes: https://issues.redhat.com/browse/RUN-3578
Depends on: containers/container-libs#411

Does this PR introduce a user-facing change?

None

@openshift-ci openshift-ci bot added the do-not-merge/release-note-label-needed Enforce release-note requirement, even if just None label Oct 22, 2025
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 22, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: inknos
Once this PR has been reviewed and has the lgtm label, please assign lsm5 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

@openshift-ci openshift-ci bot added release-note-none and removed do-not-merge/release-note-label-needed Enforce release-note requirement, even if just None labels Oct 22, 2025
@inknos
Copy link
Collaborator Author

inknos commented Oct 22, 2025

@baude @Luap99 I am confused on how to write tests for this PR and how to build in this PR when code from container-libs/common is needed

@packit-as-a-service
Copy link

[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore.

1 similar comment
@packit-as-a-service
Copy link

[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore.

@Luap99
Copy link
Member

Luap99 commented Oct 22, 2025

you need a go mod replace to build and test temporarily here in podman

https://github.com/containers/container-libs/blob/main/CONTRIBUTING_GO.md#testing-changes-in-a-dependent-repository

I am not sure we need to add a test

@baude baude added the 6.0 Breaking changes for Podman 6.0 label Oct 22, 2025
@baude
Copy link
Member

baude commented Oct 22, 2025

im ok with no new tests for now.

@baude baude added the No New Tests Allow PR to proceed without adding regression tests label Oct 22, 2025
@TomSweeneyRedHat
Copy link
Member

Changes LGTM with a quick breeze

After the tests had all but passed on containers#27343, I noted
a minor typo.  This corrects the one character flaw.

Signed-off-by: tomsweeneyredhat <[email protected]>
@inknos inknos marked this pull request as draft October 23, 2025 15:20
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 23, 2025
@inknos
Copy link
Collaborator Author

inknos commented Oct 23, 2025

@Luap99 tbh I got lost a little bit thinking how we could append the podman version as a tag. Ideally, that is only when endpoint is the same as config.Machine.Image. I think my implementation is far from being nice, but I am getting closer to the solution.

@Luap99
Copy link
Member

Luap99 commented Oct 23, 2025

Ideally, that is only when endpoint is the same as config.Machine.Image

I don't think is is important and likely not what we want in case RHEL overrides that with a custom image. I think the logic could be quite simple, if it doesn't specify a tag/digest then append the current version. That seems the most logical and consitent behavior for users and documentation purposes because the cli and config values still behave the same then.

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

Labels

6.0 Breaking changes for Podman 6.0 do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. machine No New Tests Allow PR to proceed without adding regression tests release-note-none

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants