Fixing #2112 - Transactions handle conflicting generated resources

This commit rips out all of the 'implicit resource' crap,
replacing it with a simple system that just skips
resources that the catalog says are in conflict.

Removes a bunch of code, and fixes the bug to boot.

Signed-off-by: Luke Kanies <luke@madstop.com>

5 files changed, 16 insertions, 39 deletions

diff --git a/lib/puppet/resource.rb b/lib/puppet/resource.rb
index add32b7cf..50663cb67 100644
--- a/lib/puppet/resource.rb
+++ b/lib/puppet/resource.rb
@@ -7,7 +7,7 @@ require 'puppet/resource/reference'
 class Puppet::Resource
     include Puppet::Util::Tagging
     include Enumerable
-    attr_accessor :type, :title, :file, :line, :catalog, :implicit
+    attr_accessor :type, :title, :file, :line, :catalog
 
     # Proxy these methods to the parameters hash.  It's likely they'll
     # be overridden at some point, but this works for now.
diff --git a/lib/puppet/resource/catalog.rb b/lib/puppet/resource/catalog.rb
index 266f04903..88aa9517d 100644
--- a/lib/puppet/resource/catalog.rb
+++ b/lib/puppet/resource/catalog.rb
@@ -65,7 +65,7 @@ class Puppet::Resource::Catalog < Puppet::SimpleGraph
             unless resource.respond_to?(:ref)
                 raise ArgumentError, "Can only add objects that respond to :ref, not instances of %s" % resource.class
             end
-        end.find_all { |resource| fail_or_skip_unless_unique(resource) }.each do |resource|
+        end.each { |resource| fail_unless_unique(resource) }.each do |resource|
             ref = resource.ref
 
             @transient_resources << resource if applying?
@@ -430,20 +430,9 @@ class Puppet::Resource::Catalog < Puppet::SimpleGraph
     end
 
     # Verify that the given resource isn't defined elsewhere.
-    def fail_or_skip_unless_unique(resource)
+    def fail_unless_unique(resource)
         # Short-curcuit the common case, 
-        return resource unless existing_resource = @resource_table[resource.ref]
-
-        if resource.implicit?
-            resource.debug "Generated resource conflicts with explicit resource; ignoring generated resource"
-            return nil
-        elsif old = resource(resource.ref) and old.implicit?
-            # The existing resource is implicit; remove it and replace it with
-            # the new one.
-            old.debug "Replacing with new resource"
-            remove_resource(old)
-            return resource
-        end
+        return unless existing_resource = @resource_table[resource.ref]
 
         # If we've gotten this far, it's a real conflict
 
diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index bb5adc383..37f51b2e5 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -343,9 +343,15 @@ class Transaction
         made = [made] unless made.is_a?(Array)
         made.uniq!
         made.each do |res|
-            @catalog.add_resource(res) { |r| r.finish }
+            begin
+                @catalog.add_resource(res) do |r|
+                    r.finish
+                    make_parent_child_relationship(resource, [r])
+                end
+            rescue Puppet::Resource::Catalog::DuplicateResourceError
+                next
+            end
         end
-        make_parent_child_relationship(resource, made)
         made
     end
 
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index 64cfc2540..311878841 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -617,10 +617,6 @@ class Type
 
     public
 
-    ###############################
-    # Code related to the closure-like behaviour of the resource classes.
-    attr_accessor :implicit
-
     # Is this type's name isomorphic with the object?  That is, if the
     # name conflicts, does it necessarily mean that the objects conflict?
     # Defaults to true.
@@ -632,14 +628,6 @@ class Type
         end
     end
 
-    def implicit?
-        if defined? @implicit and @implicit
-            return true
-        else
-            return false
-        end
-    end
-
     def isomorphic?
         self.class.isomorphic?
     end
@@ -1056,7 +1044,7 @@ class Type
 
         # Now create our resource.
         resource = Puppet::Resource.new(self.name, title)
-        [:catalog, :implicit].each do |attribute|
+        [:catalog].each do |attribute|
             if value = hash[attribute]
                 hash.delete(attribute)
                 resource.send(attribute.to_s + "=", value)
@@ -1914,7 +1902,7 @@ class Type
             self.title = resource.ref
         end
 
-        [:file, :line, :catalog, :implicit].each do |getter|
+        [:file, :line, :catalog].each do |getter|
             setter = getter.to_s + "="
             if val = resource.send(getter)
                 self.send(setter, val)
@@ -2012,13 +2000,7 @@ class Type
         return nil unless catalog
 
         unless defined?(@parent)
-            # This is kinda weird.
-            if implicit?
-                parents = catalog.relationship_graph.adjacent(self, :direction => :in)
-            else
-                parents = catalog.adjacent(self, :direction => :in)
-            end
-            if parents
+            if parents = catalog.adjacent(self, :direction => :in)
                 # We should never have more than one parent, so let's just ignore
                 # it if we happen to.
                 @parent = parents.shift
diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb
index 0cd9ebece..344fcb713 100644
--- a/lib/puppet/type/file.rb
+++ b/lib/puppet/type/file.rb
@@ -516,7 +516,7 @@ module Puppet
             # or so.  Unfortunately, we don't have a straightforward way to manage
             # the different lifetimes of this data, so we kludge it like this.
             # The right-side hash wins in the merge.
-            options = @original_parameters.merge(:path => full_path, :implicit => true).reject { |param, value| value.nil? }
+            options = @original_parameters.merge(:path => full_path).reject { |param, value| value.nil? }
 
             # These should never be passed to our children.
             [:parent, :ensure, :recurse, :recurselimit, :target, :alias, :source].each do |param|