blang/semver

Parsing "1.0" returns an error

vmihailenco opened this issue ยท 15 comments

Did you consider allowing versions like "1.0"? I personally did not know that semver requires X.Y.Z format.

blang commented

Yes, that's intended, because of the semver spec:
2. A normal version number MUST take the form X.Y.Z where X, Y, and Z are non-negative integers[..]

"1.0" is not valid semver version, but "1.0.0" is.

blang commented

Oh did you edit your question or did i read wrong?
Since that is a semver library only, it would be spec breaking if i would allow "1.0" as a valid version.

I added last sentence by editing the question (though before I saw your answer).

I will have to fork your lib, because all the world uses versions like "v1.0" and thinks that it is semver format. E.g. gopkg.in supports both v3, v3.N, or v3.N.M format.

blang commented

Yap, you're right but i don't think simply accepting such "wrong" versions would be an option. Maybe i could introduce a ParseTolerant method which converts such versions to valid semvers. I will think about it.
Feel free to fork :)

I don't see anything on gopkg.in that claims anything about semantic versioning (as described by semver.org.) I think the part you referenced, @vmihailenco, is about "selectors."

When using branches or tags to version the GitHub repository, gopkg.in understands that a selector in the URL such as "v1" may be satisfied by a tag or branch "v1.2" or "v1.2.1" (vMAJOR[.MINOR[.PATCH]]) in the repository, and will select the highest version satisfying the requested selector.

Thanks for the link. I am not going to argue about semver spec. I just think that v1 (or v1.0) is not any worse than v1.0.0 and I want to accept it as user input. And I don't see any complications for parser to support such format.

blang commented

I still think this repo should remain strict semver spec only, but i also think it would be a nice addition to support parsing other version variants (v1.0, 1.0, ..) in a separate library and build proper semvers out of it. Anyone interested in contributing?

@blang Would you be open to adding the ParseTolerant function you mentioned above if it was submitted in a PR?

blang commented

I guess that depends on the extend of this function. I think there is a lot of room for convenience beginning at a starting 'v' and ending up trimming spaces, removing quotes etc. If it's a small function with a limited scope and useful for most usecases i would be glad if someone could make this happen.
I think it should be fully decoupled and only preparing the version string before feeding it to Parse.

Great! Just submitted PR #19

blang commented

Thank you

Hi, I had same problem. Thank you for fixing this issue.
However, the PR implementation seems wrong to me.

Version "1.0" generally means latest version of "1.0.x", not "1.0.0". So, why don't you parse "1.0" as "1.0.INT_MAX"?

@awakia That's a fair point! I think the reason I didn't go that route was that serializing versions was a concern for my use case, and 1.0.9223372036854775807 looks a bit funky.

Since we almost certainly don't want to change the underlying data structure, one way to go would be changing the String() function here to wrap the appends of extra version parameters like so:

// Version to string
func (v Version) String() string {
    b := make([]byte, 0, 5)
    b = strconv.AppendUint(b, v.Major, 10)
     if v.Minor != math.MaxUint64 {
        b = append(b, '.')
        b = strconv.AppendUint(b, v.Minor, 10)
    }
    if v.Patch != math.MaxUint64 {
        b = append(b, '.')
        b = strconv.AppendUint(b, v.Patch, 10)
    }

    // ...

This would maintain readability of the stringified versions. @blang what do you think?

blang commented

I think 1.0 is more like a range >0.9 <1.1 than a concrete version, but in case of a version i think 1.0 should be 1.0.0. Using something like 1.0.MAX would also imply: 1.0.0 < 1.0 and 1.0.0 != 1.0 which is counter intuitive imo.
Changing the String() method would also break the spec kinda, that's a no-no.
But you might think about extending the range code, something like Return the last version included in the given range