Merge pull request #1326 from berkshelf/sethvargo/file_syncer

Do not delete the vendor directory

features/commands/vendor.feature

@@ -94,8 +94,12 @@ Feature: Vendoring cookbooks to a directory
       cookbook 'fake'
       """
     And a directory named "cukebooks"
-    When I run `berks vendor cukebooks`
-    And the exit status should be "VendorError"
+    And a directory named "cukebooks/fake/ponies"
+    And a directory named "cukebooks/existing_cookbook"
+    When I successfully run `berks vendor cukebooks`
+    And the directory "cukebooks/fake" should contain version "1.0.0" of the "fake" cookbook
+    And a directory named "cukebooks/fake/ponies" should not exist
+    And a directory named "cukebooks/existing_cookbook" should not exist
 
   Scenario: vendoring into a nested directory
     Given I have a Berksfile pointing at the local Berkshelf API with:

generator_files/Vagrantfile.erb

@@ -30,7 +30,7 @@ Vagrant.configure(VAGRANTFILE_API_VERSION) do |config|
   # The url from where the 'config.vm.box' box will be fetched if it
   # is not a Vagrant Cloud box and if it doesn't already exist on the
   # user's system.
-  # config.vm.box_url = "<%= berkshelf_config.vagrant.vm.box_url %>"
+  # config.vm.box_url = '<%= berkshelf_config.vagrant.vm.box_url %>'
 
   # Assign this VM to a host-only network IP, allowing you to access it
   # via the IP. Host-only networks can talk to the host machine as well as

lib/berkshelf.rb

@@ -26,6 +26,7 @@ module Mixin
     autoload :Logging, 'berkshelf/mixin/logging'
   end
 
+  autoload :FileSyncer, 'berkshelf/file_syncer'
   autoload :Shell,      'berkshelf/shell'
   autoload :Uploader,   'berkshelf/uploader'
   autoload :Visualizer, 'berkshelf/visualizer'

lib/berkshelf/berksfile.rb

@@ -565,25 +565,15 @@ def package(path)
       outdir
     end
 
-    # Install the Berksfile or Berksfile.lock and then copy the cached cookbooks into
-    # directories within the given destination matching their name.
+    # Install the Berksfile or Berksfile.lock and then sync the cached cookbooks
+    # into directories within the given destination matching their name.
     #
     # @param [String] destination
     #   filepath to vendor cookbooks to
     #
     # @return [String, nil]
     #   the expanded path cookbooks were vendored to or nil if nothing was vendored
     def vendor(destination)
-      destination = File.expand_path(destination)
-
-      if Dir.exist?(destination)
-        raise VendorError, "destination already exists #{destination}. Delete it and try again or use a " +
-          "different filepath."
-      end
-
-      # Ensure the parent directory exists, in case a nested path was given
-      FileUtils.mkdir_p(File.expand_path(File.join(destination, '..')))
-
       scratch          = Berkshelf.mktmpdir
       chefignore       = nil
       cached_cookbooks = install
@@ -592,12 +582,12 @@ def vendor(destination)
 
       cached_cookbooks.each do |cookbook|
         Berkshelf.formatter.vendor(cookbook, destination)
-        cookbook_destination = File.join(scratch, cookbook.cookbook_name, '/')
+        cookbook_destination = File.join(scratch, cookbook.cookbook_name)
         FileUtils.mkdir_p(cookbook_destination)
 
         # Dir.glob does not support backslash as a File separator
         src   = cookbook.path.to_s.gsub('\\', '/')
-        files = Dir.glob(File.join(src, '*'))
+        files = FileSyncer.glob(File.join(src, '*'))
 
         chefignore = Ridley::Chef::Chefignore.new(cookbook.path.to_s) rescue nil
         chefignore.apply!(files) if chefignore
@@ -606,27 +596,26 @@ def vendor(destination)
           cookbook.compile_metadata(cookbook_destination)
         end
 
-        # Don't vendor the raw metadata (metadata.rb). The raw metadata is unecessary for the
-        # client, and this is required until compiled metadata (metadata.json) takes precedence over
-        # raw metadata in the Chef-Client.
-        #
-        # We can change back to including the raw metadata in the future after this has been fixed or
-        # just remove these comments. There is no circumstance that I can currently think of where
-        # raw metadata should ever be read by the client.
-        #
-        # - Jamie
-        #
-        # See the following tickets for more information:
-        #   * https://tickets.opscode.com/browse/CHEF-4811
-        #   * https://tickets.opscode.com/browse/CHEF-4810
-        files.reject! { |file| File.basename(file) == "metadata.rb" }
-
         FileUtils.cp_r(files, cookbook_destination)
       end
 
-      FileUtils.cp(lockfile.filepath, File.join(scratch, Lockfile::DEFAULT_FILENAME))
+      # Don't vendor the raw metadata (metadata.rb). The raw metadata is
+      # unecessary for the client, and this is required until compiled metadata
+      # (metadata.json) takes precedence over raw metadata in the Chef-Client.
+      #
+      # We can change back to including the raw metadata in the future after
+      # this has been fixed or just remove these comments. There is no
+      # circumstance that I can currently think of where raw metadata should
+      # ever be read by the client.
+      #
+      # - Jamie
+      #
+      # See the following tickets for more information:
+      #
+      #   * https://tickets.opscode.com/browse/CHEF-4811
+      #   * https://tickets.opscode.com/browse/CHEF-4810
+      FileSyncer.sync(scratch, destination, exclude: ["**/*/metadata.rb"])
 
-      FileUtils.mv(scratch, destination)
       destination
     end
 

lib/berkshelf/errors.rb

@@ -394,7 +394,6 @@ def to_s
   end
 
   class DuplicateDemand < BerkshelfError; set_status_code(138); end
-  class VendorError < BerkshelfError; set_status_code(139); end
   class LockfileNotFound < BerkshelfError
     set_status_code(140)
 

lib/berkshelf/file_syncer.rb

@@ -0,0 +1,134 @@
+require 'fileutils'
+
+module Berkshelf
+  module FileSyncer
+    extend self
+
+    # Files to be ignored during a directory globbing
+    IGNORED_FILES = %w(. ..).freeze
+
+    #
+    # Glob across the given pattern, accounting for dotfiles, removing Ruby's
+    # dumb idea to include +'.'+ and +'..'+ as entries.
+    #
+    # @param [String] pattern
+    #   the path or glob pattern to get all files from
+    #
+    # @return [Array<String>]
+    #   the list of all files
+    #
+    def glob(pattern)
+      Dir.glob(pattern, File::FNM_DOTMATCH).sort.reject do |file|
+        basename = File.basename(file)
+        IGNORED_FILES.include?(basename)
+      end
+    end
+
+    #
+    # Copy the files from +source+ to +destination+, while removing any files
+    # in +destination+ that are not present in +source+.
+    #
+    # The method accepts an optional +:exclude+ parameter to ignore files and
+    # folders that match the given pattern(s). Note the exclude pattern behaves
+    # on paths relative to the given source. If you want to exclude a nested
+    # directory, you will need to use something like +**/directory+.
+    #
+    # @raise ArgumentError
+    #   if the +source+ parameter is not a directory
+    #
+    # @param [String] source
+    #   the path on disk to sync from
+    # @param [String] destination
+    #   the path on disk to sync to
+    #
+    # @option options [String, Array<String>] :exclude
+    #   a file, folder, or globbing pattern of files to ignore when syncing
+    #
+    # @return [true]
+    #
+    def sync(source, destination, options = {})
+      unless File.directory?(source)
+        raise ArgumentError, "`source' must be a directory, but was a " \
+          "`#{File.ftype(source)}'! If you just want to sync a file, use " \
+          "the `copy' method instead."
+      end
+
+      # Reject any files that match the excludes pattern
+      excludes = Array(options[:exclude]).map do |exclude|
+        [exclude, "#{exclude}/*"]
+      end.flatten
+
+      source_files = glob(File.join(source, '**/*'))
+      source_files = source_files.reject do |source_file|
+        basename = relative_path_for(source_file, source)
+        excludes.any? { |exclude| File.fnmatch?(exclude, basename, File::FNM_DOTMATCH) }
+      end
+
+      # Ensure the destination directory exists
+      FileUtils.mkdir_p(destination) unless File.directory?(destination)
+
+      # Copy over the filtered source files
+      source_files.each do |source_file|
+        relative_path = relative_path_for(source_file, source)
+
+        # Create the parent directory
+        parent = File.join(destination, File.dirname(relative_path))
+        FileUtils.mkdir_p(parent) unless File.directory?(parent)
+
+        case File.ftype(source_file).to_sym
+        when :directory
+          FileUtils.mkdir_p("#{destination}/#{relative_path}")
+        when :link
+          target = File.readlink(source_file)
+
+          Dir.chdir(destination) do
+            FileUtils.ln_sf(target, "#{destination}/#{relative_path}")
+          end
+        when :file
+          FileUtils.cp(source_file, "#{destination}/#{relative_path}")
+        else
+          type = File.ftype(source_file)
+          raise RuntimeError, "Unknown file type: `#{type}' at " \
+            "`#{source_file}'. Failed to sync `#{source_file}' to " \
+            "`#{destination}/#{relative_path}'!"
+        end
+      end
+
+      # Remove any files in the destination that are not in the source files
+      destination_files = glob("#{destination}/**/*")
+
+      # Calculate the relative paths of files so we can compare to the
+      # source.
+      relative_source_files = source_files.map do |file|
+        relative_path_for(file, source)
+      end
+      relative_destination_files = destination_files.map do |file|
+        relative_path_for(file, destination)
+      end
+
+      # Remove any extra files that are present in the destination, but are
+      # not in the source list
+      extra_files = relative_destination_files - relative_source_files
+      extra_files.each do |file|
+        FileUtils.rm_rf(File.join(destination, file))
+      end
+
+      true
+    end
+
+    private
+      #
+      # The relative path of the given +path+ to the +parent+.
+      #
+      # @param [String] path
+      #   the path to get relative with
+      # @param [String] parent
+      #   the parent where the path is contained (hopefully)
+      #
+      # @return [String]
+      #
+      def relative_path_for(path, parent)
+        Pathname.new(path).relative_path_from(Pathname.new(parent)).to_s
+      end
+  end
+end

spec/support/matchers/filepath_matchers.rb

@@ -1,3 +1,4 @@
+require 'rspec/expectations'
 require 'pathname'
 
 RSpec::Matchers.define :be_relative_path do
@@ -17,3 +18,38 @@
     "Expected '#{given}' to not be a relative path but got an absolute path."
   end
 end
+
+# expect('/path/to/directory').to be_a_directory
+RSpec::Matchers.define :be_a_directory do
+  match do |actual|
+    File.directory?(actual)
+  end
+end
+
+# expect('/path/to/directory').to be_a_file
+RSpec::Matchers.define :be_a_file do
+  match do |actual|
+    File.file?(actual)
+  end
+end
+
+# expect('/path/to/directory').to be_a_symlink
+RSpec::Matchers.define :be_a_symlink do
+  match do |actual|
+    File.symlink?(actual)
+  end
+end
+
+# expect('/path/to/directory').to be_a_symlink_to
+RSpec::Matchers.define :be_a_symlink_to do |path|
+  match do |actual|
+    File.symlink?(actual) && File.readlink(actual) == path
+  end
+end
+
+# expect('/path/to/file').to be_an_executable
+RSpec::Matchers.define :be_an_executable do
+  match do |actual|
+    File.executable?(actual)
+  end
+end