Merge pull request #961 from chef/ssd/deps-resolver-replace

WIP: Replace Molinillo-based resolver

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
-                      return nil if @dep_graph.nil?
-                      arr = @dep_graph.map(&:payload)
-                      Hash[arr.map { |e| [e.name, e] }]
-                    end
+      @dep_list
     end
 
     def to_array
-      return [] if @dep_graph.nil?
-      @dep_graph.map do |v|
-        # Resolver's list of dependency includes dependencies that
-        # 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
+      return [] if @dep_list.nil?
+      @dep_list.map do |_k, v|
+        v.to_hash
       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)
-      list
+      @dep_list = Resolver.resolve(dependencies, @vendor_index, @cwd)
     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)
-      params = spec.profile.metadata.params
-      @dep.match?(params[:name], params[:version])
+    def required_version
+      @version_requirement
+    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,71 @@
 # encoding: utf-8
-# author: Dominik Richter
-# author: Christoph Hartmann
-require 'logger'
-require 'molinillo'
+# author: Steven Danna <steve@chef.io>
+require 'inspec/log'
 require 'inspec/errors'
-require 'inspec/dependencies/requirement'
 
 module Inspec
   #
-  # Inspec::Resolver is responsible for recursively resolving all the
-  # depenendencies for a given top-level dependency set.
+  # Inspec::Resolver is a simple dependency resolver. Unlike Bundler
+  # 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 = {})
-      reqs = requirements.map do |req|
-        req = Inspec::Requirement.from_metadata(req, vendor_index, cwd: cwd)
+    def self.resolve(dependencies, vendor_index, working_dir)
+      reqs = dependencies.map do |dep|
+        req = Inspec::Requirement.from_metadata(dep, vendor_index, cwd: working_dir)
         req || fail("Cannot initialize dependency: #{req}")
       end
-
-      new(vendor_index, opts).resolve(reqs)
+      new.resolve(reqs)
     end
 
-    def initialize(vendor_index, opts = {})
-      @logger = opts[:logger] || Logger.new(nil)
-      @debug_mode = false
-
-      @vendor_index = vendor_index
-      @resolver = Molinillo::Resolver.new(self, self)
-      @search_cache = {}
+    def resolve(deps, top_level = true, seen_items = {}, path_string = '')
+      graph = {}
+      if top_level
+        Inspec::Log.debug("Starting traversal of dependencies #{deps.map(&:name)}")
+      else
+        Inspec::Log.debug("Traversing dependency tree of transitive dependency #{deps.map(&:name)}")
       end
 
-    # Resolve requirements.
-    #
-    # @param requirements [Array(Inspec::requirement)] Array of requirements
-    # @return [Array(String)] list of resolved dependency paths
-    def resolve(requirements)
-      requirements.each(&:pull)
-      @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."
+      deps.each do |dep|
+        path_string = if path_string.empty?
+                        dep.name
+                      else
+                        path_string + " -> #{dep.name}"
                       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)
+        if seen_items.key?(dep.source_url)
+          fail Inspec::CyclicDependencyError, "Dependency #{dep} would cause a dependency cycle (#{path_string})"
+        else
+          seen_items[dep.source_url] = true
         end
 
-    def uncached_search_for(dep)
-      # pre-cached and specified dependencies
-      return [dep] unless dep.profile.nil?
-
-      results = @vendor_index.find(dep)
-      return [] unless results.any?
-
-      # TODO: load dep from vendor index
-      # vertex = @dep_graph.vertex_named(dep.name)
-      # locked_requirement = vertex.payload.requirement if vertex
-      fail NotImplementedError, "load dependency #{dep} from vendor index"
+        if !dep.source_satisfies_spec?
+          fail Inspec::UnsatisfiedVersionSpecification, "The profile #{dep.name} from #{dep.source_url} has a version #{dep.source_version} which doesn't match #{dep.required_version}"
         end
 
-    # Returns the dependencies of `specification`.
-    # @note This method should be 'pure', i.e. the return value should depend
-    #   only on the `specification` parameter.
-    #
-    # @param [Object] specification
-    # @return [Array<Object>] the dependencies that are required by the given
-    #   `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
-    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
-        ]
+        Inspec::Log.debug("Adding #{dep.source_url}")
+        graph[dep.name] = dep
+        if !dep.dependencies.empty?
+          # Recursively resolve any transitive dependencies.
+          resolve(dep.dependencies, false, seen_items.dup, path_string)
         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
+      Inspec::Log.debug('Dependency traversal complete.') if top_level
+      graph
+    end
   end
 end

lib/inspec/errors.rb

@@ -7,11 +7,5 @@ module Inspec
 
   # dependency resolution
   class CyclicDependencyError < Error; end
-  class VersionConflict < Error
-    attr_reader :conflicts
-    def initialize(conflicts, msg = nil)
-      super(msg)
-      @conflicts = conflicts
-    end
-  end
+  class UnsatisfiedVersionSpecification < Error; 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 = {})
-      @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
+    extend Mixlib::Log
   end
 end

test/unit/dependencies/resolver_test.rb

@@ -0,0 +1,29 @@
+require 'helper'
+require 'inspec/errors'
+require 'inspec/dependencies/resolver'
+
+class FakeDep
+  attr_reader :name, :source_url
+  def initialize(name)
+    @name = name
+    @source_url = "file://#{name}"
+  end
+end
+
+describe Inspec::Resolver do
+  let(:subject) { Inspec::Resolver.new }
+
+  describe "#resolve" do
+    it "returns a Hash" do
+      subject.resolve([]).must_equal({})
+    end
+
+    it "errors if the source version doesn't match the requirement" do
+      dep = FakeDep.new("fake_dep_0")
+      dep.expects(:source_satisfies_spec?).returns(false)
+      dep.expects(:source_version).returns("1.0.0")
+      dep.expects(:required_version).returns(">= 1.0.1")
+      lambda { subject.resolve([dep]) }.must_raise Inspec::UnsatisfiedVersionSpecification
+    end
+  end
+end