From 9bc1ea5f04fc01f52003773955cabeaaa5908706 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 23 Jun 2012 12:48:53 -0700 Subject: [PATCH] Use config finalize to move some version-specific logic to the version This moves out the concept of a "default VM" from the Environment class and makes it the responsibility of the V1 configuration that at least one VM is defined on it. This lets the configuration ultimately decide what a "default" implementation is. --- lib/vagrant/config/v1.rb | 11 +++++++++ lib/vagrant/config/v1/base.rb | 9 +++++++ lib/vagrant/config/v1/root.rb | 9 +++++++ lib/vagrant/config/version_base.rb | 4 ++++ lib/vagrant/environment.rb | 16 ++----------- plugins/kernel_v1/config/vm.rb | 18 +++++++++++++- test/unit/vagrant/config/v1_test.rb | 37 +++++++++++++++++++++++++++++ 7 files changed, 89 insertions(+), 15 deletions(-) diff --git a/lib/vagrant/config/v1.rb b/lib/vagrant/config/v1.rb index f39311c63..1eb4a775e 100644 --- a/lib/vagrant/config/v1.rb +++ b/lib/vagrant/config/v1.rb @@ -13,6 +13,17 @@ module Vagrant new_root_object end + # Finalizes the configuration by making sure there is at least + # one VM defined in it. + def self.finalize(config) + # Call the `#finalize` method on each of the configuration keys. + # They're expected to modify themselves in our case. + config.finalize! + + # Return the object + config + end + # Loads the configuration for the given proc and returns a configuration # object. # diff --git a/lib/vagrant/config/v1/base.rb b/lib/vagrant/config/v1/base.rb index 0a05459df..349f0e973 100644 --- a/lib/vagrant/config/v1/base.rb +++ b/lib/vagrant/config/v1/base.rb @@ -5,6 +5,15 @@ module Vagrant # from this class but this class provides useful helpers that config # classes may wish to use. class Base + # This is called as a last-minute hook that allows the configuration + # object to finalize itself before it will be put into use. This is + # a useful place to do some defaults in the case the user didn't + # configure something or so on. + # + # The configuration object is expected to mutate itself. + def finalize! + end + # Merge another configuration object into this one. This assumes that # the other object is the same class as this one. # diff --git a/lib/vagrant/config/v1/root.rb b/lib/vagrant/config/v1/root.rb index 26038e2d6..d82898a9e 100644 --- a/lib/vagrant/config/v1/root.rb +++ b/lib/vagrant/config/v1/root.rb @@ -30,6 +30,15 @@ module Vagrant end end + # Called to finalize this object just prior to it being used by + # the Vagrant system. The "!" signifies that this is expected to + # mutate itself. + def finalize! + @keys.each do |_key, instance| + instance.finalize! + end + end + # Validates the configuration classes of this instance and raises an # exception if they are invalid. If you are implementing a custom configuration # class, the method you want to implement is {Base#validate}. This is diff --git a/lib/vagrant/config/version_base.rb b/lib/vagrant/config/version_base.rb index d505a810b..f1e0551fd 100644 --- a/lib/vagrant/config/version_base.rb +++ b/lib/vagrant/config/version_base.rb @@ -23,6 +23,10 @@ module Vagrant # This is an optional method to implement. The default implementation # will simply return the same object. # + # This will ONLY be called if this is the version that is being + # used. In the case that an `upgrade` is called, this will never + # be called. + # # @param [Object] obj Final configuration object. # @param [Object] Finalized configuration object. def self.finalize(obj) diff --git a/lib/vagrant/environment.rb b/lib/vagrant/environment.rb index 3d4714f06..ff0d156a7 100644 --- a/lib/vagrant/environment.rb +++ b/lib/vagrant/environment.rb @@ -13,7 +13,6 @@ module Vagrant # access to the VMs, CLI, etc. all in the scope of this environment. class Environment HOME_SUBDIRS = ["tmp", "boxes", "gems"] - DEFAULT_VM = :default DEFAULT_HOME = "~/.vagrant.d" DEFAULT_RC = "~/.vagrantrc" @@ -163,7 +162,7 @@ module Vagrant # # @return [Bool] def multivm? - vms.length > 1 || vms.keys.first != DEFAULT_VM + vms.length > 1 end # Makes a call to the CLI with the given arguments as if they @@ -418,13 +417,6 @@ module Vagrant defined_vm_keys = global.vm.defined_vm_keys.dup defined_vms = global.vm.defined_vms.dup - # If this isn't a multi-VM environment, then setup the default VM - # to simply be our configuration. - if defined_vm_keys.empty? - defined_vm_keys << DEFAULT_VM - defined_vms[DEFAULT_VM] = nil - end - vm_configs = defined_vm_keys.map do |vm_name| @logger.debug("Loading configuration for VM: #{vm_name}") @@ -434,11 +426,7 @@ module Vagrant config = inner_load[subvm] # Second pass, with the box - config = inner_load[subvm, boxes.find(config.vm.box)] - config.vm.name = vm_name - - # Return the final configuration for this VM - config + inner_load[subvm, boxes.find(config.vm.box)] end # Finally, we have our configuration. Set it and forget it. diff --git a/plugins/kernel_v1/config/vm.rb b/plugins/kernel_v1/config/vm.rb index c0bc7e1fc..10b6dfe32 100644 --- a/plugins/kernel_v1/config/vm.rb +++ b/plugins/kernel_v1/config/vm.rb @@ -8,6 +8,8 @@ require File.expand_path("../vm_subvm", __FILE__) module VagrantPlugins module Kernel_V1 class VMConfig < Vagrant::Config::V1::Base + DEFAULT_VM_NAME = :default + attr_accessor :name attr_accessor :auto_port_range attr_accessor :box @@ -99,11 +101,25 @@ module VagrantPlugins defined_vm_keys << name # Add the SubVM to the hash of defined VMs - defined_vms[name] ||= VagrantConfigSubVM.new + if !defined_vms[name] + # If it hasn't been defined before, then create the sub-VM configuration + # and configure it so that it has the proper name. + defined_vms[name] ||= VagrantConfigSubVM.new + defined_vms[name].push_proc do |config| + config.vm.name = name + end + end + defined_vms[name].options.merge!(options) defined_vms[name].push_proc(&block) if block end + def finalize! + # If we haven't defined a single VM, then we need to define a + # default VM which just inherits the rest of the configuration. + define(DEFAULT_VM_NAME) if defined_vm_keys.empty? + end + def validate(env, errors) errors.add(I18n.t("vagrant.config.vm.box_missing")) if !box errors.add(I18n.t("vagrant.config.vm.box_not_found", :name => box)) if box && !box_url && !env.boxes.find(box) diff --git a/test/unit/vagrant/config/v1_test.rb b/test/unit/vagrant/config/v1_test.rb index 546bbdacc..4e54ceb0e 100644 --- a/test/unit/vagrant/config/v1_test.rb +++ b/test/unit/vagrant/config/v1_test.rb @@ -10,6 +10,43 @@ describe Vagrant::Config::V1 do end end + describe "finalizing" do + it "should call `#finalize` on the configuration object" do + # Register a plugin for our test + plugin_class = Class.new(Vagrant.plugin("1")) do + name "test" + config "foo" do + Class.new do + attr_accessor :bar + + def finalize! + @bar = "finalized" + end + end + end + end + + # Create the proc we're testing + config_proc = Proc.new do |config| + config.foo.bar = "value" + end + + begin + # Test that it works properly + config = described_class.load(config_proc) + config.foo.bar.should == "value" + + # Finalize it + described_class.finalize(config) + config.foo.bar.should == "finalized" + ensure + # We have to unregister the plugin so that future tests + # aren't mucked up. + plugin_class.unregister! + end + end + end + describe "loading" do it "should configure with all plugin config keys loaded" do # Register a plugin for our test