-
Notifications
You must be signed in to change notification settings - Fork 223
Bump c/storage to v1.59.0, c/image to v5.36.0, c/common to v0.64.0, then v0.65.0-dev #2476
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.
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
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: TomSweeneyRedHat 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pull request #2476 has too many files changed.
We can only review pull requests with up to 300 changed files, and this pull request has 709.
|
I'm not complaining, just noting. This vendoring ripped out a number of indirects and probably over a hundred files |
|
linter is failing here for some reason, it looks like the kind of error where it fails to compile |
Luap99
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$ golangci-lint run
WARN [runner] Can't run linter goanalysis_metalinter: buildssa: failed to load package manifests: could not load export data: no export data for "github.com/containers/common/libimage/manifests"
ERRO Running error: can't run linter goanalysis_metalinter
buildssa: failed to load package manifests: could not load export data: no export data for "github.com/containers/common/libimage/manifests"
$ go build ./...
vendor/github.com/docker/docker/api/types/types.go:4:2: cannot find module providing package github.com/docker/docker/api/types/build: import lookup disabled by -mod=vendor
(Go version in go.mod is at least 1.14 and vendor directory exists.)
Something is wrong with this vendor it seems, no idea what is going on.
If I run make vendor I get zero changes but both the go build and golangci-lint commands pass all of the sudden. So somehow it seems the go module metadata that it downloaded fixed it despite not having changes anything in the repo files which is rather concerning.
cc @mtrmac Not sure if you have seen something like this before?
|
Ok so I can reproduce on a fresh system. What is interesting is that deleting the repo and cloning it again triggers the same issue again so it must be some in repo files that trigger it. I narrowed it down to running What I did notice that moby removed the old //import comments in the package definition moby/moby@bf9d739 but still doesn't really make sense to me what it would fail if go mod vendor works fine |
|
Oh wait the issue is that gitignore ignores the |
|
This needs #2478 first and then this must be rebased (and run make vendor again) |
mtrmac
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Luap99 , @TomSweeneyRedHat .
LGTM after rebase.
fa19028 to
68f2a74
Compare
Vendor in c/storage v1.59.0 and c/image v5.36.0 in preparation for Podman v5.36.0 Signed-off-by: tomsweeneyredhat <[email protected]>
Bump to c/common v0.64.0 in preparation for Podman v5.6 Signed-off-by: tomsweeneyredhat <[email protected]>
Bump to the next dev version in main, v0.65.0-dev Signed-off-by: tomsweeneyredhat <[email protected]>
68f2a74 to
6119e7a
Compare
Possible replacement for containers#2476. I'm not sure if I messed something up when trying to merge in containers#2478 there, so thought I'd create this PR to see if I messed something up there when doing the merge/vendor. Signed-off-by: tomsweeneyredhat <[email protected]>
|
LGTM.
|
Possible replacement for containers#2476. I'm not sure if I messed something up when trying to merge in containers#2478 there, so thought I'd create this PR to see if I messed something up there when doing the merge/vendor. Signed-off-by: tomsweeneyredhat <[email protected]>
|
Force-merged this, if I understand correctly, that failure should not affect future PRs. I guess removing so much code that our CI can’t handle that should be celebrated :) |
Possible replacement for containers#2476. I'm not sure if I messed something up when trying to merge in containers#2478 there, so thought I'd create this PR to see if I messed something up there when doing the merge/vendor. Signed-off-by: tomsweeneyredhat <[email protected]>
Possible replacement for containers#2476. I'm not sure if I messed something up when trying to merge in containers#2478 there, so thought I'd create this PR to see if I messed something up there when doing the merge/vendor. Signed-off-by: tomsweeneyredhat <[email protected]>
Possible replacement for containers#2476. I'm not sure if I messed something up when trying to merge in containers#2478 there, so thought I'd create this PR to see if I messed something up there when doing the merge/vendor. Signed-off-by: tomsweeneyredhat <[email protected]>
|
Doh! you beat me to the merge. I found 3 lint issues that should be tended to, I'll do them in a separate PR. |
|
Yeah these are from my debloat work in c/image. I did vendor into podman/buildah to make sure they still work without any issue and since I didn't find anything there I didn't consider trying to vendor them into c/common as I assumed that would be a trivial thing. Guess I was wrong, I will be more careful next time so we don't run into these vendor things on release day. |
Bump c/storage to v1.59.0 and c/image to v5.36.0. Then bump c/common to v0.64.0and then to the next dev version, v0.65.0-dev.
This is in preparation for Podman v5.6.0