From bf6c737472088c699850f8eec3cd37a75636e2f2 Mon Sep 17 00:00:00 2001 From: Julien Portalier Date: Mon, 7 Jan 2019 01:22:29 +0100 Subject: [PATCH 1/5] Rework NaturalSort to fix version comparison issues 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. --- Makefile | 2 +- src/helpers/natural_sort.cr | 81 ++++++++++++++++++++++--------- src/helpers/versions.cr | 24 ++++++--- src/resolvers/git.cr | 2 +- test/helpers/natural_sort_test.cr | 63 ++++++++++++++++++++++++ test/helpers/versions_test.cr | 37 ++++++++++++++ test/support/mock_resolver.cr | 2 +- 7 files changed, 179 insertions(+), 32 deletions(-) create mode 100644 test/helpers/natural_sort_test.cr create mode 100644 test/helpers/versions_test.cr diff --git a/Makefile b/Makefile index 1f505c006..3a2c8d9a6 100644 --- a/Makefile +++ b/Makefile @@ -34,7 +34,7 @@ uninstall: phony test: test_unit test_integration test_unit: phony - $(CRYSTAL) run test/*_test.cr + $(CRYSTAL) run test/*_test.cr test/helpers/*_test.cr test_integration: bin/shards phony $(CRYSTAL) run test/integration/*_test.cr diff --git a/src/helpers/natural_sort.cr b/src/helpers/natural_sort.cr index 87c9dd298..7806a84bd 100644 --- a/src/helpers/natural_sort.cr +++ b/src/helpers/natural_sort.cr @@ -1,38 +1,75 @@ module Shards module Helpers module NaturalSort - EXTRACT_ALPHA_OR_NUMBER_GROUP = /([A-Za-z]+|[\d]+)/ - IS_NUMBER_STRING = /\A\d+\Z/ + NON_ALPHANUMERIC = /[^a-zA-Z0-9]/ - def natural_sort(a, b) - NaturalSort.sort(a, b) + def natural_sort(versions) + versions.sort { |a, b| NaturalSort.compare(a, b) } end - def self.sort(a, b) - ia = ib = 0 - la, lb = a.size, b.size + def natural_compare(a, b) + NaturalSort.compare(a, b) + end + + def self.compare(a, b) + if a == b + return 0 + end loop do - return 0 if ia > la - return 0 unless a[ia..-1] =~ EXTRACT_ALPHA_OR_NUMBER_GROUP - aaa = $1 - ia += $1.size - - return 0 if ib > lb - return 0 unless b[ib..-1] =~ EXTRACT_ALPHA_OR_NUMBER_GROUP - bbb = $1 - ib += $1.size - - if aaa =~ IS_NUMBER_STRING && bbb =~ IS_NUMBER_STRING - aaa, bbb = aaa.to_i, bbb.to_i - ret = bbb <=> aaa - else - ret = bbb <=> aaa + # extract next segment from version number ("1.0.2" => "1" then "0" then "2"): + a_segment, a = next_segment(a) + b_segment, b = next_segment(b) + + # accept unbalanced version numbers ("1.0" == "1.0.0.0", "1.0" < "1.0.1") + if a_segment.empty? + only_zeroes_remaining(b_segment, b) { return 1 } + return 0 end + # accept unbalanced version numbers ("1.0.0.0" == "1.0", "1.0.1" > "1.0") + if b_segment.empty? + only_zeroes_remaining(a_segment, a) { return -1 } + return 0 + end + + # try to convert segments to numbers: + a_num = a_segment.to_i?(whitespace: false) + b_num = b_segment.to_i?(whitespace: false) + + # compare: + ret = + if a_num && b_num + b_num <=> a_num + else + b_segment <=> a_segment + end + + # if different return the result (older or newer), otherwise continue + # to the next segment: return ret unless ret == 0 end end + + private def self.next_segment(str) + segment, _, str = str.partition(NON_ALPHANUMERIC) + {segment, str} + end + + private def self.only_zeroes_remaining(segment, str) + unless segment.to_i?(whitespace: false) == 0 + yield + end + + loop do + segment, str = next_segment(str) + break if segment.empty? + + unless segment.to_i?(whitespace: false) == 0 + yield + end + end + end end end end diff --git a/src/helpers/versions.cr b/src/helpers/versions.cr index eb8f6246f..a1a2a1478 100644 --- a/src/helpers/versions.cr +++ b/src/helpers/versions.cr @@ -6,16 +6,17 @@ module Shards include NaturalSort def resolve_versions(versions, requirements) - requirements + matching_versions = requirements .map { |requirement| resolve_requirement(versions, requirement) } .reduce(versions) { |a, e| a & e } - .sort { |a, b| natural_sort(a, b) } + natural_sort(matching_versions) end def resolve_requirement(versions, requirement) case requirement when "*", "" versions + when /~>(.+)/ ver = $1.strip vver = if idx = ver.rindex('.') @@ -23,19 +24,28 @@ module Shards else ver end - versions.select { |v| v.starts_with?(vver) && (natural_sort(v, ver) <= 0) } + versions.select do |v| + v.starts_with?(vver) && + !v[vver.size]?.try(&.ascii_alphanumeric?) && + (natural_compare(v, ver) <= 0) + end + when />=(.+)/ ver = $1.strip - versions.select { |v| natural_sort(v, ver) <= 0 } + versions.select { |v| natural_compare(v, ver) <= 0 } + when /<=(.+)/ ver = $1.strip - versions.select { |v| natural_sort(v, ver) >= 0 } + versions.select { |v| natural_compare(v, ver) >= 0 } + when />(.+)/ ver = $1.strip - versions.select { |v| natural_sort(v, ver) < 0 } + versions.select { |v| natural_compare(v, ver) < 0 } + when /<(.+)/ ver = $1.strip - versions.select { |v| natural_sort(v, ver) > 0 } + versions.select { |v| natural_compare(v, ver) > 0 } + else ver = requirement.strip versions.select { |v| v == ver } diff --git a/src/resolvers/git.cr b/src/resolvers/git.cr index 4caafe87c..27930b37f 100644 --- a/src/resolvers/git.cr +++ b/src/resolvers/git.cr @@ -29,7 +29,7 @@ module Shards end protected def self.git_column_never - @@git_column_never ||= Helpers::NaturalSort.sort(git_version, "1.7.11") < 0 ? "--column=never" : "" + @@git_column_never ||= Helpers::NaturalSort.compare(git_version, "1.7.11") < 0 ? "--column=never" : "" end def read_spec(version = "*") diff --git a/test/helpers/natural_sort_test.cr b/test/helpers/natural_sort_test.cr new file mode 100644 index 000000000..fb197bea1 --- /dev/null +++ b/test/helpers/natural_sort_test.cr @@ -0,0 +1,63 @@ +require "../test_helper" + +class Shards::Helpers::NaturalSortTest < Minitest::Test + include Shards::Helpers::NaturalSort + + def test_natural_compare + # a is older than b: + assert_equal 1, natural_compare("1.0.0", "1.0.1") + assert_equal 1, natural_compare("1.0.0", "2.0.0") + assert_equal 1, natural_compare("1.0", "1.0.0.1") + assert_equal 1, natural_compare("1.0.0", "1.0.0.1") + + # a == b + assert_equal 0, natural_compare("0.1", "0.1") + assert_equal 0, natural_compare("0.1", "0.1.0.0") + assert_equal 0, natural_compare("0.1.0", "0.1") + assert_equal 0, natural_compare("2.0.0", "2.0.0") + + # a is newer than b: + assert_equal -1, natural_compare("1.0.1", "1.0.0") + assert_equal -1, natural_compare("2.0.0", "1.0.0") + assert_equal -1, natural_compare("1.0.0.1", "1.0") + assert_equal -1, natural_compare("1.0.0.1", "1.0.0") + end + + def test_natural_sort + 100.times do + versions = %w( + 0.0.1 + 0.1.0 + 0.1.1 + 0.1.2 + 0.2.0 + 0.2.1 + 0.2.10 + 0.2.10.1 + 0.2.11 + 0.10.0 + 0.11.0 + 0.20.0 + 0.20.1 + 1.0.0 + ).shuffle + + assert_equal %w( + 1.0.0 + 0.20.1 + 0.20.0 + 0.11.0 + 0.10.0 + 0.2.11 + 0.2.10.1 + 0.2.10 + 0.2.1 + 0.2.0 + 0.1.2 + 0.1.1 + 0.1.0 + 0.0.1 + ), natural_sort(versions) + end + end +end diff --git a/test/helpers/versions_test.cr b/test/helpers/versions_test.cr new file mode 100644 index 000000000..06c42ccbc --- /dev/null +++ b/test/helpers/versions_test.cr @@ -0,0 +1,37 @@ +require "../test_helper" + +class Shards::Helpers::VersionsTest < Minitest::Test + include Shards::Helpers::Versions + + def versions + ["0.0.1", "0.1.0", "0.1.1", "0.1.2", "0.2.0", "0.10.0"] + end + + def test_resolve_requirement_gt + assert_equal ["0.2.0", "0.10.0"], resolve_requirement(versions, "> 0.1.2") + assert_equal ["0.1.2", "0.2.0", "0.10.0"], resolve_requirement(versions, "> 0.1.1") + end + + def test_resolve_requirement_gte + assert_equal ["0.2.0", "0.10.0"], resolve_requirement(versions, ">= 0.2.0") + assert_equal ["0.1.2", "0.2.0", "0.10.0"], resolve_requirement(versions, ">= 0.1.2") + end + + def test_resolve_requirement_lt + assert_equal ["0.0.1"], resolve_requirement(versions, "< 0.1.0") + assert_equal ["0.0.1", "0.1.0", "0.1.1", "0.1.2"], resolve_requirement(versions, "< 0.2.0") + end + + def test_resolve_requirement_lte + assert_equal ["0.0.1", "0.1.0"], resolve_requirement(versions, "<= 0.1.0") + assert_equal ["0.0.1", "0.1.0", "0.1.1", "0.1.2", "0.2.0"], resolve_requirement(versions, "<= 0.2.0") + end + + def test_resolve_requirement_tilde + assert_equal ["0.1.0", "0.1.1", "0.1.2"], resolve_requirement(versions, "~> 0.1.0") + assert_equal ["0.1.0", "0.1.1", "0.1.2", "0.2.0", "0.10.0"], resolve_requirement(versions, "~> 0.1") + + assert_equal ["0.1"], resolve_requirement(["0.1"], "~> 0.1") + assert_equal ["0.1"], resolve_requirement(["0.1"], "~> 0.1.0") + end +end diff --git a/test/support/mock_resolver.cr b/test/support/mock_resolver.cr index 8ffba62c7..188b40dac 100644 --- a/test/support/mock_resolver.cr +++ b/test/support/mock_resolver.cr @@ -11,7 +11,7 @@ module Shards def read_spec(version = nil) specs = @@specs[dependency.name].not_nil! unless version - version = specs.keys.sort { |a, b| natural_sort(a, b) }.first? + version = natural_sort(specs.keys).first? end specs[version.to_s] end From 3b724ef5dbf826e23a89abea3717e7c1f95caf5a Mon Sep 17 00:00:00 2001 From: Julien Portalier Date: Mon, 7 Jan 2019 13:20:27 +0100 Subject: [PATCH 2/5] Merge Helpers::NaturalSort and Helpers::Versions 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. --- Makefile | 2 +- SPEC.md | 3 +- src/commands/check.cr | 8 +- src/commands/list.cr | 1 - src/commands/version.cr | 1 - src/helpers/natural_sort.cr | 75 ------------ src/helpers/versions.cr | 56 --------- src/package.cr | 6 +- src/resolvers/git.cr | 4 +- src/versions.cr | 196 ++++++++++++++++++++++++++++++ test/helpers/natural_sort_test.cr | 63 ---------- test/helpers/versions_test.cr | 37 ------ test/support/mock_resolver.cr | 6 +- test/versions_test.cr | 149 +++++++++++++++++++++++ 14 files changed, 357 insertions(+), 250 deletions(-) delete mode 100644 src/helpers/natural_sort.cr delete mode 100644 src/helpers/versions.cr create mode 100644 src/versions.cr delete mode 100644 test/helpers/natural_sort_test.cr delete mode 100644 test/helpers/versions_test.cr create mode 100644 test/versions_test.cr diff --git a/Makefile b/Makefile index 3a2c8d9a6..1f505c006 100644 --- a/Makefile +++ b/Makefile @@ -34,7 +34,7 @@ uninstall: phony test: test_unit test_integration test_unit: phony - $(CRYSTAL) run test/*_test.cr test/helpers/*_test.cr + $(CRYSTAL) run test/*_test.cr test_integration: bin/shards phony $(CRYSTAL) run test/integration/*_test.cr diff --git a/SPEC.md b/SPEC.md index 870a21697..adae531b7 100644 --- a/SPEC.md +++ b/SPEC.md @@ -79,8 +79,9 @@ The version number of the library (String, required). - It must contain digits. - It may contain dots and dashes but not consecutive ones. +- It may contain a letter to make it a 'prerelease'. -Examples: `1.2.3`, `2.0.0-rc1` or `2016.09`. +Examples: `1.2.3`, `2.0.0.1`, `1.0.0.alpha` `2.0.0-rc1` or `2016.09`. While Shards doesn't enforce it, following a rational versioning scheme like [Semantic Versioning](http://semver.org/) or [Calendar Versioning](http://calver.org/) diff --git a/src/commands/check.cr b/src/commands/check.cr index 8cfc15b21..afe8922e9 100644 --- a/src/commands/check.cr +++ b/src/commands/check.cr @@ -1,11 +1,9 @@ -require "../helpers/versions" require "./command" +require "../versions" module Shards module Commands class Check < Command - include Helpers::Versions - def run(*args) if has_dependencies? locks # ensures that lockfile exists @@ -45,7 +43,7 @@ module Shards end if version = lock["version"]? - if resolve_requirement([version], dependency.version).empty? + if Versions.resolve([version], dependency.version).empty? Shards.logger.debug { "#{dependency.name}: lock conflict" } return false else @@ -61,7 +59,7 @@ module Shards # end #end - if resolve_requirement([spec.version], dependency.version).empty? + if Versions.resolve([spec.version], dependency.version).empty? Shards.logger.debug { "#{dependency.name}: version mismatch" } return false end diff --git a/src/commands/list.cr b/src/commands/list.cr index 9c161c082..98082a92b 100644 --- a/src/commands/list.cr +++ b/src/commands/list.cr @@ -1,4 +1,3 @@ -require "../helpers/versions" require "./command" module Shards diff --git a/src/commands/version.cr b/src/commands/version.cr index 0d25642d6..86bcd718f 100644 --- a/src/commands/version.cr +++ b/src/commands/version.cr @@ -1,4 +1,3 @@ -require "../helpers/versions" require "./command" module Shards diff --git a/src/helpers/natural_sort.cr b/src/helpers/natural_sort.cr deleted file mode 100644 index 7806a84bd..000000000 --- a/src/helpers/natural_sort.cr +++ /dev/null @@ -1,75 +0,0 @@ -module Shards - module Helpers - module NaturalSort - NON_ALPHANUMERIC = /[^a-zA-Z0-9]/ - - def natural_sort(versions) - versions.sort { |a, b| NaturalSort.compare(a, b) } - end - - def natural_compare(a, b) - NaturalSort.compare(a, b) - end - - def self.compare(a, b) - if a == b - return 0 - end - - loop do - # extract next segment from version number ("1.0.2" => "1" then "0" then "2"): - a_segment, a = next_segment(a) - b_segment, b = next_segment(b) - - # accept unbalanced version numbers ("1.0" == "1.0.0.0", "1.0" < "1.0.1") - if a_segment.empty? - only_zeroes_remaining(b_segment, b) { return 1 } - return 0 - end - - # accept unbalanced version numbers ("1.0.0.0" == "1.0", "1.0.1" > "1.0") - if b_segment.empty? - only_zeroes_remaining(a_segment, a) { return -1 } - return 0 - end - - # try to convert segments to numbers: - a_num = a_segment.to_i?(whitespace: false) - b_num = b_segment.to_i?(whitespace: false) - - # compare: - ret = - if a_num && b_num - b_num <=> a_num - else - b_segment <=> a_segment - end - - # if different return the result (older or newer), otherwise continue - # to the next segment: - return ret unless ret == 0 - end - end - - private def self.next_segment(str) - segment, _, str = str.partition(NON_ALPHANUMERIC) - {segment, str} - end - - private def self.only_zeroes_remaining(segment, str) - unless segment.to_i?(whitespace: false) == 0 - yield - end - - loop do - segment, str = next_segment(str) - break if segment.empty? - - unless segment.to_i?(whitespace: false) == 0 - yield - end - end - end - end - end -end diff --git a/src/helpers/versions.cr b/src/helpers/versions.cr deleted file mode 100644 index a1a2a1478..000000000 --- a/src/helpers/versions.cr +++ /dev/null @@ -1,56 +0,0 @@ -require "./natural_sort" - -module Shards - module Helpers - module Versions - include NaturalSort - - def resolve_versions(versions, requirements) - matching_versions = requirements - .map { |requirement| resolve_requirement(versions, requirement) } - .reduce(versions) { |a, e| a & e } - natural_sort(matching_versions) - end - - def resolve_requirement(versions, requirement) - case requirement - when "*", "" - versions - - when /~>(.+)/ - ver = $1.strip - vver = if idx = ver.rindex('.') - ver[0...idx] - else - ver - end - versions.select do |v| - v.starts_with?(vver) && - !v[vver.size]?.try(&.ascii_alphanumeric?) && - (natural_compare(v, ver) <= 0) - end - - when />=(.+)/ - ver = $1.strip - versions.select { |v| natural_compare(v, ver) <= 0 } - - when /<=(.+)/ - ver = $1.strip - versions.select { |v| natural_compare(v, ver) >= 0 } - - when />(.+)/ - ver = $1.strip - versions.select { |v| natural_compare(v, ver) < 0 } - - when /<(.+)/ - ver = $1.strip - versions.select { |v| natural_compare(v, ver) > 0 } - - else - ver = requirement.strip - versions.select { |v| v == ver } - end - end - end - end -end diff --git a/src/package.cr b/src/package.cr index a69fee22e..d21ba762d 100644 --- a/src/package.cr +++ b/src/package.cr @@ -1,11 +1,9 @@ require "file_utils" require "./resolvers/*" -require "./helpers/versions" +require "./versions" module Shards class Package - include Helpers::Versions - getter requirements : Array(String) @resolver : Resolver? @available_versions : Array(String)? @@ -38,7 +36,7 @@ module Shards end def matching_versions - resolve_versions(available_versions, requirements) + Versions.resolve(available_versions, requirements) end def spec diff --git a/src/resolvers/git.cr b/src/resolvers/git.cr index 27930b37f..083b38b3c 100644 --- a/src/resolvers/git.cr +++ b/src/resolvers/git.cr @@ -1,6 +1,6 @@ require "uri" require "./resolver" -require "../helpers/natural_sort" +require "../versions" require "../helpers/path" module Shards @@ -29,7 +29,7 @@ module Shards end protected def self.git_column_never - @@git_column_never ||= Helpers::NaturalSort.compare(git_version, "1.7.11") < 0 ? "--column=never" : "" + @@git_column_never ||= Versions.compare(git_version, "1.7.11") < 0 ? "--column=never" : "" end def read_spec(version = "*") diff --git a/src/versions.cr b/src/versions.cr new file mode 100644 index 000000000..fb9924765 --- /dev/null +++ b/src/versions.cr @@ -0,0 +1,196 @@ +module Shards + module Versions + # :nodoc: + struct Segment + NON_ALPHANUMERIC = /[^a-zA-Z0-9]/ + NATURAL_SORT_EXTRACT_NEXT_CHARS_AND_DIGITS = /^(\D*)(\d*)(.*)$/ + + protected getter! segment : String + + def initialize(@str : String) + end + + def next + @segment, _, @str = @str.partition(NON_ALPHANUMERIC) + segment + end + + def empty? + segment.empty? + end + + def to_i? + segment.to_i?(whitespace: false) + end + + def <=>(b : self) + natural_sort(segment, b.segment) + end + + # Original natural sorting algorithm from: + # https://github.com/sourcefrog/natsort/blob/master/natcmp.rb + # Copyright (C) 2003 by Alan Davies . + private def natural_sort(a, b) + if (a_num = a.to_i?(whitespace: false)) && (b_num = b.to_i?(whitespace: false)) + return a_num <=> b_num + end + + loop do + return 0 if a.empty? && b.empty? + + a =~ NATURAL_SORT_EXTRACT_NEXT_CHARS_AND_DIGITS + a_chars, a_digits, a = $1, $2, $3 + + b =~ NATURAL_SORT_EXTRACT_NEXT_CHARS_AND_DIGITS + b_chars, b_digits, b = $1, $2, $3 + + ret = a_chars <=> b_chars + return ret unless ret == 0 + + a_num = a_digits.to_i?(whitespace: false) + b_num = b_digits.to_i?(whitespace: false) + + if a_num && b_num + ret = a_num.to_i <=> b_num.to_i + return ret unless ret == 0 + else + ret = a_digits <=> b_digits + return ret unless ret == 0 + end + end + end + + def only_zeroes? + return if empty? + yield unless to_i? == 0 + + loop do + self.next + + return if empty? + yield unless to_i? == 0 + end + end + + def prerelease? + segment.each_char.any?(&.ascii_letter?) + end + + def inspect(io) + @segment.inspect(io) + end + end + + def self.sort(versions) + versions.sort { |a, b| compare(a, b) } + end + + def self.compare(a, b) + if a == b + return 0 + end + + a_segment = Segment.new(a) + b_segment = Segment.new(b) + + loop do + # extract next segment from version number ("1.0.2" => "1" then "0" then "2"): + a_segment.next + b_segment.next + + # accept unbalanced version numbers ("1.0" == "1.0.0.0", "1.0" < "1.0.1") + if a_segment.empty? + b_segment.only_zeroes? { return b_segment.prerelease? ? -1 : 1 } + return 0 + end + + # accept unbalanced version numbers ("1.0.0.0" == "1.0", "1.0.1" > "1.0") + if b_segment.empty? + a_segment.only_zeroes? { return a_segment.prerelease? ? 1 : -1 } + return 0 + end + + # try to convert segments to numbers: + a_num = a_segment.to_i? + b_num = b_segment.to_i? + + ret = + if a_num && b_num + # compare numbers (for natural 1, 2, ..., 10, 11 ordering): + b_num <=> a_num + + elsif a_num + # b is preliminary version: + a_segment.only_zeroes? do + return b_segment <=> a_segment if a_segment.prerelease? + return -1 + end + return -1 + + elsif b_num + # a is preliminary version: + b_segment.only_zeroes? do + return b_segment <=> a_segment if b_segment.prerelease? + return 1 + end + return 1 + + else + # compare strings: + b_segment <=> a_segment + end + + # if different return the result (older or newer), otherwise continue + # to the next segment: + return ret unless ret == 0 + end + end + + def self.resolve(versions, requirements : Enumerable(String)) + matching_versions = requirements + .map { |requirement| resolve(versions, requirement) } + .reduce(versions) { |a, e| a & e } + sort(matching_versions) + end + + def self.resolve(versions, requirement : String) + case requirement + when "*", "" + versions + + when /~>(.+)/ + ver = $1.strip + vver = if idx = ver.rindex('.') + ver[0...idx] + else + ver + end + versions.select do |v| + v.starts_with?(vver) && + !v[vver.size]?.try(&.ascii_alphanumeric?) && + (compare(v, ver) <= 0) + end + + when />=(.+)/ + ver = $1.strip + versions.select { |v| compare(v, ver) <= 0 } + + when /<=(.+)/ + ver = $1.strip + versions.select { |v| compare(v, ver) >= 0 } + + when />(.+)/ + ver = $1.strip + versions.select { |v| compare(v, ver) < 0 } + + when /<(.+)/ + ver = $1.strip + versions.select { |v| compare(v, ver) > 0 } + + else + ver = requirement.strip + versions.select { |v| v == ver } + end + end +end +end diff --git a/test/helpers/natural_sort_test.cr b/test/helpers/natural_sort_test.cr deleted file mode 100644 index fb197bea1..000000000 --- a/test/helpers/natural_sort_test.cr +++ /dev/null @@ -1,63 +0,0 @@ -require "../test_helper" - -class Shards::Helpers::NaturalSortTest < Minitest::Test - include Shards::Helpers::NaturalSort - - def test_natural_compare - # a is older than b: - assert_equal 1, natural_compare("1.0.0", "1.0.1") - assert_equal 1, natural_compare("1.0.0", "2.0.0") - assert_equal 1, natural_compare("1.0", "1.0.0.1") - assert_equal 1, natural_compare("1.0.0", "1.0.0.1") - - # a == b - assert_equal 0, natural_compare("0.1", "0.1") - assert_equal 0, natural_compare("0.1", "0.1.0.0") - assert_equal 0, natural_compare("0.1.0", "0.1") - assert_equal 0, natural_compare("2.0.0", "2.0.0") - - # a is newer than b: - assert_equal -1, natural_compare("1.0.1", "1.0.0") - assert_equal -1, natural_compare("2.0.0", "1.0.0") - assert_equal -1, natural_compare("1.0.0.1", "1.0") - assert_equal -1, natural_compare("1.0.0.1", "1.0.0") - end - - def test_natural_sort - 100.times do - versions = %w( - 0.0.1 - 0.1.0 - 0.1.1 - 0.1.2 - 0.2.0 - 0.2.1 - 0.2.10 - 0.2.10.1 - 0.2.11 - 0.10.0 - 0.11.0 - 0.20.0 - 0.20.1 - 1.0.0 - ).shuffle - - assert_equal %w( - 1.0.0 - 0.20.1 - 0.20.0 - 0.11.0 - 0.10.0 - 0.2.11 - 0.2.10.1 - 0.2.10 - 0.2.1 - 0.2.0 - 0.1.2 - 0.1.1 - 0.1.0 - 0.0.1 - ), natural_sort(versions) - end - end -end diff --git a/test/helpers/versions_test.cr b/test/helpers/versions_test.cr deleted file mode 100644 index 06c42ccbc..000000000 --- a/test/helpers/versions_test.cr +++ /dev/null @@ -1,37 +0,0 @@ -require "../test_helper" - -class Shards::Helpers::VersionsTest < Minitest::Test - include Shards::Helpers::Versions - - def versions - ["0.0.1", "0.1.0", "0.1.1", "0.1.2", "0.2.0", "0.10.0"] - end - - def test_resolve_requirement_gt - assert_equal ["0.2.0", "0.10.0"], resolve_requirement(versions, "> 0.1.2") - assert_equal ["0.1.2", "0.2.0", "0.10.0"], resolve_requirement(versions, "> 0.1.1") - end - - def test_resolve_requirement_gte - assert_equal ["0.2.0", "0.10.0"], resolve_requirement(versions, ">= 0.2.0") - assert_equal ["0.1.2", "0.2.0", "0.10.0"], resolve_requirement(versions, ">= 0.1.2") - end - - def test_resolve_requirement_lt - assert_equal ["0.0.1"], resolve_requirement(versions, "< 0.1.0") - assert_equal ["0.0.1", "0.1.0", "0.1.1", "0.1.2"], resolve_requirement(versions, "< 0.2.0") - end - - def test_resolve_requirement_lte - assert_equal ["0.0.1", "0.1.0"], resolve_requirement(versions, "<= 0.1.0") - assert_equal ["0.0.1", "0.1.0", "0.1.1", "0.1.2", "0.2.0"], resolve_requirement(versions, "<= 0.2.0") - end - - def test_resolve_requirement_tilde - assert_equal ["0.1.0", "0.1.1", "0.1.2"], resolve_requirement(versions, "~> 0.1.0") - assert_equal ["0.1.0", "0.1.1", "0.1.2", "0.2.0", "0.10.0"], resolve_requirement(versions, "~> 0.1") - - assert_equal ["0.1"], resolve_requirement(["0.1"], "~> 0.1") - assert_equal ["0.1"], resolve_requirement(["0.1"], "~> 0.1.0") - end -end diff --git a/test/support/mock_resolver.cr b/test/support/mock_resolver.cr index 188b40dac..a8b9e2268 100644 --- a/test/support/mock_resolver.cr +++ b/test/support/mock_resolver.cr @@ -1,9 +1,7 @@ -require "../../src/helpers/natural_sort" +require "../../src/versions" module Shards class MockResolver < Resolver - include Helpers::NaturalSort - def self.key "mock" end @@ -11,7 +9,7 @@ module Shards def read_spec(version = nil) specs = @@specs[dependency.name].not_nil! unless version - version = natural_sort(specs.keys).first? + version = Versions.sort(specs.keys).first? end specs[version.to_s] end diff --git a/test/versions_test.cr b/test/versions_test.cr new file mode 100644 index 000000000..61a5f4951 --- /dev/null +++ b/test/versions_test.cr @@ -0,0 +1,149 @@ +require "./test_helper" + +module Shards + class VersionsTest < Minitest::Test + def test_compare + # a is older than b: + assert_equal 1, Versions.compare("1.0.0", "1.0.1") + assert_equal 1, Versions.compare("1.0.0", "2.0.0") + assert_equal 1, Versions.compare("1.0", "1.0.0.1") + assert_equal 1, Versions.compare("1.0.0", "1.0.0.1") + + # a == b + assert_equal 0, Versions.compare("0.1", "0.1") + assert_equal 0, Versions.compare("0.1", "0.1.0.0") + assert_equal 0, Versions.compare("0.1.0", "0.1") + assert_equal 0, Versions.compare("2.0.0", "2.0.0") + + # a is newer than b: + assert_equal -1, Versions.compare("1.0.1", "1.0.0") + assert_equal -1, Versions.compare("2.0.0", "1.0.0") + assert_equal -1, Versions.compare("1.0.0.1", "1.0") + assert_equal -1, Versions.compare("1.0.0.1", "1.0.0") + end + + def test_compare_preversions + # a is older than b: + assert_equal 1, Versions.compare("1.0.0-beta", "1.0.0") + assert_equal 1, Versions.compare("1.0.0.alpha", "1.0.0") + assert_equal 1, Versions.compare("1.0.0.alpha", "1.0.0.beta") + assert_equal 1, Versions.compare("1.0.beta", "1.0.0") + assert_equal 1, Versions.compare("1.0.alpha", "1.0.0-beta") + + assert_equal 1, Versions.compare("1.0-pre1", "1.0-pre2") + assert_equal 1, Versions.compare("1.0-pre2", "1.0-pre10") + + # a == b + assert_equal 0, Versions.compare("1.0.0-beta", "1.0.0-beta") + assert_equal 0, Versions.compare("1.0.0-alpha", "1.0.0.alpha") + assert_equal 0, Versions.compare("1.0.beta", "1.0.0.beta") + assert_equal 0, Versions.compare("1.0.beta", "1.0.0.0.0.0.beta") + + # a is newer than b: + assert_equal -1, Versions.compare("1.0.0", "1.0.0-beta") + assert_equal -1, Versions.compare("1.0.0", "1.0.0.alpha") + assert_equal -1, Versions.compare("1.0.0.beta", "1.0.0.alpha") + assert_equal -1, Versions.compare("1.0.0", "1.0.beta") + assert_equal -1, Versions.compare("1.0.0-beta", "1.0.alpha") + + assert_equal -1, Versions.compare("1.0-pre2", "1.0-pre1") + assert_equal -1, Versions.compare("1.0-pre10", "1.0-pre2") + end + + def test_sort + 100.times do + versions = %w( + 0.0.1 + 0.1.0 + 0.1.1 + 0.1.2 + 0.2.0 + 0.2.1 + 0.2.10 + 0.2.10.1 + 0.2.11 + 0.10.0 + 0.11.0 + 0.20.0 + 0.20.1 + 1.0.0-alpha + 1.0.0.beta + 1.0.0-pre1 + 1.0.0-pre2 + 1.0.0-rc1 + 1.0.0-rc2 + 1.0.0-rc10 + 1.0.0 + ).shuffle + + assert_equal %w( + 1.0.0 + 1.0.0-rc10 + 1.0.0-rc2 + 1.0.0-rc1 + 1.0.0-pre2 + 1.0.0-pre1 + 1.0.0.beta + 1.0.0-alpha + 0.20.1 + 0.20.0 + 0.11.0 + 0.10.0 + 0.2.11 + 0.2.10.1 + 0.2.10 + 0.2.1 + 0.2.0 + 0.1.2 + 0.1.1 + 0.1.0 + 0.0.1 + ), Versions.sort(versions) + end + end + + def versions + ["0.0.1", "0.1.0", "0.1.1", "0.1.2", "0.2.0", "0.10.0"] + end + + def test_resolve_any + assert_equal versions, Versions.resolve(versions, "*") + end + + def test_resolve_eq + assert_equal ["0.2.0"], Versions.resolve(versions, "0.2.0") + assert_equal ["0.1.1"], Versions.resolve(versions, "0.1.1") + assert_equal ["0.10.0"], Versions.resolve(versions, "0.10.0") + assert_empty Versions.resolve(versions, "1.0.0") + assert_empty Versions.resolve(versions, "0.0.1.alpha") + end + + def test_resolve_gt + assert_equal ["0.2.0", "0.10.0"], Versions.resolve(versions, "> 0.1.2") + assert_equal ["0.1.2", "0.2.0", "0.10.0"], Versions.resolve(versions, "> 0.1.1") + end + + def test_resolve_gte + assert_equal ["0.2.0", "0.10.0"], Versions.resolve(versions, ">= 0.2.0") + assert_equal ["0.1.2", "0.2.0", "0.10.0"], Versions.resolve(versions, ">= 0.1.2") + end + + def test_resolve_lt + assert_equal ["0.0.1"], Versions.resolve(versions, "< 0.1.0") + assert_equal ["0.0.1", "0.1.0", "0.1.1", "0.1.2"], Versions.resolve(versions, "< 0.2.0") + end + + def test_resolve_lte + assert_equal ["0.0.1", "0.1.0"], Versions.resolve(versions, "<= 0.1.0") + assert_equal ["0.0.1", "0.1.0", "0.1.1", "0.1.2", "0.2.0"], Versions.resolve(versions, "<= 0.2.0") + end + + def test_resolve_approximate + assert_equal ["0.1.0", "0.1.1", "0.1.2"], Versions.resolve(versions, "~> 0.1.0") + assert_equal ["0.1.0", "0.1.1", "0.1.2", "0.2.0", "0.10.0"], Versions.resolve(versions, "~> 0.1") + + assert_equal ["0.1"], Versions.resolve(["0.1"], "~> 0.1") + assert_equal ["0.1"], Versions.resolve(["0.1"], "~> 0.1.0") + end + end +end From b896ced6c63ced04eacf153ab60eb6536c881986 Mon Sep 17 00:00:00 2001 From: Julien Portalier Date: Mon, 7 Jan 2019 16:34:19 +0100 Subject: [PATCH 3/5] Support prereleases 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`. --- src/cli.cr | 10 ------ src/config.cr | 3 ++ src/resolvers/git.cr | 6 +--- src/versions.cr | 9 +++++ test/integration/install_test.cr | 22 +++++++++++++ test/integration/update_test.cr | 56 ++++++++++++++++++++++++++++++++ test/integration_helper.cr | 5 ++- 7 files changed, 95 insertions(+), 16 deletions(-) diff --git a/src/cli.cr b/src/cli.cr index 975a84cb1..eab6a86e1 100644 --- a/src/cli.cr +++ b/src/cli.cr @@ -19,9 +19,6 @@ module Shards Options: HELP puts opts - # info - # search - # update [package package ...] exit end @@ -42,9 +39,6 @@ module Shards build(path, args[1..-1]) when "check" Commands::Check.run(path) - #when "info" - # display_help_and_exit(opts) unless args[1]? - # Commands::Info.run(args[1]) when "init" Commands::Init.run(path) when "install" @@ -53,12 +47,8 @@ module Shards Commands::List.run(path, tree: args.includes?("--tree")) when "prune" Commands::Prune.run(path) - # when "search" - # display_help_and_exit(opts) unless args[1]? - # Commands::Search.run(path, args[1]) when "update" Commands::Update.run(path) - # Commands.update(path, *args[1..-1]) when "version" Commands::Version.run(args[1]? || path) else diff --git a/src/config.cr b/src/config.cr index 17bebc2b2..0bf96168a 100644 --- a/src/config.cr +++ b/src/config.cr @@ -5,6 +5,9 @@ module Shards DEFAULT_COMMAND = "install" DEFAULT_VERSION = "0" + VERSION_REFERENCE = /^v?\d+[-.][-.a-zA-Z\d]+$/ + VERSION_TAG = /^v(\d+[-.][-.a-zA-Z\d]+)$/ + def self.cache_path @@cache_path ||= find_or_create_cache_path end diff --git a/src/resolvers/git.cr b/src/resolvers/git.cr index 083b38b3c..ad3742af1 100644 --- a/src/resolvers/git.cr +++ b/src/resolvers/git.cr @@ -4,8 +4,6 @@ require "../versions" require "../helpers/path" module Shards - VERSION_REFERENCE = /^v?[\d\.]+$/ - class GitResolver < Resolver @@has_git_command : Bool? @@git_column_never : String? @@ -60,14 +58,12 @@ module Shards end end - RELEASE_VERSION_TAG = /^v([\d\.]+)$/ - protected def versions_from_tags(refs = nil) options = "--contains #{refs}" if refs capture("git tag --list #{options} #{GitResolver.git_column_never}") .split('\n') - .compact_map { |tag| $1 if tag =~ RELEASE_VERSION_TAG } + .compact_map { |tag| $1 if tag =~ VERSION_TAG } end def matches?(commit) diff --git a/src/versions.cr b/src/versions.cr index fb9924765..2092e94a8 100644 --- a/src/versions.cr +++ b/src/versions.cr @@ -146,10 +146,19 @@ module Shards end end + def self.prerelease?(str) + str.each_char.any?(&.ascii_letter?) + end + def self.resolve(versions, requirements : Enumerable(String)) + unless requirements.any? { |r| prerelease?(r) } + versions = versions.reject { |v| prerelease?(v) } + end + matching_versions = requirements .map { |requirement| resolve(versions, requirement) } .reduce(versions) { |a, e| a & e } + sort(matching_versions) end diff --git a/test/integration/install_test.cr b/test/integration/install_test.cr index 28d5d4820..c5de57299 100644 --- a/test/integration/install_test.cr +++ b/test/integration/install_test.cr @@ -49,6 +49,28 @@ class InstallCommandTest < Minitest::Test end end + def test_wont_install_prerelease_version + metadata = { + dependencies: {unstable: "*"}, + } + with_shard(metadata) do + run "shards install" + assert_installed "unstable", "0.2.0" + assert_locked "unstable", "0.2.0" + end + end + + def test_installs_specified_prerelease_version + metadata = { + dependencies: {unstable: "0.3.0.alpha"}, + } + with_shard(metadata) do + run "shards install" + assert_installed "unstable", "0.3.0.alpha" + assert_locked "unstable", "0.3.0.alpha" + end + end + def test_installs_dependencies_at_locked_version metadata = { dependencies: {web: "1.0.0"}, diff --git a/test/integration/update_test.cr b/test/integration/update_test.cr index 083ce5c5f..47811743a 100644 --- a/test/integration/update_test.cr +++ b/test/integration/update_test.cr @@ -54,6 +54,62 @@ class UpdateCommandTest < Minitest::Test end end + def test_wont_install_prerelease_version + metadata = { dependencies: {unstable: "*"} } + lock = {unstable: "0.1.0"} + + with_shard(metadata, lock) do + run "shards update" + assert_installed "unstable", "0.2.0" + assert_locked "unstable", "0.2.0" + end + end + + def test_installs_specified_prerelease_version + metadata = { dependencies: {unstable: "~> 0.3.0.alpha"} } + lock = {unstable: "0.3.0.alpha"} + + with_shard(metadata, lock) do + run "shards update" + assert_installed "unstable", "0.3.0.beta" + assert_locked "unstable", "0.3.0.beta" + end + end + + def test_updates_locked_specified_prerelease + metadata = { dependencies: {unstable: "~> 0.3.0.alpha"} } + lock = {unstable: "0.3.0.alpha"} + + with_shard(metadata, lock) do + run "shards update" + assert_installed "unstable", "0.3.0.beta" + assert_locked "unstable", "0.3.0.beta" + end + end + + def test_updates_from_prerelease_to_release_with_approximate_operator + metadata = { dependencies: {preview: "~> 0.3.0.a"} } + lock = {preview: "0.3.0.alpha"} + + with_shard(metadata, lock) do + run "shards update" + assert_installed "preview", "0.3.0" + assert_locked "preview", "0.3.0" + end + end + + # TODO: detect version, and prefer release (0.3.0) over further prereleases (?) + def test_updates_to_latest_prerelease_with_gte_operator + metadata = { dependencies: {preview: ">= 0.3.0.a"} } + lock = {preview: "0.3.0.a"} + + with_shard(metadata, lock) do + run "shards update" + assert_installed "preview", "0.4.0.a" + assert_locked "preview", "0.4.0.a" + end + end + def test_updates_locked_commit metadata = { dependencies: {web: {git: git_url(:web), branch: "master"}}, diff --git a/test/integration_helper.cr b/test/integration_helper.cr index 278fccba7..f85a3921c 100644 --- a/test/integration_helper.cr +++ b/test/integration_helper.cr @@ -62,6 +62,9 @@ class Minitest::Test create_file "binary", "bin/foobar", "#! /usr/bin/env sh\necho 'OK'", perm: 0o755 create_file "binary", "bin/baz", "#! /usr/bin/env sh\necho 'KO'", perm: 0o755 + create_git_repository "unstable", "0.1.0", "0.2.0", "0.3.0.alpha", "0.3.0.beta" + create_git_repository "preview", "0.1.0", "0.2.0", "0.3.0.a", "0.3.0.b", "0.3.0", "0.4.0.a" + Minitest::Test.created_repositories! end @@ -103,7 +106,7 @@ class Minitest::Test assert lock = locks.find { |d| d.name == name }, "expected #{name} dependency to have been locked", file, line if lock && version - if version =~ /^[\d.]+$/ + if version =~ Shards::VERSION_REFERENCE assert_equal version, lock.version, "expected #{name} dependency to have been locked at version #{version}", file, line else assert_equal version, lock.refs, "expected #{name} dependency to have been locked at commit #{version}", file, line From 7121263e38124c6b05cb1d0b94f7b09a4271c35a Mon Sep 17 00:00:00 2001 From: Julien Portalier Date: Tue, 8 Jan 2019 16:13:39 +0100 Subject: [PATCH 4/5] Update CHANGELOG --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 590d83ce4..cf4e5877c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,15 @@ ## UNRELEASED +Features: +- Experimental support for prereleases. Add a letter to a version number to + declare a pre-release. For example `1.2.3.alpha` or `1.0.0-rc1`. + Fixes: +- Approximate operator used to match invalid version numbers (e.g. `~> 0.1.0` + wrongly matched `0.10.0`). +- Unbalanced version numbers, such as `1.0.0` and `1.0.0.1` are now correctly + ordered and compared as `1.0.0.1 > 1.0.0`. - `install -t` isn't supported on macOS. ## v0.8.1 - 2018-06-17 From ccd53b52fd83cba94810f0b2cbfff225de219af1 Mon Sep 17 00:00:00 2001 From: Julien Portalier Date: Thu, 10 Jan 2019 13:47:19 +0100 Subject: [PATCH 5/5] Support semver +metadata (ignore it). --- src/versions.cr | 5 ++++- test/versions_test.cr | 16 ++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/versions.cr b/src/versions.cr index 2092e94a8..661349452 100644 --- a/src/versions.cr +++ b/src/versions.cr @@ -8,6 +8,9 @@ module Shards protected getter! segment : String def initialize(@str : String) + if index = @str.index('+') + @str = @str[0...index] + end end def next @@ -201,5 +204,5 @@ module Shards versions.select { |v| v == ver } end end -end + end end diff --git a/test/versions_test.cr b/test/versions_test.cr index 61a5f4951..679c9db92 100644 --- a/test/versions_test.cr +++ b/test/versions_test.cr @@ -2,6 +2,16 @@ require "./test_helper" module Shards class VersionsTest < Minitest::Test + def prerelease? + refute Versions.prerelease?("1.0") + refute Versions.prerelease?("1.0.0.1") + + assert Versions.prerelease?("1.0a") + assert Versions.prerelease?("1.0.alpha") + assert Versions.prerelease?("1.0.0-rc1") + assert Versions.prerelease?("1.0.0-pre.1.2.x.y") + end + def test_compare # a is older than b: assert_equal 1, Versions.compare("1.0.0", "1.0.1") @@ -50,6 +60,12 @@ module Shards assert_equal -1, Versions.compare("1.0-pre10", "1.0-pre2") end + def test_compare_ignores_semver_metadata + assert_equal -1, Versions.compare("1.1+20180110", "1.0+20180110") + assert_equal 0, Versions.compare("1.0+build1", "1.0+build2") + assert_equal 1, Versions.compare("1.0+20180110", "1.1+20180110") + end + def test_sort 100.times do versions = %w(