-
Notifications
You must be signed in to change notification settings - Fork 1.4k
downgrade go-difflib and go-spew to tagged releases #5654
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
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
what modules? (I guess nydus-snapshotter and stargz-snapshotter but not sure)
Uh oh!
There was an error while loading. Please reload this page.
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.
spf13/viperwas the initial one, but that made it through to kustomize, kustomize to kubernetes, kubernetes to containerd, containerd to stargzThere 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.
If it is already in containerd then what is it breaking for "all users of the containerd module" ? Or doesn't this need to go to containerd's go.mod then?
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.
Trying to break the cycle; currently all users of buildkit and moby will be forced to update the dependency to a version that will never be released.
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.
hmm, I'm looking at containerd v2.0.2 (vendored in buildkit) and it already has this same patch containerd/containerd@f341477 (from you). So the newer versions are not breaking anything for buildkit itself, but afaics your claim is that they may break something for someone importing buildkit (and through it containerd). But if these exclude rules do not carry over to the caller (as they have not carried from containerd to buildkit) then any caller would need to add their own exclude rules anyway. If that's the case then what's the improvement from buildkit adding these extra rules.
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.
s/archived/ or marked "unmaintained"
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.
Are these excludes more "dependency hygiene" than fixing an actual issue? I'm trying to figure out if we are going to keep these excludes "forever".
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.
(Assuming exclude propagates but gets overwritten by other deps)
Can we add a comment that these are downgraded in containerd 2.0.2 but because
nydus-snapshotter,stargz-snapshotterdo not depend on 2.0.2 yet that downgrade does not work unless we make one explicitly.Uh oh!
There was an error while loading. Please reload this page.
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.
There is no particular issue. The world still exists, the wheels keep rolling.
I look at go.mod and see a noticeable amount of untagged deps. But for some reason, @thaJeztah cares a lot more about these two than about others.
@thaJeztah hopes he can stop the plague by quick downgrade of every place where these deps already got (though nothing prevents this from happening again).
To be honest, I'd declare a dep with last commit in 2018 as dead and requiring a complete replacement.
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 other untagged versions don't tag releases, so there's no release to choose from.