-
Notifications
You must be signed in to change notification settings - Fork 18k
go/version: IsValid reports false on inputs like "go1.8.5rc5", "go1.9.2rc2" #68634
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
Comments
The reason go1.8.5rc5 sorts as oldest is because the go/version package doesn't consider it valid; go1 isn't very relevant as you could pick any other valid version to compare with. Compare docs include:
And version.IsValid("go1.8.5rc5") reports false.
Interestingly, I asked about the intended behavior these IsValid edge cases in #62039 (comment) but I don't see an answer there. |
I found this point too when I tested it more deeply, and
I don't think this is a rational reason to disallowing prereleases for patch releases
So I'd like to create a pull request to solve this. |
…invalid version go/version.IsValid do not consider prerelease patch as valid version, but some prerelease patches actually do exist in history versions, such as go1.8.5rc5, go1.8.5rc4, go1.9.2rc2 .The version go1.9.2rc2 even could be found from https://go.dev/dl/?mode=json&include=all, and downloaded from https://dl.google.com/go/go1.9.2rc2.linux-amd64.tar.gz. It's unreasonable to treat an existing version as an invalid version, so it should be fixed. Fixes golang#68634
Change https://go.dev/cl/602096 mentions this issue: |
At this time it's not clear to me that the scope of this package intends to consider a number of one-off old (pre-go1.21.0) releases that followed the "goA.B.CrcN" pattern as valid. The package comment says:
Only "goA.BrcN" pre-release version strings come up in https://go.dev/doc/toolchain. The current release process includes only those pre-release versions, there aren't pre-releases for minor Go releases, so version.IsValid correctly reports true for all planned future Go release versions. Maybe @rsc has more thoughts on this, otherwise I don't expect the behavior of |
Thanks for your patient replying.
I think I should handle all possible situations, so I tried to depend on std pkg Although I no longer rely on this pkg now, I still have some questions:
I'd be glad if you could answer these questions |
I've had a chance to talk with Russ about this, and he confirmed that these "go1.8.5rc5" and "go1.9.2rc2" versions aren't really proper Go releases—they're side-effects that happened to be produced during testing of what eventually became x/build/cmd/releasebot (CL 73333). We would consider having go/version parse them if they were real Go releases that we told users to use, but that's not the case. If there's anything to do here, it would be to consider removing those legacy artifacts that came around from doing testing of early release automation work. Given that it seems they're not really causing that much trouble, it's probably fine to leave them alone. Closing since there's nothing to do for the go/version package here. It's working as intended on these inputs. |
Go version
go version go1.22.5 windows/amd64
Output of
go env
in your module/workspace:What did you do?
When I try to sort go version list that got from below command
I got an incredible result that the version
go1.8.5rc5
is the minimum version, even less thango1
.The reason is that
go/version.IsValid
do not consider prerelease patch as valid version, but some prerelease patch really do exist in history versions, such asgo1.8.5rc5
,go1.8.5rc4
,go1.9.2rc2
.The versiongo1.9.2rc2
even could be discovered from https://go.dev/dl/?mode=json&include=all, and downloaded from https://dl.google.com/go/go1.9.2rc2.linux-amd64.tar.gz, it's very unreasonable to treat it as an invalid version.Here is a snippset to prove this: https://go.dev/play/p/TBz7px5tMd_8
What did you see happen?
output
What did you expect to see?
output
The text was updated successfully, but these errors were encountered: