Skip to content
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

Version comparison is naive #160

Closed
scop opened this issue Jun 22, 2024 · 2 comments · Fixed by #162
Closed

Version comparison is naive #160

scop opened this issue Jun 22, 2024 · 2 comments · Fixed by #162
Labels
bug Something isn't working

Comments

@scop
Copy link
Contributor

scop commented Jun 22, 2024

gup version**

v0.27.1

Description (About the problem)

gup's version comparison is using a simple lexicographic string comparison. This does not work properly e.g. when the number of digits in a version changes.

Steps to reproduce

Example case added in #159 -- 1.2.0 is treated as newer than 1.11.5.

Expected behavior

1.2.0 is older than 1.11.5.

Additional details**

I don't know if the version comparison implementation used by go mod is exposed somewhere. But that would be a good one to use for this.

@scop scop added the bug Something isn't working label Jun 22, 2024
@nao1215
Copy link
Owner

nao1215 commented Jun 22, 2024

@scop
Thank you for your contributions.
I believe that using the following library could resolve this bug in many cases.
https://github.com/hashicorp/go-version

I can create a new PR and fix it myself.
However, I think it would be better for you to update PR #159 so that your contribution record remains. What do you think?

@scop
Copy link
Contributor Author

scop commented Jun 26, 2024

Thanks for addressing this! And no problem at all with not taking #159.

I had a look at that go-version lib myself, but seeing that it is documented to take semver and for example go module snapshot version numbers not following semver, I wasn't quite sure if it's the best choice. But I'm sure it will work fine for the majority of cases.

I would however suggest adding some snapshot versions to the test suite (e.g. grabbed with gup update -m something) to verify that it indeed works as intended for those versions as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants