Skip to content

Vendor also the current c/storage and c/common into Skopeo#304

Merged
Luap99 merged 1 commit intocontainers:mainfrom
mtrmac:test_skopeo-vendor-all
Sep 3, 2025
Merged

Vendor also the current c/storage and c/common into Skopeo#304
Luap99 merged 1 commit intocontainers:mainfrom
mtrmac:test_skopeo-vendor-all

Conversation

@mtrmac
Copy link
Contributor

@mtrmac mtrmac commented Sep 2, 2025

This is ~necessary to benefit from the monorepo, so that interdependent updates to c/storage and c/image are tested as a single unit by the Skopeo test.

Proven in #299 .

@github-actions github-actions bot added the image Related to "image" package label Sep 2, 2025
msg "Replacing upstream skopeo $SKOPEO_CI_BRANCH branch $project_module module"
go mod edit -replace ${project_module}=$GOSRC
for subdir in storage image common; do
project_module=$(sed -n 's/^module //p' < "$GOSRC/$subdir/go.mod")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not the proper way to parse the information, but I can’t quickly find a clean way.

The previous cd $dir; go list . doesn’t work in common which has no code in the root of the directory hierarchy.

cd $dir; go list -m looks like it should work, but in all three directories it lists all of …/common, …/image/v5, …/storage, always in that order. That’s… sort of the data we want, but I don’t understand why it is listing three items, so I’m not comfortable on relying on it.

The sed does not handle comments and other syntax, but at least it’s likely to fail if something unexpected happens.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does the sed gain us anything here, might as well hard code the go.podman.io module paths I think to not have to worry about the go commands behaviours.

Right now, the main thing I can see is the /v5 part of c/image — and that iterating over tuples of (module name, path) is a bit awkward in shell. (Hypothetically, I suppose the vanity names for various libraries in the common-libs repo might use entirely different domains, or something, but that’s not worth worrying about.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

… Ultimately just hard-coding 3 go mod edit lines might be simplest.

Copy link
Member

Choose a reason for hiding this comment

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

… Ultimately just hard-coding 3 go mod edit lines might be simplest.

yeah agree

@mtrmac mtrmac changed the title Vendor Also the current c/storage and c/common into Skopeo Vendor also the current c/storage and c/common into Skopeo Sep 2, 2025
@mtrmac mtrmac force-pushed the test_skopeo-vendor-all branch from 25f0186 to 8b6e19d Compare September 2, 2025 17:28
@packit-as-a-service
Copy link

Packit jobs failed. @containers/packit-build please check.

3 similar comments
@packit-as-a-service
Copy link

Packit jobs failed. @containers/packit-build please check.

@packit-as-a-service
Copy link

Packit jobs failed. @containers/packit-build please check.

@packit-as-a-service
Copy link

Packit jobs failed. @containers/packit-build please check.

This is ~necessary to benefit from the monorepo, so that
interdependent updates to c/storage and c/image are tested as a single
unit by the Skopeo test.

Proven in containers#299 .

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
@mtrmac mtrmac force-pushed the test_skopeo-vendor-all branch from 8b6e19d to 595243d Compare September 2, 2025 17:57
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

@Luap99 Luap99 merged commit ce6777d into containers:main Sep 3, 2025
37 checks passed
@mtrmac mtrmac deleted the test_skopeo-vendor-all branch September 3, 2025 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

image Related to "image" package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants