From d5ad387f277f5e29abe97ffbd8fb82e6c31e15be Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 12 Sep 2010 23:01:55 -0600 Subject: [PATCH] New `vagrant package` option `--vagrantfile` and changed semantics of `--include` (see CHANGELOG) --- CHANGELOG.md | 10 +++++ lib/vagrant/action/general/package.rb | 32 +++++++++----- lib/vagrant/command/package.rb | 1 + templates/locales/en.yml | 2 +- templates/package_Vagrantfile.erb | 2 +- test/vagrant/action/general/package_test.rb | 48 +++++++++++++++------ 6 files changed, 70 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 18adf1b9244..c3851a5cc54 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,15 @@ ## 0.6.0 (unreleased) + - `vagrant package` now takes a `--vagrantfile` option to specify a + Vagrantfile to package. The `--include` approach for including a Vagrantfile + no longer works (previously built boxes will continue to work). + - `vagrant package` has new logic with regards to the `--include` option + depending on if the file path is relative or absolute (they can be + intermixed): + * _Relative_ paths are copied directly into the box, preserving + their path. So `--include lib/foo` would be in the box as "lib/foo" + * _Absolute_ paths are simply copied files into the root of the + box. So `--include /lib/foo` would be in the box as "foo" - "vagrant_main" is no longer the default run list. Instead, chef run list starts empty. It is up to you to specify all recipes in the Vagrantfile now. diff --git a/lib/vagrant/action/general/package.rb b/lib/vagrant/action/general/package.rb index 61531ced9a2..438647df145 100644 --- a/lib/vagrant/action/general/package.rb +++ b/lib/vagrant/action/general/package.rb @@ -22,6 +22,7 @@ def initialize(app, env) @env = env @env["package.output"] ||= env["config"].package.name @env["package.include"] ||= [] + @env["package.vagrantfile"] ||= nil end def call(env) @@ -30,7 +31,7 @@ def call(env) raise Errors::PackageOutputExists.new if File.exist?(tar_path) raise Errors::PackageRequiresDirectory.new if !@env["package.directory"] || !File.directory?(@env["package.directory"]) - verify_included_files + verify_files_to_copy compress @app.call(env) @@ -41,8 +42,21 @@ def recover(env) File.delete(tar_path) if File.exist?(tar_path) end - def verify_included_files - @env["package.include"].each do |file| + def files_to_copy + package_dir = Pathname.new(@env["package.directory"]).join("include") + + files = @env["package.include"].inject({}) do |acc, file| + source = Pathname.new(file) + acc[file] = source.relative? ? package_dir.join(source) : package_dir.join(source.basename) + acc + end + + files[@env["package.vagrantfile"]] = package_dir.join("_Vagrantfile") if @env["package.vagrantfile"] + files + end + + def verify_files_to_copy + files_to_copy.each do |file, _| raise Errors::PackageIncludeMissing.new(:file => file) if !File.exist?(file) end end @@ -51,14 +65,10 @@ def verify_included_files # to the temporary directory so they are included in a sub-folder within # the actual box def copy_include_files - if @env["package.include"].length > 0 - include_dir = File.join(@env["package.directory"], "include") - FileUtils.mkdir_p(include_dir) - - @env["package.include"].each do |f| - @env.ui.info "vagrant.actions.general.package.packaging", :file => f - FileUtils.cp(f, include_dir) - end + files_to_copy.each do |from, to| + @env.ui.info "vagrant.actions.general.package.packaging", :file => from + FileUtils.mkdir_p(to.parent) + FileUtils.cp(from, to) end end diff --git a/lib/vagrant/command/package.rb b/lib/vagrant/command/package.rb index 90de8eee376..eed73ca2413 100644 --- a/lib/vagrant/command/package.rb +++ b/lib/vagrant/command/package.rb @@ -5,6 +5,7 @@ class PackageCommand < NamedBase class_option :base, :type => :string, :default => nil class_option :output, :type => :string, :default => nil class_option :include, :type => :array, :default => nil + class_option :vagrantfile, :type => :string, :default => nil register "package" def execute diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 25c9d4c6265..6b0da901253 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -322,7 +322,7 @@ en: general: package: packaging: "Packaging additional file: %{file}" - compressing: "Compressing package to: %w{tar_path}" + compressing: "Compressing package to: %{tar_path}" output_exists: |- The specified file to save the package as already exists. Please remove this file or specify a different file name for outputting. diff --git a/templates/package_Vagrantfile.erb b/templates/package_Vagrantfile.erb index f3d5d652304..0091ccc6818 100644 --- a/templates/package_Vagrantfile.erb +++ b/templates/package_Vagrantfile.erb @@ -7,5 +7,5 @@ end # Load include vagrant file if it exists after the auto-generated # so it can override any of the settings -include_vagrantfile = File.expand_path(File.join(File.dirname(__FILE__), "include", "Vagrantfile")) +include_vagrantfile = File.expand_path("../include/_Vagrantfile", __FILE__) load include_vagrantfile if File.exist?(include_vagrantfile) diff --git a/test/vagrant/action/general/package_test.rb b/test/vagrant/action/general/package_test.rb index bd376ef4510..049365d0899 100644 --- a/test/vagrant/action/general/package_test.rb +++ b/test/vagrant/action/general/package_test.rb @@ -55,7 +55,7 @@ class PackageGeneralActionTest < Test::Unit::TestCase context "calling" do should "call the proper methods then continue chain" do seq = sequence("seq") - @instance.expects(:verify_included_files).in_sequence(seq).returns(true) + @instance.expects(:verify_files_to_copy).in_sequence(seq).returns(true) @instance.expects(:compress).in_sequence(seq) @app.expects(:call).with(@env).in_sequence(seq) @instance.call(@env) @@ -109,7 +109,34 @@ class PackageGeneralActionTest < Test::Unit::TestCase end end - context "verifying included files" do + context "files to copy" do + setup do + @env["package.include"] = [] + @package_dir = Pathname.new(@env["package.directory"]).join("include") + end + + should "have included files whole path if relative" do + path = "lib/foo" + @env["package.include"] = [path] + result = @instance.files_to_copy + assert_equal @package_dir.join(path), result[path] + end + + should "have the filename if an absolute path" do + path = "/foo/bar" + @env["package.include"] = [path] + result = @instance.files_to_copy + assert_equal @package_dir.join("bar"), result[path] + end + + should "include the Vagrantfile if specified" do + @env["package.vagrantfile"] = "foo" + result = @instance.files_to_copy + assert_equal @package_dir.join("_Vagrantfile"), result["foo"] + end + end + + context "verifying files to copy" do setup do @env["package.include"] = ["foo"] File.stubs(:exist?).returns(true) @@ -118,13 +145,13 @@ class PackageGeneralActionTest < Test::Unit::TestCase should "error if included file is not found" do File.expects(:exist?).with("foo").returns(false) assert_raises(Vagrant::Errors::PackageIncludeMissing) { - @instance.verify_included_files + @instance.verify_files_to_copy } end should "return true if all exist" do assert_nothing_raised { - assert @instance.verify_included_files + assert @instance.verify_files_to_copy } end end @@ -142,14 +169,11 @@ class PackageGeneralActionTest < Test::Unit::TestCase end should "create the include directory and copy files to it" do - include_dir = File.join(@env["package.directory"], "include") - copy_seq = sequence("copy_seq") - FileUtils.expects(:mkdir_p).with(include_dir).once.in_sequence(copy_seq) - - 5.times do |i| - file = mock("f#{i}") - @env["package.include"] << file - FileUtils.expects(:cp).with(file, include_dir).in_sequence(copy_seq) + @env["package.include"] = ["/foo/bar", "lib/foo"] + seq = sequence("seq") + @instance.files_to_copy.each do |from, to| + FileUtils.expects(:mkdir_p).with(to.parent).in_sequence(seq) + FileUtils.expects(:cp).with(from, to).in_sequence(seq) end @instance.copy_include_files