Filter pre-releases in #requirement_satified_by? rather than when sea…

…rching

lib/molinillo/resolution.rb

@@ -351,7 +351,15 @@ def attempt_to_activate
           debug(depth) { "Found existing spec (#{existing_vertex.payload})" }
           attempt_to_filter_existing_spec(existing_vertex)
         else
-          activate_new_spec
+          possibility.possibilities.select! do |possibility|
+            requirement_satisfied_by?(requirement, activated, possibility)
+          end
+          if possibility.latest_version.nil?
+            create_conflict
+            unwind_for_conflict
+          else
+            activate_new_spec
+          end
         end
       end
 
@@ -360,8 +368,7 @@ def attempt_to_activate
       def attempt_to_filter_existing_spec(vertex)
         filtered_set = filtered_possibility_set(vertex)
         if !filtered_set.possibilities.empty? &&
-            (vertex.payload.dependencies == dependencies_for(possibility) ||
-             possibility_set_contains_prereleases(filtered_set))
+            (vertex.payload.dependencies == dependencies_for(possibility))
           activated.set_payload(name, filtered_set)
           new_requirements = requirements.dup
           push_state_for_requirements(new_requirements, false)
@@ -388,21 +395,7 @@ def filtered_possibility_set(vertex)
           vertex.requirements.uniq.all? { |req| requirement_satisfied_by?(req, activated, poss) }
         end
 
-        # TODO: ideally we should ensure the possibilities set is unique here
-
-        PossibilitySet.new(vertex.payload.dependencies, (filtered_old_values + filtered_new_values))
-      end
-
-      # Checks whether a `PossibilitySet` contains any pre-release versions. These
-      # need special treatment, as calling `search_for` on a previous, non-pre-release
-      # requirement won't have returned any pre-release versions
-      # @param [PossibilitySet] the possibility set to check
-      # @return [Boolean] whether or not the given `PossibilitySet` contains any
-      #   pre-release version
-      def possibility_set_contains_prereleases(possibility_set)
-        # TODO: figure out a way to check for pre-release versions that doesn't
-        # call `.version` on user-provided possibilities objects
-        possibility_set.possibilities.any? { |poss| poss.version.prerelease? }
+        PossibilitySet.new(vertex.payload.dependencies, filtered_old_values | filtered_new_values)
       end
 
       # @param [String] requirement_name the spec name to search for

spec/resolver_spec.rb

@@ -46,12 +46,12 @@ def initialize(fixture_path)
     end
 
     def run(index_class, context)
-      return if ignore?(index_class)
-
       test_case = self
 
       context.instance_eval do
         it test_case.name do
+          skip 'does not yet reliably pass' if test_case.ignore?(index_class)
+
           resolve = lambda do
             index = index_class.new(test_case.index.specs)
             resolver = Resolver.new(index, TestUI.new)
@@ -84,7 +84,7 @@ def run(index_class, context)
     end
 
     def ignore?(index_class)
-      if [BerkshelfIndex, ReverseBundlerIndex].include?(index_class) &&
+      if [BerkshelfIndex, ReverseBundlerIndex, RandomSortIndex].include?(index_class) &&
           name == 'can resolve when two specs have the same dependencies and swapping happens'
 
         # These indexes don't do a great job sorting, and segiddins has been

spec/spec_helper/index.rb

@@ -22,7 +22,12 @@ def initialize(specs_by_name)
       self.specs = specs_by_name
     end
 
-    def requirement_satisfied_by?(requirement, _activated, spec)
+    def requirement_satisfied_by?(requirement, activated, spec)
+      if spec.version.prerelease? && !requirement.prerelease?
+        vertex = activated.vertex_named(spec.name)
+        return false if vertex.requirements.none?(&:prerelease?)
+      end
+
       case requirement
       when TestSpecification
         requirement.version == spec.version
@@ -34,10 +39,8 @@ def requirement_satisfied_by?(requirement, _activated, spec)
     def search_for(dependency)
       @search_for ||= {}
       @search_for[dependency] ||= begin
-        prerelease = dependency_prerelease?(dependency)
-        Array(specs[dependency.name]).select do |spec|
-          (prerelease ? true : !spec.version.prerelease?) &&
-            dependency.requirement.satisfied_by?(spec.version)
+        specs[dependency.name].select do |spec|
+          dependency.requirement.satisfied_by?(spec.version)
         end
       end
       @search_for[dependency].dup
@@ -55,18 +58,12 @@ def sort_dependencies(dependencies, activated, conflicts)
       dependencies.sort_by do |d|
         [
           activated.vertex_named(d.name).payload ? 0 : 1,
-          dependency_prerelease?(d) ? 0 : 1,
+          d.prerelease? ? 0 : 1,
           conflicts[d.name] ? 0 : 1,
           activated.vertex_named(d.name).payload ? 0 : search_for(d).count,
         ]
       end
     end
-
-    private
-
-    def dependency_prerelease?(dependency)
-      dependency.prerelease?
-    end
   end
 
   class BundlerIndex < TestIndex
@@ -126,14 +123,14 @@ def sort_dependencies(dependencies, activated, conflicts)
       dependencies.sort_by do |d|
         [
           activated.vertex_named(d.name).payload ? 0 : 1,
-          dependency_prerelease?(d) ? 0 : 1,
+          d.prerelease? ? 0 : 1,
           conflicts[d.name] ? 0 : 1,
           search_for(d).count,
         ]
       end
     end
 
-    def requirement_satisfied_by?(requirement, activated, spec)
+    def requirement_satisfied_by?(requirement, activated, spec) # rubocop:disable Metrics/CyclomaticComplexity
       requirement = case requirement
                     when TestSpecification
                       Gem::Dependency.new(requirement.name, requirement.version)
@@ -142,10 +139,14 @@ def requirement_satisfied_by?(requirement, activated, spec)
                     end
 
       shared_possibility_versions =
-        existing_possibility_versions_for_namespace(requirement, activated).
-        inject { |agg, set| agg & set }
+        existing_possibility_versions_for_namespace(requirement, activated)
+
+      if spec.version.prerelease? && !requirement.prerelease?
+        vertex = activated.vertex_named(spec.name)
+        return false if vertex.requirements.none?(&:prerelease?)
+      end
 
-      if existing_possibility_versions_for_namespace(requirement, activated).any?
+      if !existing_possibility_versions_for_namespace(requirement, activated).empty?
         shared_possibility_versions.include?(spec.version) &&
           requirement.requirement.satisfied_by?(spec.version)
       else
@@ -156,15 +157,22 @@ def requirement_satisfied_by?(requirement, activated, spec)
     private
 
     def existing_possibility_versions_for_namespace(requirement, activated)
-      activated.vertices.values.map do |vertex|
+      prerelease_requirement = requirement.prerelease?
+      existing = activated.vertices.values.map do |vertex|
         next unless vertex.payload
         next unless vertex.name.split('/').first == requirement.name.split('/').first
+
+        prerelease_requirement ||= vertex.requirements.any?(&:prerelease?)
+
         if vertex.payload.respond_to?(:possibilities)
           vertex.payload.possibilities.map(&:version)
         else
           [vertex.payload.version]
         end
-      end.compact
+      end.compact.flatten(1)
+      # must use #select! instead of #reject! for 1.8.7 compatibility
+      existing.select! { |possibility| !possibility.prerelease? } unless prerelease_requirement
+      existing
     end
   end
 
@@ -194,7 +202,7 @@ def versions_of(dependency_name)
     BundlerIndex,
     ReverseBundlerIndex,
     BundlerSingleAllNoPenaltyIndex,
-    # RandomSortIndex, this isn't yet always passing
+    RandomSortIndex,
     CocoaPodsIndex,
     BerkshelfIndex,
   ].freeze

spec/spec_helper/naive_resolver.rb

@@ -17,7 +17,7 @@ def self.resolve(index, dependencies)
         break unless vertex
         possibilities = possibilities_by_level[level] ||= index.search_for(Gem::Dependency.new(vertex.name, '>= 0.0.0-a'))
         possibilities.select! do |spec|
-          vertex.requirements.all? { |r| r.requirement.satisfied_by?(spec.version) && (!spec.version.prerelease? || index.send(:dependency_prerelease?, r)) } &&
+          vertex.requirements.all? { |r| r.requirement.satisfied_by?(spec.version) && (!spec.version.prerelease? || r.prerelease?) } &&
             spec.dependencies.all? { |d| v = activated.vertex_named(d.name); !v || !v.payload || d.satisfied_by?(v.payload.version) }
         end
         warn "level = #{level} possibilities = #{possibilities.map(&:to_s)} requirements = #{vertex.requirements.map(&:to_s)}"