Replace SemanticVersion with SoftwareVersion

[straight-shoota]

Fixes #7637

This allows to handle a wider variety of version numbers than only semver compatible ones.

The implementation can certainly be improved for performance. This PR focuses on establishing a more versatile behaviour and API. I'd like to postpone performance issues in a subsequent discussion. This can always be improved once the API is settled.

[ghost]

Perhaps this should just be called Version? It's obviously the version of a software.

[j8r]

Release, prerelease can be memoized. I don't really like matching a huge regex to validate the version, parse? is better for this than valid. It will use the actual parser and not assuming a parsable pattern.

[straight-shoota]

@j8r The regex is not optimal and probably a major point to improve (btw. that's why the constant shouldn't be public @r00ster91, I'd like it to vanish eventually). But I really want to focus this PR on the API, not the implementation.

It's designed that the regex can be removed and instead use a custom parser + array storage (for example) without breaking the API. But before we can do that, I'd like to be able to use the new behaviour to help decide on how performance can be improved more effectively. There are a number of different possibilities and it's hard to tell which way to got without further research.

@r00ster91 Version is to unspecific IMO. SoftwareVersion is explicit and unambiguous.

[straight-shoota]

How did the first build on darwin succeed? https://circleci.com/gh/crystal-lang/crystal/21290?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link
It contained an unguarded @[Deprecated] annotation which should fail on 0.27.2...

[yxhuvud]

I find it quite unintuitive that these are not equal. What is the motivation?

[straight-shoota]

They are in fact different with a different number of segments. Both interpretations are legitimate and I don't think there is a definitive argument for one or the other. It mostly depends on the context which behaviour would be expected. The practical relevance however is pretty small anyway. You're usually comparing software versions in a specific context (such as a versioned software package) where changes in version format are pretty rare and unlikely to assign a version that could be considered equal twice. So I don't think it matters much what the comparison returns.

When in doubt, I'd usually prefer to stick to the example of Gem::Version, but there are implications on hash performance. Equality in <=> should also result in equal hashes and ignoring trailing zero segments would add more complexity to the hasher, thus reducing performance.

[ysbaddaden]

It has been a cause of bugs in shards, and now shards considers 1 == 1.0 == 1.0.0 = ...

[straight-shoota]

@ysbaddaden What kind of bugs?

[straight-shoota]

It seems that for Gem::Version the equality of 1.0 == 1.00 has not matched the hashes of these version numbers. This was only fixed relatively recently and there is a current discussion about the inconsistencies this caused in the Ruby implementation.

ruby/rubygems#2595
ruby/rubygems#2597

[RX14]

I think things like Gem::Version prove that comparing version numbers isn't nearly as easy as it looks, there's a bunch of edgcases and tradeoffs. Semver fixed that, by being a rigid spec with rigid semantics. That means it's simple and unopinionated to make a SemanticVersion class.

I'd prefer this more generic class to be in a shard, where there are no backwards-compatibility guarantees past 1.0 so there won't be problems with locking in a bad behaviour. The shard should still have a way to validate that a string is exactly a semver string.

As for what happens to SemanticVersion i'd be fine with moving it into the compiler sources and making it private.

[j8r]

Can someone remind me why the compiler absolutely needs SemanticVersion?
For example with OpenSSL versions there was one approach by comparing integers and an other with the compare_versions macro.

It may be worth it to see to what extent this module is used, and how, before taking any decision.

[straight-shoota]

Can we please confine the general feature discussion to #7637? This PR is about specific implementation.

[straight-shoota]

Closing stale PR.