Sorry for the duplication - Qwest (my new, not-for-much-longer ISP) has apparently been blocking port 25 or something.
On Jul 23, 2009, at 12:50 AM, Luke Kanies wrote: > > 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 | 26 ++++++++++- > 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, 48 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..3103c5d 100644 > --- a/lib/puppet/type/file.rb > +++ b/lib/puppet/type/file.rb > @@ -580,7 +580,31 @@ 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. > + other_paths = catalog.vertices.find_all { |r| r.is_a? > (self.class) and r[:path].include?(self[:path]) }.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.include?(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 > > > > -- A diplomat is a man who can convince his wife she'd look stout in a fur coat. --------------------------------------------------------------------- Luke Kanies | http://reductivelabs.com | http://madstop.com --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
