deep_merge! weirdness


#1

i came across something i’m unsure about in deep_merge!. it seems that when
merging hash deep_merge! will re-assign dest source.length times if dest is
not a hash. i’ve patched this and specs are green, but i wonder if i’m
missing something.

— a/lib/chef/mixin/deep_merge.rb
+++ b/lib/chef/mixin/deep_merge.rb
@@ -84,17 +84,17 @@ class Chef
when nil
dest
when Hash

  •      source.each do |src_key, src_value|
    
  •        if dest.kind_of?(Hash)
    
  •      if dest.kind_of?(Hash)
    
  •        source.each do |src_key, src_value|
             if dest[src_key]
               dest[src_key] = deep_merge!(src_value, dest[src_key])
             else # dest[src_key] doesn't exist so we take whatever
    

source has
raise_if_knockout_used!(src_value)
dest[src_key] = src_value
end

  •        else # dest isn't a hash, so we overwrite it completely
    
  •          dest = source
           end
    
  •      else # dest isn't a hash, so we overwrite it completely
    
  •        dest = source
         end
       when Array
         if dest.kind_of?(Array)

#2

This looks good to me Avishai. That would be great if you can file a ticket
and even greater if you can submit this patch :slight_smile:

Thanks a lot for taking time to think about this.

– Serdar

On Sun, Oct 13, 2013 at 2:49 PM, Avishai Ish-Shalom avishai@fewbytes.comwrote:

i came across something i’m unsure about in deep_merge!. it seems that
when merging hash deep_merge! will re-assign dest source.length times if
dest is not a hash. i’ve patched this and specs are green, but i wonder if
i’m missing something.

— a/lib/chef/mixin/deep_merge.rb
+++ b/lib/chef/mixin/deep_merge.rb
@@ -84,17 +84,17 @@ class Chef
when nil
dest
when Hash

  •      source.each do |src_key, src_value|
    
  •        if dest.kind_of?(Hash)
    
  •      if dest.kind_of?(Hash)
    
  •        source.each do |src_key, src_value|
             if dest[src_key]
               dest[src_key] = deep_merge!(src_value, dest[src_key])
             else # dest[src_key] doesn't exist so we take whatever
    

source has
raise_if_knockout_used!(src_value)
dest[src_key] = src_value
end

  •        else # dest isn't a hash, so we overwrite it completely
    
  •          dest = source
           end
    
  •      else # dest isn't a hash, so we overwrite it completely
    
  •        dest = source
         end
       when Array
         if dest.kind_of?(Array)