Fixing #2296 - overlapping recursions work again

This fixes the behaviour when you have file recursions
that overlap - we again correctly use the most
specific information.

It's still a bit expensive when you do this, but
at least it behaves correctly, and it should be
a rare circumstance.

Signed-off-by: Luke Kanies <luke@madstop.com>

5 files changed, 52 insertions, 108 deletions

diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb
index 4efd901e5..79ab8eb95 100644
--- a/lib/puppet/type/file.rb
+++ b/lib/puppet/type/file.rb
@@ -580,7 +580,35 @@ module Puppet
             # remote system.
             mark_children_for_purging(children) if self.purge?
 
-            return children.values.sort { |a, b| a[:path] <=> b[:path] }
+            result = children.values.sort { |a, b| a[:path] <=> b[:path] }
+            remove_less_specific_files(result)
+        end
+
+        # This is to fix bug #2296, where two files recurse over the same
+        # set of files.  It's a rare case, and when it does happen you're
+        # not likely to have many actual conflicts, which is good, because
+        # this is a pretty inefficient implementation.
+        def remove_less_specific_files(files)
+            # We sort the paths so we can short-circuit some tests.
+            mypath = self[:path]
+            other_paths = catalog.vertices.find_all do |r|
+                r.is_a?(self.class) and r[:path][0..(mypath.length - 1)] == mypath
+            end.collect { |r| r[:path] }.sort { |a, b| a.length <=> b.length } - [self[:path]]
+
+            return files if other_paths.empty?
+
+            remove = []
+            files.each do |file|
+                path = file[:path]
+                other_paths.each do |p|
+                    if path[0..(p.length - 1)] == p
+                        remove << file
+                        break
+                    end
+                end
+            end
+
+            files - remove
         end
 
         # A simple method for determining whether we should be recursing.
diff --git a/spec/integration/type/file.rb b/spec/integration/type/file.rb
index cced1ed1c..3b03d53e3 100755
--- a/spec/integration/type/file.rb
+++ b/spec/integration/type/file.rb
@@ -115,6 +115,27 @@ describe Puppet::Type.type(:file) do
                 File.lstat(newpath).ftype.should == "file"
             end
         end
+
+        it "should not recursively manage files managed by a more specific explicit file" do
+            dir = tmpfile("file_source_integration_source")
+
+            subdir = File.join(dir, "subdir")
+            file = File.join(subdir, "file")
+
+            FileUtils.mkdir_p(subdir)
+            File.open(file, "w") { |f| f.puts "" }
+
+            base = Puppet::Type::File.new(:name => dir, :recurse => true, :backup => false, :mode => "755")
+            sub = Puppet::Type::File.new(:name => subdir, :recurse => true, :backup => false, :mode => "644")
+
+            @catalog = Puppet::Resource::Catalog.new
+            @catalog.add_resource base
+            @catalog.add_resource sub
+
+            @catalog.apply
+
+            (File.stat(file).mode & 007777).should == 0644
+        end
     end
 
     describe "when generating resources" do
diff --git a/spec/unit/type/file.rb b/spec/unit/type/file.rb
index 627cac41a..6ec86e7a1 100755
--- a/spec/unit/type/file.rb
+++ b/spec/unit/type/file.rb
@@ -12,8 +12,7 @@ describe Puppet::Type.type(:file) do
         @path = pathname
         @file = Puppet::Type::File.new(:name => @path)
 
-        @catalog = mock 'catalog'
-        @catalog.stub_everything
+        @catalog = Puppet::Resource::Catalog.new
         @file.catalog = @catalog
     end
 
@@ -108,7 +107,6 @@ describe Puppet::Type.type(:file) do
                 :path => @link,
                 :mode => "755"
             )
-            @catalog = Puppet::Resource::Catalog.new
             @catalog.add_resource @resource
         end
 
@@ -513,9 +511,6 @@ describe Puppet::Type.type(:file) do
 
     describe "when returning resources with :eval_generate" do
         before do
-            @catalog = mock 'catalog'
-            @catalog.stub_everything
-
             @graph = stub 'graph', :add_edge => nil
             @catalog.stubs(:relationship_graph).returns @graph
 
diff --git a/test/other/overrides.rb b/test/other/overrides.rb
deleted file mode 100755
index 22c3d2ac1..000000000
--- a/test/other/overrides.rb
+++ /dev/null
@@ -1,101 +0,0 @@
-#!/usr/bin/env ruby
-
-require File.dirname(__FILE__) + '/../lib/puppettest'
-
-require 'puppet'
-require 'puppettest'
-
-class TestOverrides < Test::Unit::TestCase
-    include PuppetTest
-    def mksubdirs(basedir, level)
-        @@tmpfiles << basedir
-        dir = basedir.dup
-
-        (level + 1).times { |index|
-            Dir.mkdir(dir)
-            path = File.join(dir, "file")
-            File.open(path, "w") { |f| f.puts "yayness" }
-            dir = File.join(dir, index.to_s)
-        }
-    end
-
-    def test_simpleoverride
-        basedir = File.join(tmpdir(), "overridetesting")
-        mksubdirs(basedir, 1)
-
-        basefile = File.join(basedir, "file")
-        baseobj = Puppet::Type.type(:file).new(
-            :title => "base",
-            :path => basedir,
-            :recurse => true,
-            :mode => "755"
-        )
-
-        subdir = File.join(basedir, "0")
-        subfile = File.join(subdir, "file")
-        subobj = Puppet::Type.type(:file).new(
-            :title => "sub",
-            :path => subdir,
-            :recurse => true,
-            :mode => "644"
-        )
-
-        assert_apply(baseobj, subobj)
-
-        assert_equal(0755, File.stat(basefile).mode & 007777, "Did not set base mode")
-        assert_equal(0644, File.stat(subfile).mode & 007777, "Did not set overridden mode")
-    end
-
-    def test_deepoverride
-        basedir = File.join(tmpdir(), "deepoverridetesting")
-        mksubdirs(basedir, 10)
-
-        baseobj = nil
-        assert_nothing_raised("Could not create base obj") {
-            baseobj = Puppet::Type.type(:file).new(
-                :path => basedir,
-                :recurse => true,
-                :mode => "755"
-            )
-        }
-
-        children = []
-        files = {}
-        subdir = basedir.dup
-        mode = nil
-        10.times { |index|
-            next unless index % 3
-            subdir = File.join(subdir, index.to_s)
-            path = File.join(subdir, "file")
-            if index % 2
-                mode = "644"
-                files[path] = 0644
-            else
-                mode = "750"
-                files[path] = 0750
-            end
-
-            assert_nothing_raised("Could not create sub obj") {
-                children << Puppet::Type.type(:file).new(
-                    :path => subdir,
-                    :recurse => true,
-                    :mode => mode
-                )
-            }
-        }
-
-        config = mk_catalog(baseobj, *children)
-
-        assert_nothing_raised("Could not eval component") {
-            config.apply
-        }
-
-        files.each { |path, mode|
-            assert(FileTest.exists?(path), "File %s does not exist" % path)
-            curmode = File.stat(path).mode & 007777
-            assert(curmode == mode,
-                "File %s was incorrect mode %o instead of %o" % [path, curmode, mode])
-        }
-    end
-end
-
diff --git a/test/ral/type/file.rb b/test/ral/type/file.rb
index 829310ed4..7d6ec659f 100755
--- a/test/ral/type/file.rb
+++ b/test/ral/type/file.rb
@@ -5,6 +5,7 @@ require File.dirname(__FILE__) + '/../../lib/puppettest'
 require 'puppettest'
 require 'puppettest/support/utils'
 require 'fileutils'
+require 'mocha'
 
 class TestFile < Test::Unit::TestCase
     include PuppetTest::Support::Utils