scaling back aspirations.lcg/suppress-3694

this works, but only in the most trivial of cases.

3 files changed, 20 insertions, 15 deletions

diff --git a/lib/chef/dsl/recipe.rb b/lib/chef/dsl/recipe.rb
index a5a6f820c7..798744677a 100644
--- a/lib/chef/dsl/recipe.rb
+++ b/lib/chef/dsl/recipe.rb
@@ -105,7 +105,7 @@ class Chef
         # This behavior is very counter-intuitive and should be removed.
         # See CHEF-3694, https://tickets.opscode.com/browse/CHEF-3694
         # Moved to this location to resolve CHEF-5052, https://tickets.opscode.com/browse/CHEF-5052
-        prior_resource = resource.load_prior_resource(type, name)
+        trivial_prior_resource = resource.load_prior_resource(type, name)
         cloned_resource = resource.dup
         resource.cookbook_name = cookbook_name
         resource.recipe_name = recipe_name
@@ -120,7 +120,9 @@ class Chef
         resource.instance_eval(&resource_attrs_block) if block_given?
 
         # emit a cloned resource warning if it is warranted
-        resource.maybe_emit_cloned_resource_warning(cloned_resource) unless prior_resource.nil?
+        if !trivial_prior_resource.nil? && ( !trivial_prior_resource || !resource.identicalish_resource?(cloned_resource))
+          resource.emit_cloned_resource_warning(cloned_resource)
+        end
 
         # Run optional resource hook
         resource.after_created
diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb
index 7ae6206897..14e1f3e976 100644
--- a/lib/chef/resource.rb
+++ b/lib/chef/resource.rb
@@ -298,7 +298,7 @@ F
 
     def identicalish_resource?(cloned_resource)
       skipped_ivars = [ :@source_line, :@cookbook_name, :@recipe_name, :@params, :@elapsed_time ]
-      checked_ivars = cloned_resource.instance_variables | self.instance_variables - skipped_ivars
+      checked_ivars = ( cloned_resource.instance_variables | self.instance_variables ) - skipped_ivars
       non_matching_ivars = checked_ivars.reject do |iv|
         if iv == :@action && ( [self.instance_variable_get(iv)].flatten == [:nothing] || [cloned_resource.instance_variable_get(iv)].flatten == [:nothing] )
           # :nothing action on either side of the comparison always matches
@@ -311,14 +311,14 @@ F
       non_matching_ivars.empty?
     end
 
-    def maybe_emit_cloned_resource_warning(cloned_resource)
-      unless identicalish_resource?(cloned_resource)
+    def emit_cloned_resource_warning(cloned_resource)
+      #unless identicalish_resource?(cloned_resource)
         Chef::Log.warn("Cloning resource attributes for #{self} from prior resource (CHEF-3694)")
         Chef::Log.warn("Previous #{cloned_resource}: #{cloned_resource.source_line}") if cloned_resource.source_line
         Chef::Log.warn("Current  #{self}: #{self.source_line}") if self.source_line
-      else
-        Chef::Log.debug("Harmless resource cloning from #{cloned_resource}: #{cloned_resource.source_line} to #{self}: #{self.source_line}")
-      end
+      #else
+      #  Chef::Log.debug("Harmless resource cloning from #{cloned_resource}: #{cloned_resource.source_line} to #{self}: #{self.source_line}")
+      #end
     end
 
     def load_prior_resource(resource_type, instance_name)
@@ -327,12 +327,13 @@ F
         prior_resource = run_context.resource_collection.lookup(key)
         # if we get here, there is a prior resource (otherwise we'd have jumped
         # to the rescue clause).
+        ret = identicalish_resource?(prior_resource)
         prior_resource.instance_variables.each do |iv|
           unless iv == :@source_line || iv == :@action || iv == :@not_if || iv == :@only_if
             self.instance_variable_set(iv, prior_resource.instance_variable_get(iv))
           end
         end
-        prior_resource
+        ret
       rescue Chef::Exceptions::ResourceNotFound
         nil
       end
diff --git a/spec/unit/recipe_spec.rb b/spec/unit/recipe_spec.rb
index e27e6c708d..afa47f5da3 100644
--- a/spec/unit/recipe_spec.rb
+++ b/spec/unit/recipe_spec.rb
@@ -195,36 +195,38 @@ describe Chef::Recipe do
         end
       end
 
-      it "should not emit a 3694 warning when attributes do not change" do
+      it "should emit a 3694 warning for non-trivial attributes (unfortunately)" do
         recipe.zen_master "klopp" do
           something "bvb"
         end
-        expect(Chef::Log).to_not receive(:warn)
+        expect(Chef::Log).to receive(:warn).at_least(:once)
         recipe.zen_master "klopp" do
           something "bvb"
         end
       end
 
+      it "should not emit a 3694 warning for completely trivial resource cloning" do
+        recipe.zen_master "klopp"
+        expect(Chef::Log).to_not receive(:warn)
+        recipe.zen_master "klopp"
+      end
+
       it "should not emit a 3694 warning when attributes do not change and the first action is :nothing" do
         recipe.zen_master "klopp" do
-          something "bvb"
           action :nothing
         end
         expect(Chef::Log).to_not receive(:warn)
         recipe.zen_master "klopp" do
-          something "bvb"
           action :score
         end
       end
 
       it "should not emit a 3694 warning when attributes do not change and the second action is :nothing" do
         recipe.zen_master "klopp" do
-          something "bvb"
           action :score
         end
         expect(Chef::Log).to_not receive(:warn)
         recipe.zen_master "klopp" do
-          something "bvb"
           action :nothing
         end
       end