diff --git a/lib/vagrant/action/general/package.rb b/lib/vagrant/action/general/package.rb index beb6f05fc..ee9904e89 100644 --- a/lib/vagrant/action/general/package.rb +++ b/lib/vagrant/action/general/package.rb @@ -20,25 +20,63 @@ module Vagrant class Package include Util + # Perform sanity validations that the provided output filepath is sane. + # In particular, this function validates: + # + # - The output path is a regular file (not a directory or symlink) + # - No file currently exists at the given path + # - A directory of package files was actually provided (internal) + # + # @param [String] output path to the output file + # @param [String] directory path to a directory containing the files + def self.validate!(output, directory) + filename = File.basename(output.to_s) + output = fullpath(output) + + if File.directory?(output) + raise Vagrant::Errors::PackageOutputDirectory + end + + if File.exist?(output) + raise Vagrant::Errors::PackageOutputExists, filename: filename + end + + if !Vagrant::Util::Presence.present?(directory) || !File.directory?(directory) + raise Vagrant::Errors::PackageRequiresDirectory + end + end + + # Calculate the full path of the given path, relative to the current + # working directory (where the command was run). + # + # @param [String] output the relative path + def self.fullpath(output) + File.expand_path(output, Dir.pwd) + end + + # The path to the final output file. + # @return [String] + attr_reader :fullpath + def initialize(app, env) @app = app env["package.files"] ||= {} env["package.output"] ||= "package.box" + + @fullpath = self.class.fullpath(env["package.output"]) end def call(env) @env = env - file_name = File.basename(@env["package.output"].to_s) - raise Errors::PackageOutputDirectory if File.directory?(tar_path) - raise Errors::PackageOutputExists, file_name:file_name if File.exist?(tar_path) - raise Errors::PackageRequiresDirectory if !env["package.directory"] || - !File.directory?(env["package.directory"]) + self.class.validate!(env["package.output"], env["package.directory"]) + + raise Errors::PackageOutputDirectory if File.directory?(fullpath) @app.call(env) - @env[:ui].info I18n.t("vagrant.actions.general.package.compressing", tar_path: tar_path) + @env[:ui].info I18n.t("vagrant.actions.general.package.compressing", fullpath: fullpath) copy_include_files setup_private_key compress @@ -54,7 +92,7 @@ module Vagrant end # Cleanup any packaged files if the packaging failed at some point. - File.delete(tar_path) if File.exist?(tar_path) + File.delete(fullpath) if File.exist?(fullpath) end # This method copies the include files (passed in via command line) @@ -88,7 +126,7 @@ module Vagrant def compress # Get the output path. We have to do this up here so that the # pwd returns the proper thing. - output_path = tar_path.to_s + output_path = fullpath.to_s # Switch into that directory and package everything up Util::SafeChdir.safe_chdir(@env["package.directory"]) do @@ -147,11 +185,6 @@ module Vagrant f.puts %Q[end] end end - - # Path to the final box output file - def tar_path - File.expand_path(@env["package.output"], FileUtils.pwd) - end end end end diff --git a/plugins/providers/virtualbox/action.rb b/plugins/providers/virtualbox/action.rb index 7da0c06ae..f43846191 100644 --- a/plugins/providers/virtualbox/action.rb +++ b/plugins/providers/virtualbox/action.rb @@ -33,6 +33,8 @@ module VagrantPlugins autoload :Network, File.expand_path("../action/network", __FILE__) autoload :NetworkFixIPv6, File.expand_path("../action/network_fix_ipv6", __FILE__) autoload :Package, File.expand_path("../action/package", __FILE__) + autoload :PackageSetupFiles, File.expand_path("../action/package_setup_files", __FILE__) + autoload :PackageSetupFolders, File.expand_path("../action/package_setup_folders", __FILE__) autoload :PackageVagrantfile, File.expand_path("../action/package_vagrantfile", __FILE__) autoload :PrepareCloneSnapshot, File.expand_path("../action/prepare_clone_snapshot", __FILE__) autoload :PrepareNFSSettings, File.expand_path("../action/prepare_nfs_settings", __FILE__) @@ -41,12 +43,14 @@ module VagrantPlugins autoload :Resume, File.expand_path("../action/resume", __FILE__) autoload :SaneDefaults, File.expand_path("../action/sane_defaults", __FILE__) autoload :SetName, File.expand_path("../action/set_name", __FILE__) - autoload :SetupPackageFiles, File.expand_path("../action/setup_package_files", __FILE__) autoload :SnapshotDelete, File.expand_path("../action/snapshot_delete", __FILE__) autoload :SnapshotRestore, File.expand_path("../action/snapshot_restore", __FILE__) autoload :SnapshotSave, File.expand_path("../action/snapshot_save", __FILE__) autoload :Suspend, File.expand_path("../action/suspend", __FILE__) + # @deprecated use {PackageSetupFiles} instead + autoload :SetupPackageFiles, File.expand_path("../action/setup_package_files", __FILE__) + # Include the built-in modules so that we can use them as top-level # things. include Vagrant::Action::Builtin @@ -150,7 +154,8 @@ module VagrantPlugins next end - b2.use SetupPackageFiles + b2.use PackageSetupFolders + b2.use PackageSetupFiles b2.use CheckAccessible b2.use action_halt b2.use ClearForwardedPorts diff --git a/plugins/providers/virtualbox/action/package.rb b/plugins/providers/virtualbox/action/package.rb index 62a564ac9..fb5ebdabf 100644 --- a/plugins/providers/virtualbox/action/package.rb +++ b/plugins/providers/virtualbox/action/package.rb @@ -1,6 +1,4 @@ -require 'fileutils' - -require 'vagrant/action/general/package' +require_relative "../../../../lib/vagrant/action/general/package" module VagrantPlugins module ProviderVirtualBox @@ -10,33 +8,7 @@ module VagrantPlugins # called in the unit tests. alias_method :general_call, :call def call(env) - # Setup the temporary directory - @temp_dir = env[:tmp_path].join(Time.now.to_i.to_s) - env["export.temp_dir"] = @temp_dir - FileUtils.mkpath(env["export.temp_dir"]) - - # Just match up a couple environmental variables so that - # the superclass will do the right thing. Then, call the - # superclass - env["package.directory"] = env["export.temp_dir"] - general_call(env) - - # Always call recover to clean up the temp dir - clean_temp_dir - end - - def recover(env) - clean_temp_dir - super - end - - protected - - def clean_temp_dir - if @temp_dir && File.exist?(@temp_dir) - FileUtils.rm_rf(@temp_dir) - end end end end diff --git a/plugins/providers/virtualbox/action/package_setup_files.rb b/plugins/providers/virtualbox/action/package_setup_files.rb new file mode 100644 index 000000000..a70c7751a --- /dev/null +++ b/plugins/providers/virtualbox/action/package_setup_files.rb @@ -0,0 +1,51 @@ +module VagrantPlugins + module ProviderVirtualBox + module Action + class PackageSetupFiles + def initialize(app, env) + @app = app + + env["package.include"] ||= [] + env["package.vagrantfile"] ||= nil + end + + def call(env) + files = {} + env["package.include"].each do |file| + source = Pathname.new(file) + dest = nil + + # If the source is relative then we add the file as-is to the include + # directory. Otherwise, we copy only the file into the root of the + # include directory. Kind of strange, but seems to match what people + # expect based on history. + if source.relative? + dest = source + else + dest = source.basename + end + + # Assign the mapping + files[file] = dest + end + + if env["package.vagrantfile"] + # Vagrantfiles are treated special and mapped to a specific file + files[env["package.vagrantfile"]] = "_Vagrantfile" + end + + # Verify the mapping + files.each do |from, _| + raise Vagrant::Errors::PackageIncludeMissing, + file: from if !File.exist?(from) + end + + # Save the mapping + env["package.files"] = files + + @app.call(env) + end + end + end + end +end diff --git a/plugins/providers/virtualbox/action/package_setup_folders.rb b/plugins/providers/virtualbox/action/package_setup_folders.rb new file mode 100644 index 000000000..a045122b7 --- /dev/null +++ b/plugins/providers/virtualbox/action/package_setup_folders.rb @@ -0,0 +1,38 @@ +require "fileutils" + +require_relative "../../../../lib/vagrant/action/general/package" + +module VagrantPlugins + module ProviderVirtualBox + module Action + class PackageSetupFolders + include Vagrant::Util::Presence + + def initialize(app, env) + @app = app + end + + def call(env) + env["package.output"] ||= "package.box" + env["package.directory"] ||= Dir.mktmpdir("package-", env[:tmp_path]) + + # Match up a couple environmental variables so that the other parts of + # Vagrant will do the right thing. + env["export.temp_dir"] = env["package.directory"] + + Vagrant::Action::General::Package.validate!( + env["package.output"], env["package.directory"]) + + @app.call(env) + end + + def recover(env) + dir = env["package.directory"] + if File.exist?(dir) + FileUtils.rm_rf(dir) + end + end + end + end + end +end diff --git a/plugins/providers/virtualbox/action/setup_package_files.rb b/plugins/providers/virtualbox/action/setup_package_files.rb index 1654393c4..d7bc93da3 100644 --- a/plugins/providers/virtualbox/action/setup_package_files.rb +++ b/plugins/providers/virtualbox/action/setup_package_files.rb @@ -1,49 +1,15 @@ +require "log4r" + +require_relative "package_setup_files" + module VagrantPlugins module ProviderVirtualBox module Action - class SetupPackageFiles - def initialize(app, env) - @app = app - - env["package.include"] ||= [] - env["package.vagrantfile"] ||= nil - end - - def call(env) - files = {} - env["package.include"].each do |file| - source = Pathname.new(file) - dest = nil - - # If the source is relative then we add the file as-is to the include - # directory. Otherwise, we copy only the file into the root of the - # include directory. Kind of strange, but seems to match what people - # expect based on history. - if source.relative? - dest = source - else - dest = source.basename - end - - # Assign the mapping - files[file] = dest - end - - if env["package.vagrantfile"] - # Vagrantfiles are treated special and mapped to a specific file - files[env["package.vagrantfile"]] = "_Vagrantfile" - end - - # Verify the mapping - files.each do |from, _| - raise Vagrant::Errors::PackageIncludeMissing, - file: from if !File.exist?(from) - end - - # Save the mapping - env["package.files"] = files - - @app.call(env) + class SetupPackageFiles < PackageSetupFiles + def initialize(*) + @logger = Log4r::Logger.new("vagrant::plugins::virtualbox::setup_package_files") + @logger.warn { "SetupPackageFiles has been renamed to PackageSetupFiles" } + super end end end diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 481e9b754..7a887e133 100755 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -1734,10 +1734,10 @@ en: Sometimes, Vagrant will attempt to auto-correct this for you. In this case, Vagrant was unable to. This is usually because the guest machine is in a state which doesn't allow modifying port forwarding. You could - try 'vagrant reload' (equivalent of running a halt followed by an up) - so vagrant can attempt to auto-correct this upon booting. Be warned + try 'vagrant reload' (equivalent of running a halt followed by an up) + so vagrant can attempt to auto-correct this upon booting. Be warned that any unsaved work might be lost. - + fixed_collision: |- Fixed port collision for %{guest_port} => %{host_port}. Now on port %{new_port}. forwarding: Forwarding ports... @@ -1883,16 +1883,17 @@ en: general: package: packaging: "Packaging additional file: %{file}" - compressing: "Compressing package to: %{tar_path}" + compressing: "Compressing package to: %{fullpath}" output_exists: |- - The specified file '%{file_name}' to save the package as already exists. Please + The specified file '%{filename}' to save the package as already exists. Please remove this file or specify a different file name for outputting. output_is_directory: |- The specified output is a directory. Please specify a path including a filename. requires_directory: |- A directory was not specified to package. This should never happen - and is a result of an internal inconsistency. + and is a result of an internal inconsistency. Please report a bug + on the Vagrant bug tracker. include_file_missing: |- Package include file doesn't exist: %{file}