Skip to content

vendor to latest c/{common,image,storage}#5929

Merged
openshift-merge-bot[bot] merged 2 commits intocontainers:mainfrom
Luap99:vendor
Jan 24, 2025
Merged

vendor to latest c/{common,image,storage}#5929
openshift-merge-bot[bot] merged 2 commits intocontainers:mainfrom
Luap99:vendor

Conversation

@Luap99
Copy link
Copy Markdown
Member

@Luap99 Luap99 commented Jan 21, 2025

Make sure all the test pass before we do a final vendor dance.

What type of PR is this?

/kind api-change
/kind bug
/kind cleanup
/kind deprecation
/kind design
/kind documentation
/kind failing-test
/kind feature
/kind flake
/kind other

What this PR does / why we need it:

How to verify it

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

None

@nalind
Copy link
Copy Markdown
Member

nalind commented Jan 21, 2025

Looks like we need to update

buildah/tests/run.bats

Line 974 in eac9331

for mask in /proc/acpi /proc/kcore /proc/keys /proc/latency_stats /proc/sched_debug /proc/scsi /proc/timer_list /proc/timer_stats /sys/dev/block /sys/devices/virtual/powercap /sys/firmware /sys/fs/selinux; do
to account for containers/common#2278.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 22, 2025
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 22, 2025
@Luap99
Copy link
Copy Markdown
Member Author

Luap99 commented Jan 22, 2025

@mtrmac Tests are broken, it seems to be from your new code containers/image#2613?!

[+1757s] # Error: writing blob: layer 0 (blob "sha256:85a67008b2828ae9236a430a302a14053eaec77addb721865cc4857147a94b2f"/""/"sha256:85a67008b2828ae9236a430a302a14053eaec77addb721865cc4857147a94b2f") does not match config's DiffID "sha256:d2421964bad195c959ba147ad21626ccddc73a4f2638664ad1c07bd9df48a675"

@mtrmac
Copy link
Copy Markdown
Contributor

mtrmac commented Jan 22, 2025

@mtrmac Tests are broken, it seems to be from your new code containers/image#2613?!

That very likely needs an equivalent of containers/podman@5235ee5 ; I’ll take a look and file a PR today.

@mtrmac mtrmac mentioned this pull request Jan 22, 2025
Merged
mtrmac
mtrmac reviewed Jan 22, 2025
View reviewed changes
Comment thread go.mod Outdated
Comment on lines +10 to +11
github.com/containers/common v0.61.1-0.20250121185748-34a90afcdc6d
github.com/containers/image/v5 v5.33.1-0.20250121231649-a45ebe065b9e
Copy link
Copy Markdown
Contributor

@mtrmac mtrmac Jan 22, 2025

Choose a reason for hiding this comment

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

These are, nominally, downgrades.

For c/image, containers/image#2691 should help.

Copy link
Copy Markdown
Member Author

@Luap99 Luap99 Jan 23, 2025

Choose a reason for hiding this comment

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

They are not downgrades?!
34a90afcdc6d (containers/common@34a90af)
a45ebe065b9e (containers/image@a45ebe0)

These are (or were) commits from the latest main branch at the time I did the vendor. So they are not downgrades if we consider main always to be newest which it must be.

Copy link
Copy Markdown
Contributor

@mtrmac mtrmac Jan 23, 2025

Choose a reason for hiding this comment

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

You’re right that main is newer — it’s just that Go doesn’t know that.

@mtrmac
Copy link
Copy Markdown
Contributor

mtrmac commented Jan 22, 2025

I’ll take a look and file a PR today.

#5932 .

@Luap99 Luap99 mentioned this pull request Jan 23, 2025
Closed
@TomSweeneyRedHat
Copy link
Copy Markdown
Member

TomSweeneyRedHat commented Jan 23, 2025

Changes LGTM, looks like you may need to rebase.

Luap99 added 2 commits January 24, 2025 11:45
@Luap99
vendor to latest c/{common,image,storage}
4887a4f 
Make sure all the test pass before we do a final vendor dance.

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
@Luap99
tests: remove masked /sys/dev/block check
2119c8d 
The c/common defaults were changed to no longer mask this path[1]. As
such we need to remove it from this test.

[1] containers/common#2278

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
@Luap99
Copy link
Copy Markdown
Member Author

Luap99 commented Jan 24, 2025

@TomSweeneyRedHat @nalind This should be good to merge. Then we can vendor buildah@main on the podman side

@nalind
Copy link
Copy Markdown
Member

nalind commented Jan 24, 2025

/approve
/lgtm

@openshift-ci openshift-ci Bot added the lgtm label Jan 24, 2025
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Jan 24, 2025

[APPROVALNOTIFIER] This PR is APPROVED

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

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details 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-merge-bot openshift-merge-bot Bot merged commit 21fe6dc into containers:main Jan 24, 2025
@Luap99 Luap99 deleted the vendor branch January 24, 2025 18:39
@stale-locking-app stale-locking-app Bot locked as resolved and limited conversation to collaborators Apr 25, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@mtrmac mtrmac mtrmac left review comments

Assignees

@nalind nalind

Labels

approved lgtm locked - please file new issue/PR

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Luap99 @nalind @mtrmac @TomSweeneyRedHat @openshift-merge-robot