Go: Better handle pre-release versions #15327
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
owen-mc
reviewed
Jan 15, 2024
| // which is compatible with the SemVer specification | ||
| rcIndex := strings.Index(goVersion, "rc") | ||
| if rcIndex != -1 { | ||
| return semver.Canonical("v"+goVersion[2:rcIndex]) + "-" + goVersion[rcIndex:] |
There was a problem hiding this comment.
I find it confusing that we call semver.Canonical here and also on line 798 (but not at other call-sites). Is that intentional?
There was a problem hiding this comment.
I think L798 is leftover from the first attempt at getting this to work and is no longer necessary, so can be removed.
There was a problem hiding this comment.
That makes sense. Would there be any benefit in also using semver.Canonical on the other branch, so that we know we're always returning a canonical version?
owen-mc
approved these changes
Jan 15, 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.
The
go versioncommand returns a version identifier which is not a valid semantic version. This is noticeable for pre-release builds of Go, such asgo1.22rc1. Our existing handling of such versions meant that they were treated as invalid bysemver.Compareand were therefore considered lower than all other versions. This PR implements a workaround for version identifiers that containrcin them so that they are converted into a valid semantic version and therefore compared correctly.