+1 On Fri, Apr 30, 2010 at 5:38 PM, Markus Roberts <[email protected]> wrote:
> The plussignment operator was constructing the new parameter value by > modifying the param object's value in place (so as to preserve the file > and line information for debugging). However, when multiple resources > are overridden by the same plussignment this would result in all of the > resources sharing the same value (the union of all the prior values and > the new value), which is wrong. > > Instead, we need to give each resource its own copy of the value (e.g., > a copy of the param object), which this patch implements. > > Signed-off-by: Markus Roberts <[email protected]> > --- > lib/puppet/parser/resource.rb | 14 ++++++++++---- > spec/unit/parser/resource.rb | 18 ++++++++++++++++++ > 2 files changed, 28 insertions(+), 4 deletions(-) > > diff --git a/lib/puppet/parser/resource.rb b/lib/puppet/parser/resource.rb > index 651ed42..ee87886 100644 > --- a/lib/puppet/parser/resource.rb > +++ b/lib/puppet/parser/resource.rb > @@ -377,10 +377,16 @@ class Puppet::Parser::Resource > > # If we've gotten this far, we're allowed to override. > > - # Merge with previous value, if the parameter was generated with > the +> syntax. > - # It's important that we use the new param instance here, not the > old one, > - # so that the source is registered correctly for later overrides. > - param.value = [current.value, param.value].flatten if param.add > + # Merge with previous value, if the parameter was generated with > the +> > + # syntax. It's important that we use a copy of the new param > instance > + # here, not the old one, and not the original new one, so that the > source > + # is registered correctly for later overrides but the values > aren't > + # implcitly shared when multiple resources are overrriden at once > (see > + # ticket #3556). > + if param.add > + param = param.dup > + param.value = [current.value, param.value].flatten > + end > > set_parameter(param) > end > diff --git a/spec/unit/parser/resource.rb b/spec/unit/parser/resource.rb > index d41d4f4..9924bbe 100755 > --- a/spec/unit/parser/resource.rb > +++ b/spec/unit/parser/resource.rb > @@ -381,6 +381,24 @@ describe Puppet::Parser::Resource do > @resource[:testing].should == %w{other testing} > end > > + it "should not merge parameter values when multiple resources are > overriden with '+>' at once " do > + @resource_2 = mkresource :source => @source > + > + @resource. set_parameter(:testing, "old_val_1") > + @resource_2.set_parameter(:testing, "old_val_2") > + > + @source.stubs(:child_of?).returns true > + param = Puppet::Parser::Resource::Param.new(:name => :testing, > :value => "new_val", :source => @resource.source) > + param.add = true > + @override.set_parameter(param) > + > + @resource. merge(@override) > + @resource_2.merge(@override) > + > + @resource [:testing].should == %w{old_val_1 new_val} > + @resource_2[:testing].should == %w{old_val_2 new_val} > + end > + > it "should promote tag overrides to real tags" do > @source.stubs(:child_of?).returns true > param = Puppet::Parser::Resource::Param.new(:name => :tag, > :value => "testing", :source => @resource.source) > -- > 1.6.4 > > -- > You received this message because you are subscribed to the Google Groups > "Puppet Developers" group. > To post to this group, send email to [email protected]. > To unsubscribe from this group, send email to > [email protected]<puppet-dev%[email protected]> > . > For more options, visit this group at > http://groups.google.com/group/puppet-dev?hl=en. > > -- You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
