From d272a4e6e65ca8fb2986d77d3bb711f4f7f62528 Mon Sep 17 00:00:00 2001 From: Lamont Granquist Date: Fri, 9 Jan 2015 10:32:37 -0800 Subject: scaling back aspirations. this works, but only in the most trivial of cases. --- lib/chef/dsl/recipe.rb | 6 ++++-- lib/chef/resource.rb | 15 ++++++++------- spec/unit/recipe_spec.rb | 14 ++++++++------ 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 -- cgit v1.2.1