Mostly +1; comment below.
On Apr 15, 2010, at 4:35 PM, Markus Roberts 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
It would actually probably make more sense to just turn this around
and use current instead of param:
current.value = [current.value, param.value].flatten
You don't need the dupe, because each resource already has its own
param.
Not a huge difference, but it should have been written this way in the
first place, and avoids the dupe.
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]
.
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en
.
--
If there is anything the nonconformist hates worse than a conformist,
it's another nonconformist who doesn't conform to the prevailing
standard of nonconformity. --Bill Vaughan
---------------------------------------------------------------------
Luke Kanies -|- http://puppetlabs.com -|- +1(615)594-8199
--
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.
