From e258395346ab544419db6006004cde9fdb039a3d Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 6 Jan 2011 18:35:58 -0800 Subject: [PATCH] Rewrite of Vagrant::Config, on the path to fix bug with multiple loading Vagrantfiles --- lib/vagrant/config.rb | 144 +++++++++++++++---------------- lib/vagrant/environment.rb | 15 ++-- test/vagrant/config_test.rb | 140 ++++++++++-------------------- test/vagrant/environment_test.rb | 2 + 4 files changed, 127 insertions(+), 174 deletions(-) diff --git a/lib/vagrant/config.rb b/lib/vagrant/config.rb index 05ad481ea..88be0d2f7 100644 --- a/lib/vagrant/config.rb +++ b/lib/vagrant/config.rb @@ -17,92 +17,90 @@ module Vagrant # 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(env) - # config.queue << "/path/to/some/Vagrantfile" - # result = config.load! + # 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 - extend Util::StackedProcRunner + # An array of symbols specifying the load order for the procs. + attr_accessor :load_order - @@config = nil - - attr_reader :queue - - class << self - # Resets the current loaded config object to the specified environment. - # This clears the proc stack and initializes a new {Top} for loading. - # This method shouldn't be called directly, instead use an instance of this - # class for config loading. - # - # @param [Environment] env - def reset!(env=nil) - @@config = nil - proc_stack.clear - - # Reset the configuration to the specified environment - config(env) - end - - # Returns the current {Top} configuration object. While this is still - # here for implementation purposes, it shouldn't be called directly. Instead, - # use an instance of this class. - def config(env=nil) - @@config ||= Config::Top.new(env) - end - - # Adds the given proc/block to the stack of config procs which are all - # run later on a single config object. This is the main way to configure - # Vagrant, and is how all Vagrantfiles are formatted: - # - # Vagrant::Config.run do |config| - # # ... - # end - # - def run(&block) - push_proc(&block) - end - - # Executes all the config procs onto the currently loaded {Top} object, - # and returns the final configured object. This also validates the - # configuration by calling {Top#validate!} on every configuration - # class. - def execute! - config_object ||= config - run_procs!(config_object) - config_object - end - end - - # Initialize a {Config} object for the given {Environment}. + # This is the method which is called by all Vagrantfiles to configure Vagrant. + # This method expects a block which accepts a single argument representing + # an instance of the {Config::Top} class. # - # @param [Environment] env Environment which config object will be part - # of. - def initialize(env) - @env = env - @queue = [] + # Note that the block is not run immediately. Instead, it's proc is stored + # away for execution later. + def self.run(&block) + # Store it for later + @last_proc = block end - # Loads the queue of files/procs, executes them in the proper - # sequence, and returns the resulting configuration object. - def load! - self.class.reset!(@env) + # 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. + # + # @return [Proc] + def self.last_proc + value = @last_proc + @last_proc = nil + value + end - queue.flatten.each do |item| - if item.is_a?(String) && File.exist?(item) - begin - load item - rescue SyntaxError => e - # Report syntax errors in a nice way for Vagrantfiles - raise Errors::VagrantfileSyntaxError, :file => e.message + def initialize + @procs = {} + @load_order = [] + 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) + @procs[key] = [path].flatten.map(&method(:proc_for)) + 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 - elsif item.is_a?(Proc) - self.class.run(&item) end end - return self.class.execute! + 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 + end end end diff --git a/lib/vagrant/environment.rb b/lib/vagrant/environment.rb index 0b1951de3..7339775b1 100644 --- a/lib/vagrant/environment.rb +++ b/lib/vagrant/environment.rb @@ -320,22 +320,23 @@ module Vagrant first_run = @config.nil? # First load the initial, non config-dependent Vagrantfiles - loader = Config.new(self) - loader.queue << File.expand_path("config/default.rb", Vagrant.source_root) - loader.queue << File.join(box.directory, ROOTFILE_NAME) if !first_run && box - loader.queue << File.join(home_path, ROOTFILE_NAME) if !first_run && home_path - loader.queue << File.join(root_path, ROOTFILE_NAME) if root_path + loader = Config.new + loader.load_order = [:default, :box, :home, :root, :sub_vm] + loader.set(:default, File.expand_path("config/default.rb", Vagrant.source_root)) + loader.set(:box, File.join(box.directory, ROOTFILE_NAME)) if !first_run && box + loader.set(:home, File.join(home_path, ROOTFILE_NAME)) if !first_run && home_path + loader.set(:root, File.join(root_path, ROOTFILE_NAME)) if root_path # If this environment is representing a sub-VM, then we push that # proc on as the last configuration. if vm subvm = parent.config.vm.defined_vms[vm.name] - loader.queue << subvm.proc_stack if subvm + loader.set(:sub_vm, subvm.proc_stack) if subvm end # Execute the configuration stack and store the result as the final # value in the config ivar. - @config = loader.load! + @config = loader.load(self) # (re)load the logger @logger = nil diff --git a/test/vagrant/config_test.rb b/test/vagrant/config_test.rb index a5372d98a..985639ee6 100644 --- a/test/vagrant/config_test.rb +++ b/test/vagrant/config_test.rb @@ -5,120 +5,72 @@ class ConfigTest < Test::Unit::TestCase @klass = Vagrant::Config end + context "with the class" do + should "allow access to the last proc" do + foo = mock("object") + foo.expects(:call).once + + @klass.run { |config| foo.call } + value = @klass.last_proc + assert value.is_a?(Proc) + value.call + + assert @klass.last_proc.nil? + end + end + context "with an instance" do setup do - @env = vagrant_env - @instance = @klass.new(@env) + # @env = vagrant_env + @instance = @klass.new end - should "initially have an empty queue" do - assert @instance.queue.empty? + should "load the config files in the given order" do + names = %w{alpha beta gamma} + + @instance.load_order = [:alpha, :beta] + + names.each do |name| + vagrantfile(vagrant_box(name), "config.vm.box = '#{name}'") + @instance.set(name.to_sym, vagrant_box(name).join("Vagrantfile")) + end + + config = @instance.load(nil) + assert_equal "beta", config.vm.box end - should "reset the config class on load, then execute" do - seq = sequence("sequence") - @klass.expects(:reset!).with(@env).in_sequence(seq) - @klass.expects(:execute!).in_sequence(seq) - @instance.load! + should "load the config as procs" do + @instance.set(:proc, Proc.new { |config| config.vm.box = "proc" }) + @instance.load_order = [:proc] + config = @instance.load(nil) + + assert_equal "proc", config.vm.box end - should "run the queue in the order given" do - @instance.queue << Proc.new { |config| config.vm.box = "foo" } - @instance.queue << Proc.new { |config| config.vm.box = "bar" } - result = @instance.load! + should "load an array of procs" do + @instance.set(:proc, [Proc.new { |config| config.vm.box = "proc" }, + Proc.new { |config| config.vm.box = "proc2" }]) + @instance.load_order = [:proc] + config = @instance.load(nil) - assert_equal "bar", result.vm.box + assert_equal "proc2", config.vm.box end - should "allow nested arrays" do - queue = [] - queue << Proc.new { |config| config.vm.box = "foo" } - queue << Proc.new { |config| config.vm.box = "bar" } - @instance.queue << queue - result = @instance.load! - - assert_equal "bar", result.vm.box - end - - should "load a file if it exists" do - filename = "foo" - File.expects(:exist?).with(filename).returns(true) - @instance.expects(:load).with(filename).once - - @instance.queue << filename - @instance.load! - end - - should "not load a file if it doesn't exist" do - filename = "foo" - File.expects(:exist?).with(filename).returns(false) - @instance.expects(:load).with(filename).never - - @instance.queue << filename - @instance.load! + should "not care if a file doesn't exist" do + @instance.load_order = [:foo] + assert_nothing_raised { @instance.set(:foo, "i/dont/exist") } + assert_nothing_raised { @instance.load(nil) } end should "raise an exception if there is a syntax error in a file" do - @instance.queue << "foo" - File.expects(:exist?).with("foo").returns(true) - @instance.expects(:load).with("foo").raises(SyntaxError.new) + vagrantfile(vagrant_box("foo"), "^%&8318") assert_raises(Vagrant::Errors::VagrantfileSyntaxError) { - @instance.load! + @instance.set(:foo, vagrant_box("foo").join("Vagrantfile")) } end end - context "resetting" do - setup do - @klass.reset!(vagrant_env) - @klass::Top.any_instance.stubs(:validate!) - @klass.run { |config| } - @klass.execute! - end - - should "return the same config object typically" do - config = @klass.config - assert config.equal?(@klass.config) - end - - should "create a new object if cleared" do - config = @klass.config - @klass.reset! - assert !config.equal?(@klass.config) - end - - should "empty the proc stack" do - assert !@klass.proc_stack.empty? - @klass.reset! - assert @klass.proc_stack.empty? - end - - should "reload the config object based on the given environment" do - env = mock("env") - @klass.expects(:config).with(env).once - @klass.reset!(env) - end - end - - context "initializing" do - setup do - @klass.reset!(vagrant_env) - end - - should "add the given block to the proc stack" do - proc = Proc.new {} - @klass.run(&proc) - assert_equal [proc], @klass.proc_stack - end - - should "return the configuration on execute!" do - @klass.run {} - result = @klass.execute! - assert result.is_a?(@klass::Top) - end - end - context "top config class" do setup do @configures_list = {} diff --git a/test/vagrant/environment_test.rb b/test/vagrant/environment_test.rb index 7e1c3c6f8..a6cd69ac7 100644 --- a/test/vagrant/environment_test.rb +++ b/test/vagrant/environment_test.rb @@ -4,6 +4,8 @@ require "pathname" class EnvironmentTest < Test::Unit::TestCase setup do @klass = Vagrant::Environment + + clean_paths end context "class method check virtualbox version" do