Fixing #343. Collections and definition evaluation both now happen on every iterative evaluation, with collections being evaluated first. This way collections can find resources that either are inside defined types or are the types themselves.

git-svn-id: https://reductivelabs.com/svn/puppet/trunk@1967 980ebf18-57e1-0310-9a29-db15c13687c0

4 files changed, 40 insertions, 25 deletions

diff --git a/lib/puppet/parser/collector.rb b/lib/puppet/parser/collector.rb
index 841d8585f..bd0496cf9 100644
--- a/lib/puppet/parser/collector.rb
+++ b/lib/puppet/parser/collector.rb
@@ -88,12 +88,17 @@ class Puppet::Parser::Collector
     # and then delete this object from the list of collections to evaluate.
     def evaluate
         if self.resources
-            return collect_resources
-        end
-
-        method = "collect_#{@form.to_s}"
-        objects = send(method).each do |obj|
-            obj.virtual = false
+            # We don't want to get rid of the collection unless it actually finds something,
+            # so that the collection will keep trying until all of the definitions are
+            # evaluated.
+            unless objects = collect_resources
+                return
+            end
+        else
+            method = "collect_#{@form.to_s}"
+            objects = send(method).each do |obj|
+                obj.virtual = false
+            end
         end
 
         # And then remove us from the list of collections, since we've
diff --git a/lib/puppet/parser/interpreter.rb b/lib/puppet/parser/interpreter.rb
index a81b7939f..988ff478b 100644
--- a/lib/puppet/parser/interpreter.rb
+++ b/lib/puppet/parser/interpreter.rb
@@ -92,21 +92,36 @@ class Puppet::Parser::Interpreter
         initparsevars
     end
 
-    # Iteratively evaluate all of the objects.  This finds all fo the
+    # Iteratively evaluate all of the objects.  This finds all of the
     # objects that represent definitions and evaluates the definitions appropriately.
     # It also adds defaults and overrides as appropriate.
     def evaliterate(scope)
         count = 0
-        begin
-            timeout 300 do
-                while ary = scope.unevaluated
-                    ary.each do |resource|
-                        resource.evaluate
+        loop do
+            count += 1
+            done = true
+            # First perform collections, so we can collect defined types.
+            if coll = scope.collections and ! coll.empty?
+                exceptwrap do
+                    coll.each do |c|
+                        c.evaluate
                     end
                 end
+                done = false
+            end
+            
+            # Then evaluate any defined types.
+            if ary = scope.unevaluated
+                ary.each do |resource|
+                    resource.evaluate
+                end
+                done = false
+            end
+            break if done
+            
+            if count > 1000
+                raise Puppet::ParseError, "Got 1000 class levels, which is unsupported"
             end
-        rescue Timeout::Error
-            raise Puppet::DevError, "Got a timeout trying to evaluate all definitions"
         end
     end
 
@@ -182,13 +197,6 @@ class Puppet::Parser::Interpreter
         # Now make sure we fail if there's anything left to do
         failonleftovers(scope)
 
-        # Now perform the collections
-        exceptwrap do
-            scope.collections.each do |coll|
-                coll.evaluate
-            end
-        end
-
         # Now finish everything.  This recursively calls finish on the
         # contained scopes and resources.
         scope.finish
diff --git a/lib/puppet/parser/lexer.rb b/lib/puppet/parser/lexer.rb
index 39fef8668..7f537eafc 100644
--- a/lib/puppet/parser/lexer.rb
+++ b/lib/puppet/parser/lexer.rb
@@ -81,6 +81,8 @@ module Puppet
                 array = []
 
                 self.scan { |token,str|
+                    # Ignore any definition nesting problems
+                    @indefine = false
                     #Puppet.debug("got token '%s' => '%s'" % [token,str])
                     if token.nil?
                         return array
@@ -252,12 +254,12 @@ module Puppet
 
                     if @lasttoken == :DEFINE
                         if indefine?
+                            msg = "Cannot nest definition %s inside %s" % [value, @indefine]
                             self.indefine = false
-                            raise Puppet::ParseError,
-                                "Definitions cannot nest"
+                            raise Puppet::ParseError, msg
                         end
 
-                        @indefine = true
+                        @indefine = value
                     end
 
                     @last = value
diff --git a/lib/puppet/parser/resource/reference.rb b/lib/puppet/parser/resource/reference.rb
index f71b3719f..9557d8c54 100644
--- a/lib/puppet/parser/resource/reference.rb
+++ b/lib/puppet/parser/resource/reference.rb
@@ -50,7 +50,7 @@ class Puppet::Parser::Resource::Reference
 
     def to_s
         unless defined? @namestring
-            @namestring = "%s[%s]" % [type, title]
+            @namestring = "%s[%s]" % [type.capitalize, title]
         end
         @namestring
     end