From 73a672cff1151934339dec154e180c01228cf735 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 3 Dec 2011 18:31:17 -0800 Subject: [PATCH] load_config! is kind of working again. Specifically: Global configuration load appears to be working. More unit tests should reveal if proper VM configuration is loading. --- BRANCH_TODO | 10 +++ lib/vagrant/config/container.rb | 27 +++---- lib/vagrant/config/loader.rb | 4 +- lib/vagrant/config/vm.rb | 1 + lib/vagrant/environment.rb | 103 ++++++++++++++---------- templates/locales/en.yml | 1 - test/unit/vagrant/config/loader_test.rb | 5 ++ test/unit/vagrant/environment_test.rb | 33 ++++++++ 8 files changed, 126 insertions(+), 58 deletions(-) create mode 100644 BRANCH_TODO diff --git a/BRANCH_TODO b/BRANCH_TODO new file mode 100644 index 000000000..804dec5c2 --- /dev/null +++ b/BRANCH_TODO @@ -0,0 +1,10 @@ +This is a TODO file for only this branch (config-overhaul) + +* Config validation +* Separation of global vs VM configuration. +* Finish Config::Container +* Have Vagrant::Environment use new global config, put VM-specific + config directly into Vagrant::VM objects +* Only allow one kind of vagrantfile to be loaded (i.e. if both + Vagrantfile and vagrantfile exist, throw an error) +* Nicer error if can't setup home directory \ No newline at end of file diff --git a/lib/vagrant/config/container.rb b/lib/vagrant/config/container.rb index 6c109c421..8d5aae7a4 100644 --- a/lib/vagrant/config/container.rb +++ b/lib/vagrant/config/container.rb @@ -7,30 +7,27 @@ module Vagrant # the completely loaded configuration values. This class is meant to # be immutable. class Container + attr_reader :global + # Initializes the configuration container. # - # A `Vagrant::Config::top` should be passed in to initialize this. - # The container will use this top in order to separate and provide - # access to the configuration. - def initialize(top) - @top = top - end + # @param [Top] global Top-level configuration for the global + # applicatoin. + # @param [Array] vms Array of VM configurations. + def initialize(global, vms) + @global = global + @vms = {} - # This returns the global configuration values. These are values - # that apply to the system as a whole, and not to a specific virtual - # machine or so on. Examples of this sort of configuration: the - # class of the host system, name of the Vagrant dotfile, etc. - def global - # For now, we just return all the configuration, until we - # separate out global vs. non-global configuration keys. - @top + vms.each do |vm_config| + @vms[vm_config.vm.name] = vm_config + end end # This returns the configuration for a specific virtual machine. # The values for this configuration are usually pertinent to a # single virtual machine and do not affect the system globally. def for_vm(name) - @top + @vms[name] end end end diff --git a/lib/vagrant/config/loader.rb b/lib/vagrant/config/loader.rb index 4dd4e32a1..b0367d10d 100644 --- a/lib/vagrant/config/loader.rb +++ b/lib/vagrant/config/loader.rb @@ -52,9 +52,12 @@ module Vagrant @logger.error("Unknown config sources: #{unknown_sources.inspect}") end + # Create the top-level configuration which will hold all the config. top = Top.new @load_order.each do |key| + next if !@sources.has_key?(key) + @sources[key].each do |source| @logger.debug("Loading from: #{key}") @@ -73,7 +76,6 @@ module Vagrant end @logger.debug("Configuration loaded successfully") - top end diff --git a/lib/vagrant/config/vm.rb b/lib/vagrant/config/vm.rb index 64db899d3..a20198cd4 100644 --- a/lib/vagrant/config/vm.rb +++ b/lib/vagrant/config/vm.rb @@ -8,6 +8,7 @@ module Vagrant include Util::StackedProcRunner + attr_accessor :name attr_accessor :auto_port_range attr_accessor :box attr_accessor :box_url diff --git a/lib/vagrant/environment.rb b/lib/vagrant/environment.rb index e64861517..274a85d70 100644 --- a/lib/vagrant/environment.rb +++ b/lib/vagrant/environment.rb @@ -28,9 +28,6 @@ module Vagrant # The {UI} object to communicate with the outside world. attr_writer :ui - # The {Config} object representing the Vagrantfile loader - attr_reader :config_loader - #--------------------------------------------------------------- # Class Methods #--------------------------------------------------------------- @@ -64,7 +61,8 @@ module Vagrant :cwd => nil, :vagrantfile_name => nil, :lock_path => nil, - :ui_class => nil + :ui_class => nil, + :home_path => nil }.merge(opts || {}) # Set the default working directory to look for the vagrantfile @@ -83,6 +81,7 @@ module Vagrant @vagrantfile_name = opts[:vagrantfile_name] @lock_path = opts[:lock_path] @ui_class = opts[:ui_class] + @home_path = opts[:home_path] @loaded = false @lock_acquired = false @@ -113,8 +112,14 @@ module Vagrant return parent.home_path if parent return @_home_path if defined?(@_home_path) - @_home_path ||= Pathname.new(File.expand_path(ENV["VAGRANT_HOME"] || DEFAULT_HOME)) + @_home_path ||= Pathname.new(File.expand_path(@home_path || + ENV["VAGRANT_HOME"] || + DEFAULT_HOME)) @logger.info("Home path: #{@_home_path}") + + # Make sure the home directory is properly setup + load_home_directory! + @_home_path end @@ -378,7 +383,6 @@ module Vagrant # Reloads the configuration of this environment. def reload_config! @config = nil - @config_loader = nil load_config! self end @@ -388,50 +392,67 @@ module Vagrant # this environment, meaning that it will use the given root directory # to load the Vagrantfile into that context. def load_config! - first_run = @config.nil? + # Initialize the config loader + config_loader = Config::Loader.new + config_loader.load_order = [:default, :box, :home, :root, :vm] - # First load the initial, non config-dependent Vagrantfiles - @config_loader ||= Config.new(parent ? parent.config_loader : nil) - @config_loader.load_order = [:default, :box, :home, :root, :sub_vm] - @config_loader.set(:default, File.expand_path("config/default.rb", Vagrant.source_root)) + inner_load = lambda do |subvm=nil, box=nil| + # Default Vagrantfile first. This is the Vagrantfile that ships + # with Vagrant. + config_loader.set(:default, File.expand_path("config/default.rb", Vagrant.source_root)) - vagrantfile_name.each do |rootfile| - if !first_run && vm && box - # We load the box Vagrantfile - box_vagrantfile = box.directory.join(rootfile) - @config_loader.set(:box, box_vagrantfile) if box_vagrantfile.exist? + vagrantfile_name.each do |rootfile| + if box + # We load the box Vagrantfile + box_vagrantfile = box.directory.join(rootfile) + config_loader.set(:box, box_vagrantfile) if box_vagrantfile.exist? + end + + if home_path + # Load the home Vagrantfile + home_vagrantfile = home_path.join(rootfile) + config_loader.set(:home, home_vagrantfile) if home_vagrantfile.exist? + end + + if root_path + # Load the Vagrantfile in this directory + root_vagrantfile = root_path.join(rootfile) + config_loader.set(:root, root_vagrantfile) if root_vagrantfile.exist? + end end - if !first_run && home_path - # Load the home Vagrantfile - home_vagrantfile = home_path.join(rootfile) - @config_loader.set(:home, home_vagrantfile) if home_vagrantfile.exist? + if subvm + # We have subvm configuration, so set that up as well. + config_loader.set(:vm, subvm.proc_stack) end - if root_path - # Load the Vagrantfile in this directory - root_vagrantfile = root_path.join(rootfile) - @config_loader.set(:root, root_vagrantfile) if root_vagrantfile.exist? - end + # Execute the configuration stack and store the result as the final + # value in the config ivar. + config_loader.load end - # 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] - @config_loader.set(:sub_vm, subvm.proc_stack) if subvm + # For the global configuration, we only need to load the configuration + # in a single pass, since nothing is conditional on the configuration. + global = inner_load.call + + # For each virtual machine represented by this environment, we have + # to load the configuration in two-passes. We do this because the + # first pass is used to determine the box for the VM. The second pass + # is used to also load the box Vagrantfile. + vm_configs = global.vm.defined_vm_keys.map do |vm_name| + # First pass, first run. + config = inner_load[vm_name] + + # Second pass, with the box + config = inner_load[global.vm.defined_vms[vm_name], config.vm.box] + config.vm.name = vm_name + + # Return the final configuration for this VM + config end - # Execute the configuration stack and store the result as the final - # value in the config ivar. - @config = @config_loader.load(self) - - if first_run - # After the first run we want to load the configuration again since - # it can change due to box Vagrantfiles and home directory Vagrantfiles - load_home_directory! - load_config! - end + # Finally, we have our configuration. Set it and forget it. + @config = Config::Container.new(global, vm_configs) end # Loads the home directory path and creates the necessary subdirectories @@ -445,7 +466,7 @@ module Vagrant dirs.each do |dir| next if File.directory?(dir) - ui.info I18n.t("vagrant.general.creating_home_dir", :directory => dir) + @logger.info("Creating: #{dir}") FileUtils.mkdir_p(dir) end end diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 15df63895..7f5f6168d 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -1,7 +1,6 @@ en: vagrant: general: - creating_home_dir: "Creating home directory since it doesn't exist: %{directory}" moving_home_dir: "Moving old Vagrant home directory to new location: %{directory}" home_dir_migration_failed: |- Both an old and new Vagrant home directory exist. Only the new one will diff --git a/test/unit/vagrant/config/loader_test.rb b/test/unit/vagrant/config/loader_test.rb index baf833b72..ceb8b732f 100644 --- a/test/unit/vagrant/config/loader_test.rb +++ b/test/unit/vagrant/config/loader_test.rb @@ -5,6 +5,11 @@ describe Vagrant::Config::Loader do let(:instance) { described_class.new } + it "should ignore non-existent load order keys" do + instance.load_order = [:foo] + instance.load + end + it "should load and return the configuration" do proc = Proc.new do |config| config.vagrant.dotfile_name = "foo" diff --git a/test/unit/vagrant/environment_test.rb b/test/unit/vagrant/environment_test.rb index 9a9847c82..86bfb00a3 100644 --- a/test/unit/vagrant/environment_test.rb +++ b/test/unit/vagrant/environment_test.rb @@ -14,6 +14,39 @@ describe Vagrant::Environment do end end + describe "home path" do + it "is set to the home path given" do + instance = described_class.new(:home_path => "/tmp/foo") + instance.home_path.should == Pathname.new("/tmp/foo") + end + + it "is set to the environmental variable VAGRANT_HOME" do + pending "A good temporary ENV thing" + end + + it "is set to the DEFAULT_HOME by default" do + expected = Pathname.new(File.expand_path(described_class::DEFAULT_HOME)) + described_class.new.home_path.should == expected + end + end + + describe "loading configuration" do + let(:home_path) { Pathname.new("/tmp/foo") } + let(:instance) { described_class.new(:home_path => home_path) } + + it "should load global configuration" do + File.open(home_path.join("Vagrantfile"), "w+") do |f| + f.write(<<-VF) +Vagrant::Config.run do |config| + config.vagrant.dotfile_name = "foo" +end +VF + end + + instance.config.global.vagrant.dotfile_name.should == "foo" + end + end + describe "ui" do it "should be a silent UI by default" do described_class.new.ui.should be_kind_of(Vagrant::UI::Silent)