Add support for optional dependencies

Author: segiddins

lib/molinillo/delegates/specification_provider.rb

@@ -60,6 +60,13 @@ def allow_missing?(dependency)
         end
       end
 
+      # (see Molinillo::SpecificationProvider#optional_dependency?)
+      def optional_dependency?(dependency)
+        with_no_such_dependency_error_handling do
+          specification_provider.optional_dependency?(dependency)
+        end
+      end
+
       private
 
       # Ensures any raised {NoSuchDependencyError} has its

lib/molinillo/modules/specification_provider.rb

@@ -96,5 +96,18 @@ def sort_dependencies(dependencies, activated, conflicts)
     def allow_missing?(dependency)
       false
     end
+
+    # Returns whether this dependency is considered optional.
+    #
+    # If the only dependencies for a specification name are optional, that
+    # specification will not be activated. If there is a strong dependency as
+    # well, than the requirements imposed by optional dependencies will be taken
+    # into account.
+    #
+    # @param [Object] dependency
+    # @return [Boolean] whether this dependency is optional.
+    def optional_dependency?(dependency)
+      false
+    end
   end
 end

lib/molinillo/resolution.rb

@@ -163,6 +163,7 @@ def initial_state
         end
 
         requirements = sort_dependencies(original_requested, graph, {})
+        requirements.reject! { |r| optional_dependency?(r) }
         initial_requirement = requirements.shift
         DependencyState.new(
           initial_requirement && name_for(initial_requirement),
@@ -382,13 +383,26 @@ def attempt_to_activate_new_spec
       # @return [Boolean] whether the current spec is satisfied as a new
       # possibility.
       def new_spec_satisfied?
+        unless requirement_satisfied_by?(requirement, activated, possibility)
+          debug(depth) { 'Requirement unsatisfied by requested spec' }
+          return false
+        end
+
+        vertex = activated.vertex_named(name)
+        vertex.requirements.each do |d|
+          next unless optional_dependency?(d) # TODO: investigate getting rid of this
+          next if requirement_satisfied_by?(d, activated, possibility)
+          debug(depth) { "Vertex requirement #{d} unsatisfied by the possibility" }
+          return false
+        end
+
         locked_requirement = locked_requirement_named(name)
-        requested_spec_satisfied = requirement_satisfied_by?(requirement, activated, possibility)
-        locked_spec_satisfied = !locked_requirement ||
-          requirement_satisfied_by?(locked_requirement, activated, possibility)
-        debug(depth) { 'Unsatisfied by requested spec' } unless requested_spec_satisfied
-        debug(depth) { 'Unsatisfied by locked spec' } unless locked_spec_satisfied
-        requested_spec_satisfied && locked_spec_satisfied
+        if locked_requirement && !requirement_satisfied_by?(locked_requirement, activated, possibility)
+          debug(depth) { "Possibility unsatisfied by locked spec #{locked_requirement}" }
+          return false
+        end
+
+        true
       end
 
       # @param [String] requirement_name the spec name to search for
@@ -416,9 +430,14 @@ def activate_spec
       def require_nested_dependencies_for(activated_spec)
         nested_dependencies = dependencies_for(activated_spec)
         debug(depth) { "Requiring nested dependencies (#{nested_dependencies.join(', ')})" }
-        nested_dependencies.each do |d|
-          activated.add_child_vertex(name_for(d), nil, [name_for(activated_spec)], d)
+        nested_dependencies = nested_dependencies.reject do |d|
+          name = name_for(d)
+          activated.add_child_vertex(name, nil, [name_for(activated_spec)], d)
           @parent_of[d] = requirement
+          next false unless optional_dependency?(d)
+          next true unless existing = activated.vertex_named(name).payload
+          next true if requirement_satisfied_by?(d, activated, existing)
+          false
         end
 
         push_state_for_requirements(requirements + nested_dependencies, !nested_dependencies.empty?)

spec/dependency_graph_spec.rb

@@ -57,7 +57,7 @@ module Molinillo
         @graph.detach_vertex_named(root.name)
         expect(@graph.vertex_named(root.name)).to be_nil
         expect(@graph.vertex_named(child.name)).to eq(child)
-        expect(child.predecessors).to eq([root2])
+        expect(child.predecessors).to contain_exactly(root2)
         expect(@graph.vertices.count).to eq(2)
       end
 

spec/resolver_spec.rb

@@ -19,7 +19,7 @@ def initialize(fixture_path)
       File.open(fixture_path) do |fixture|
         JSON.load(fixture).tap do |test_case|
           self.name = test_case['name']
-          self.index = TestIndex.new(test_case['index'] || 'awesome')
+          self.index = TestIndex.from_fixture(test_case['index'] || 'awesome')
           self.requested = test_case['requested'].map do |(name, reqs)|
             VersionKit::Dependency.new name, reqs.split(',').map(&:chomp)
           end
@@ -89,8 +89,9 @@ def initialize(fixture_path)
     end
 
     describe 'in general' do
+      let(:index) { TestIndex.from_fixture('awesome') }
       before do
-        @resolver = described_class.new(TestIndex.new('awesome'), TestUI.new)
+        @resolver = described_class.new(index, TestUI.new)
       end
 
       it 'can resolve a list of 0 requirements' do
@@ -121,6 +122,61 @@ def initialize(fixture_path)
         expect(resolved.map(&:payload).map(&:to_s)).to eq(['actionpack (1.2.3)'])
       end
 
+      describe 'optional dependencies' do
+        let(:index) do
+          TestIndex.new(
+            'no_deps' => [
+              TestSpecification.new('name' => 'no_deps', 'version' => '1.0'),
+              TestSpecification.new('name' => 'no_deps', 'version' => '2.0'),
+              TestSpecification.new('name' => 'no_deps', 'version' => '3.0'),
+            ],
+            'strong_dep' => [
+              TestSpecification.new('name' => 'strong_dep', 'version' => '1.0',
+                                    'dependencies' => { 'no_deps' => '< 3' }),
+            ],
+            'weak_dep' => [
+              TestSpecification.new('name' => 'weak_dep', 'version' => '1.0',
+                                    'dependencies' => { 'no_deps' => '< 2 optional' }),
+            ]
+          )
+        end
+
+        let(:no_deps) { VersionKit::Dependency.new('no_deps', '> 1') }
+        let(:weak_dep) { VersionKit::Dependency.new('weak_dep', '>= 0') }
+        let(:strong_dep) { VersionKit::Dependency.new('strong_dep', '>= 0') }
+
+        it 'ignores optional explicit dependencies' do
+          no_deps.optional = true
+          expect(@resolver.resolve([no_deps], DependencyGraph.new).map(&:payload).compact).to be_empty
+        end
+
+        it 'ignores nested optional dependencies' do
+          expect(@resolver.resolve([weak_dep], DependencyGraph.new).map(&:payload).compact.map(&:to_s)).to eq [
+            'weak_dep (1.0.0)',
+            'no_deps (1.0.0)',
+          ]
+        end
+
+        it 'uses nested optional dependencies' do
+          expect(@resolver.resolve([weak_dep, strong_dep], DependencyGraph.new).map(&:payload).compact.map(&:to_s)).
+            to eq [
+              'weak_dep (1.0.0)',
+              'strong_dep (1.0.0)',
+              'no_deps (1.0.0)',
+            ]
+        end
+
+        it 'raises when an optional dependency conflicts' do
+          resolve = proc { @resolver.resolve([weak_dep, no_deps], DependencyGraph.new) }
+          expect(&resolve).to raise_error(Molinillo::VersionConflict, <<-EOS.strip)
+Unable to satisfy the following requirements:
+
+- `no_deps (> 1.0.0)` required by `user-specified dependency`
+- `no_deps (< 2.0.0)` required by `weak_dep (1.0.0)`
+          EOS
+        end
+      end
+
       # Regression test. See: https://github.com/CocoaPods/Molinillo/pull/38
       it 'can resolve when swapping children with successors' do
         swap_child_with_successors_index = Class.new(TestIndex) do
@@ -144,7 +200,7 @@ def versions_of(dependency_name)
           end
         end
 
-        index = swap_child_with_successors_index.new('swap_child_with_successors')
+        index = swap_child_with_successors_index.from_fixture('swap_child_with_successors')
         @resolver = described_class.new(index, TestUI.new)
         demands = [
           VersionKit::Dependency.new('build-essential', '>= 0.0.0'),

spec/spec_helper/index.rb

@@ -4,16 +4,21 @@ class TestIndex
     attr_accessor :specs
     include SpecificationProvider
 
-    def initialize(fixture_name)
+    def self.from_fixture(fixture_name)
       File.open(FIXTURE_INDEX_DIR + (fixture_name + '.json'), 'r') do |fixture|
-        self.specs = JSON.load(fixture).reduce(Hash.new([])) do |specs_by_name, (name, versions)|
+        sorted_specs = JSON.load(fixture).reduce(Hash.new([])) do |specs_by_name, (name, versions)|
           specs_by_name.tap do |specs|
             specs[name] = versions.map { |s| TestSpecification.new s }.sort_by(&:version)
           end
         end
+        return new(sorted_specs)
       end
     end
 
+    def initialize(specs_by_name)
+      self.specs = specs_by_name
+    end
+
     def requirement_satisfied_by?(requirement, _activated, spec)
       case requirement
       when TestSpecification
@@ -53,6 +58,10 @@ def sort_dependencies(dependencies, activated, conflicts)
       end
     end
 
+    def optional_dependency?(dependency)
+      dependency.optional?
+    end
+
     private
 
     def dependency_pre_release?(dependency)

spec/spec_helper/specification.rb

@@ -1,12 +1,21 @@
 # frozen_string_literal: true
+module VersionKit
+  class Dependency
+    attr_accessor :optional
+    alias optional? optional
+  end
+end
+
 module Molinillo
   class TestSpecification
     attr_accessor :name, :version, :dependencies
     def initialize(hash)
       self.name = hash['name']
       self.version = VersionKit::Version.new(hash['version'])
-      self.dependencies = hash['dependencies'].map do |(name, requirement)|
-        VersionKit::Dependency.new(name, requirement.split(',').map(&:chomp))
+      self.dependencies = hash.fetch('dependencies') { Hash.new }.map do |(name, requirement)|
+        requirements = requirement.split(',').map(&:chomp)
+        optional = requirements.delete('optional')
+        VersionKit::Dependency.new(name, requirements).tap { |d| d.optional = optional }
       end
     end