Skip to content
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.

Sign up for GitHub

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

Jump to bottom

vendor: bump c/common to dbeb17e40c80 #25468

Merged
merged 1 commit into from
Mar 21, 2025
Merged

vendor: bump c/common to dbeb17e40c80 #25468

merged 1 commit into from
Mar 21, 2025
+117 −98

Conversation

flouthoc
Copy link
Collaborator

@flouthoc flouthoc commented Mar 4, 2025

Does this PR introduce a user-facing change?

None

@flouthoc flouthoc marked this pull request as draft March 4, 2025 22:24
@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 Mar 4, 2025
@flouthoc flouthoc added the No New Tests Allow PR to proceed without adding regression tests label Mar 4, 2025
@flouthoc
Copy link
Collaborator Author

flouthoc commented Mar 4, 2025

Test containers/common#2339

@flouthoc
Copy link
Collaborator Author

flouthoc commented Mar 6, 2025

I think podman save --multi-image-archive (untagged images) and podman image scp transfer are expecting wrong output before this new c/common patch podman was attempting to save images to tar or copy to remote machine via id then while loading both test expect the images to not have names but now since we work directly with images so we can always get their original names back.

@mtrmac Any thoughts, while debugging I think both of these test are expecting wrong outputs.

@mtrmac
Copy link
Collaborator

mtrmac commented Mar 7, 2025

I think the podman image scp transfer test is definitely correct and the code is buggy: It should not say “Loaded image” for a name which was not present at the source at all.

The podman save --multi-image-archive (untagged images) case seems to be similar: (Assuming the created archive does not contain tags) podman load should say which image names it loaded, not what it was deduplicated to.

See the review comments about reporting all of Image.Names() and not just the actually-loaded values.

@mtrmac
Copy link
Collaborator

mtrmac commented Mar 7, 2025

… the c/common review comments that is

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 10, 2025
@flouthoc flouthoc force-pushed the libimage-refactor branch from 37a6c3b to cc05190 Compare March 14, 2025 04:38
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 14, 2025
@flouthoc flouthoc force-pushed the libimage-refactor branch from cc05190 to 6bc49d2 Compare March 17, 2025 18:28
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 17, 2025
@packit-as-a-service Packit-as-a-Service
Copy link

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

@flouthoc flouthoc force-pushed the libimage-refactor branch from 6bc49d2 to 265fd27 Compare March 17, 2025 19:19
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 17, 2025
@flouthoc flouthoc force-pushed the libimage-refactor branch 2 times, most recently from 882cc03 to 5b43f88 Compare March 21, 2025 03:02
@flouthoc
vendor: bump c/common to dbeb17e40c80
f91aca8 
Signed-off-by: flouthoc <flouthoc.git@gmail.com>
@flouthoc flouthoc force-pushed the libimage-refactor branch from 5b43f88 to f91aca8 Compare March 21, 2025 17:25
@flouthoc flouthoc changed the title vendor: bump c/common to PR c/common@2339 vendor: bump c/common to dbeb17e40c80 Mar 21, 2025
@flouthoc flouthoc marked this pull request as ready for review March 21, 2025 17:25
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 21, 2025
@flouthoc
Copy link
Collaborator Author

Includes changes from containers/common#2339

@flouthoc flouthoc changed the title vendor: bump c/common to dbeb17e40c80 vendor: bump c/common to dbeb17e40c80 Mar 21, 2025
@flouthoc
Copy link
Collaborator Author

@containers/podman-maintainers PTAL

@mheon
Copy link
Member

mheon commented Mar 21, 2025

LGTM

Luap99
Luap99 approved these changes Mar 21, 2025
View reviewed changes
Copy link
Member

@Luap99 Luap99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 21, 2025
@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Mar 21, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: flouthoc, Luap99

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 21, 2025
@openshift-merge-bot openshift-merge-bot bot merged commit a444a2a into containers:main Mar 21, 2025
88 of 89 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Luap99 Luap99

Assignees

@Luap99 Luap99

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. No New Tests Allow PR to proceed without adding regression tests release-note-none
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@flouthoc @mtrmac @mheon @Luap99 @openshift-merge-robot