Fixing a problem in collecting exported resources. Virtual resources worked fine, but exported resources resulted in an essentially infinite loop.

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

4 files changed, 44 insertions, 10 deletions

diff --git a/lib/puppet/parser/collector.rb b/lib/puppet/parser/collector.rb
index 9eae4cc4a..daed149f3 100644
--- a/lib/puppet/parser/collector.rb
+++ b/lib/puppet/parser/collector.rb
@@ -7,7 +7,7 @@ class Puppet::Parser::Collector
     def collect_exported
         # First get everything from the export table.  Just reuse our
         # collect_virtual method but tell it to use 'exported? for the test.
-        resources = collect_virtual(true)
+        resources = collect_virtual(true).reject { |r| ! r.virtual? }
 
         count = 0
 
@@ -43,7 +43,7 @@ class Puppet::Parser::Collector
             end
         end
 
-        scope.debug("Collected %s %s resource%s in %.2f seconds" %
+        scope.info("Collected %s %s resource%s in %.2f seconds" %
             [count, @type, count == 1 ? "" : "s", time])
 
         return resources
@@ -113,6 +113,13 @@ class Puppet::Parser::Collector
                 return objects
             end
         end
+
+#        if objects and ! objects.empty?
+#            objects.each { |r| r.virtual = false }
+#            return objects
+#        else
+#            return false
+#        end
     end
 
     def initialize(scope, type, equery, vquery, form)
diff --git a/lib/puppet/parser/interpreter.rb b/lib/puppet/parser/interpreter.rb
index 24de0904e..5a8227ec0 100644
--- a/lib/puppet/parser/interpreter.rb
+++ b/lib/puppet/parser/interpreter.rb
@@ -135,7 +135,11 @@ class Puppet::Parser::Interpreter
                     coll.each do |c|
                         # Only keep the loop going if we actually successfully
                         # collected something.
-                        if c.evaluate
+                        if o = c.evaluate
+                            o.each do |r|
+                                Puppet.info "Resource %s: v: %s, e: %s" %
+                                    [r.ref, r.virtual?.inspect, r.exported.inspect]
+                            end
                             done = false
                         end
                     end
@@ -145,6 +149,7 @@ class Puppet::Parser::Interpreter
             # Then evaluate any defined types.
             if ary = scope.unevaluated
                 ary.each do |resource|
+                    Puppet.info "Evaluated %s" % resource.ref
                     resource.evaluate
                 end
                 # If we evaluated, then loop through again.
diff --git a/test/language/collector.rb b/test/language/collector.rb
index 761d80df8..549ed7c9a 100755
--- a/test/language/collector.rb
+++ b/test/language/collector.rb
@@ -187,8 +187,15 @@ class TestCollector < Test::Unit::TestCase
             ret = coll.evaluate
         end
 
-        # Make sure it got deleted from the collection list
-        assert_equal([], @scope.collections)
+        # Make sure that the collection does not find the resource on the
+        # next run.
+        ret = nil
+        assert_nothing_raised do
+            ret = coll.collect_exported
+        end
+
+        assert(ret.empty?, "Exported resource was collected on the second run")
+
 
         # And make sure our exported object is no longer exported
         assert(! exported.virtual?, "Virtual object did not get realized")
@@ -209,7 +216,7 @@ class TestCollector < Test::Unit::TestCase
             ret = coll.evaluate
         end
 
-        assert_equal([], ret)
+        assert(! ret, "got resources back")
 
         # Now create a whole new scope and make sure we can actually retrieve
         # the resource from the database, not just from the scope.
@@ -261,9 +268,6 @@ class TestCollector < Test::Unit::TestCase
         assert_nothing_raised("Collection found same resource twice") do
             ret = coll.evaluate
         end
-
-        # Make sure it got deleted from the collection list
-        assert_equal([], scope.collections)
     end
 
     def test_collection_conflicts
diff --git a/test/language/interpreter.rb b/test/language/interpreter.rb
index 72f2e0f1e..2122d5a30 100755
--- a/test/language/interpreter.rb
+++ b/test/language/interpreter.rb
@@ -811,6 +811,15 @@ class TestInterpreter < Test::Unit::TestCase
         virt_three.virtual = true
         scope.setresource(virt_three)
 
+        # Create a normal, virtual resource
+        plainvirt = Puppet::Parser::Resource.new(
+            :type => "user", :title => "five",
+            :scope => scope, :source => source,
+            :params => paramify(source, :uid => "root")
+        )
+        plainvirt.virtual = true
+        scope.setresource(plainvirt)
+
         # Now create some collections for our virtual resources
         %w{Three[three] One[two]}.each do |ref|
             coll = Puppet::Parser::Collector.new(scope, "file", nil, nil, :virtual)
@@ -818,6 +827,10 @@ class TestInterpreter < Test::Unit::TestCase
             scope.newcollection(coll)
         end
 
+        # And create a generic user collector for our plain resource
+        coll = Puppet::Parser::Collector.new(scope, "user", nil, nil, :virtual)
+        scope.newcollection(coll)
+
         ret = nil
         assert_nothing_raised do
             ret = scope.unevaluated
@@ -830,7 +843,9 @@ class TestInterpreter < Test::Unit::TestCase
 
         # Now translate the whole tree
         assert_nothing_raised do
-            interp.evaliterate(scope)
+            Timeout::timeout(2) do
+                interp.evaliterate(scope)
+            end
         end
 
         # Now make sure we've got all of our files
@@ -841,6 +856,9 @@ class TestInterpreter < Test::Unit::TestCase
             assert_equal("root", file[:owner])
             assert(! file.virtual?, "file %s is still virtual" % name)
         end
+
+        # Now make sure we found the user
+        assert(! plainvirt.virtual?, "user was not realized")
     end
 
     # Make sure we fail if there are any leftover overrides to perform.