ensure deterministic results for SortBySemVer() #559
Merged
Conversation
This file contains 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
Signed-off-by: Kent <[email protected]>
|
CI failure does not appear to be related to the change. |
|
Gentle bump |
|
Thank you for the PR and your patience @krancour, much appreciated. Apologies for the delay in review. I think this is a very useful contribution. |
dlactin
pushed a commit
to dlactin/argocd-image-updater
that referenced
this pull request
May 9, 2024
Signed-off-by: Kent <[email protected]>
sribiere-jellysmack
pushed a commit
to sribiere-jellysmack/argocd-image-updater
that referenced
this pull request
Aug 13, 2024
Signed-off-by: Kent <[email protected]>
This was referenced Oct 18, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #375
The heart of the issue is that https://github.com/masterminds/semver does not yield deterministic results when sorting versions. That package (rightfully) considers
1.24and1.24.0, for example, to be equal. This is fine in nearly all circumstances, but when sorting a list of versions containing both of these, these two can appear in either order, making the results of the sort non-deterministic. If one's goal were to select the latest1.24.x, you'd sometimes end up withlist[len(list) - 1]being1.24and other times being1.24.0.This PR furnishes a drop-in replacement for the
semver.Collectiontype that breaks comparison ties with a lexical comparison of the origin strings from which bothVersionobjects were created.fwiw, I do plan to offer this improvement upstream to https://github.com/masterminds/semver, but am also offering it here because the
semverpackage has moved on to av3which is API-compatible, but has breaking changes in its functionality. I am reluctant to suggest that the solution is the get this merged upstream and then upgrade to v3, as that upgrade would surely affect things in other, unanticipated ways.