Fix: install must validate selected packages against lock file

Introduces Versions.matches? to compare a version against a given
requirement, and refactors Versions.resolve to avoid duplication.

Author: ysbaddaden

src/commands/install.cr

@@ -17,6 +17,10 @@ module Shards
         solver.prepare(development: !Shards.production?)
 
         if packages = solver.solve
+          if lockfile?
+            validate(packages)
+          end
+
           install(packages)
 
           if generate_lockfile?(packages)
@@ -30,6 +34,33 @@ module Shards
         end
       end
 
+      private def validate(packages)
+        packages.each do |package|
+          if lock = locks.find { |d| d.name == package.name }
+            if version = lock.version?
+              validate_locked_version(package, version)
+            elsif commit = lock["commit"]?
+              validate_locked_commit(package, commit)
+            else
+              raise InvalidLock.new # unknown lock resolver
+            end
+          elsif Shards.production?
+            raise LockConflict.new("can't install new dependency #{package.name} in production")
+          end
+        end
+      end
+
+      private def validate_locked_version(package, version)
+        return if Shards.production? && package.version == version
+        return if Versions.matches?(version, package.spec.version)
+        raise LockConflict.new("#{package.name} requirements changed")
+      end
+
+      private def validate_locked_commit(package, commit)
+        return if commit == package.commit
+        raise LockConflict.new("#{package.name} requirements changed")
+      end
+
       private def install(packages : Array(Package))
         # first install all dependencies:
         installed = packages.compact_map { |package| install(package) }

src/versions.cr

@@ -177,39 +177,56 @@ module Shards
       case requirement
       when "*", ""
         versions
+      when /~>\s*([^\s]+)/
+        ver = if idx = $1.rindex('.')
+                $1[0...idx]
+              else
+                $1
+              end
+        versions.select { |version| matches_approximate?(version, $1, ver) }
+      when /\s*(~>|>=|<=|>|<|=)\s*([^~<>=\s]+)\s*/
+        versions.select { |version| matches_operator?(version, $1, $2) }
+      else
+        versions.select { |version| matches_operator?(version, "=", requirement) }
+      end
+    end
 
-      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 }
+    def self.matches?(version : String, requirement : String)
+      case requirement
+      when "*", ""
+        true
+      when /~>\s*([^\s]+)\d*/
+        ver = if idx = $1.rindex('.')
+                $1[0...idx]
+              else
+                $1
+              end
+        matches_approximate?(version, $1, ver)
+      when /\s*(~>|>=|<=|>|<|=)\s*([^~<>=\s]+)\s*/
+        matches_operator?(version, $1, $2)
+      else
+        matches_operator?(version, "=", requirement)
+      end
+    end
 
-      when /<(.+)/
-        ver = $1.strip
-        versions.select { |v| compare(v, ver) > 0 }
+    private def self.matches_approximate?(version, requirement, ver)
+      version.starts_with?(ver) &&
+        !version[ver.size]?.try(&.ascii_alphanumeric?) &&
+        (compare(version, requirement) <= 0)
+    end
 
+    private def self.matches_operator?(version, operator, requirement)
+      case operator
+      when ">="
+        compare(version, requirement) <= 0
+      when "<="
+        compare(version, requirement) >= 0
+      when ">"
+        compare(version, requirement) < 0
+      when "<"
+        compare(version, requirement) > 0
       else
-        ver = requirement.strip
-        versions.select { |v| v == ver }
+        compare(version, requirement) == 0
       end
     end
   end

test/versions_test.cr

@@ -165,5 +165,40 @@ module Shards
       assert_equal ["0.1"], Versions.resolve(["0.1"], "~> 0.1")
       assert_equal ["0.1"], Versions.resolve(["0.1"], "~> 0.1.0")
     end
+
+    def test_matches?
+      assert Versions.matches?("0.1.0", "*")
+      assert Versions.matches?("1.0.0", "*")
+
+      assert Versions.matches?("1.0.0", "1.0.0")
+      assert Versions.matches?("1.0.0", "1.0")
+      refute Versions.matches?("1.0.0", "1.0.1")
+
+      assert Versions.matches?("1.0.0", ">= 1.0.0")
+      assert Versions.matches?("1.0.0", ">= 1.0")
+      assert Versions.matches?("1.0.1", ">= 1.0.0")
+      refute Versions.matches?("1.0.0", ">= 1.0.1")
+
+      refute Versions.matches?("1.0.0", "> 1.0.0")
+      refute Versions.matches?("1.0.0", "> 1.0")
+      assert Versions.matches?("1.0.1", "> 1.0.0")
+      refute Versions.matches?("1.0.0", "> 1.0.1")
+
+      assert Versions.matches?("1.0.0", "<= 1.0.0")
+      assert Versions.matches?("1.0.0", "<= 1.0")
+      refute Versions.matches?("1.0.1", "<= 1.0.0")
+      assert Versions.matches?("1.0.0", "<= 1.0.1")
+
+      refute Versions.matches?("1.0.0", "< 1.0.0")
+      refute Versions.matches?("1.0.0", "< 1.0")
+      refute Versions.matches?("1.0.1", "< 1.0.0")
+      assert Versions.matches?("1.0.0", "< 1.0.1")
+
+      assert Versions.matches?("1.0.0", "~> 1.0.0")
+      assert Versions.matches?("1.0.0", "~> 1.0")
+      refute Versions.matches?("1.0.0", "~> 1.1")
+      assert Versions.matches?("1.0.1", "~> 1.0.0")
+      refute Versions.matches?("1.0.0", "~> 1.0.1")
+    end
   end
 end