berkshelf / ridley

A reliable Chef API client with a clean syntax
Other
231 stars 85 forks source link

Ignore .DS_Store files during upload #284

Closed sethvargo closed 8 years ago

sethvargo commented 9 years ago

Migrated from https://github.com/berkshelf/berkshelf/issues/706, Ridley should ignore .DS_Store files from the cookbook. It causes strange problems during upload as noted in that issue.

tas50 commented 9 years ago

I will send an Oregon gift basket to anyone that fixes this bug. One of the more annoying bugs out there for my workflow

florianb commented 9 years ago

I am really not sure, if this bug report belongs really to ridley - i posted the same comment in https://github.com/berkshelf/berkshelf/issues/706 because i assume that berkshelf is collecting the data for uploading them to the chef-server.

Unfortunately this problem was not solved for me (berks version 3.2.3) and it also had nothing to do with any .DS_Store-files.

My error was caused by having files placed directly in the files/template-folder.

I was able to solve the problem by moving the designated files into an additional subdirectory:

Wrong

files/
└── distribute.sh

lead to: E, [2015-03-16T17:57:59.138178 #71706] ERROR -- : Ridley::Errors::HTTPBadRequest: {"error":["Invalid element in array value of 'files'."]}

Right

files/
└── postgres
    └── distribute.sh
grelkin commented 9 years ago

I guess that the problem may be about that ridley and knife are ignoring files in a little bit different way: ridley uses the buff-ignore library to determine if a file should be ignored, and knife uses Chef::Cookbook::Chefignore class.

Ridley chef/cookbook.rb and chef/chefignore.rb

# chef/cookbook.rb
308: def ignored?(file)
309:   !!chefignore && chefignore.ignored?(file)
310: end

# chef/chefignore.rb
004: class Chefignore < Buff::Ignore::IgnoreFile

Knife chef/cookbook/cookbook_version_loader.rb

243: def remove_ignored_files
244:   cookbook_settings.each_value do |file_list|
245:     file_list.reject! do |relative_path, full_path|
246:       chefignore.ignored?(relative_path)
247:     end
248:   end
249: end

Both buff-ignore and Chef::Cookbook::Chefignore are using File.fnmatch? method to check file name against chefignore pattern:

Buff-Ignore ignore/ignore_file.rb

073: def ignored?(filename)
074:   base = File.expand_path(options[:base] || File.dirname(filepath))
075:   basename = filename.sub(base + File::SEPARATOR, '')
076: 
077:   ignores.any? { |ignore| File.fnmatch?(ignore, basename) }
078: end

Knife chef/cookbook/chefignore.rb

042: def ignored?(file_name)
043:   @ignores.any? {|glob| File.fnmatch?(glob, file_name)}
044: end

And both are trying to use filename relative to the cookbook folder. But since buff-ignore created without the options hash: Ridley chef/cookbook.rb

090: @chefignore    = Ridley::Chef::Chefignore.new(@path) rescue nil

It will use File.dirname(filepath) which will be evaluated not to the cookbooks folder but to the folder one level up.

Thus, for example, if one has chefignore.rb with the following content

README.*

then ridley will use cookboook/README.md as filename to define if the file should be ignored, and knife will use README.md. And, that's why in this case the readme will be ignored by knife but not by ridley and berkshelf.

Probably this can solve the bug: https://github.com/sethvargo/buff-ignore/pull/2

jhsubscribe commented 8 years ago

@sethvargo - When you get time, I think this just needs a new release of the buff-ignore gem so that Berks will receive this fix? (you merged Grelkins PR back in March) (CAVEAT: I may be confused about how github/gems etc work)

tas50 commented 8 years ago

Closing this since https://github.com/sethvargo/buff-ignore/pull/3 releases buff-ignore. Thanks for pointing that out @jhsubscribe