Introduce Config::Loader

Config::Loader will be the new class responsible for loading configuration
and replaces the previous dual-role "Vagrant::Config" played. While this
commit is very early-stage, once this new architecture is flushed out, it
will make loading, using, and extending configuration much easier and cleaner.

Additionally, I believe this will help post Vagrant 1.0 if multi-language
configuration is implemented.

lib/vagrant/config.rb

@@ -1,46 +1,18 @@
 require 'vagrant/config/base'
-require 'vagrant/config/error_recorder'
+require 'vagrant/config/loader'
+# require 'vagrant/config/error_recorder'
 require 'vagrant/config/top'
 
-# The built-in configuration classes
-require 'vagrant/config/vagrant'
-require 'vagrant/config/ssh'
-require 'vagrant/config/nfs'
-require 'vagrant/config/vm'
-require 'vagrant/config/package'
+# # The built-in configuration classes
+# require 'vagrant/config/vagrant'
+# require 'vagrant/config/ssh'
+# require 'vagrant/config/nfs'
+# require 'vagrant/config/vm'
+# require 'vagrant/config/package'
 
 module Vagrant
-  # The config class is responsible for loading Vagrant configurations, which
-  # are usually found in Vagrantfiles but may also be procs. The loading is done
-  # by specifying a queue of files or procs that are for configuration, and then
-  # executing them. The config loader will run each item in the queue, so that
-  # configuration from later items overwrite that from earlier items. This is how
-  # Vagrant "scoping" of Vagranfiles is implemented.
-  #
-  # If you're looking to create your own configuration classes, see {Base}.
-  #
-  # # Loading Configuration Files
-  #
-  # If you are in fact looking to load configuration files, then this is the
-  # class you are looking for. Loading configuration is quite easy. The following
-  # example assumes `env` is already a loaded instance of {Environment}:
-  #
-  #     config = Vagrant::Config.new
-  #     config.set(:first, "/path/to/some/Vagrantfile")
-  #     config.set(:second, "/path/to/another/Vagrantfile")
-  #     config.load_order = [:first, :second]
-  #     result = config.load(env)
-  #
-  #     p "Your box is: #{result.vm.box}"
-  #
-  # The load order determines what order the config files specified are loaded.
-  # If a key is not mentioned (for example if above the load order was set to
-  # `[:first]`, therefore `:second` was not mentioned), then that config file
-  # won't be loaded.
-  class Config
-    # An array of symbols specifying the load order for the procs.
-    attr_accessor :load_order
-    attr_reader :procs
+  module Config
+    CONFIGURE_MUTEX = Mutex.new
 
     # This is the method which is called by all Vagrantfiles to configure Vagrant.
     # This method expects a block which accepts a single argument representing
@@ -54,69 +26,23 @@ module Vagrant
       @last_procs << block
     end
 
-    # Returns the last proc which was activated for the class via {run}. This
-    # also sets the last proc to `nil` so that calling this method multiple times
-    # will not return duplicates.
+    # This is a method which will yield to a block and will capture all
+    # ``Vagrant.configure`` calls, returning an array of `Proc`s.
     #
-    # @return [Proc]
-    def self.last_proc
-      value = @last_procs
-      @last_procs = nil
-      value
-    end
+    # Wrapping this around anytime you call code which loads configurations
+    # will force a mutex so that procs never get mixed up. This keeps
+    # the configuration loading part of Vagrant thread-safe.
+    def self.capture_configures
+      CONFIGURE_MUTEX.synchronize do
+        # Reset the last procs so that we start fresh
+        @last_procs = []
 
-    def initialize(parent=nil)
-      @procs = {}
-      @load_order = []
+        # Yield to allow the caller to do whatever loading needed
+        yield
 
-      if parent
-        # Shallow copy the procs and load order from parent if given
-        @procs = parent.procs.dup
-        @load_order = parent.load_order.dup
-      end
-    end
-
-    # Adds a Vagrantfile to be loaded to the queue of config procs. Note
-    # that this causes the Vagrantfile file to be loaded at this point,
-    # and it will never be loaded again.
-    def set(key, path)
-      return if @procs.has_key?(key)
-      @procs[key] = [path].flatten.map(&method(:proc_for)).flatten
-    end
-
-    # Loads the added procs using the set `load_order` attribute and returns
-    # the {Config::Top} object result. The configuration is loaded for the
-    # given {Environment} object.
-    #
-    # @param [Environment] env
-    def load(env)
-      config = Top.new(env)
-
-      # Only run the procs specified in the load order, in the order
-      # specified.
-      load_order.each do |key|
-        if @procs[key]
-          @procs[key].each do |proc|
-            proc.call(config) if proc
-          end
-        end
-      end
-
-      config
-    end
-
-    protected
-
-    def proc_for(path)
-      return nil if !path
-      return path if path.is_a?(Proc)
-
-      begin
-        Kernel.load path if File.exist?(path)
-        return self.class.last_proc
-      rescue SyntaxError => e
-        # Report syntax errors in a nice way for Vagrantfiles
-        raise Errors::VagrantfileSyntaxError, :file => e.message
+        # Return the last procs we've seen while still in the mutex,
+        # knowing we're safe.
+        return @last_procs
       end
     end
   end

lib/vagrant/config/base.rb

@@ -1,5 +1,5 @@
 module Vagrant
-  class Config
+  module Config
     # The base class for all configuration classes. This implements
     # basic things such as the environment instance variable which all
     # config classes need as well as a basic `to_json` implementation.

lib/vagrant/config/loader.rb

@@ -0,0 +1,90 @@
+require "pathname"
+
+require "log4r"
+
+module Vagrant
+  module Config
+    # This class is responsible for loading Vagrant configuration,
+    # usually in the form of Vagrantfiles.
+    #
+    # Loading works by specifying the sources for the configuration
+    # as well as the order the sources should be loaded. Configuration
+    # set later always overrides those set earlier; this is how
+    # configuration "scoping" is implemented.
+    class Loader
+      # This is an array of symbols specifying the order in which
+      # configuration is loaded. For examples, see the class documentation.
+      attr_accessor :load_order
+
+      def initialize
+        @logger  = Log4r::Logger.new("vagrant::config::loader")
+        @sources = {}
+      end
+
+      # Set the configuration data for the given name.
+      #
+      # The `name` should be a symbol and must uniquely identify the data
+      # being given.
+      #
+      # `data` can either be a path to a Ruby Vagrantfile or a `Proc` directly.
+      # `data` can also be an array of such values.
+      #
+      # At this point, no configuration is actually loaded. Note that calling
+      # `set` multiple times with the same name will override any previously
+      # set values. In this way, the last set data for a given name wins.
+      def set(name, data)
+        @logger.debug("Set #{name.inspect} = #{data.inspect}")
+
+        # Make all sources an array.
+        data = [data] if !data.kind_of?(Array)
+        @sources[name] = data
+      end
+
+      # This loads the configured sources in the configured order and returns
+      # an actual configuration object that is ready to be used.
+      def load
+        unknown_sources = @sources.keys - @load_order
+        if !unknown_sources.empty?
+          # TODO: Raise exception here perhaps.
+          @logger.error("Unknown config sources: #{unknown_sources.inspect}")
+        end
+
+        @load_order.each do |key|
+          @sources[key].each do |source|
+            procs_for_source(source).each do |proc|
+              # TODO: Call the proc with a configuration object.
+            end
+          end
+        end
+      end
+
+      protected
+
+      # This returns an array of `Proc` objects for the given source.
+      # The `Proc` objects returned will expect a single argument for
+      # the configuration object and are expected to mutate this
+      # configuration object.
+      def procs_for_source(source)
+        return source if source.is_a?(Proc)
+
+        # Assume all string sources are actually pathnames
+        source = Pathname.new(source) if source.is_a?(String)
+
+        if source.is_a?(Pathname)
+          @logger.debug("Load procs for pathname: #{source.inspect}")
+
+          begin
+            return Config.capture_configures do
+              Kernel.load source
+            end
+          rescue SyntaxError => e
+            # Report syntax errors in a nice way.
+            raise Errors::VagrantfileSyntaxError, :file => e.message
+          end
+        end
+
+        raise Exception, "Unknown configuration source: #{source.inspect}"
+      end
+    end
+  end
+end

lib/vagrant/config/top.rb

@@ -1,5 +1,5 @@
 module Vagrant
-  class Config
+  module Config
     # This class is the "top" configure class, which handles registering
     # other configuration classes as well as validation of all configured
     # classes. This is the object which is returned by {Environment#config}

test/unit/base.rb

@@ -5,6 +5,13 @@ require "rspec/autorun"
 # classes to test.
 require "vagrant"
 
+# Add this directory to the load path, since it just makes
+# everything else easier.
+$:.unshift File.expand_path("../", __FILE__)
+
+# Load in helpers
+require "support/shared/base_context"
+
 # Do not buffer output
 $stdout.sync = true
 $stderr.sync = true

test/unit/support/shared/base_context.rb

@@ -0,0 +1,16 @@
+require "tempfile"
+
+shared_context "unit" do
+  # This helper creates a temporary file and returns a Pathname
+  # object pointed to it.
+  def temporary_file(contents=nil)
+    f = Tempfile.new("vagrant-unit")
+
+    if contents
+      f.write(contents)
+      f.flush
+    end
+
+    return Pathname.new(f.path)
+  end
+end

test/unit/vagrant/config/loader_test.rb

@@ -0,0 +1,13 @@
+require File.expand_path("../../../base", __FILE__)
+
+describe Vagrant::Config::Loader do
+  include_context "unit"
+
+  let(:instance) { described_class.new }
+
+  it "should raise proper error if there is a syntax error in a Vagrantfile" do
+    instance.load_order = [:file]
+    instance.set(:file, temporary_file("Vagrant:^Config"))
+    expect { instance.load }.to raise_exception(Vagrant::Errors::VagrantfileSyntaxError)
+  end
+end

test/unit/vagrant/config_test.rb

@@ -0,0 +1,27 @@
+require File.expand_path("../../base", __FILE__)
+
+describe Vagrant::Config do
+  it "should not execute the proc on configuration" do
+    described_class.run do
+      raise Exception, "Failure."
+    end
+  end
+
+  it "should capture configuration procs" do
+    receiver = double()
+
+    procs = described_class.capture_configures do
+      described_class.run do
+        receiver.hello!
+      end
+    end
+
+    # Verify the structure of the result
+    procs.should be_kind_of(Array)
+    procs.length.should == 1
+
+    # Verify that the proper proc was captured
+    receiver.should_receive(:hello!).once
+    procs[0].call
+  end
+end