Skip to content

Conversation

@jankaluza
Copy link
Member

Does this PR introduce a user-facing change?


@openshift-ci openshift-ci bot added the do-not-merge/release-note-label-needed Enforce release-note requirement, even if just None label Aug 19, 2025
@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 Aug 19, 2025

you need to replace all import paths in the repo with the new ones and run make vendor for the new paths to work, so yeah you need something like find . -type f -name '*.go' -exec sed -i -e 's,{OLD},{NEW},g' {} \; And AFAICS you have not fixed the imports in the monrepo either, you must rename all import paths there as well. Everything must be imported as go.podman.io/...

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 20, 2025
@github-actions github-actions bot added machine kind/api-change Change to remote API; merits scrutiny labels Aug 20, 2025
@jankaluza jankaluza force-pushed the monorepo branch 8 times, most recently from 02ffbf1 to 050c638 Compare August 21, 2025 07:34
@jankaluza jankaluza changed the title Do not merge: Try building with monorepo. Switch common, storage and image to monorepo. Sep 1, 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 Sep 1, 2025
github.com/containerd/stargz-snapshotter/estargz v0.17.0 // indirect
github.com/containerd/typeurl/v2 v2.2.3 // indirect
github.com/containernetworking/cni v1.3.0 // indirect
github.com/containers/common v0.62.2 // indirect
Copy link
Member

Choose a reason for hiding this comment

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

That does not seem right we still depend on the old repo!

Copy link
Member Author

@jankaluza jankaluza Sep 1, 2025

Choose a reason for hiding this comment

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

Yes, this is caused by the crc-org/crc ecosystem. I'm preparing PRs for them, but I think this will take some time for them to review and merge. I was wondering if we can merge this PR and update to newer crc once they are done with their part.

Copy link
Member Author

@jankaluza jankaluza Sep 1, 2025

Choose a reason for hiding this comment

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

crc-org/crc#4902, but more will follow. There are more packages which depend on containers/common there directly.

Copy link
Member

Choose a reason for hiding this comment

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

It seems to compile find so I guess it works and it is only the one package pkg/strongunits which is not to bad.
I dislike it but I dislike not getting this merged soon more but this is 100% something that we must fix before the 5.7 release.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah. I wanted to explain this once the tests are green, but you have asked faster than that :-).

I share the same opinion - it is just a single import and I believe it should work fine. I will actively monitor the crc PRs and will bump the crc in our go.mod to remove the dependency asap.

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 approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 1, 2025
@Luap99
Copy link
Member

Luap99 commented Sep 1, 2025

@containers/podman-maintainers PTAL

Copy link
Member

@giuseppe giuseppe 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 Sep 1, 2025
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 1, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: giuseppe, jankaluza, 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

@Luap99 Luap99 added release-note-none and removed do-not-merge/release-note-label-needed Enforce release-note requirement, even if just None labels Sep 1, 2025
@openshift-merge-bot openshift-merge-bot bot merged commit 39072f7 into containers:main Sep 1, 2025
82 of 89 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/api-change Change to remote API; merits scrutiny lgtm Indicates that a PR is ready to be merged. machine release-note-none

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants