Merge commit 'tags/2.6.0' into next

16 files changed, 202 insertions, 125 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 74a5470b8..53124a13b 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,5 +1,16 @@
+2.6.0
+=====
+42a475e  Fixing #4268 - manifests always imported
+06fc40c  [#4269] Undef variables interpolate to empty string
+1288f8c  [#4270] Force inherited classes to load into the correct environment
+539d716  [#4287] Fix the undefined evaluate_match error when comparing functions
+d2da1d4  Tweak to tweak to fix for #4233 -- ":" is valid in type names
+bbc07f2  Bandaid for #4285 -- :name vs <namevar>
+40e6f02  Tweak to fix for #4233 -- only accept word chars in types
+
 2.6.0rc4
 ========
+d87a2e3  Updated CHANGELOG for 2.6.0RC4
 cf597d7  [#4233] Ruby regexps are not multiline by default, but Resource titles can be multiline
 d6cbb21  Fix for #4234 -- ruby DSL fails on second resource
 4822de3  Fix for #4236 -- Only interpolate $ if followed by a variable
diff --git a/lib/puppet/parser/ast.rb b/lib/puppet/parser/ast.rb
index 2773a240e..54e034acb 100644
--- a/lib/puppet/parser/ast.rb
+++ b/lib/puppet/parser/ast.rb
@@ -87,6 +87,20 @@ class Puppet::Parser::AST
   def initialize(args)
     set_options(args)
   end
+
+  # evaluate ourselves, and match
+  def evaluate_match(value, scope)
+    obj = self.safeevaluate(scope)
+
+    obj   = obj.downcase   if obj.respond_to?(:downcase)
+    value = value.downcase if value.respond_to?(:downcase)
+
+    obj   = Puppet::Parser::Scope.number?(obj)   || obj
+    value = Puppet::Parser::Scope.number?(value) || value
+
+    # "" == undef for case/selector/if
+    obj == value or (obj == "" and value == :undef)
+  end
 end
 
 # And include all of the AST subclasses.
diff --git a/lib/puppet/parser/ast/leaf.rb b/lib/puppet/parser/ast/leaf.rb
index 49f430278..49cde63ca 100644
--- a/lib/puppet/parser/ast/leaf.rb
+++ b/lib/puppet/parser/ast/leaf.rb
@@ -10,20 +10,6 @@ class Puppet::Parser::AST
       @value
     end
 
-    # evaluate ourselves, and match
-    def evaluate_match(value, scope)
-      obj = self.safeevaluate(scope)
-
-      obj   = obj.downcase   if obj.respond_to?(:downcase)
-      value = value.downcase if value.respond_to?(:downcase)
-
-      obj   = Puppet::Parser::Scope.number?(obj)   || obj
-      value = Puppet::Parser::Scope.number?(value) || value
-
-      # "" == undef for case/selector/if
-      obj == value or (obj == "" and value == :undef)
-    end
-
     def match(value)
       @value == value
     end
@@ -77,7 +63,7 @@ class Puppet::Parser::AST
 
   class Concat < AST::Leaf
     def evaluate(scope)
-      @value.collect { |x| x.evaluate(scope) }.join
+      @value.collect { |x| x.evaluate(scope) }.collect{ |x| x == :undef ? '' : x }.join
     end
 
     def to_s
diff --git a/lib/puppet/parser/type_loader.rb b/lib/puppet/parser/type_loader.rb
index c33f90d11..35ad49593 100644
--- a/lib/puppet/parser/type_loader.rb
+++ b/lib/puppet/parser/type_loader.rb
@@ -88,7 +88,7 @@ class Puppet::Parser::TypeLoader
         nil
       end
       if result = yield(filename)
-        Puppet.info "Automatically imported #{name} from #{filename}"
+        Puppet.info "Automatically imported #{name} from #{filename} into #{environment}"
         result.module_name = modname if modname and result.respond_to?(:module_name=)
         return result
       end
diff --git a/lib/puppet/resource.rb b/lib/puppet/resource.rb
index 55874aec8..96d22e414 100644
--- a/lib/puppet/resource.rb
+++ b/lib/puppet/resource.rb
@@ -217,7 +217,11 @@ class Puppet::Resource
   end
 
   def uniqueness_key
-    self.to_hash.values_at(*key_attributes.sort_by { |k| k.to_s })
+    # Temporary kludge to deal with inconsistant use patters
+    h = self.to_hash
+    h[namevar] ||= h[:name]
+    h[:name]   ||= h[namevar]
+    h.values_at(*key_attributes.sort_by { |k| k.to_s })
   end
 
   def key_attributes
@@ -353,7 +357,8 @@ class Puppet::Resource
 
   def find_resource_type(type)
     # It still works fine without the type == 'class' short-cut, but it is a lot slower.
-    find_builtin_resource_type(type) || find_defined_resource_type(type) unless type.to_s.downcase == 'class'
+    return nil if ["class", "node"].include? type.to_s.downcase
+    find_builtin_resource_type(type) || find_defined_resource_type(type)
   end
 
   def find_builtin_resource_type(type)
diff --git a/lib/puppet/resource/catalog.rb b/lib/puppet/resource/catalog.rb
index 4ac99eeea..4b4342d11 100644
--- a/lib/puppet/resource/catalog.rb
+++ b/lib/puppet/resource/catalog.rb
@@ -57,7 +57,7 @@ class Puppet::Resource::Catalog < Puppet::SimpleGraph
   end
 
   def title_key_for_ref( ref )
-    ref =~ /^(.+)\[(.*)\]/m
+    ref =~ /^([\w:]+)\[(.*)\]$/m
     [$1, $2]
   end
 
diff --git a/lib/puppet/resource/type.rb b/lib/puppet/resource/type.rb
index c2b4271e6..85c0979c1 100644
--- a/lib/puppet/resource/type.rb
+++ b/lib/puppet/resource/type.rb
@@ -145,7 +145,12 @@ class Puppet::Resource::Type
     resource_type = type == :hostclass ? :class : :node
 
     # Make sure our parent class has been evaluated, if we have one.
-    parent_type.mk_plain_resource(scope) if parent and ! scope.catalog.resource(resource_type, parent)
+    if parent
+      parent_resource = scope.catalog.resource(resource_type, parent)
+      unless parent_resource
+        parent_type(scope).mk_plain_resource(scope)
+      end
+    end
 
     # Do nothing if the resource already exists; this makes sure we don't
     # get multiple copies of the class resource, which helps provide the
@@ -169,11 +174,14 @@ class Puppet::Resource::Type
     @name.is_a?(Regexp)
   end
 
-  def parent_type
+  def parent_type(scope = nil)
     return nil unless parent
 
-    unless @parent_type ||= resource_type_collection.send(type, parent)
-      fail Puppet::ParseError, "Could not find parent resource type '#{parent}' of type #{type}"
+    unless @parent_type
+      raise "Must pass scope to parent_type when called first time" unless scope
+      unless @parent_type = scope.environment.known_resource_types.send("find_#{type}", scope.namespaces, parent)
+        fail Puppet::ParseError, "Could not find parent resource type '#{parent}' of type #{type} in #{scope.environment}"
+      end
     end
 
     @parent_type
@@ -255,7 +263,7 @@ class Puppet::Resource::Type
   end
 
   def evaluate_parent_type(resource)
-    return unless klass = parent_type and parent_resource = resource.scope.compiler.catalog.resource(:class, klass.name) || resource.scope.compiler.catalog.resource(:node, klass.name)
+    return unless klass = parent_type(resource.scope) and parent_resource = resource.scope.compiler.catalog.resource(:class, klass.name) || resource.scope.compiler.catalog.resource(:node, klass.name)
     parent_resource.evaluate unless parent_resource.evaluated?
     parent_scope(resource.scope, klass)
   end
diff --git a/lib/puppet/resource/type_collection.rb b/lib/puppet/resource/type_collection.rb
index 9ed27332d..6a933362f 100644
--- a/lib/puppet/resource/type_collection.rb
+++ b/lib/puppet/resource/type_collection.rb
@@ -134,7 +134,7 @@ class Puppet::Resource::TypeCollection
     loader.load_until(namespaces, name) { find(namespaces, name, type) }
   end
 
-  def find_node(name)
+  def find_node(namespaces, name)
     find("", name, :node)
   end
 
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index 880711066..c3855a400 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -723,6 +723,10 @@ class Type
 
   # Are we running in noop mode?
   def noop?
+    # If we're not a host_config, we're almost certainly part of
+    # Settings, and we want to ignore 'noop'
+    return false if catalog and ! catalog.host_config?
+
     if defined?(@noop)
       @noop
     else
diff --git a/lib/puppet/util/settings.rb b/lib/puppet/util/settings.rb
index cbb12a816..ca4ecda35 100644
--- a/lib/puppet/util/settings.rb
+++ b/lib/puppet/util/settings.rb
@@ -157,13 +157,6 @@ class Puppet::Util::Settings
     set_value(str, value, :cli)
   end
 
-  def without_noop
-    old_noop = value(:noop,:cli) and set_value(:noop, false, :cli) if valid?(:noop)
-    yield
-  ensure
-    set_value(:noop, old_noop, :cli) if valid?(:noop)
-  end
-
   def include?(name)
     name = name.intern if name.is_a? String
     @config.include?(name)
@@ -635,14 +628,12 @@ if @config.include?(:run_mode)
         return
       end
 
-      without_noop do
-        catalog.host_config = false
-        catalog.apply do |transaction|
-          if transaction.any_failed?
-            report = transaction.report
-            failures = report.logs.find_all { |log| log.level == :err }
-            raise "Got #{failures.length} failure(s) while initializing: #{failures.collect { |l| l.to_s }.join("; ")}"
-          end
+      catalog.host_config = false
+      catalog.apply do |transaction|
+        if transaction.any_failed?
+          report = transaction.report
+          failures = report.logs.find_all { |log| log.level == :err }
+          raise "Got #{failures.length} failure(s) while initializing: #{failures.collect { |l| l.to_s }.join("; ")}"
         end
       end
 
diff --git a/spec/unit/parser/ast/leaf_spec.rb b/spec/unit/parser/ast/leaf_spec.rb
index d21cbf573..5bdca67fa 100755
--- a/spec/unit/parser/ast/leaf_spec.rb
+++ b/spec/unit/parser/ast/leaf_spec.rb
@@ -13,63 +13,6 @@ describe Puppet::Parser::AST::Leaf do
     Puppet::Parser::AST::Leaf.new(:value => "value").should respond_to(:evaluate_match)
   end
 
-  describe "when evaluate_match is called" do
-    it "should evaluate itself" do
-      @leaf.expects(:safeevaluate).with(@scope)
-
-      @leaf.evaluate_match("value", @scope)
-    end
-
-    it "should match values by equality" do
-      @value.stubs(:==).returns(false)
-      @leaf.stubs(:safeevaluate).with(@scope).returns(@value)
-      @value.expects(:==).with("value")
-
-      @leaf.evaluate_match("value", @scope)
-    end
-
-    it "should downcase the evaluated value if wanted" do
-      @leaf.stubs(:safeevaluate).with(@scope).returns(@value)
-      @value.expects(:downcase).returns("value")
-
-      @leaf.evaluate_match("value", @scope)
-    end
-
-    it "should convert values to number" do
-      @leaf.stubs(:safeevaluate).with(@scope).returns(@value)
-      Puppet::Parser::Scope.expects(:number?).with(@value).returns(2)
-      Puppet::Parser::Scope.expects(:number?).with("23").returns(23)
-
-      @leaf.evaluate_match("23", @scope)
-    end
-
-    it "should compare 'numberized' values" do
-      @leaf.stubs(:safeevaluate).with(@scope).returns(@value)
-      two = stub_everything 'two'
-      one = stub_everything 'one'
-
-      Puppet::Parser::Scope.stubs(:number?).with(@value).returns(one)
-      Puppet::Parser::Scope.stubs(:number?).with("2").returns(two)
-
-      one.expects(:==).with(two)
-
-      @leaf.evaluate_match("2", @scope)
-    end
-
-    it "should match undef if value is an empty string" do
-      @leaf.stubs(:safeevaluate).with(@scope).returns("")
-
-      @leaf.evaluate_match(:undef, @scope).should be_true
-    end
-
-    it "should downcase the parameter value if wanted" do
-      parameter = stub 'parameter'
-      parameter.expects(:downcase).returns("value")
-
-      @leaf.evaluate_match(parameter, @scope)
-    end
-  end
-
   describe "when converting to string" do
     it "should transform its value to string" do
       value = stub 'value', :is_a? => true
@@ -107,6 +50,37 @@ describe Puppet::Parser::AST::String do
   end
 end
 
+describe Puppet::Parser::AST::Concat do
+  describe "when evaluating" do
+    before :each do
+      @scope = stub_everything 'scope'
+    end
+    it "should interpolate variables and concatenate their values" do
+      one = Puppet::Parser::AST::String.new(:value => "one")
+      one.stubs(:evaluate).returns("one ")
+      two = Puppet::Parser::AST::String.new(:value => "two")
+      two.stubs(:evaluate).returns(" two ")
+      three = Puppet::Parser::AST::String.new(:value => "three")
+      three.stubs(:evaluate).returns(" three")
+      var = Puppet::Parser::AST::Variable.new(:value => "myvar")
+      var.stubs(:evaluate).returns("foo")
+      array = Puppet::Parser::AST::Variable.new(:value => "array")
+      array.stubs(:evaluate).returns(["bar","baz"])
+      concat = Puppet::Parser::AST::Concat.new(:value => [one,var,two,array,three])
+
+      concat.evaluate(@scope).should == 'one foo two barbaz three'
+    end
+
+    it "should transform undef variables to empty string" do
+      var = Puppet::Parser::AST::Variable.new(:value => "myvar")
+      var.stubs(:evaluate).returns(:undef)
+      concat = Puppet::Parser::AST::Concat.new(:value => [var])
+
+      concat.evaluate(@scope).should == ''
+    end
+  end
+end
+
 describe Puppet::Parser::AST::Undef do
   before :each do
     @scope = stub 'scope'
diff --git a/spec/unit/parser/ast_spec.rb b/spec/unit/parser/ast_spec.rb
index b743cea2e..29dce2b9c 100644
--- a/spec/unit/parser/ast_spec.rb
+++ b/spec/unit/parser/ast_spec.rb
@@ -39,3 +39,73 @@ describe Puppet::Parser::AST do
   end
 
 end
+
+describe 'AST Generic Child' do
+  before :each do
+    @value = stub 'value'
+    class Evaluateable < Puppet::Parser::AST
+      attr_accessor :value
+      def safeevaluate(*options)
+        return value
+      end
+    end
+    @evaluateable = Evaluateable.new(:value => @value)
+    @scope = stubs 'scope'
+  end
+
+  describe "when evaluate_match is called" do
+    it "should evaluate itself" do
+      @evaluateable.expects(:safeevaluate).with(@scope)
+
+      @evaluateable.evaluate_match("value", @scope)
+    end
+
+    it "should match values by equality" do
+      @value.expects(:==).with("value").returns(true)
+
+      @evaluateable.evaluate_match("value", @scope)
+    end
+
+    it "should downcase the evaluated value if wanted" do
+      @value.expects(:downcase).returns("value")
+
+      @evaluateable.evaluate_match("value", @scope)
+    end
+
+    it "should convert values to number" do
+      Puppet::Parser::Scope.expects(:number?).with(@value).returns(2)
+      Puppet::Parser::Scope.expects(:number?).with("23").returns(23)
+
+      @evaluateable.evaluate_match("23", @scope)
+    end
+
+    it "should compare 'numberized' values" do
+      two = stub_everything 'two'
+      one = stub_everything 'one'
+
+      Puppet::Parser::Scope.stubs(:number?).with(@value).returns(one)
+      Puppet::Parser::Scope.stubs(:number?).with("2").returns(two)
+
+      one.expects(:==).with(two)
+
+      @evaluateable.evaluate_match("2", @scope)
+    end
+
+    it "should match undef if value is an empty string" do
+      @evaluateable.value = ''
+      @evaluateable.evaluate_match(:undef, @scope).should be_true
+    end
+
+    it "should downcase the parameter value if wanted" do
+      parameter = stub 'parameter'
+      parameter.expects(:downcase).returns("value")
+
+      @evaluateable.evaluate_match(parameter, @scope)
+    end
+
+    it "should not match '' if value is undef" do
+      @evaluateable.value = :undef
+      @evaluateable.evaluate_match('', @scope).should be_false
+    end
+  end
+end
diff --git a/spec/unit/resource/type_collection_spec.rb b/spec/unit/resource/type_collection_spec.rb
index 788ea418e..09643cd24 100644
--- a/spec/unit/resource/type_collection_spec.rb
+++ b/spec/unit/resource/type_collection_spec.rb
@@ -263,7 +263,7 @@ describe Puppet::Resource::TypeCollection do
   it "should use the generic 'find' method with an empty namespace to find nodes" do
     loader = Puppet::Resource::TypeCollection.new("env")
     loader.expects(:find).with("", "bar", :node)
-    loader.find_node("bar")
+    loader.find_node(stub("ignored"), "bar")
   end
 
   it "should use the 'find_or_load' method to find hostclasses" do
diff --git a/spec/unit/resource/type_spec.rb b/spec/unit/resource/type_spec.rb
index be2e6b1cf..0ef4a5166 100755
--- a/spec/unit/resource/type_spec.rb
+++ b/spec/unit/resource/type_spec.rb
@@ -346,6 +346,9 @@ describe Puppet::Resource::Type do
 
       @child = Puppet::Resource::Type.new(:hostclass, "foo", :parent => "bar")
       @code.add @child
+
+      @env   = stub "environment", :known_resource_types => @code
+      @scope = stub "scope", :environment => @env, :namespaces => [""]
     end
 
     it "should be able to define a parent" do
@@ -353,7 +356,7 @@ describe Puppet::Resource::Type do
     end
 
     it "should use the code collection to find the parent resource type" do
-      @child.parent_type.should equal(@parent)
+      @child.parent_type(@scope).should equal(@parent)
     end
 
     it "should be able to find parent nodes" do
@@ -362,16 +365,17 @@ describe Puppet::Resource::Type do
       child = Puppet::Resource::Type.new(:node, "foo", :parent => "bar")
       @code.add child
 
-      child.parent_type.should equal(parent)
+      child.parent_type(@scope).should equal(parent)
     end
 
     it "should cache a reference to the parent type" do
       @code.expects(:hostclass).once.with("bar").returns @parent
-      @child.parent_type
+      @child.parent_type(@scope)
       @child.parent_type
     end
 
     it "should correctly state when it is another type's child" do
+      @child.parent_type(@scope)
       @child.should be_child_of(@parent)
     end
 
@@ -379,6 +383,9 @@ describe Puppet::Resource::Type do
       @grandchild = Puppet::Resource::Type.new(:hostclass, "baz", :parent => "foo")
       @code.add @grandchild
 
+      @child.parent_type(@scope)
+      @grandchild.parent_type(@scope)
+
       @grandchild.should be_child_of(@parent)
     end
 
@@ -470,6 +477,8 @@ describe Puppet::Resource::Type do
       end
 
       it "should evaluate the parent's resource" do
+        @type.parent_type(@scope)
+        
         @type.evaluate_code(@resource)
 
         @scope.class_scope(@parent_type).should_not be_nil
@@ -477,6 +486,8 @@ describe Puppet::Resource::Type do
 
       it "should not evaluate the parent's resource if it has already been evaluated" do
         @parent_resource.evaluate
+        
+        @type.parent_type(@scope)
 
         @parent_resource.expects(:evaluate).never
 
@@ -484,6 +495,8 @@ describe Puppet::Resource::Type do
       end
 
       it "should use the parent's scope as its base scope" do
+        @type.parent_type(@scope)
+
         @type.evaluate_code(@resource)
 
         @scope.class_scope(@type).parent.object_id.should == @scope.class_scope(@parent_type).object_id
@@ -505,6 +518,8 @@ describe Puppet::Resource::Type do
       end
 
       it "should evaluate the parent's resource" do
+        @type.parent_type(@scope)
+
         @type.evaluate_code(@resource)
 
         @scope.class_scope(@parent_type).should_not be_nil
@@ -512,6 +527,8 @@ describe Puppet::Resource::Type do
 
       it "should not evaluate the parent's resource if it has already been evaluated" do
         @parent_resource.evaluate
+        
+        @type.parent_type(@scope)
 
         @parent_resource.expects(:evaluate).never
 
@@ -519,6 +536,8 @@ describe Puppet::Resource::Type do
       end
 
       it "should use the parent's scope as its base scope" do
+        @type.parent_type(@scope)
+
         @type.evaluate_code(@resource)
 
         @scope.class_scope(@type).parent.object_id.should == @scope.class_scope(@parent_type).object_id
@@ -528,7 +547,7 @@ describe Puppet::Resource::Type do
 
   describe "when creating a resource" do
     before do
-      @node = Puppet::Node.new("foo")
+      @node = Puppet::Node.new("foo", :environment => 'env')
       @compiler = Puppet::Parser::Compiler.new(@node)
       @scope = Puppet::Parser::Scope.new(:compiler => @compiler)
 
@@ -538,6 +557,8 @@ describe Puppet::Resource::Type do
       @code = Puppet::Resource::TypeCollection.new("env")
       @code.add @top
       @code.add @middle
+      
+      @node.environment.stubs(:known_resource_types).returns(@code)
     end
 
     it "should create a resource instance" do
diff --git a/spec/unit/type_spec.rb b/spec/unit/type_spec.rb
index 71d415dc6..487750e52 100755
--- a/spec/unit/type_spec.rb
+++ b/spec/unit/type_spec.rb
@@ -123,6 +123,22 @@ describe Puppet::Type do
     Puppet::Type.type(:mount).new(:name => "foo").type.should == :mount
   end
 
+  it "should use any provided noop value" do
+    Puppet::Type.type(:mount).new(:name => "foo", :noop => true).must be_noop
+  end
+
+  it "should use the global noop value if none is provided" do
+    Puppet[:noop] = true
+    Puppet::Type.type(:mount).new(:name => "foo").must be_noop
+  end
+
+  it "should not be noop if in a non-host_config catalog" do
+    resource = Puppet::Type.type(:mount).new(:name => "foo")
+    catalog = Puppet::Resource::Catalog.new
+    catalog.add_resource resource
+    resource.should_not be_noop
+  end
+
   describe "when creating an event" do
     before do
       @resource = Puppet::Type.type(:mount).new :name => "foo"
diff --git a/spec/unit/util/settings_spec.rb b/spec/unit/util/settings_spec.rb
index 81cab79e7..7bca44b76 100755
--- a/spec/unit/util/settings_spec.rb
+++ b/spec/unit/util/settings_spec.rb
@@ -1096,27 +1096,4 @@ describe Puppet::Util::Settings do
 
     it "should cache the result"
   end
-
-  describe "#without_noop" do
-    before do
-      @settings = Puppet::Util::Settings.new
-      @settings.setdefaults :main, :noop => [true, ""]
-    end
-
-    it "should set noop to false for the duration of the block" do
-      @settings.without_noop { @settings.value(:noop, :cli).should be_false }
-    end
-
-    it "should ensure that noop is returned to its previous value" do
-      @settings.without_noop { raise } rescue nil
-      @settings.value(:noop, :cli).should be_true
-    end
-
-    it "should work even if no 'noop' setting is available" do
-      settings = Puppet::Util::Settings.new
-      stuff = nil
-      settings.without_noop { stuff = "yay" }
-      stuff.should == "yay"
-    end
-  end
 end