[Pkg-puppet-devel] [SCM] Puppet packaging for Debian branch, experimental, updated. debian/2.6.8-1-844-g7ec39d5

[Daniel Pittman]

The following commit has been merged in the experimental branch:
commit 1b42725b5caab6f8e457e11fec2488fbe94e8e43
Author: Daniel Pittman <daniel at puppetlabs.com>
Date:   Mon May 2 15:14:25 2011 -0700

    (#7314) Faces fail horribly when one has a syntax error.
    
    When we hit a syntax error in any face, a whole bunch of unrelated face things
    would blow up in horrible ways.  Stack traces for all...
    
    Now, instead, we catch that fault   but specifically only in the face file
    and report it through our error logs, then quietly ignore the face.
    
    Reviewed-By: Nick Lewis <nick at puppetlabs.com>

diff --git a/lib/puppet/interface/face_collection.rb b/lib/puppet/interface/face_collection.rb
index 6e6afc5..baa4246 100644
--- a/lib/puppet/interface/face_collection.rb
+++ b/lib/puppet/interface/face_collection.rb
@@ -10,21 +10,12 @@ module Puppet::Interface::FaceCollection
     unless @loaded
       @loaded = true
       $LOAD_PATH.each do |dir|
-        next unless FileTest.directory?(dir)
-        Dir.chdir(dir) do
-          Dir.glob("puppet/face/*.rb").collect { |f| f.sub(/\.rb/, '') }.each do |file|
-            iname = file.sub(/\.rb/, '')
-            begin
-              require iname
-            rescue Exception => detail
-              puts detail.backtrace if Puppet[:trace]
-              raise "Could not load #{iname} from #{dir}/#{file}: #{detail}"
-            end
-          end
-        end
+        Dir.glob("#{dir}/puppet/face/*.rb").
+          collect {|f| File.basename(f, '.rb') }.
+          each {|name| self[name, :current] }
       end
     end
-    return @faces.keys.select {|name| @faces[name].length > 0 }
+    @faces.keys.select {|name| @faces[name].length > 0 }
   end
 
   def self.validate_version(version)
@@ -124,6 +115,10 @@ module Puppet::Interface::FaceCollection
     rescue LoadError => e
       raise unless e.message =~ %r{-- puppet/face/#{name}$}
       # ...guess we didn't find the file; return a much better problem.
+    rescue SyntaxError => e
+      raise unless e.message =~ %r{puppet/face/#{name}\.rb:\d+: }
+      Puppet.err "Failed to load face #{name}:\n#{e}"
+      # ...but we just carry on after complaining.
     end
 
     return get_face(name, version)
diff --git a/spec/fixtures/faulty_face/puppet/face/syntax.rb b/spec/fixtures/faulty_face/puppet/face/syntax.rb
new file mode 100644
index 0000000..3b1e36c
--- /dev/null
+++ b/spec/fixtures/faulty_face/puppet/face/syntax.rb
@@ -0,0 +1,8 @@
+Puppet::Face.define(:syntax, '1.0.0') do
+  action :foo do
+    when_invoked do |whom|
+      "hello, #{whom}"
+    end
+  # This 'end' is deliberately omitted, to induce a syntax error.
+  # Please don't fix that, as it is used for testing. --daniel 2011-05-02
+end
diff --git a/spec/unit/interface/face_collection_spec.rb b/spec/unit/interface/face_collection_spec.rb
index 4358ef4..4ad8787 100755
--- a/spec/unit/interface/face_collection_spec.rb
+++ b/spec/unit/interface/face_collection_spec.rb
@@ -22,7 +22,7 @@ describe Puppet::Interface::FaceCollection do
 
   after :each do
     subject.instance_variable_set(:@faces, @original_faces)
-    $".clear ; @original_required.each do |item| $" << item end
+    @original_required.each {|f| $".push f unless $".include? f }
   end
 
   describe "::prefix_match?" do
@@ -160,4 +160,21 @@ describe Puppet::Interface::FaceCollection do
       end
     end
   end
+
+  context "faulty faces" do
+    before :each do
+      $:.unshift "#{PuppetSpec::FIXTURE_DIR}/faulty_face"
+    end
+
+    after :each do
+      $:.delete_if {|x| x == "#{PuppetSpec::FIXTURE_DIR}/faulty_face"}
+    end
+
+    it "should not die if a face has a syntax error" do
+      subject.faces.should be_include :help
+      subject.faces.should_not be_include :syntax
+      @logs.should_not be_empty
+      @logs.first.message.should =~ /syntax error/
+    end
+  end
 end

-- 
Puppet packaging for Debian