Don't display/diff empty/ignored cookbook directories

Also, add many chefignore tests

4 files changed, 377 insertions, 62 deletions

diff --git a/lib/chef/chef_fs/file_system/chef_repository_file_system_entry.rb b/lib/chef/chef_fs/file_system/chef_repository_file_system_entry.rb
index 3017a90269..b29e18b79c 100644
--- a/lib/chef/chef_fs/file_system/chef_repository_file_system_entry.rb
+++ b/lib/chef/chef_fs/file_system/chef_repository_file_system_entry.rb
@@ -17,7 +17,6 @@
 #
 
 require 'chef/chef_fs/file_system/file_system_entry'
-require 'chef/cookbook/chefignore'
 require 'chef/cookbook/cookbook_version_loader'
 require 'chef/node'
 require 'chef/role'
@@ -31,23 +30,16 @@ class Chef
       # ChefRepositoryFileSystemEntry works just like FileSystemEntry,
       # except can inflate Chef objects
       class ChefRepositoryFileSystemEntry < FileSystemEntry
-        def initialize(name, parent, file_path = nil)
+        def initialize(name, parent, file_path = nil, ignore_empty_directories = nil, chefignore = nil)
           super(name, parent, file_path)
-          # Load /cookbooks/chefignore
-          if path == '/cookbooks'
-            @chefignore = Chef::Cookbook::Chefignore.new(self.file_path)
-            @ignore_empty_directories = true
-          # If we are a cookbook or a cookbook subdirectory, empty directories
-          # underneath us are ignored (since they cannot be uploaded)
-          elsif parent && parent.ignore_empty_directories?
-            @ignore_empty_directories = true
-          end
         end
 
-        attr_reader :chefignore
+        def chefignore
+          nil
+        end
 
         def ignore_empty_directories?
-          @ignore_empty_directories
+          parent.ignore_empty_directories?
         end
 
         def chef_object
@@ -69,43 +61,36 @@ class Chef
         def children
           @children ||=
             Dir.entries(file_path).
-                select { |entry| entry != '.' && entry != '..' && !ignored?(entry) }.
-                map { |entry| ChefRepositoryFileSystemEntry.new(entry, self) }
+                select { |entry| entry != '.' && entry != '..' }.
+                map { |entry| ChefRepositoryFileSystemEntry.new(entry, self) }.
+                select { |entry| !ignored?(entry) }
         end
 
-        attr_reader :chefignore
-
         private
 
-        def is_cookbooks_dir?
-          # We check name first because it's a faster fail than path
-          path == "/cookbooks"
-        end
-
-        def ignored?(child_name)
-          # empty directories inside a cookbook are ignored
-          if ignore_empty_directories?
-            child_path = PathUtils.join(file_path, child_name)
-            if File.directory?(child_path) && Dir.entries(child_path) == [ '.', '..' ]
+        def ignored?(child_entry)
+          if child_entry.dir?
+            # empty cookbooks and cookbook directories are ignored
+            if ignore_empty_directories? && child_entry.children.size == 0
               return true
             end
-          end
-
-          ignorer = parent
-          begin
-            if ignorer.chefignore
-              # Grab the path from entry to child
-              path_to_child = child_name
-              child = self
-              while child.parent != ignorer
-                path_to_child = PathUtils.join(child.name, path_to_child)
-                child = child.parent
+          else
+            ignorer = parent
+            begin
+              if ignorer.chefignore
+                # Grab the path from entry to child
+                path_to_child = child_entry.name
+                child = self
+                while child.parent != ignorer
+                  path_to_child = PathUtils.join(child.name, path_to_child)
+                  child = child.parent
+                end
+                # Check whether that relative path is ignored
+                return ignorer.chefignore.ignored?(path_to_child)
               end
-              # Check whether that relative path is ignored
-              return ignorer.chefignore.ignored?(path_to_child)
-            end
-            ignorer = ignorer.parent
-          end while ignorer
+              ignorer = ignorer.parent
+            end while ignorer
+          end
         end
 
       end
diff --git a/lib/chef/chef_fs/file_system/chef_repository_file_system_root_dir.rb b/lib/chef/chef_fs/file_system/chef_repository_file_system_root_dir.rb
index 4ca674c627..0855a18f1d 100644
--- a/lib/chef/chef_fs/file_system/chef_repository_file_system_root_dir.rb
+++ b/lib/chef/chef_fs/file_system/chef_repository_file_system_root_dir.rb
@@ -18,6 +18,7 @@
 
 require 'chef/chef_fs/file_system/base_fs_dir'
 require 'chef/chef_fs/file_system/chef_repository_file_system_entry'
+require 'chef/chef_fs/file_system/chef_repository_file_system_cookbooks_dir'
 require 'chef/chef_fs/file_system/multiplexed_dir'
 
 class Chef
@@ -63,7 +64,11 @@ class Chef
           if paths.size == 0
             return nil
           end
-          dirs = paths.map { |path| ChefRepositoryFileSystemEntry.new(name, self, path) }
+          if name == 'cookbooks'
+            dirs = paths.map { |path| ChefRepositoryFileSystemCookbooksDir.new(name, self, path) }
+          else
+            dirs = paths.map { |path| ChefRepositoryFileSystemEntry.new(name, self, path) }
+          end
           MultiplexedDir.new(dirs)
         end
       end
diff --git a/spec/integration/knife/chef_repository_file_system_spec.rb b/spec/integration/knife/chef_repository_file_system_spec.rb
index decf53d25e..e815f2248b 100644
--- a/spec/integration/knife/chef_repository_file_system_spec.rb
+++ b/spec/integration/knife/chef_repository_file_system_spec.rb
@@ -53,13 +53,11 @@ cookbooks
       directory 'cookbooks/cookbook1/recipes'
 
       it "knife list --local -R / does not return it" do
-        pending 'figure out if knife cookbook upload -a ignores it too' do
-          knife('list', '--local', '-R', '/').stdout.should == "/:
+        knife('list', '--local', '-R', '/').stdout.should == "/:
 cookbooks
 
 /cookbooks:
 "
-        end
       end
     end
 
@@ -90,13 +88,11 @@ x.txt
       directory 'cookbooks/cookbook1/templates/default'
 
       it "knife list --local -R / does not return it" do
-        pending 'figure out if knife cookbook upload -a ignores it too' do
-          knife('list', '--local', '-R', '/').stdout.should == "/:
+        knife('list', '--local', '-R', '/').stdout.should == "/:
 cookbooks
 
 /cookbooks:
 "
-        end
       end
     end
 
@@ -106,8 +102,7 @@ cookbooks
       directory 'cookbooks/cookbook1/files/default'
 
       it "knife list --local -R / does not return the empty ones" do
-        pending 'exclude directories with only empty children' do
-          knife('list', '--local', '-R', '/').stdout.should == "/:
+        knife('list', '--local', '-R', '/').stdout.should == "/:
 cookbooks
 
 /cookbooks:
@@ -116,13 +111,12 @@ cookbook1
 /cookbooks/cookbook1:
 templates
 
-/cookbooks/cookbook1/:
+/cookbooks/cookbook1/templates:
 default
 
 /cookbooks/cookbook1/templates/default:
 x.txt
 "
-        end
       end
     end
 
@@ -296,9 +290,338 @@ e.json
         end
       end
     end
+
+    when_the_repository "has a file in cookbooks/" do
+      file 'cookbooks/file', ''
+      it 'does not show up in list -R' do
+        pending "don't show files when only directories are allowed" do
+          knife('list', '--local', '-R', '/').stdout.should == "/:
+cookbooks
+
+/cookbooks:
+"
+        end
+      end
+    end
+
+    when_the_repository "has a file in data_bags/" do
+      file 'data_bags/file', ''
+      it 'does not show up in list -R' do
+        pending "don't show files when only directories are allowed" do
+          knife('list', '--local', '-R', '/').stdout.should == "/:
+data_bags
+
+/data_bags:
+"
+        end
+      end
+    end
+  end
+
+  context 'chefignore tests' do
+    when_the_repository "has lots of stuff in it" do
+      file 'roles/x.json', {}
+      file 'environments/x.json', {}
+      file 'data_bags/bag1/x.json', {}
+      file 'cookbooks/cookbook1/x.json', {}
+
+      context "and has a chefignore everywhere except cookbooks" do
+        chefignore = "x.json\nroles/x.json\nenvironments/x.json\ndata_bags/bag1/x.json\nbag1/x.json\ncookbooks/cookbook1/x.json\ncookbook1/x.json\n"
+        file 'chefignore', chefignore
+        file 'roles/chefignore', chefignore
+        file 'environments/chefignore', chefignore
+        file 'data_bags/chefignore', chefignore
+        file 'data_bags/bag1/chefignore', chefignore
+        file 'cookbooks/cookbook1/chefignore', chefignore
+
+        it 'nothing is ignored' do
+          # NOTE: many of the "chefignore" files should probably not show up
+          # themselves, but we have other tests that talk about that
+          knife('list', '--local', '-R', '/').stdout.should == "/:
+cookbooks
+data_bags
+environments
+roles
+
+/cookbooks:
+cookbook1
+
+/cookbooks/cookbook1:
+chefignore
+x.json
+
+/data_bags:
+bag1
+chefignore
+
+/data_bags/bag1:
+chefignore
+x.json
+
+/environments:
+chefignore
+x.json
+
+/roles:
+chefignore
+x.json
+"
+        end
+      end
+    end
+
+    when_the_repository 'has a cookbook with only chefignored files' do
+      file 'cookbooks/cookbook1/templates/default/x.rb', ''
+      file 'cookbooks/cookbook1/libraries/x.rb', ''
+      file 'cookbooks/chefignore', "libraries/x.rb\ntemplates/default/x.rb\n"
+
+      it 'the cookbook is not listed' do
+        knife('list', '--local', '-R', '/').stdout.should == "/:
+cookbooks
+
+/cookbooks:
+chefignore
+"
+      end
+    end
+
+    when_the_repository "has multiple cookbooks" do
+      file 'cookbooks/cookbook1/x.json', {}
+      file 'cookbooks/cookbook1/y.json', {}
+      file 'cookbooks/cookbook2/x.json', {}
+      file 'cookbooks/cookbook2/y.json', {}
+
+      context 'and has a chefignore with filenames' do
+        file 'cookbooks/chefignore', "x.json\n"
+
+        it 'matching files and directories get ignored in all cookbooks' do
+          knife('list', '--local', '-R', '/').stdout.should == "/:
+cookbooks
+
+/cookbooks:
+chefignore
+cookbook1
+cookbook2
+
+/cookbooks/cookbook1:
+y.json
+
+/cookbooks/cookbook2:
+y.json
+"
+        end
+      end
+
+      context "and has a chefignore with wildcards" do
+        file 'cookbooks/chefignore', "x.*\n"
+        file 'cookbooks/cookbook1/x.rb', ''
+
+        it 'matching files and directories get ignored in all cookbooks' do
+          knife('list', '--local', '-R', '/').stdout.should == "/:
+cookbooks
+
+/cookbooks:
+chefignore
+cookbook1
+cookbook2
+
+/cookbooks/cookbook1:
+y.json
+
+/cookbooks/cookbook2:
+y.json
+"
+        end
+      end
+
+      context "and has a chefignore with relative paths" do
+        file 'cookbooks/cookbook1/recipes/x.rb', ''
+        file 'cookbooks/cookbook2/recipes/y.rb', ''
+        file 'cookbooks/chefignore', "recipes/x.rb\n"
+
+        it 'matching directories get ignored' do
+          knife('list', '--local', '-R', '/').stdout.should == "/:
+cookbooks
+
+/cookbooks:
+chefignore
+cookbook1
+cookbook2
+
+/cookbooks/cookbook1:
+x.json
+y.json
+
+/cookbooks/cookbook2:
+recipes
+x.json
+y.json
+
+/cookbooks/cookbook2/recipes:
+y.rb
+"
+        end
+      end
+
+      context "and has a chefignore with subdirectories" do
+        file 'cookbooks/cookbook1/recipes/y.rb', ''
+        file 'cookbooks/chefignore', "recipes\n"
+
+        it 'matching directories do NOT get ignored' do
+          knife('list', '--local', '-R', '/').stdout.should == "/:
+cookbooks
+
+/cookbooks:
+chefignore
+cookbook1
+cookbook2
+
+/cookbooks/cookbook1:
+recipes
+x.json
+y.json
+
+/cookbooks/cookbook1/recipes:
+y.rb
+
+/cookbooks/cookbook2:
+x.json
+y.json
+"
+        end
+      end
+
+      context "and has a chefignore that ignores all files in a subdirectory" do
+        file 'cookbooks/cookbook1/templates/default/x.rb', ''
+        file 'cookbooks/cookbook1/libraries/x.rb', ''
+        file 'cookbooks/chefignore', "libraries/x.rb\ntemplates/default/x.rb\n"
+
+        it 'ignores the subdirectory entirely' do
+          knife('list', '--local', '-R', '/').stdout.should == "/:
+cookbooks
+
+/cookbooks:
+chefignore
+cookbook1
+cookbook2
+
+/cookbooks/cookbook1:
+x.json
+y.json
+
+/cookbooks/cookbook2:
+x.json
+y.json
+"
+        end
+      end
+
+      context "and has an empty chefignore" do
+        file 'cookbooks/chefignore', "\n"
+
+        it 'nothing is ignored' do
+          knife('list', '--local', '-R', '/').stdout.should == "/:
+cookbooks
+
+/cookbooks:
+chefignore
+cookbook1
+cookbook2
+
+/cookbooks/cookbook1:
+x.json
+y.json
+
+/cookbooks/cookbook2:
+x.json
+y.json
+"
+        end
+      end
+
+      context "and has a chefignore with comments and empty lines" do
+        file 'cookbooks/chefignore', "\n\n # blah\n#\nx.json\n\n"
+
+        it 'matching files and directories get ignored in all cookbooks' do
+          knife('list', '--local', '-R', '/').stdout.should == "/:
+cookbooks
+
+/cookbooks:
+chefignore
+cookbook1
+cookbook2
+
+/cookbooks/cookbook1:
+y.json
+
+/cookbooks/cookbook2:
+y.json
+"
+        end
+      end
+    end
+
+    when_the_repository "has multiple cookbook paths" do
+      before :each do
+        Chef::Config.cookbook_path = [
+          File.join(Chef::Config.chef_repo_path, 'cookbooks1'),
+          File.join(Chef::Config.chef_repo_path, 'cookbooks2')
+        ]
+      end
+
+      file 'cookbooks1/mycookbook/metadata.rb', ''
+      file 'cookbooks1/mycookbook/x.json', {}
+      file 'cookbooks2/yourcookbook/metadata.rb', ''
+      file 'cookbooks2/yourcookbook/x.json', ''
+
+      context "and multiple chefignores" do
+        file 'cookbooks1/chefignore', "metadata.rb\n"
+        file 'cookbooks2/chefignore', "x.json\n"
+        it "chefignores apply only to the directories they are in" do
+          knife('list', '--local', '-R', '/').stdout.should == "/:
+cookbooks
+
+/cookbooks:
+chefignore
+mycookbook
+yourcookbook
+
+/cookbooks/mycookbook:
+x.json
+
+/cookbooks/yourcookbook:
+metadata.rb
+"
+        end
+
+        context "and conflicting cookbooks" do
+          file 'cookbooks1/yourcookbook/metadata.rb', ''
+          file 'cookbooks1/yourcookbook/x.json', ''
+          file 'cookbooks1/yourcookbook/onlyincookbooks1.rb', ''
+          file 'cookbooks2/yourcookbook/onlyincookbooks2.rb', ''
+
+          it "chefignores apply only to the winning cookbook" do
+            knife('list', '--local', '-R', '/').stdout.should == "/:
+cookbooks
+
+/cookbooks:
+chefignore
+mycookbook
+yourcookbook
+
+/cookbooks/mycookbook:
+x.json
+
+/cookbooks/yourcookbook:
+onlyincookbooks1.rb
+x.json
+"
+          end
+        end
+      end
+    end
   end
 
-  # TODO chefignore
   # TODO alternate repo_path / *_path
   # TODO multiple *_path
   # TODO nonexistent repo_path / *_path
diff --git a/spec/support/shared/integration/integration_helper.rb b/spec/support/shared/integration/integration_helper.rb
index fcbacf7c8f..fde763c5e4 100644
--- a/spec/support/shared/integration/integration_helper.rb
+++ b/spec/support/shared/integration/integration_helper.rb
@@ -40,13 +40,15 @@ module IntegrationSupport
 
       after :each do
         if @repository_dir
-          Chef::Config.chef_repo_path = @old_chef_repo_path
-          Chef::Config.cookbook_path = @old_cookbook_path
-          FileUtils.remove_entry_secure(@repository_dir)
-
-          @old_chef_repo_path = nil
-          @old_cookbook_path = nil
-          @repository_dir = nil
+          begin
+            Chef::Config.chef_repo_path = @old_chef_repo_path
+            Chef::Config.cookbook_path = @old_cookbook_path
+            FileUtils.remove_entry_secure(@repository_dir)
+          ensure
+            @old_chef_repo_path = nil
+            @old_cookbook_path = nil
+            @repository_dir = nil
+          end
         end
       end