diff --git a/lib/vagrant/action.rb b/lib/vagrant/action.rb index 4b37c69f1..e5f384838 100644 --- a/lib/vagrant/action.rb +++ b/lib/vagrant/action.rb @@ -15,6 +15,7 @@ module Vagrant autoload :Confirm, "vagrant/action/builtin/confirm" autoload :ConfigValidate, "vagrant/action/builtin/config_validate" autoload :DestroyConfirm, "vagrant/action/builtin/destroy_confirm" + autoload :PackageOutputOverwriteConfirm, "vagrant/action/builtin/package_output_overwrite_confirm" autoload :EnvSet, "vagrant/action/builtin/env_set" autoload :GracefulHalt, "vagrant/action/builtin/graceful_halt" autoload :HandleBox, "vagrant/action/builtin/handle_box" diff --git a/lib/vagrant/action/builtin/package_output_overwrite_confirm.rb b/lib/vagrant/action/builtin/package_output_overwrite_confirm.rb new file mode 100644 index 000000000..98e6a26cc --- /dev/null +++ b/lib/vagrant/action/builtin/package_output_overwrite_confirm.rb @@ -0,0 +1,28 @@ +require_relative "confirm" + +module Vagrant + module Action + module Builtin + class PackageOutputOverwriteConfirm < Confirm + def initialize(app, env) + force_key = "package.force_output_overwrite" + message = I18n.t("vagrant.actions.general.package.output_exists.overwrite_confirmation", + name: env["package.output"]) + + super(app, env, message, force_key, allowed: %w(y n Y N)) + end + + def call(env) + output = File.expand_path(env["package.output"], Dir.pwd) + + if File.exist?(output) + super + else + env[:result] = true + @app.call(env) + end + end + end + end + end +end diff --git a/lib/vagrant/action/general/package.rb b/lib/vagrant/action/general/package.rb index ca1293ad6..aebe56583 100644 --- a/lib/vagrant/action/general/package.rb +++ b/lib/vagrant/action/general/package.rb @@ -31,17 +31,12 @@ module Vagrant # @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 @@ -101,7 +96,7 @@ module Vagrant @env = env # There are certain exceptions that we don't delete the file for. - ignore_exc = [Errors::PackageOutputDirectory, Errors::PackageOutputExists] + ignore_exc = [Errors::PackageOutputDirectory] ignore_exc.each do |exc| return if env["vagrant.error"].is_a?(exc) end diff --git a/lib/vagrant/errors.rb b/lib/vagrant/errors.rb index 44bed4d4c..300b65e30 100644 --- a/lib/vagrant/errors.rb +++ b/lib/vagrant/errors.rb @@ -552,10 +552,6 @@ module Vagrant error_key(:output_is_directory, "vagrant.actions.general.package") end - class PackageOutputExists < VagrantError - error_key(:output_exists, "vagrant.actions.general.package") - end - class PackageRequiresDirectory < VagrantError error_key(:requires_directory, "vagrant.actions.general.package") end diff --git a/plugins/commands/package/command.rb b/plugins/commands/package/command.rb index 4d468b533..4b106c3b1 100644 --- a/plugins/commands/package/command.rb +++ b/plugins/commands/package/command.rb @@ -32,6 +32,10 @@ module VagrantPlugins o.on("--vagrantfile FILE", "Vagrantfile to package with the box") do |v| options[:vagrantfile] = v end + + o.on("-f", "--force", "Vagrantfile to package with the box") do |f| + options[:force_output_overwrite] = f + end end # Parse the options diff --git a/plugins/providers/hyperv/action.rb b/plugins/providers/hyperv/action.rb index c7e1fb939..21c101683 100644 --- a/plugins/providers/hyperv/action.rb +++ b/plugins/providers/hyperv/action.rb @@ -79,10 +79,16 @@ module VagrantPlugins b2.use PackageSetupFiles b2.use action_halt b2.use SyncedFolderCleanup - b2.use Package - b2.use PackageVagrantfile - b2.use PackageMetadataJson - b2.use Export + b2.use Call, PackageOutputOverwriteConfirm do |env2, b3| + if env2[:result] + b3.use Package + b3.use PackageVagrantfile + b3.use PackageMetadataJson + b3.use Export + else + b3.use MessageWillNotOverwritePackageOutput + end + end end end end diff --git a/plugins/providers/virtualbox/action.rb b/plugins/providers/virtualbox/action.rb index bb4e677af..36ede2614 100644 --- a/plugins/providers/virtualbox/action.rb +++ b/plugins/providers/virtualbox/action.rb @@ -30,6 +30,7 @@ module VagrantPlugins autoload :MessageNotCreated, File.expand_path("../action/message_not_created", __FILE__) autoload :MessageNotRunning, File.expand_path("../action/message_not_running", __FILE__) autoload :MessageWillNotDestroy, File.expand_path("../action/message_will_not_destroy", __FILE__) + autoload :MessageWillNotOverwritePackageOutput, File.expand_path("../action/message_will_not_overwrite_package_output", __FILE__) 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__) @@ -163,11 +164,18 @@ module VagrantPlugins b2.use ClearForwardedPorts b2.use PrepareNFSValidIds b2.use SyncedFolderCleanup - b2.use Package - b2.use Export - b2.use PackageVagrantfile + b2.use Call, PackageOutputOverwriteConfirm do |env2, b3| + if env2[:result] + b3.use Package + b3.use Export + b3.use PackageVagrantfile + else + b3.use MessageWillNotOverwritePackageOutput + end + end end end + end # This action just runs the provisioners on the machine. diff --git a/plugins/providers/virtualbox/action/message_will_not_overwrite_package_output.rb b/plugins/providers/virtualbox/action/message_will_not_overwrite_package_output.rb new file mode 100644 index 000000000..690f7e930 --- /dev/null +++ b/plugins/providers/virtualbox/action/message_will_not_overwrite_package_output.rb @@ -0,0 +1,17 @@ +module VagrantPlugins + module ProviderVirtualBox + module Action + class MessageWillNotOverwritePackageOutput + def initialize(app, env) + @app = app + end + + def call(env) + env[:ui].info I18n.t("vagrant.actions.general.package.output_exists.will_not_overwrite", + name: env[:machine].name) + @app.call(env) + end + end + end + end +end diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 2b63dd383..c4731be73 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -2426,9 +2426,11 @@ en: packaging: "Packaging additional file: %{file}" compressing: "Compressing package to: %{fullpath}" box_folder: "Creating new folder: %{folder_path}" - output_exists: |- - The specified file '%{filename}' to save the package as already exists. Please - remove this file or specify a different file name for outputting. + output_exists: + overwrite_confirmation: "Are you sure you want to overwrite '%{name}'? [y/N] " + will_not_overwrite: |- + The VM '%{name}' will not be packaged, since the confirmation + was declined. output_is_directory: |- The specified output is a directory. Please specify a path including a filename. diff --git a/test/unit/vagrant/action/builtin/package_output_overwrite_confirm_test.rb b/test/unit/vagrant/action/builtin/package_output_overwrite_confirm_test.rb new file mode 100644 index 000000000..b80e09c82 --- /dev/null +++ b/test/unit/vagrant/action/builtin/package_output_overwrite_confirm_test.rb @@ -0,0 +1,43 @@ +require File.expand_path("../../../../base", __FILE__) + +describe Vagrant::Action::Builtin::PackageOutputOverwriteConfirm do + let(:app) {lambda {|env|}} + let(:output) {"output.box"} + let(:env) {{"package.output" => output, ui: double("ui")}} + let(:message) {"Are you sure you want to overwrite 'output.box'? [y/N] "} + + context "when package output file exist" do + before do + allow(File).to receive(:expand_path).with(output, any_args).and_return(output) + allow(File).to receive(:exist?).with(output).and_return(true) + end + + %w(y Y).each do |valid| + it "should set the result to true if '#{valid}' is given" do + expect(env[:ui]).to receive(:ask).with(message).and_return(valid) + described_class.new(app, env).call(env) + expect(env[:result]).to be + end + end + + %w(n N).each do |valid| + it "should set the result to true if '#{valid}' is given" do + expect(env[:ui]).to receive(:ask).with(message).and_return(valid) + described_class.new(app, env).call(env) + expect(env[:result]).to be false + end + end + end + + context "when package output file don't exist" do + before do + allow(File).to receive(:expand_path).with(output, any_args).and_return(output) + allow(File).to receive(:exist?).with(output).and_return(false) + end + + it "should set the result to true" do + described_class.new(app, env).call(env) + expect(env[:result]).to be + end + end +end diff --git a/test/unit/vagrant/action/general/package_test.rb b/test/unit/vagrant/action/general/package_test.rb index 19f334b5b..f6c58f732 100644 --- a/test/unit/vagrant/action/general/package_test.rb +++ b/test/unit/vagrant/action/general/package_test.rb @@ -26,7 +26,6 @@ describe Vagrant::Action::General::Package do allow(described_class).to receive(:fullpath).and_return(output) allow(File).to receive(:directory?).with(output).and_return(false) allow(File).to receive(:directory?).with(directory).and_return(true) - allow(File).to receive(:exist?).and_return(false) allow(Vagrant::Util::Presence).to receive(:present?).with(directory).and_return(true) end @@ -34,20 +33,13 @@ describe Vagrant::Action::General::Package do expect { described_class.validate!(output, directory) }.not_to raise_error end - it "should raise error when output directory exists" do + it "should raise error when output is a directory" do expect(File).to receive(:directory?).with(output).and_return(true) expect { described_class.validate!(output, directory) }.to raise_error(Vagrant::Errors::PackageOutputDirectory) end - it "should raise error if output path exists" do - expect(File).to receive(:exist?).with(output).and_return(true) - expect { - described_class.validate!(output, directory) - }.to raise_error(Vagrant::Errors::PackageOutputExists) - end - it "should raise error if directory value not provided" do expect(Vagrant::Util::Presence).to receive(:present?).and_call_original expect { @@ -131,16 +123,6 @@ describe Vagrant::Action::General::Package do subject.recover(env) end end - - context "when vagrant error is PackageOutputExists" do - let(:error) { Vagrant::Errors::PackageOutputExists.new } - - it "should not do anything" do - expect(File).not_to receive(:exist?) - expect(File).not_to receive(:delete) - subject.recover(env) - end - end end describe "#copy_include_files" do