Merge pull request #1324 from nanoc/gh-1313-fix-exclusion-handling-in…

…-pruner

Really ignore output_dir when checking for excludes in pruner

nanoc/lib/nanoc/base/services/pruner.rb

@@ -6,6 +6,8 @@ module Nanoc
   #
   # @api private
   class Pruner
+    include Nanoc::Int::ContractsSupport
+
     # @param [Nanoc::Int::Configuration] config
     #
     # @param [Nanoc::Int::ItemRepRepo] reps
@@ -22,9 +24,6 @@ def initialize(config, reps, dry_run: false, exclude: [])
       @exclude = Set.new(exclude)
     end
 
-    # Prunes all output files not managed by Nanoc.
-    #
-    # @return [void]
     def run
       return unless File.directory?(@config[:output_dir])
 
@@ -35,18 +34,13 @@ def run
       remove_empty_directories(present_dirs)
     end
 
-    def exclude?(component)
-      @exclude.include?(component)
-    end
-
-    # @param [String] filename The filename to check
-    #
-    # @return [Boolean] true if the given file is excluded, false otherwise
+    contract String => C::Bool
     def filename_excluded?(filename)
       pathname = Pathname.new(strip_output_dir(filename))
       @exclude.any? { |e| pathname_components(pathname).include?(e) }
     end
 
+    contract String => String
     def strip_output_dir(filename)
       if filename.start_with?(@config[:output_dir])
         filename[@config[:output_dir].size..-1]
@@ -55,6 +49,7 @@ def strip_output_dir(filename)
       end
     end
 
+    contract Pathname => C::ArrayOf[String]
     def pathname_components(pathname)
       components = []
       tmp = pathname
@@ -67,22 +62,27 @@ def pathname_components(pathname)
       components.reverse
     end
 
+    contract C::ArrayOf[String], C::ArrayOf[String] => self
     # @api private
     def remove_stray_files(present_files, compiled_files)
       (present_files - compiled_files).each do |f|
-        delete_file(f) unless exclude?(f)
+        delete_file(f) unless filename_excluded?(f)
       end
+      self
     end
 
+    contract C::ArrayOf[String] => self
     # @api private
     def remove_empty_directories(present_dirs)
       present_dirs.reverse_each do |dir|
         next if Dir.foreach(dir) { |n| break true if n !~ /\A\.\.?\z/ }
-        next if exclude?(dir)
+        next if filename_excluded?(dir)
         delete_dir(dir)
       end
+      self
     end
 
+    contract String => C::ArrayOf[C::ArrayOf[String]]
     # @api private
     def files_and_dirs_in(dir)
       present_files = []
@@ -91,15 +91,13 @@ def files_and_dirs_in(dir)
       expanded_dir = File.expand_path(dir)
 
       Find.find(dir) do |f|
-        basename = File.basename(f)
-
         case File.ftype(f)
         when 'file'
-          unless exclude?(basename)
+          unless filename_excluded?(f)
             present_files << f
           end
         when 'directory'
-          if exclude?(basename)
+          if filename_excluded?(f)
             Find.prune
           elsif expanded_dir != File.expand_path(f)
             present_dirs << f

nanoc/spec/nanoc/base/services/pruner_spec.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-describe Nanoc::Pruner do
+describe Nanoc::Pruner, stdio: true do
   subject(:pruner) { described_class.new(config, reps, dry_run: dry_run, exclude: exclude) }
 
   let(:config) { Nanoc::Int::Configuration.new({}).with_defaults }
@@ -50,6 +50,268 @@
     end
   end
 
+  describe '#run' do
+    subject { pruner.run }
+
+    describe 'it removes stray files' do
+      let(:present_files) do
+        [
+          'output/foo.html',
+          'output/foo.txt',
+          'output/bar.html',
+          'output/foo/bar.html',
+          'output/foo/bar.txt',
+          'output/output/asdf.txt',
+        ]
+      end
+
+      let(:reps) do
+        Nanoc::Int::ItemRepRepo.new.tap do |reps|
+          reps << Nanoc::Int::ItemRep.new(item, :a).tap do |rep|
+            rep.raw_paths = { last: ['output/foo.html'] }
+          end
+
+          reps << Nanoc::Int::ItemRep.new(item, :b).tap do |rep|
+            rep.raw_paths = { last: ['output/bar.html'] }
+          end
+
+          reps << Nanoc::Int::ItemRep.new(item, :c).tap do |rep|
+            rep.raw_paths = { last: ['output/foo/bar.html'] }
+          end
+        end
+      end
+
+      before do
+        present_files.each do |fn|
+          FileUtils.mkdir_p(File.dirname(fn))
+          File.write(fn, 'asdf')
+        end
+      end
+
+      context 'nothing excluded' do
+        it 'removes /foo.txt' do
+          expect { subject }
+            .to change { File.file?('output/foo.txt') }
+            .from(true)
+            .to(false)
+        end
+
+        it 'removes /foo/bar.txt' do
+          expect { subject }
+            .to change { File.file?('output/foo/bar.txt') }
+            .from(true)
+            .to(false)
+        end
+
+        it 'removes /output/asdf.txt' do
+          expect { subject }
+            .to change { File.file?('output/output/asdf.txt') }
+            .from(true)
+            .to(false)
+        end
+      end
+
+      context 'foo excluded' do
+        let(:exclude) { ['foo'] }
+
+        it 'removes /foo.txt' do
+          expect { subject }
+            .to change { File.file?('output/foo.txt') }
+            .from(true)
+            .to(false)
+        end
+
+        it 'keeps /foo/bar.txt' do
+          expect { subject }
+            .not_to change { File.file?('output/foo/bar.txt') }
+            .from(true)
+        end
+
+        it 'removes /output/asdf.txt' do
+          expect { subject }
+            .to change { File.file?('output/output/asdf.txt') }
+            .from(true)
+            .to(false)
+        end
+      end
+
+      context 'output excluded' do
+        let(:exclude) { ['output'] }
+
+        it 'removes /foo.txt' do
+          expect { subject }
+            .to change { File.file?('output/foo.txt') }
+            .from(true)
+            .to(false)
+        end
+
+        it 'removes /foo/bar.txt' do
+          expect { subject }
+            .to change { File.file?('output/foo/bar.txt') }
+            .from(true)
+            .to(false)
+        end
+
+        it 'keeps /output/asdf.txt' do
+          expect { subject }
+            .not_to change { File.file?('output/output/asdf.txt') }
+            .from(true)
+        end
+      end
+    end
+
+    describe 'it removes empty directories' do
+      let(:present_dirs) do
+        [
+          'output/.foo',
+          'output/foo',
+          'output/foo/bar',
+          'output/bar',
+          'output/output',
+          'output/output/asdf',
+        ]
+      end
+
+      before do
+        present_dirs.each do |fn|
+          FileUtils.mkdir_p(fn)
+        end
+      end
+
+      context 'nothing excluded' do
+        it 'removes /.foo' do
+          expect { subject }
+            .to change { File.directory?('output/.foo') }
+            .from(true)
+            .to(false)
+        end
+
+        it 'removes /foo' do
+          expect { subject }
+            .to change { File.directory?('output/foo') }
+            .from(true)
+            .to(false)
+        end
+
+        it 'removes /foo/bar' do
+          expect { subject }
+            .to change { File.directory?('output/foo/bar') }
+            .from(true)
+            .to(false)
+        end
+
+        it 'removes /bar' do
+          expect { subject }
+            .to change { File.directory?('output/bar') }
+            .from(true)
+            .to(false)
+        end
+
+        it 'removes /output' do
+          expect { subject }
+            .to change { File.directory?('output/output') }
+            .from(true)
+            .to(false)
+        end
+
+        it 'removes /output/asdf' do
+          expect { subject }
+            .to change { File.directory?('output/output/asdf') }
+            .from(true)
+            .to(false)
+        end
+      end
+
+      context 'foo excluded' do
+        let(:exclude) { ['foo'] }
+
+        it 'removes /.foo' do
+          expect { subject }
+            .to change { File.directory?('output/.foo') }
+            .from(true)
+            .to(false)
+        end
+
+        it 'removes /bar' do
+          expect { subject }
+            .to change { File.directory?('output/bar') }
+            .from(true)
+            .to(false)
+        end
+
+        it 'keeps /foo' do
+          expect { subject }
+            .not_to change { File.directory?('output/foo') }
+            .from(true)
+        end
+
+        it 'keeps /foo/bar' do
+          expect { subject }
+            .not_to change { File.directory?('output/foo/bar') }
+            .from(true)
+        end
+
+        it 'removes /output' do
+          expect { subject }
+            .to change { File.directory?('output/output') }
+            .from(true)
+            .to(false)
+        end
+
+        it 'removes /output/asdf' do
+          expect { subject }
+            .to change { File.directory?('output/output/asdf') }
+            .from(true)
+            .to(false)
+        end
+      end
+
+      context 'output excluded' do
+        let(:exclude) { ['output'] }
+
+        it 'removes /.foo' do
+          expect { subject }
+            .to change { File.directory?('output/.foo') }
+            .from(true)
+            .to(false)
+        end
+
+        it 'removes /bar' do
+          expect { subject }
+            .to change { File.directory?('output/bar') }
+            .from(true)
+            .to(false)
+        end
+
+        it 'removes /foo' do
+          expect { subject }
+            .to change { File.directory?('output/foo') }
+            .from(true)
+            .to(false)
+        end
+
+        it 'removes /foo/bar' do
+          expect { subject }
+            .to change { File.directory?('output/foo/bar') }
+            .from(true)
+            .to(false)
+        end
+
+        it 'keeps /output' do
+          expect { subject }
+            .not_to change { File.directory?('output/output') }
+            .from(true)
+        end
+
+        it 'keeps /output/asdf' do
+          expect { subject }
+            .not_to change { File.directory?('output/output/asdf') }
+            .from(true)
+        end
+      end
+    end
+  end
+
   describe '#pathname_components' do
     subject { pruner.pathname_components(pathname) }
 

nanoc/spec/nanoc/regressions/gh_1313_spec.rb

@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+describe 'GH-1313', site: true, stdio: true do
+  before do
+    File.write('nanoc.yaml', <<~CONFIG)
+      output_dir: build/bin/web/bin
+      prune:
+        auto_prune: true
+        exclude:
+          - bin
+    CONFIG
+  end
+
+  before do
+    FileUtils.mkdir_p('build/bin/web/bin')
+    File.write('build/bin/web/bin/should-be-pruned', 'asdf')
+  end
+
+  example do
+    expect { Nanoc::CLI.run(%w[compile]) }
+      .to change { File.file?('build/bin/web/bin/should-be-pruned') }
+      .from(true)
+      .to(false)
+  end
+end