[Pkg-puppet-devel] [SCM] Puppet packaging for Debian branch, upstream, updated. 0.25.5-663-g71824ee

[Markus Roberts]

The following commit has been merged in the upstream branch:
commit 8c8c1469ae9f1dd11c567d89a27be81653ca2052
Author: Markus Roberts <Markus at reality.com>
Date:   Thu Jul 15 19:44:35 2010 -0700

    Minimal fix for #4243 -- import isn't thread safe
    
    The import function was calling type_loader#import directly so that it could
    pass in the current file name, but by doing so it was thwarting the thread-
    safety locking level.  This patch rearanges things so that all imports go
    through the same (thread safe) code path while retaining the current_file
    passing, error handling, etc. from the old structure.

diff --git a/lib/puppet/parser/parser_support.rb b/lib/puppet/parser/parser_support.rb
index 4f3a4dd..c0fd371 100644
--- a/lib/puppet/parser/parser_support.rb
+++ b/lib/puppet/parser/parser_support.rb
@@ -111,7 +111,7 @@ class Puppet::Parser::Parser
   end
 
   def import(file)
-    known_resource_types.loader.import(file, @lexer.file)
+    known_resource_types.loader.import_if_possible(file, @lexer.file)
   end
 
   def initialize(env)
diff --git a/lib/puppet/parser/type_loader.rb b/lib/puppet/parser/type_loader.rb
index cb8657f..c33f90d 100644
--- a/lib/puppet/parser/type_loader.rb
+++ b/lib/puppet/parser/type_loader.rb
@@ -70,7 +70,7 @@ class Puppet::Parser::TypeLoader
 
   def initialize(env)
     self.environment = env
-    @loaded = []
+    @loaded = {}
     @loading = Helper.new
 
     @imported = {}
@@ -79,10 +79,13 @@ class Puppet::Parser::TypeLoader
   def load_until(namespaces, name)
     return nil if name == "" # special-case main.
     name2files(namespaces, name).each do |filename|
-      modname = nil
-      import_if_possible(filename) do
-        modname = import(filename)
-        @loaded << filename
+      modname = begin
+        import_if_possible(filename)
+      rescue Puppet::ImportError => detail
+        # We couldn't load the item
+        # I'm not convienced we should just drop these errors, but this
+        # preserves existing behaviours.
+        nil
       end
       if result = yield(filename)
         Puppet.info "Automatically imported #{name} from #{filename}"
@@ -124,23 +127,18 @@ class Puppet::Parser::TypeLoader
     parser.parse
   end
 
-  private
-
   # Utility method factored out of load for handling thread-safety.
   # This isn't tested in the specs, because that's basically impossible.
-  def import_if_possible(file, &blk)
-    return if @loaded.include?(file)
-    begin
+  def import_if_possible(file, current_file = nil)
+    @loaded[file] || begin
       case @loading.owner_of(file)
       when :this_thread
-        return
+        nil
       when :another_thread
-        return import_if_possible(file, &blk)
+        import_if_possible(file,current_file)
       when :nobody
-        blk.call
+        @loaded[file] = import(file,current_file)
       end
-    rescue Puppet::ImportError => detail
-      # We couldn't load the item
     ensure
       @loading.done_with(file)
     end
diff --git a/spec/unit/parser/type_loader_spec.rb b/spec/unit/parser/type_loader_spec.rb
index db72a23..8f005d5 100644
--- a/spec/unit/parser/type_loader_spec.rb
+++ b/spec/unit/parser/type_loader_spec.rb
@@ -38,16 +38,16 @@ describe Puppet::Parser::TypeLoader do
 
     it "should attempt to import each generated name" do
       @loader.expects(:name2files).returns %w{foo bar}
-      @loader.expects(:import).with("foo")
-      @loader.expects(:import).with("bar")
+      @loader.expects(:import).with("foo",nil)
+      @loader.expects(:import).with("bar",nil)
       @loader.load_until(["foo"], "bar") { |f| false }
     end
 
     it "should yield after each import" do
       yielded = []
       @loader.expects(:name2files).returns %w{foo bar}
-      @loader.expects(:import).with("foo")
-      @loader.expects(:import).with("bar")
+      @loader.expects(:import).with("foo",nil)
+      @loader.expects(:import).with("bar",nil)
       @loader.load_until(["foo"], "bar") { |f| yielded << f; false }
       yielded.should == %w{foo bar}
     end
@@ -55,31 +55,31 @@ describe Puppet::Parser::TypeLoader do
     it "should stop importing when the yielded block returns true" do
       yielded = []
       @loader.expects(:name2files).returns %w{foo bar baz}
-      @loader.expects(:import).with("foo")
-      @loader.expects(:import).with("bar")
-      @loader.expects(:import).with("baz").never
+      @loader.expects(:import).with("foo",nil)
+      @loader.expects(:import).with("bar",nil)
+      @loader.expects(:import).with("baz",nil).never
       @loader.load_until(["foo"], "bar") { |f| true if f == "bar" }
     end
 
     it "should return the result of the block" do
       yielded = []
       @loader.expects(:name2files).returns %w{foo bar baz}
-      @loader.expects(:import).with("foo")
-      @loader.expects(:import).with("bar")
-      @loader.expects(:import).with("baz").never
+      @loader.expects(:import).with("foo",nil)
+      @loader.expects(:import).with("bar",nil)
+      @loader.expects(:import).with("baz",nil).never
       @loader.load_until(["foo"], "bar") { |f| 10 if f == "bar" }.should == 10
     end
 
     it "should return nil if the block never returns true" do
       @loader.expects(:name2files).returns %w{foo bar}
-      @loader.expects(:import).with("foo")
-      @loader.expects(:import).with("bar")
+      @loader.expects(:import).with("foo",nil)
+      @loader.expects(:import).with("bar",nil)
       @loader.load_until(["foo"], "bar") { |f| false }.should be_nil
     end
 
     it "should know when a given name has been loaded" do
       @loader.expects(:name2files).returns %w{file}
-      @loader.expects(:import).with("file")
+      @loader.expects(:import).with("file",nil)
       @loader.load_until(["foo"], "bar") { |f| true }
       @loader.should be_loaded("file")
     end

-- 
Puppet packaging for Debian