Refactor out shared utility

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

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"
@@ -55,22 +51,32 @@ module VagrantPlugins
               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

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

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

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