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 <[email protected]> --- lib/puppet/type/file.rb | 30 ++++++++++++- spec/integration/type/file.rb | 21 +++++++++ spec/unit/type/file.rb | 7 +--- test/other/overrides.rb | 101 ----------------------------------------- test/ral/type/file.rb | 1 + 5 files changed, 52 insertions(+), 108 deletions(-) delete mode 100755 test/other/overrides.rb diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb index 55d4ec7..f3ccd7f 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 cced1ed..3b03d53 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 58cd4ad..d6dcb00 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 22c3d2a..0000000 --- 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 829310e..7d6ec65 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 -- 1.6.1 --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en -~----------~----~----~----~------~----~------~--~---
