From f5a957f949d77a18cc33db28c056e66897195ff2 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Tue, 24 Jul 2018 14:03:53 -0700 Subject: [PATCH 1/6] WIP --- plugins/hosts/darwin/cap/fs_iso.rb | 63 ++++++++++++++++++++++++++ plugins/hosts/darwin/plugin.rb | 10 ++++ plugins/kernel_v2/config/cloud_init.rb | 50 ++++++++++++++++++++ plugins/kernel_v2/plugin.rb | 11 +++-- 4 files changed, 131 insertions(+), 3 deletions(-) create mode 100644 plugins/hosts/darwin/cap/fs_iso.rb create mode 100644 plugins/kernel_v2/config/cloud_init.rb diff --git a/plugins/hosts/darwin/cap/fs_iso.rb b/plugins/hosts/darwin/cap/fs_iso.rb new file mode 100644 index 000000000..8a2d858eb --- /dev/null +++ b/plugins/hosts/darwin/cap/fs_iso.rb @@ -0,0 +1,63 @@ +require "tempfile" + +module VagrantPlugins + module HostDarwin + module Cap + class FsISO + @@logger = Log4r::Logger.new("vagrant::host::darwin::fs_iso") + + # Check that the host has the ability to generate ISOs + # + # @param [Vagrant::Environment] env + # @return [Boolean] + def self.isofs_available(env) + !!Vagrant::Util::Which.which("hdiutil") + end + + # Generate an ISO file of the given source directory + # + # @param [Vagrant::Environment] env + # @param [String, Pathname] source_directory Contents of ISO + # @param [String, Pathname, nil] file_destination Location to store ISO + # @return [Pathname] ISO location + # @note If file_destination exists, source_directory will be checked + # for recent modifications and a new ISO will be generated if requried. + def self.create_iso(env, source_directory, file_destination=nil) + if file_destination.nil? + tmpfile = Tempfile.new("vagrant-iso") + file_destination = Pathname.new(tmpfile.path) + tmpfile.delete + else + file_destination = Pathname.new(file_destination.to_s) + end + source_directory = Pathname.new(source_directory) + if iso_update_required?(file_destination, source_directory) + # Ensure destination directory is available + FileUtils.mkdir_p(file_destination.to_s) + result = Vagrant::Util::Subprocess.execute("hdiutil", "makehybrid", "-o", + file_destination.to_s, "-hfs", "-joliet", "-iso", "-default-volume-name", + "cidata", source_directory.to_s) + if result.exit_code != 0 + raise "Failed to create ISO!" + end + end + file_destination + end + + # Check if source directory has any new updates + # + # @param [Pathname] iso_path Path to ISO file + # @param [Pathname] dir_path Path to source directory + # @return [Boolean] + def self.iso_update_required?(iso_path, dir_path) + Dir.glob(dir_path.join("**/**/*")).each do |path| + if File.mtime > iso_path.mtime + return true + end + end + false + end + end + end + end +end diff --git a/plugins/hosts/darwin/plugin.rb b/plugins/hosts/darwin/plugin.rb index d27e2eda4..cbc1fe1d7 100644 --- a/plugins/hosts/darwin/plugin.rb +++ b/plugins/hosts/darwin/plugin.rb @@ -11,6 +11,16 @@ module VagrantPlugins Host end + host_capability("darwin", "iso_available") do + require_relative "cap/fs_iso" + Cap::FsISO + end + + host_capability("darwin", "create_iso") do + require_relative "cap/fs_iso" + Cap::FsISO + end + host_capability("darwin", "provider_install_virtualbox") do require_relative "cap/provider_install_virtualbox" Cap::ProviderInstallVirtualBox diff --git a/plugins/kernel_v2/config/cloud_init.rb b/plugins/kernel_v2/config/cloud_init.rb new file mode 100644 index 000000000..271e9df83 --- /dev/null +++ b/plugins/kernel_v2/config/cloud_init.rb @@ -0,0 +1,50 @@ +require "vagrant" + +module VagrantPlugins + module Kernel_V2 + class CloudInit < Vagrant.plugin("2", :config) + + DEFAULT_SOURCE_PATTERN = "**/**/*".freeze + + attr_accessor :iso_path + attr_accessor :source_files_pattern + attr_accessor :source_directory + + def initialize + @iso_path = UNSET_VALUE + @source_files_pattern = UNSET_VALUE + @source_directory = UNSET_VALUE + end + + def finalize! + if @iso_path != UNSET_VALUE + @iso_path = Pathname.new(@iso_path.to_s) + else + @iso_path = nil + end + if @source_files_pattern == UNSET_VALUE + @source_files_pattern = DEFAULT_SOURCE_PATTERN + end + if @source_directory != UNSET_VALUE + @source_directory = Pathname.new(@source_directory.to_s) + else + @source_directory = nil + end + end + + def validate(machine) + errors = _detected_errors + if @source_directory.nil? || !@source_directory.exist? + errors << I18n.t("vagrant.config.cloud_init.invalid_source_directory", + directory: source_directory.to_s) + end + + {"cloud_init" => errors} + end + + def to_s + "CloudInit" + end + end + end +end diff --git a/plugins/kernel_v2/plugin.rb b/plugins/kernel_v2/plugin.rb index 280470d19..846106828 100644 --- a/plugins/kernel_v2/plugin.rb +++ b/plugins/kernel_v2/plugin.rb @@ -15,9 +15,9 @@ module VagrantPlugins # "kernel_v1", none of these configuration classes are upgradable. # This is by design, since we can't be sure if they're upgradable # until another version is available. - config("ssh") do - require File.expand_path("../config/ssh", __FILE__) - SSHConfig + config("cloud_init") do + require File.expand_path("../config/cloud_init", __FILE__) + CloudInit end config("package") do @@ -30,6 +30,11 @@ module VagrantPlugins PushConfig end + config("ssh") do + require File.expand_path("../config/ssh", __FILE__) + SSHConfig + end + config("vagrant") do require File.expand_path("../config/vagrant", __FILE__) VagrantConfig From e1d104a8e3cf4c50b22afd62286e32ce79d99aa7 Mon Sep 17 00:00:00 2001 From: sophia Date: Wed, 10 Jun 2020 15:53:41 -0500 Subject: [PATCH 2/6] Build iso for Darwin host --- lib/vagrant/errors.rb | 4 ++ plugins/hosts/darwin/cap/fs_iso.rb | 35 ++++++++--- plugins/kernel_v2/config/cloud_init.rb | 50 --------------- plugins/kernel_v2/plugin.rb | 11 +--- templates/locales/en.yml | 12 ++++ .../plugins/hosts/darwin/cap/fs_iso_test.rb | 61 +++++++++++++++++++ 6 files changed, 105 insertions(+), 68 deletions(-) delete mode 100644 plugins/kernel_v2/config/cloud_init.rb create mode 100644 test/unit/plugins/hosts/darwin/cap/fs_iso_test.rb diff --git a/lib/vagrant/errors.rb b/lib/vagrant/errors.rb index 05e821bba..d2c137af0 100644 --- a/lib/vagrant/errors.rb +++ b/lib/vagrant/errors.rb @@ -428,6 +428,10 @@ module Vagrant error_key(:host_explicit_not_detected) end + class ISOBuildFailed < VagrantError + error_key(:iso_build_failed) + end + class LinuxMountFailed < VagrantError error_key(:linux_mount_failed) end diff --git a/plugins/hosts/darwin/cap/fs_iso.rb b/plugins/hosts/darwin/cap/fs_iso.rb index 8a2d858eb..07c0e853f 100644 --- a/plugins/hosts/darwin/cap/fs_iso.rb +++ b/plugins/hosts/darwin/cap/fs_iso.rb @@ -1,4 +1,8 @@ require "tempfile" +require 'fileutils' +require 'pathname' +require "vagrant/util/subprocess" +require "vagrant/util/map_command_options" module VagrantPlugins module HostDarwin @@ -6,12 +10,14 @@ module VagrantPlugins class FsISO @@logger = Log4r::Logger.new("vagrant::host::darwin::fs_iso") + BUILD_ISO_CMD = "hdiutil".freeze + # Check that the host has the ability to generate ISOs # # @param [Vagrant::Environment] env # @return [Boolean] def self.isofs_available(env) - !!Vagrant::Util::Which.which("hdiutil") + !!Vagrant::Util::Which.which(BUILD_ISO_CMD) end # Generate an ISO file of the given source directory @@ -19,26 +25,34 @@ module VagrantPlugins # @param [Vagrant::Environment] env # @param [String, Pathname] source_directory Contents of ISO # @param [String, Pathname, nil] file_destination Location to store ISO + # @param [Map] extra arguments to pass to the iso building command # @return [Pathname] ISO location # @note If file_destination exists, source_directory will be checked # for recent modifications and a new ISO will be generated if requried. - def self.create_iso(env, source_directory, file_destination=nil) + def self.create_iso(env, source_directory, file_destination=nil, extra_opts={}) if file_destination.nil? tmpfile = Tempfile.new("vagrant-iso") file_destination = Pathname.new(tmpfile.path) tmpfile.delete else file_destination = Pathname.new(file_destination.to_s) - end - source_directory = Pathname.new(source_directory) - if iso_update_required?(file_destination, source_directory) # Ensure destination directory is available FileUtils.mkdir_p(file_destination.to_s) - result = Vagrant::Util::Subprocess.execute("hdiutil", "makehybrid", "-o", - file_destination.to_s, "-hfs", "-joliet", "-iso", "-default-volume-name", - "cidata", source_directory.to_s) + end + source_directory = Pathname.new(source_directory) + if iso_update_required?(file_destination, source_directory) + iso_command = [BUILD_ISO_CMD, "makehybrid"] + iso_command << "-hfs" + iso_command << "-iso" + iso_command << "-joliet" + iso_command.concat(Vagrant::Util::MapCommandOptions.map_to_command_options(extra_opts, cmd_flag="-")) + iso_command << "-o" + iso_command << file_destination.to_s + iso_command << source_directory.to_s + result = Vagrant::Util::Subprocess.execute(*iso_command) + if result.exit_code != 0 - raise "Failed to create ISO!" + raise Vagrant::Errors::ISOBuildFailed, cmd: iso_command.join(" "), stdout: result.stdout, stderr: result.stderr end end file_destination @@ -51,10 +65,11 @@ module VagrantPlugins # @return [Boolean] def self.iso_update_required?(iso_path, dir_path) Dir.glob(dir_path.join("**/**/*")).each do |path| - if File.mtime > iso_path.mtime + if Pathname.new(path).mtime > iso_path.mtime return true end end + @@logger.info("ISO update not required! No changes found in source path #{dir_path}") false end end diff --git a/plugins/kernel_v2/config/cloud_init.rb b/plugins/kernel_v2/config/cloud_init.rb deleted file mode 100644 index 271e9df83..000000000 --- a/plugins/kernel_v2/config/cloud_init.rb +++ /dev/null @@ -1,50 +0,0 @@ -require "vagrant" - -module VagrantPlugins - module Kernel_V2 - class CloudInit < Vagrant.plugin("2", :config) - - DEFAULT_SOURCE_PATTERN = "**/**/*".freeze - - attr_accessor :iso_path - attr_accessor :source_files_pattern - attr_accessor :source_directory - - def initialize - @iso_path = UNSET_VALUE - @source_files_pattern = UNSET_VALUE - @source_directory = UNSET_VALUE - end - - def finalize! - if @iso_path != UNSET_VALUE - @iso_path = Pathname.new(@iso_path.to_s) - else - @iso_path = nil - end - if @source_files_pattern == UNSET_VALUE - @source_files_pattern = DEFAULT_SOURCE_PATTERN - end - if @source_directory != UNSET_VALUE - @source_directory = Pathname.new(@source_directory.to_s) - else - @source_directory = nil - end - end - - def validate(machine) - errors = _detected_errors - if @source_directory.nil? || !@source_directory.exist? - errors << I18n.t("vagrant.config.cloud_init.invalid_source_directory", - directory: source_directory.to_s) - end - - {"cloud_init" => errors} - end - - def to_s - "CloudInit" - end - end - end -end diff --git a/plugins/kernel_v2/plugin.rb b/plugins/kernel_v2/plugin.rb index 846106828..280470d19 100644 --- a/plugins/kernel_v2/plugin.rb +++ b/plugins/kernel_v2/plugin.rb @@ -15,9 +15,9 @@ module VagrantPlugins # "kernel_v1", none of these configuration classes are upgradable. # This is by design, since we can't be sure if they're upgradable # until another version is available. - config("cloud_init") do - require File.expand_path("../config/cloud_init", __FILE__) - CloudInit + config("ssh") do + require File.expand_path("../config/ssh", __FILE__) + SSHConfig end config("package") do @@ -30,11 +30,6 @@ module VagrantPlugins PushConfig end - config("ssh") do - require File.expand_path("../config/ssh", __FILE__) - SSHConfig - end - config("vagrant") do require File.expand_path("../config/vagrant", __FILE__) VagrantConfig diff --git a/templates/locales/en.yml b/templates/locales/en.yml index a6f8feea9..6a443055f 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -939,6 +939,18 @@ en: ("%{value}") could not be found. Please verify that the plugin is installed which implements this host and that the value you used for `config.vagrant.host` is correct. + iso_build_failed: |- + Failed to build iso image. The following command returned an error: + + %{cmd} + + Stdout from the command: + + %{stdout} + + Stderr from the command: + + %{stderr} hyperv_virtualbox_error: |- Hyper-V and VirtualBox cannot be used together and will result in a system crash. Vagrant will now exit. Please disable Hyper-V if you wish diff --git a/test/unit/plugins/hosts/darwin/cap/fs_iso_test.rb b/test/unit/plugins/hosts/darwin/cap/fs_iso_test.rb new file mode 100644 index 000000000..08d5eb564 --- /dev/null +++ b/test/unit/plugins/hosts/darwin/cap/fs_iso_test.rb @@ -0,0 +1,61 @@ +require 'pathname' + +require_relative "../../../../base" +require_relative "../../../../../../plugins/hosts/darwin/cap/fs_iso" + +describe VagrantPlugins::HostDarwin::Cap::FsISO do + include_context "unit" + + let(:subject){ VagrantPlugins::HostDarwin::Cap::FsISO } + + let(:env){ double("env") } + + describe ".isofs_available" do + it "finds iso building utility when available" do + expect(Vagrant::Util::Which).to receive(:which).and_return(true) + expect(subject.isofs_available(env)).to eq(true) + end + + it "does not find iso building utility when not available" do + expect(Vagrant::Util::Which).to receive(:which).and_return(false) + expect(subject.isofs_available(env)).to eq(false) + end + end + + describe ".create_iso" do + before do + allow(subject).to receive(:iso_update_required?).and_return(true) + allow(FileUtils).to receive(:mkdir_p) + end + + it "builds an iso" do + expect(Vagrant::Util::Subprocess).to receive(:execute).with( + "hdiutil", "makehybrid", "-hfs", "-iso", "-joliet", "-o", /.iso/, /\/foo\/src/ + ).and_return(double(exit_code: 0)) + + output = subject.create_iso(env, "/foo/src", "/woo/out.iso") + expect(output.to_s).to eq("/woo/out.iso") + end + + it "builds an iso with args" do + expect(Vagrant::Util::Subprocess).to receive(:execute).with( + "hdiutil", "makehybrid", "-hfs", "-iso", "-joliet", "-default-volume-name", "cidata", "-o", /.iso/, /\/foo\/src/ + ).and_return(double(exit_code: 0)) + + output = subject.create_iso(env, "/foo/src", "/woo/out.iso", extra_opts={"default-volume-name" => "cidata"}) + expect(output.to_s).to eq("/woo/out.iso") + end + + it "raises an error if iso build failed" do + allow(Vagrant::Util::Subprocess).to receive(:execute).with(any_args).and_return(double(stdout: "nope", stderr: "nope", exit_code: 1)) + expect{ subject.create_iso(env, "/foo/src", "/woo/out.iso") }.to raise_error(Vagrant::Errors::ISOBuildFailed) + end + + it "does not build iso if no changes required" do + allow(subject).to receive(:iso_update_required?).and_return(false) + expect(Vagrant::Util::Subprocess).to_not receive(:execute) + output = subject.create_iso(env, "/foo/src", "/woo/out.iso", extra_opts={"default-volume-name" => "cidata"}) + expect(output.to_s).to eq("/woo/out.iso") + end + end +end From 1ff6582fff5a1b3334a49901b9eaa059ee65dbe2 Mon Sep 17 00:00:00 2001 From: sophia Date: Thu, 11 Jun 2020 14:34:16 -0500 Subject: [PATCH 3/6] Refactor out shared utility --- lib/vagrant/util/directory.rb | 19 +++++++ plugins/hosts/darwin/cap/fs_iso.rb | 52 +++++++++++-------- plugins/hosts/darwin/plugin.rb | 2 +- .../plugins/hosts/darwin/cap/fs_iso_test.rb | 20 +++---- test/unit/vagrant/util/directory_test.rb | 22 ++++++++ 5 files changed, 78 insertions(+), 37 deletions(-) create mode 100644 lib/vagrant/util/directory.rb create mode 100644 test/unit/vagrant/util/directory_test.rb diff --git a/lib/vagrant/util/directory.rb b/lib/vagrant/util/directory.rb new file mode 100644 index 000000000..1e5a8cb7e --- /dev/null +++ b/lib/vagrant/util/directory.rb @@ -0,0 +1,19 @@ +require 'pathname' + +module Vagrant + module Util + class Directory + # Check if directory has any new updates + # + # @param [Pathname, String] Path to directory + # @param [Time] time to compare to eg. has any file in dir_path + # changed since this time + # @return [Boolean] + def self.directory_changed?(dir_path, threshold_time) + Dir.glob(Pathname.new(dir_path).join("**", "*")).any? do |path| + Pathname.new(path).mtime > threshold_time + end + end + end + end +end diff --git a/plugins/hosts/darwin/cap/fs_iso.rb b/plugins/hosts/darwin/cap/fs_iso.rb index 07c0e853f..eec21a264 100644 --- a/plugins/hosts/darwin/cap/fs_iso.rb +++ b/plugins/hosts/darwin/cap/fs_iso.rb @@ -3,6 +3,7 @@ require 'fileutils' require 'pathname' require "vagrant/util/subprocess" require "vagrant/util/map_command_options" +require "vagrant/util/directory" module VagrantPlugins module HostDarwin @@ -23,24 +24,19 @@ module VagrantPlugins # Generate an ISO file of the given source directory # # @param [Vagrant::Environment] env - # @param [String, Pathname] source_directory Contents of ISO - # @param [String, Pathname, nil] file_destination Location to store ISO + # @param [String] source_directory Contents of ISO + # @param [String, nil] file_destination Location to store ISO # @param [Map] extra arguments to pass to the iso building command # @return [Pathname] ISO location # @note If file_destination exists, source_directory will be checked # for recent modifications and a new ISO will be generated if requried. def self.create_iso(env, source_directory, file_destination=nil, extra_opts={}) - if file_destination.nil? - tmpfile = Tempfile.new("vagrant-iso") - file_destination = Pathname.new(tmpfile.path) - tmpfile.delete - else - file_destination = Pathname.new(file_destination.to_s) - # Ensure destination directory is available - FileUtils.mkdir_p(file_destination.to_s) - end + file_destination = output_file(file_destination) source_directory = Pathname.new(source_directory) - if iso_update_required?(file_destination, source_directory) + + # If the destrination does not exist or there have been changes in the source directory since the last build, then build + if !file_destination.exist? || Vagrant::Util::Directory.directory_changed?(source_directory, file_destination.mtime) + @@logger.info("Building ISO from source #{source_directory}") iso_command = [BUILD_ISO_CMD, "makehybrid"] iso_command << "-hfs" iso_command << "-iso" @@ -50,27 +46,37 @@ module VagrantPlugins iso_command << file_destination.to_s iso_command << source_directory.to_s result = Vagrant::Util::Subprocess.execute(*iso_command) - + if result.exit_code != 0 raise Vagrant::Errors::ISOBuildFailed, cmd: iso_command.join(" "), stdout: result.stdout, stderr: result.stderr end end + + @@logger.info("ISO available at #{file_destination}") file_destination end - # Check if source directory has any new updates + # Determines a valid file path for an output file + # and ensures parent directory exists # - # @param [Pathname] iso_path Path to ISO file - # @param [Pathname] dir_path Path to source directory - # @return [Boolean] - def self.iso_update_required?(iso_path, dir_path) - Dir.glob(dir_path.join("**/**/*")).each do |path| - if Pathname.new(path).mtime > iso_path.mtime - return true + # @param [String, nil] (optional) path to output file + # @return [Pathname] path to output file + def self.output_file(file_destination=nil) + if file_destination.nil? + @@logger.info("No file destination specified, creating temp location") + tmpfile = Tempfile.new("vagrant-iso") + file_destination = Pathname.new(tmpfile.path) + tmpfile.delete + else + file_destination = Pathname.new(file_destination.to_s) + if file_destination.extname != ".iso" + file_destination = file_destination.join("#{rand(36**6).to_s(36)}_vagrant-iso") end end - @@logger.info("ISO update not required! No changes found in source path #{dir_path}") - false + @@logger.info("Targeting to create ISO at #{file_destination}") + # Ensure destination directory is available + FileUtils.mkdir_p(File.dirname(file_destination.to_s)) + file_destination end end end diff --git a/plugins/hosts/darwin/plugin.rb b/plugins/hosts/darwin/plugin.rb index cbc1fe1d7..f3a7e6a55 100644 --- a/plugins/hosts/darwin/plugin.rb +++ b/plugins/hosts/darwin/plugin.rb @@ -11,7 +11,7 @@ module VagrantPlugins Host end - host_capability("darwin", "iso_available") do + host_capability("darwin", "isofs_available") do require_relative "cap/fs_iso" Cap::FsISO end diff --git a/test/unit/plugins/hosts/darwin/cap/fs_iso_test.rb b/test/unit/plugins/hosts/darwin/cap/fs_iso_test.rb index 08d5eb564..9a107c693 100644 --- a/test/unit/plugins/hosts/darwin/cap/fs_iso_test.rb +++ b/test/unit/plugins/hosts/darwin/cap/fs_iso_test.rb @@ -1,5 +1,4 @@ -require 'pathname' - +require "pathname" require_relative "../../../../base" require_relative "../../../../../../plugins/hosts/darwin/cap/fs_iso" @@ -7,8 +6,7 @@ describe VagrantPlugins::HostDarwin::Cap::FsISO do include_context "unit" let(:subject){ VagrantPlugins::HostDarwin::Cap::FsISO } - - let(:env){ double("env") } + let(:env) { double("env") } describe ".isofs_available" do it "finds iso building utility when available" do @@ -23,9 +21,12 @@ describe VagrantPlugins::HostDarwin::Cap::FsISO do end describe ".create_iso" do + let(:file_destination) { "/woo/out.iso" } + let(:file_destination_path) { Pathname.new(file_destination)} + before do - allow(subject).to receive(:iso_update_required?).and_return(true) - allow(FileUtils).to receive(:mkdir_p) + allow(subject).to receive(:output_file).with(any_args).and_return(file_destination_path) + allow(file_destination_path).to receive(:exist?).and_return(false) end it "builds an iso" do @@ -50,12 +51,5 @@ describe VagrantPlugins::HostDarwin::Cap::FsISO do allow(Vagrant::Util::Subprocess).to receive(:execute).with(any_args).and_return(double(stdout: "nope", stderr: "nope", exit_code: 1)) expect{ subject.create_iso(env, "/foo/src", "/woo/out.iso") }.to raise_error(Vagrant::Errors::ISOBuildFailed) end - - it "does not build iso if no changes required" do - allow(subject).to receive(:iso_update_required?).and_return(false) - expect(Vagrant::Util::Subprocess).to_not receive(:execute) - output = subject.create_iso(env, "/foo/src", "/woo/out.iso", extra_opts={"default-volume-name" => "cidata"}) - expect(output.to_s).to eq("/woo/out.iso") - end end end diff --git a/test/unit/vagrant/util/directory_test.rb b/test/unit/vagrant/util/directory_test.rb new file mode 100644 index 000000000..71bc4675c --- /dev/null +++ b/test/unit/vagrant/util/directory_test.rb @@ -0,0 +1,22 @@ +require File.expand_path("../../../base", __FILE__) +require "vagrant/util/directory" +require "time" + +describe Vagrant::Util::Directory do + include_context "unit" + + let(:subject){ Vagrant::Util::Directory } + + describe ".directory_changed?" do + + it "should return false if the threshold time is larger the all mtimes" do + t = Time.new("3008", "09", "09") + expect(subject.directory_changed?(Dir.getwd, t)).to eq(false) + end + + it "should return true if the threshold time is less than any mtimes" do + t = Time.new("1990", "06", "06") + expect(subject.directory_changed?(Dir.getwd, t)).to eq(true) + end + end +end From decd489a6d0e86a9f45cae65680342a0bc571f13 Mon Sep 17 00:00:00 2001 From: sophia Date: Mon, 15 Jun 2020 11:14:44 -0500 Subject: [PATCH 4/6] Overwrite outputfile if exists and update available --- plugins/hosts/darwin/cap/fs_iso.rb | 40 ++++++++----------- .../plugins/hosts/darwin/cap/fs_iso_test.rb | 18 ++++++--- 2 files changed, 29 insertions(+), 29 deletions(-) diff --git a/plugins/hosts/darwin/cap/fs_iso.rb b/plugins/hosts/darwin/cap/fs_iso.rb index eec21a264..98ed30d2c 100644 --- a/plugins/hosts/darwin/cap/fs_iso.rb +++ b/plugins/hosts/darwin/cap/fs_iso.rb @@ -31,8 +31,22 @@ module VagrantPlugins # @note If file_destination exists, source_directory will be checked # for recent modifications and a new ISO will be generated if requried. def self.create_iso(env, source_directory, file_destination=nil, extra_opts={}) - file_destination = output_file(file_destination) source_directory = Pathname.new(source_directory) + if file_destination.nil? + @@logger.info("No file destination specified, creating temp location") + tmpfile = Tempfile.new(["vagrant", ".iso"]) + file_destination = Pathname.new(tmpfile.path) + tmpfile.delete + else + file_destination = Pathname.new(file_destination.to_s) + # If the file destination path is a folder, target the output to a randomly named + # file in that dir + if file_destination.extname != ".iso" + file_destination = file_destination.join("#{rand(36**6).to_s(36)}_vagrant.iso") + end + end + # Ensure destination directory is available + FileUtils.mkdir_p(File.dirname(file_destination.to_s)) # If the destrination does not exist or there have been changes in the source directory since the last build, then build if !file_destination.exist? || Vagrant::Util::Directory.directory_changed?(source_directory, file_destination.mtime) @@ -41,6 +55,7 @@ module VagrantPlugins iso_command << "-hfs" iso_command << "-iso" iso_command << "-joliet" + iso_command << "-ov" iso_command.concat(Vagrant::Util::MapCommandOptions.map_to_command_options(extra_opts, cmd_flag="-")) iso_command << "-o" iso_command << file_destination.to_s @@ -55,29 +70,6 @@ module VagrantPlugins @@logger.info("ISO available at #{file_destination}") file_destination end - - # Determines a valid file path for an output file - # and ensures parent directory exists - # - # @param [String, nil] (optional) path to output file - # @return [Pathname] path to output file - def self.output_file(file_destination=nil) - if file_destination.nil? - @@logger.info("No file destination specified, creating temp location") - tmpfile = Tempfile.new("vagrant-iso") - file_destination = Pathname.new(tmpfile.path) - tmpfile.delete - else - file_destination = Pathname.new(file_destination.to_s) - if file_destination.extname != ".iso" - file_destination = file_destination.join("#{rand(36**6).to_s(36)}_vagrant-iso") - end - end - @@logger.info("Targeting to create ISO at #{file_destination}") - # Ensure destination directory is available - FileUtils.mkdir_p(File.dirname(file_destination.to_s)) - file_destination - end end end end diff --git a/test/unit/plugins/hosts/darwin/cap/fs_iso_test.rb b/test/unit/plugins/hosts/darwin/cap/fs_iso_test.rb index 9a107c693..47e213e71 100644 --- a/test/unit/plugins/hosts/darwin/cap/fs_iso_test.rb +++ b/test/unit/plugins/hosts/darwin/cap/fs_iso_test.rb @@ -22,16 +22,15 @@ describe VagrantPlugins::HostDarwin::Cap::FsISO do describe ".create_iso" do let(:file_destination) { "/woo/out.iso" } - let(:file_destination_path) { Pathname.new(file_destination)} before do - allow(subject).to receive(:output_file).with(any_args).and_return(file_destination_path) - allow(file_destination_path).to receive(:exist?).and_return(false) + allow(file_destination).to receive(:nil?).and_return(false) + allow(FileUtils).to receive(:mkdir_p) end it "builds an iso" do expect(Vagrant::Util::Subprocess).to receive(:execute).with( - "hdiutil", "makehybrid", "-hfs", "-iso", "-joliet", "-o", /.iso/, /\/foo\/src/ + "hdiutil", "makehybrid", "-hfs", "-iso", "-joliet", "-ov", "-o", /.iso/, /\/foo\/src/ ).and_return(double(exit_code: 0)) output = subject.create_iso(env, "/foo/src", "/woo/out.iso") @@ -40,13 +39,22 @@ describe VagrantPlugins::HostDarwin::Cap::FsISO do it "builds an iso with args" do expect(Vagrant::Util::Subprocess).to receive(:execute).with( - "hdiutil", "makehybrid", "-hfs", "-iso", "-joliet", "-default-volume-name", "cidata", "-o", /.iso/, /\/foo\/src/ + "hdiutil", "makehybrid", "-hfs", "-iso", "-joliet", "-ov", "-default-volume-name", "cidata", "-o", /.iso/, /\/foo\/src/ ).and_return(double(exit_code: 0)) output = subject.create_iso(env, "/foo/src", "/woo/out.iso", extra_opts={"default-volume-name" => "cidata"}) expect(output.to_s).to eq("/woo/out.iso") end + it "builds an iso given a file destination without an extension" do + expect(Vagrant::Util::Subprocess).to receive(:execute).with( + "hdiutil", "makehybrid", "-hfs", "-iso", "-joliet", "-ov", "-o", /.iso/, /\/foo\/src/ + ).and_return(double(exit_code: 0)) + + output = subject.create_iso(env, "/foo/src", "/woo/out_dir") + expect(output.to_s).to match(/\/woo\/out_dir\/[\w]{6}_vagrant.iso/) + end + it "raises an error if iso build failed" do allow(Vagrant::Util::Subprocess).to receive(:execute).with(any_args).and_return(double(stdout: "nope", stderr: "nope", exit_code: 1)) expect{ subject.create_iso(env, "/foo/src", "/woo/out.iso") }.to raise_error(Vagrant::Errors::ISOBuildFailed) From 7322c74eef0ac8eb84fd98eddd713f46df55ce68 Mon Sep 17 00:00:00 2001 From: sophia Date: Thu, 18 Jun 2020 11:24:27 -0500 Subject: [PATCH 5/6] Add optional arguments as a hash --- plugins/hosts/darwin/cap/fs_iso.rb | 10 +++++---- .../plugins/hosts/darwin/cap/fs_iso_test.rb | 22 ++++++++++++++----- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/plugins/hosts/darwin/cap/fs_iso.rb b/plugins/hosts/darwin/cap/fs_iso.rb index 98ed30d2c..7f9d819a1 100644 --- a/plugins/hosts/darwin/cap/fs_iso.rb +++ b/plugins/hosts/darwin/cap/fs_iso.rb @@ -25,12 +25,14 @@ module VagrantPlugins # # @param [Vagrant::Environment] env # @param [String] source_directory Contents of ISO - # @param [String, nil] file_destination Location to store ISO # @param [Map] extra arguments to pass to the iso building command + # :file_destination (string) location to store ISO + # :volume_id (String) to set the volume name # @return [Pathname] ISO location # @note If file_destination exists, source_directory will be checked # for recent modifications and a new ISO will be generated if requried. - def self.create_iso(env, source_directory, file_destination=nil, extra_opts={}) + def self.create_iso(env, source_directory, **extra_opts) + file_destination = extra_opts[:file_destination] source_directory = Pathname.new(source_directory) if file_destination.nil? @@logger.info("No file destination specified, creating temp location") @@ -42,7 +44,7 @@ module VagrantPlugins # If the file destination path is a folder, target the output to a randomly named # file in that dir if file_destination.extname != ".iso" - file_destination = file_destination.join("#{rand(36**6).to_s(36)}_vagrant.iso") + file_destination = file_destination.join("#{SecureRandom.hex(3)}_vagrant.iso") end end # Ensure destination directory is available @@ -56,7 +58,7 @@ module VagrantPlugins iso_command << "-iso" iso_command << "-joliet" iso_command << "-ov" - iso_command.concat(Vagrant::Util::MapCommandOptions.map_to_command_options(extra_opts, cmd_flag="-")) + iso_command.concat(["-default-volume-name", extra_opts[:volume_id]]) if extra_opts[:volume_id] iso_command << "-o" iso_command << file_destination.to_s iso_command << source_directory.to_s diff --git a/test/unit/plugins/hosts/darwin/cap/fs_iso_test.rb b/test/unit/plugins/hosts/darwin/cap/fs_iso_test.rb index 47e213e71..199b65cbb 100644 --- a/test/unit/plugins/hosts/darwin/cap/fs_iso_test.rb +++ b/test/unit/plugins/hosts/darwin/cap/fs_iso_test.rb @@ -33,16 +33,16 @@ describe VagrantPlugins::HostDarwin::Cap::FsISO do "hdiutil", "makehybrid", "-hfs", "-iso", "-joliet", "-ov", "-o", /.iso/, /\/foo\/src/ ).and_return(double(exit_code: 0)) - output = subject.create_iso(env, "/foo/src", "/woo/out.iso") + output = subject.create_iso(env, "/foo/src", file_destination: "/woo/out.iso") expect(output.to_s).to eq("/woo/out.iso") end - it "builds an iso with args" do + it "builds an iso with volume_id" do expect(Vagrant::Util::Subprocess).to receive(:execute).with( "hdiutil", "makehybrid", "-hfs", "-iso", "-joliet", "-ov", "-default-volume-name", "cidata", "-o", /.iso/, /\/foo\/src/ ).and_return(double(exit_code: 0)) - output = subject.create_iso(env, "/foo/src", "/woo/out.iso", extra_opts={"default-volume-name" => "cidata"}) + output = subject.create_iso(env, "/foo/src", file_destination: "/woo/out.iso", volume_id: "cidata") expect(output.to_s).to eq("/woo/out.iso") end @@ -51,13 +51,25 @@ describe VagrantPlugins::HostDarwin::Cap::FsISO do "hdiutil", "makehybrid", "-hfs", "-iso", "-joliet", "-ov", "-o", /.iso/, /\/foo\/src/ ).and_return(double(exit_code: 0)) - output = subject.create_iso(env, "/foo/src", "/woo/out_dir") + output = subject.create_iso(env, "/foo/src", file_destination: "/woo/out_dir") expect(output.to_s).to match(/\/woo\/out_dir\/[\w]{6}_vagrant.iso/) end + it "builds an iso when no file destination is given" do + allow(Tempfile).to receive(:new).and_return(file_destination) + allow(file_destination).to receive(:path).and_return(file_destination) + allow(file_destination).to receive(:delete) + expect(Vagrant::Util::Subprocess).to receive(:execute).with( + "hdiutil", "makehybrid", "-hfs", "-iso", "-joliet", "-ov", "-o", /.iso/, /\/foo\/src/ + ).and_return(double(exit_code: 0)) + + output = subject.create_iso(env, "/foo/src") + expect(output.to_s).to eq(file_destination) + end + it "raises an error if iso build failed" do allow(Vagrant::Util::Subprocess).to receive(:execute).with(any_args).and_return(double(stdout: "nope", stderr: "nope", exit_code: 1)) - expect{ subject.create_iso(env, "/foo/src", "/woo/out.iso") }.to raise_error(Vagrant::Errors::ISOBuildFailed) + expect{ subject.create_iso(env, "/foo/src") }.to raise_error(Vagrant::Errors::ISOBuildFailed) end end end From afae8c564029894be473337d08e7adb75f19c025 Mon Sep 17 00:00:00 2001 From: sophia Date: Thu, 2 Jul 2020 09:53:40 -0500 Subject: [PATCH 6/6] Make tests check for output directory creation --- plugins/hosts/darwin/cap/fs_iso.rb | 2 +- test/unit/plugins/hosts/darwin/cap/fs_iso_test.rb | 15 +++++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/plugins/hosts/darwin/cap/fs_iso.rb b/plugins/hosts/darwin/cap/fs_iso.rb index 7f9d819a1..e5e5bd4fd 100644 --- a/plugins/hosts/darwin/cap/fs_iso.rb +++ b/plugins/hosts/darwin/cap/fs_iso.rb @@ -48,7 +48,7 @@ module VagrantPlugins end end # Ensure destination directory is available - FileUtils.mkdir_p(File.dirname(file_destination.to_s)) + FileUtils.mkdir_p(file_destination.dirname) # If the destrination does not exist or there have been changes in the source directory since the last build, then build if !file_destination.exist? || Vagrant::Util::Directory.directory_changed?(source_directory, file_destination.mtime) diff --git a/test/unit/plugins/hosts/darwin/cap/fs_iso_test.rb b/test/unit/plugins/hosts/darwin/cap/fs_iso_test.rb index 199b65cbb..d44a7747c 100644 --- a/test/unit/plugins/hosts/darwin/cap/fs_iso_test.rb +++ b/test/unit/plugins/hosts/darwin/cap/fs_iso_test.rb @@ -25,15 +25,15 @@ describe VagrantPlugins::HostDarwin::Cap::FsISO do before do allow(file_destination).to receive(:nil?).and_return(false) - allow(FileUtils).to receive(:mkdir_p) end it "builds an iso" do expect(Vagrant::Util::Subprocess).to receive(:execute).with( "hdiutil", "makehybrid", "-hfs", "-iso", "-joliet", "-ov", "-o", /.iso/, /\/foo\/src/ ).and_return(double(exit_code: 0)) + expect(FileUtils).to receive(:mkdir_p).with(Pathname.new(file_destination).dirname) - output = subject.create_iso(env, "/foo/src", file_destination: "/woo/out.iso") + output = subject.create_iso(env, "/foo/src", file_destination: file_destination) expect(output.to_s).to eq("/woo/out.iso") end @@ -41,8 +41,9 @@ describe VagrantPlugins::HostDarwin::Cap::FsISO do expect(Vagrant::Util::Subprocess).to receive(:execute).with( "hdiutil", "makehybrid", "-hfs", "-iso", "-joliet", "-ov", "-default-volume-name", "cidata", "-o", /.iso/, /\/foo\/src/ ).and_return(double(exit_code: 0)) + expect(FileUtils).to receive(:mkdir_p).with(Pathname.new(file_destination).dirname) - output = subject.create_iso(env, "/foo/src", file_destination: "/woo/out.iso", volume_id: "cidata") + output = subject.create_iso(env, "/foo/src", file_destination: file_destination, volume_id: "cidata") expect(output.to_s).to eq("/woo/out.iso") end @@ -50,6 +51,8 @@ describe VagrantPlugins::HostDarwin::Cap::FsISO do expect(Vagrant::Util::Subprocess).to receive(:execute).with( "hdiutil", "makehybrid", "-hfs", "-iso", "-joliet", "-ov", "-o", /.iso/, /\/foo\/src/ ).and_return(double(exit_code: 0)) + expect(FileUtils).to receive(:mkdir_p).with(Pathname.new("/woo/out_dir")) + output = subject.create_iso(env, "/foo/src", file_destination: "/woo/out_dir") expect(output.to_s).to match(/\/woo\/out_dir\/[\w]{6}_vagrant.iso/) @@ -62,6 +65,8 @@ describe VagrantPlugins::HostDarwin::Cap::FsISO do expect(Vagrant::Util::Subprocess).to receive(:execute).with( "hdiutil", "makehybrid", "-hfs", "-iso", "-joliet", "-ov", "-o", /.iso/, /\/foo\/src/ ).and_return(double(exit_code: 0)) + # Should create a directory wherever Tempfile creates files by default + expect(FileUtils).to receive(:mkdir_p).with(Pathname.new(file_destination).dirname) output = subject.create_iso(env, "/foo/src") expect(output.to_s).to eq(file_destination) @@ -69,7 +74,9 @@ describe VagrantPlugins::HostDarwin::Cap::FsISO do it "raises an error if iso build failed" do allow(Vagrant::Util::Subprocess).to receive(:execute).with(any_args).and_return(double(stdout: "nope", stderr: "nope", exit_code: 1)) - expect{ subject.create_iso(env, "/foo/src") }.to raise_error(Vagrant::Errors::ISOBuildFailed) + expect(FileUtils).to receive(:mkdir_p).with(Pathname.new(file_destination).dirname) + + expect{ subject.create_iso(env, "/foo/src", file_destination: file_destination) }.to raise_error(Vagrant::Errors::ISOBuildFailed) end end end