Replace Molinillo-based resolver

The Molinillo library is a good library for systems that need a
constraint solver that will solve dependency problems requiring a single
version of each named dependency.

In our case, the eventual goal is to allow libraries to have conflicting
transitive dependencies at runtime. Isolation will be provided by
restricting all calls within a given profile to scope which can only see
that profile's dependencies.

To facilitate working on the isolation feature, I've replaced the
Molinillo-based resolver with a minimal resolver which will allow us to
load multiple versions of the same library.

Since we will likely want a good amount of logging around this feature
in the future, I've added a Inspec::Log singleton-style class, replacing
the previous Inpsec::Log which appeared unused in the code base.

Signed-off-by: Steven Danna <steve@chef.io>

inspec.gemspec

@@ -34,6 +34,6 @@ Gem::Specification.new do |spec|
   spec.add_dependency 'rspec-its', '~> 1.2'
   spec.add_dependency 'pry', '~> 0'
   spec.add_dependency 'hashie', '~> 3.4'
-  spec.add_dependency 'molinillo', '~> 0'
+  spec.add_dependency 'mixlib-log'
   spec.add_dependency 'sslshake', '~> 1'
 end

lib/inspec/base_cli.rb

@@ -3,6 +3,7 @@
 # author: Dominik Richter
 
 require 'thor'
+require 'inspec/log'
 
 module Inspec
   class BaseCLI < Thor # rubocop:disable Metrics/ClassLength
@@ -128,6 +129,14 @@ module Inspec
     end
 
     def configure_logger(o)
+      #
+      # TODO(ssd): This is a big gross, but this configures the
+      # logging singleton Inspec::Log. Eventually it would be nice to
+      # move internal debug logging to use this logging singleton.
+      #
+      Inspec::Log.init(o.log_location)
+      Inspec::Log.level = get_log_level(o.log_level)
+
       o[:logger] = Logger.new(STDOUT)
       # output json if we have activated the json formatter
       if opts['log-format'] == 'json'

lib/inspec/cli.rb

@@ -15,6 +15,12 @@ require 'inspec/runner_mock'
 require 'inspec/env_printer'
 
 class Inspec::InspecCLI < Inspec::BaseCLI # rubocop:disable Metrics/ClassLength
+  class_option :log_level, aliases: :l, type: :string,
+               desc: 'Set the log level: info (default), debug, warn, error'
+
+  class_option :log_location, type: :string, default: STDOUT,
+               desc: 'Location to send diagnostic log messages to. (default: STDOUT)'
+
   class_option :diagnose, type: :boolean,
     desc: 'Show diagnostics (versions, configurations)'
 
@@ -95,6 +101,7 @@ class Inspec::InspecCLI < Inspec::BaseCLI # rubocop:disable Metrics/ClassLength
 
   desc 'vendor', 'Download all dependencies and generate a lockfile'
   def vendor(path = nil)
+    configure_logger(opts)
     profile = Inspec::Profile.for_target('./', opts)
     lockfile = profile.generate_lockfile(path)
     File.write('inspec.lock', lockfile.to_yaml)
@@ -135,6 +142,7 @@ class Inspec::InspecCLI < Inspec::BaseCLI # rubocop:disable Metrics/ClassLength
   exec_options
   def exec(*targets)
     diagnose
+    configure_logger(opts)
     o = opts.dup
 
     # run tests

lib/inspec/dependencies/dependency_set.rb

@@ -54,31 +54,16 @@ module Inspec
       @cwd = cwd
       @vendor_path = vendor_path
       @dep_list = dep_list
-      @dep_graph = nil
     end
 
-    #
-    # Returns a flat list of all dependencies since that is all we
-    # know how to load at the moment.
-    #
     def list
-      @dep_list ||= begin
+      @dep_list
-                      return nil if @dep_graph.nil?
-                      arr = @dep_graph.map(&:payload)
-                      Hash[arr.map { |e| [e.name, e] }]
-                    end
     end
 
     def to_array
-      return [] if @dep_graph.nil?
+      return [] if @dep_list.nil?
-      @dep_graph.map do |v|
+      @dep_list.map do |_k, v|
-        # Resolver's list of dependency includes dependencies that
+        v.to_hash
-        # we'll find further down the tree We don't want those at the
-        # top level as they should already be included in the to_hash
-        # output of the nodes that connect them.
-        if v.incoming_edges.empty?
-          v.payload.to_hash
-        end
       end.compact
     end
 
@@ -92,8 +77,7 @@ module Inspec
     def vendor(dependencies)
       return nil if dependencies.nil? || dependencies.empty?
       @vendor_index ||= VendorIndex.new(@vendor_path)
-      @dep_graph = Resolver.resolve(dependencies, @vendor_index, @cwd)
+      @dep_list = Resolver.resolve(dependencies, @vendor_index, @cwd)
-      list
     end
   end
 end

lib/inspec/dependencies/requirement.rb

@@ -7,7 +7,7 @@ module Inspec
   # Inspec::Requirement represents a given profile dependency, where
   # appropriate we delegate to Inspec::Profile directly.
   #
-  class Requirement
+  class Requirement # rubocop:disable Metrics/ClassLength
     attr_reader :name, :dep, :cwd, :opts
     attr_writer :dependencies
 
@@ -42,9 +42,18 @@ module Inspec
       @cwd = cwd
     end
 
-    def matches_spec?(spec)
+    def required_version
-      params = spec.profile.metadata.params
+      @version_requirement
-      @dep.match?(params[:name], params[:version])
+    end
+
+    def source_version
+      profile.metadata.params[:version]
+    end
+
+    def source_satisfies_spec?
+      name = profile.metadata.params[:name]
+      version = profile.metadata.params[:version]
+      @dep.match?(name, version)
     end
 
     def to_hash

lib/inspec/dependencies/resolver.rb

@@ -1,183 +1,75 @@
 # encoding: utf-8
-# author: Dominik Richter
+# author: Steven Danna <steve@chef.io>
-# author: Christoph Hartmann
+require 'inspec/log'
-require 'logger'
-require 'molinillo'
 require 'inspec/errors'
-require 'inspec/dependencies/requirement'
 
 module Inspec
   #
-  # Inspec::Resolver is responsible for recursively resolving all the
+  # Inspec::Resolver is a simple dependency resolver. Unlike Bundler
-  # depenendencies for a given top-level dependency set.
+  # or Berkshelf, it does not attempt to resolve each named dependency
+  # to a single version. Rather, it traverses down the dependency tree
+  # and:
+  #
+  # - Fetches the dependency from the source
+  # - Checks the presence of cycles, and
+  # - Checks that the specified dependency source satisfies the
+  #   specified version constraint
+  #
+  # The full dependency tree is then available for the loader, which
+  # will provide the isolation necessary to support multiple versions
+  # of the same profile being used at runtime.
+  #
+  # Currently the fetching happens somewhat lazily depending on the
+  # implementation of the fetcher being used.
   #
   class Resolver
-    def self.resolve(requirements, vendor_index, cwd, opts = {})
+    def self.resolve(dependencies, vendor_index, working_dir)
-      reqs = requirements.map do |req|
+      reqs = dependencies.map do |dep|
-        req = Inspec::Requirement.from_metadata(req, vendor_index, cwd: cwd)
+        req = Inspec::Requirement.from_metadata(dep, vendor_index, cwd: working_dir)
         req || fail("Cannot initialize dependency: #{req}")
       end
-
+      new(vendor_index).resolve(reqs)
-      new(vendor_index, opts).resolve(reqs)
     end
 
-    def initialize(vendor_index, opts = {})
+    def initialize(vendor_index)
-      @logger = opts[:logger] || Logger.new(nil)
-      @debug_mode = false
-
       @vendor_index = vendor_index
-      @resolver = Molinillo::Resolver.new(self, self)
-      @search_cache = {}
     end
 
-    # Resolve requirements.
+    def resolve(deps, top_level = true, seen_items = {}, path_string = '')
-    #
+      graph = {}
-    # @param requirements [Array(Inspec::requirement)] Array of requirements
+      if top_level
-    # @return [Array(String)] list of resolved dependency paths
+        Inspec::Log.debug("Starting traversal of dependencies #{deps.map(&:name)}")
-    def resolve(requirements)
+      else
-      requirements.each(&:pull)
+        Inspec::Log.debug("Traversing dependency tree of transitive dependency #{deps.map(&:name)}")
-      @base_dep_graph = Molinillo::DependencyGraph.new
-      @dep_graph = @resolver.resolve(requirements, @base_dep_graph)
-    rescue Molinillo::VersionConflict => e
-      raise VersionConflict.new(e.conflicts.keys.uniq, e.message)
-    rescue Molinillo::CircularDependencyError => e
-      names = e.dependencies.sort_by(&:name).map { |d| "profile '#{d.name}'" }
-      raise CyclicDependencyError,
-            'Your profile has requirements that depend on each other, creating '\
-            "an infinite loop. Please remove #{names.count > 1 ? 'either ' : ''} "\
-            "#{names.join(' or ')} and try again."
-    end
-
-    # --------------------------------------------------------------------------
-    # SpecificationProvider
-
-    # Search for the specifications that match the given dependency.
-    # The specifications in the returned array will be considered in reverse
-    # order, so the latest version ought to be last.
-    # @note This method should be 'pure', i.e. the return value should depend
-    #   only on the `dependency` parameter.
-    #
-    # @param [Object] dependency
-    # @return [Array<Object>] the specifications that satisfy the given
-    #   `dependency`.
-    def search_for(dep)
-      unless dep.is_a?(Inspec::Requirement)
-        fail 'Internal error: Dependency resolver requires an Inspec::Requirement object for #search_for(dependency)'
       end
-      @search_cache[dep] ||= uncached_search_for(dep)
-    end
 
-    def uncached_search_for(dep)
+      deps.each do |dep|
-      # pre-cached and specified dependencies
+        path_string = if path_string.empty?
-      return [dep] unless dep.profile.nil?
+                        dep.name
+                      else
+                        path_string + " -> #{dep.name}"
+                      end
 
-      results = @vendor_index.find(dep)
+        if seen_items.key?(dep.source_url)
-      return [] unless results.any?
+          fail Inspec::CyclicDependencyError, "Dependency #{dep} would cause a dependency cycle (#{path_string})"
+        else
+          seen_items[dep.source_url] = true
+        end
 
-      # TODO: load dep from vendor index
+        if !dep.source_satisfies_spec?
-      # vertex = @dep_graph.vertex_named(dep.name)
+          fail Inspec::UnsatisfiedVersionSpecification, "The profile #{dep.name} from #{dep.source_url} has a version #{dep.source_version} which doesn't match #{dep.required_version}"
-      # locked_requirement = vertex.payload.requirement if vertex
+        end
-      fail NotImplementedError, "load dependency #{dep} from vendor index"
-    end
 
-    # Returns the dependencies of `specification`.
+        Inspec::Log.debug("Adding #{dep.source_url}")
-    # @note This method should be 'pure', i.e. the return value should depend
+        graph[dep.name] = dep
-    #   only on the `specification` parameter.
+        if !dep.dependencies.empty?
-    #
+          # Recursively resolve any transitive dependencies.
-    # @param [Object] specification
+          resolve(dep.dependencies, false, seen_items.dup, path_string)
-    # @return [Array<Object>] the dependencies that are required by the given
+        end
-    #   `specification`.
-    def dependencies_for(specification)
-      specification.dependencies
-    end
-
-    # Determines whether the given `requirement` is satisfied by the given
-    # `spec`, in the context of the current `activated` dependency graph.
-    #
-    # @param [Object] requirement
-    # @param [DependencyGraph] activated the current dependency graph in the
-    #   resolution process.
-    # @param [Object] spec
-    # @return [Boolean] whether `requirement` is satisfied by `spec` in the
-    #   context of the current `activated` dependency graph.
-    def requirement_satisfied_by?(requirement, _activated, spec)
-      requirement.matches_spec?(spec) || spec.is_a?(Inspec::Profile)
-    end
-
-    # Returns the name for the given `dependency`.
-    # @note This method should be 'pure', i.e. the return value should depend
-    #   only on the `dependency` parameter.
-    #
-    # @param [Object] dependency
-    # @return [String] the name for the given `dependency`.
-    def name_for(dependency)
-      unless dependency.is_a?(Inspec::Requirement)
-        fail 'Internal error: Dependency resolver requires an Inspec::Requirement object for #name_for(dependency)'
       end
-      dependency.name
+
+      Inspec::Log.debug('Dependency traversal complete.') if top_level
+      graph
     end
-
-    # @return [String] the name of the source of explicit dependencies, i.e.
-    #   those passed to {Resolver#resolve} directly.
-    def name_for_explicit_dependency_source
-      'inspec.yml'
-    end
-
-    # @return [String] the name of the source of 'locked' dependencies, i.e.
-    #   those passed to {Resolver#resolve} directly as the `base`
-    def name_for_locking_dependency_source
-      'inspec.lock'
-    end
-
-    # Sort dependencies so that the ones that are easiest to resolve are first.
-    # Easiest to resolve is (usually) defined by:
-    #   1) Is this dependency already activated?
-    #   2) How relaxed are the requirements?
-    #   3) Are there any conflicts for this dependency?
-    #   4) How many possibilities are there to satisfy this dependency?
-    #
-    # @param [Array<Object>] dependencies
-    # @param [DependencyGraph] activated the current dependency graph in the
-    #   resolution process.
-    # @param [{String => Array<Conflict>}] conflicts
-    # @return [Array<Object>] a sorted copy of `dependencies`.
-    def sort_dependencies(dependencies, activated, conflicts)
-      dependencies.sort_by do |dependency|
-        name = name_for(dependency)
-        [
-          activated.vertex_named(name).payload ? 0 : 1,
-          # amount_constrained(dependency), # TODO
-          conflicts[name] ? 0 : 1,
-          # activated.vertex_named(name).payload ? 0 : search_for(dependency).count, # TODO
-        ]
-      end
-    end
-
-    # Returns whether this dependency, which has no possible matching
-    # specifications, can safely be ignored.
-    #
-    # @param [Object] dependency
-    # @return [Boolean] whether this dependency can safely be skipped.
-    def allow_missing?(_dependency)
-      # TODO
-      false
-    end
-
-    # --------------------------------------------------------------------------
-    # UI
-
-    include Molinillo::UI
-
-    # The {IO} object that should be used to print output. `STDOUT`, by default.
-    #
-    # @return [IO]
-    def output
-      self
-    end
-
-    def print(what = '')
-      @logger.info(what)
-    end
-    alias puts print
   end
 end

lib/inspec/errors.rb

@@ -7,11 +7,5 @@ module Inspec
 
   # dependency resolution
   class CyclicDependencyError < Error; end
-  class VersionConflict < Error
+  class UnsatisfiedVersionSpecification < Error; end
-    attr_reader :conflicts
-    def initialize(conflicts, msg = nil)
-      super(msg)
-      @conflicts = conflicts
-    end
-  end
 end

lib/inspec/log.rb

@@ -1,34 +1,11 @@
 # encoding: utf-8
-# Copyright 2015 Dominik Richter. All rights reserved.
 # author: Dominik Richter
 # author: Christoph Hartmann
 
-require 'rainbow/ext/string'
+require 'mixlib/log'
 
 module Inspec
   class Log
-    def initialize(opts = {})
+    extend Mixlib::Log
-      @quiet = opts[:quiet] || false
-    end
-
-    def show(msg)
-      puts msg unless @quiet
-    end
-
-    def info(msg)
-      show '  .  '.color(:white) + msg
-    end
-
-    def error(msg)
-      show '  ✖  '.color(:red).bright + msg
-    end
-
-    def warn(msg)
-      show '  !  '.color(:yellow).bright + msg
-    end
-
-    def ok(msg)
-      show '  ✔  '.color(:green).bright + msg
-    end
   end
 end