Implements pre-releases #249

ysbaddaden · 2019-01-08T15:32:39Z

I wanted to fix the issue found by tests in #248 but actually refactored Helper::NaturalSort and Helper::Versions as a single Versions module, responsible for comparing and sorting version numbers correctly:

support for unbalanced version numbers (1.0.0 == 1.0 and 1.0.1 > 1.0);
support for pre-release version numbers;
fixed approximate resolver where ~> 0.1.0 matched 0.10.0.

Supporting pre-releases in install/update commands was then just extracting pre-release versions correctly, and not filtering them out unless a requirement specifically requires one.

closes #184
fixes #183

@straight-shoota

It now accepts unbalanced versions numbers, for example: - 1.0 == 1.0.0.0 - 1.0 < 1.0.1 - 1.0.2 > 1.0 The tilde operator was matching invalid version numbers, because we only checked that the version number started with the loose requirement, not that it matched the version number segment. For example "~> 0.1.0" matched "0.10.0" which was invalid. Added some tests from @straight-shoota for Helpers::Versions and some more tests for the lower Helpers::NaturalSort module.

Helpers::NaturalSort wasn't a real natural sort algorithm, it was sorting segments partitioned by non alphanumeric chars. There is now a single Versions object responsible for sorting and comparing version numbers. Introduces support for prerelease versions, defined as a version number that contains an ASCII letter. They're correctly compared using a natural sort algorithm.

A version number is considered a prerelease when it has at least one letter. For example `1.0.alpha` and `2.0.0-pre10` are valid prereleases for release versions `1.0` and `2.0.0` respectively. Prereleases will never be selected until a `shard.yml` specifically requires one. For example `version: 1.0-alpha`. The approximate operator is recommended since it will allow upgrading to the next prereleases and eventually to the release. For example `version: ~> 1.0.0.pre` can be updated to `1.0.0.rc` then `1.0.0` but not `1.1.0.beta`.

ysbaddaden · 2019-01-08T17:36:34Z

Also happens to fix #183 where unbalanced version numbers since 1.0 and 1.0.0 are now considered the same.

src/config.cr

straight-shoota · 2019-01-08T20:45:35Z

src/versions.cr

+
+      when />=(.+)/
+        ver = $1.strip
+        versions.select { |v| compare(v, ver) <= 0 }


Minor detail: If you switch the argument positions of v and ver, the comparator lines up nicely with the restriction >=.

straight-shoota · 2019-01-08T21:08:54Z

test/versions_test.cr

+    end
+
+    def test_sort
+      100.times do


Why so many randomized iterations? The comparisons between items always stay the same, so this doesn't provide any additional coverage.
You don't get any more complete result than iterating through the sorted list and asserting that every item compares lower than its successors.

The original NaturalSort had a bug where the initial order influenced the output ordering. Adding randomization catched it. 100 is a magic number, chances of not catching the issue is veeery low, and this has no impact on speed test (fast).

It just has some smell to use randomized brute force specs 😄

straight-shoota · 2019-01-08T21:13:10Z

Hm, I was already working on a patch for this 🤔 That's why I assigned #184 to myself.

Both result and key implementation are pretty similar in your approach, so 👍

ysbaddaden · 2019-01-09T09:11:55Z

Sorry for duplicating work. I went overboard on some free time I had 🙇

straight-shoota · 2019-01-09T09:12:28Z

src/versions.cr

@@ -146,10 +146,19 @@ module Shards
      end
    end

+    def self.prerelease?(str)
+      str.each_char.any?(&.ascii_letter?)


Hyphens are often also used to determine pre-releases. Rubygems considers any version with a letter or hyphen as pre-release.
This obviously only makes a difference when the pre-release tag doesn't contain any letters (like 0.1-1), which is probably pretty rare.

Does it really? Sigh. 0.1.1-1 looks like a build number that supersedes a 0.1.1 release, it doesn't look like a pre-release to me.

Shall we really support that?

Rubygems doesn't look for a hyphen, only for a letter:
https://github.com/rubygems/rubygems/blob/master/lib/rubygems/version.rb#L290-L297

Take a look at the constructor. Internally it actually replaces a hyphen with .pre.

I'm not sure if this needs to be supported. But rubygems made extra effort to achieve this, so maybe they have a good reason?

Anyway, it's really trivial to implement here, so why not?

it's really trivial to implement here

Actually, it's not, because dashes and dots are equivalent, so 1.0.0-1 == 1.0.0.1. I must use the same trick than rubygems and replace - with a .pre. so 1.0.0-1 becomes 1.0.0.pre.1 to be considered a prerelease, and thus older to 1.0.0.

The replace leads to oddities, like 0.0.1-alpha > 0.0.1.beta because we parse 0.0.1-alpha as 0.0.1.pre.alpha but don't transform 0.0.1.beta.

We should probably check the partition result (dot or dash) in addition to the segment, but... I honestly, I'm tired, here.

If we don't currently support semver perfectly then we don't have to support it perfectly in this PR.

btw, n.n.n-n notation is used by homebrew, where last part denotes revision, far from being pre-release.

@Sija This is pretty common with any package distribution. Crystal's apt and rpm packages also use this release + iteration format.
But in semver, everything following a hyphen makes a pre-release. We can't fit both. But I agree with @RX14.

So every version containing a letter is considered a pre-release.

straight-shoota · 2019-01-09T12:35:24Z

test/versions_test.cr

+module Shards
+  class VersionsTest < Minitest::Test
+    def test_compare
+      # a is older than b:


I would't use older and newer here. Version comparison is not temporal.

ysbaddaden · 2019-01-10T12:52:43Z

I added a few tests, and also ignore semver metadata. I failed to add support for prereleases made of only numbers. Still, it's better than the current one, and there shouldn't any regression since the overall algorithm is the same.

straight-shoota · 2019-01-10T13:42:24Z

CHANGELOG.md

@@ -2,7 +2,15 @@

 ## UNRELEASED

+Features:
+- Experimental support for prereleases. Add a letter to a version number to


Is it really just experimental?

I mean experimental as in "works on my computer, not tested in the wild".

straight-shoota · 2019-01-10T15:04:19Z

src/versions.cr

@@ -146,10 +146,19 @@ module Shards
      end
    end

+    def self.prerelease?(str)
+      str.each_char.any?(&.ascii_letter?)


@Sija This is pretty common with any package distribution. Crystal's apt and rpm packages also use this release + iteration format.
But in semver, everything following a hyphen makes a pre-release. We can't fit both. But I agree with @RX14.

So every version containing a letter is considered a pre-release.

vladfaust · 2019-01-11T09:26:34Z

Thanks, @ysbaddaden 👍

ysbaddaden · 2019-01-11T09:39:30Z

You're welcome —even if it took 2 years.

…natural-sort Implements pre-releases

ysbaddaden added 4 commits January 8, 2019 16:18

Update CHANGELOG

7121263

ysbaddaden added kind:bug kind:feature labels Jan 8, 2019

ysbaddaden added this to the v0.9.0 milestone Jan 8, 2019

ysbaddaden force-pushed the rework-and-fix-natural-sort branch from eb1f1e2 to 7121263 Compare January 8, 2019 15:34

ysbaddaden mentioned this pull request Jan 8, 2019

Add version_restriction_test #248

Closed

ysbaddaden changed the title ~~Implemente pre-releases~~ Implements pre-releases Jan 8, 2019

RX14 approved these changes Jan 8, 2019

View reviewed changes

straight-shoota reviewed Jan 8, 2019

View reviewed changes

straight-shoota reviewed Jan 9, 2019

View reviewed changes

Support semver +metadata (ignore it).

ccd53b5

straight-shoota reviewed Jan 10, 2019

View reviewed changes

ysbaddaden merged commit 301c0ff into master Jan 11, 2019

ysbaddaden deleted the rework-and-fix-natural-sort branch January 11, 2019 09:08

Nicolab mentioned this pull request Jan 30, 2020

Doesn't seem to support versions like 1.0.0-beta1? #318

Closed

f-fr pushed a commit to f-fr/shards that referenced this pull request Jan 2, 2021

Merge pull request crystal-lang#249 from crystal-lang/rework-and-fix-…

b9ff918

…natural-sort Implements pre-releases

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implements pre-releases #249

Implements pre-releases #249

ysbaddaden commented Jan 8, 2019 •

edited

ysbaddaden commented Jan 8, 2019

straight-shoota Jan 8, 2019

straight-shoota Jan 8, 2019

ysbaddaden Jan 9, 2019

straight-shoota Jan 9, 2019

straight-shoota commented Jan 8, 2019

ysbaddaden commented Jan 9, 2019

straight-shoota Jan 9, 2019

ysbaddaden Jan 9, 2019

ysbaddaden Jan 9, 2019

straight-shoota Jan 9, 2019

straight-shoota Jan 9, 2019

ysbaddaden Jan 9, 2019

ysbaddaden Jan 9, 2019

RX14 Jan 10, 2019

Sija Jan 10, 2019

straight-shoota Jan 10, 2019

straight-shoota Jan 9, 2019

ysbaddaden commented Jan 10, 2019

straight-shoota Jan 10, 2019

ysbaddaden Jan 10, 2019

straight-shoota Jan 10, 2019

vladfaust commented Jan 11, 2019

ysbaddaden commented Jan 11, 2019

Implements pre-releases #249

Implements pre-releases #249

Conversation

ysbaddaden commented Jan 8, 2019 • edited

ysbaddaden commented Jan 8, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

straight-shoota commented Jan 8, 2019

ysbaddaden commented Jan 9, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ysbaddaden commented Jan 10, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vladfaust commented Jan 11, 2019

ysbaddaden commented Jan 11, 2019

ysbaddaden commented Jan 8, 2019 •

edited