Bug #446

Require should stack not override

Added by Mark Chappell over 5 years ago. Updated over 4 years ago.

Status:Closed Start date:
Priority:High Due date:
Assignee:Andrew Shafer % Done:

0%
Category:-
Target version:0.24.0
Affected Puppet version:0.25.4 Branch:
Keywords:
Votes: 0

Description

When passing require in to a definition it makes more sense for the require argument to stack rather than override as it does at the minute.

define alpha {
        file { "/tmp/one" : content => "One" }
        file { "/tmp/two" : content => "Two" , require => File[[tmppre2]] }
}

file { "/tmp/pre" : content => "Pre" }
file { "/tmp/pre2" : content => "Pre" }

alpha { "Wobble" : require => File[[tmppre]] }

File /tmp/two should depend on both /tmp/pre and /tmp/pre2 as opposed to just depending on /tmp/pre2

History

#1
Updated by Luke Kanies over 5 years ago

  • Status changed from 1 to Closed
  • 7 set to wontfix

As long as relationships are specified using normal parameters, this will never change.

I think at some point relationships should get their own syntax, so they can be managed separately from other parameters, but for now, this is a wontfix bug. There’s no way the language is ever going to treat this kind of parameter specially, which is what you’re asking for.

#2
Updated by Marcin Owsiany almost 5 years ago

Here is a patch against 0.23.0 which I think fixes this. It essentially makes all relationship metaparameters stack up, rather than get overriden.

--- puppet-0.23.0.orig/lib/puppet/parser/resource.rb    2007-06-17 23:06:43.000000000 +0100
+++ puppet-0.23.0/lib/puppet/parser/resource.rb 2007-06-28 10:27:04.000000000 +0100
@@ -59,8 +59,9 @@
     # Add any metaparams defined in our scope. This actually adds any metaparams
     # from any parent scope, and there's currently no way to turn that off.
     def addmetaparams
+        relationship_names = Puppet::Type.relationship_params.collect { |p| p.name }
         Puppet::Type.eachmetaparam do |name|
-            next if self[name]
+            next if self[name] and not relationship_names.include?(name)
             if val = scope.lookupvar(name.to_s, false)
                 unless val == :undefined
                     set Param.new(:name => name, :value => val,
@@ -262,7 +263,15 @@
         # Because definitions are now parse-time, I can paramcheck immediately.
         paramcheck(param.name)

-        if current = @params[param.name]
+        # First, do merging on some metaparams
+        relationship_names = Puppet::Type.relationship_params.collect { |p| p.name }
+        if current = @params[param.name] and relationship_names.include?(param.name)
+            if current.value.is_a?(Array)
+                current.value << param.value
+            else
+                current.value = [ current.value, param.value ]
+            end
+        elsif current = @params[param.name]
             # XXX Should we ignore any settings that have the same values?
             if param.source.child_of?(current.source)
                 # Replace it, keeping all of its info.

I strongly feel that without this feature, definitions don’t work as people expect them to. In the original report, alpha { “Wobble”: … } looks like a regular type, and people don’t expect the require parameter to be dropped silently in some cases, which is what happens now.

#3
Updated by Luke Kanies almost 5 years ago

  • Status changed from Closed to 4
  • 7 deleted (wontfix)

I’ve been convinced to accept this patch.

#4
Updated by Luke Kanies over 4 years ago

  • Status changed from 4 to 1

#5
Updated by Luke Kanies over 4 years ago

  • Status changed from 1 to Closed
  • 7 set to fixed

Fixed in commit:8ad27328850c5acf67548c7ad6c93d0c4a43e1ec.

Also available in: Atom PDF