From 6dbd4771265173a9d4c3e7756c35c9ca371ca394 Mon Sep 17 00:00:00 2001 From: Jesse Wolfe Date: Wed, 28 Jul 2010 14:54:23 -0700 Subject: [#4397]+[#4344] Move type-name resolution out of Puppet::Resource into the AST resources. Move type-name resolution out of Puppet::Resource into the AST resources. Move find_resource_type out of Puppet::Resource into Scope Thus, never pass unqualified type names to Puppet::Resource objects. Thus, Puppet::Resource objects don't need the namespace property, and Puppet::Resource objects never consult the harddrive to look for .pp files that might contain their type definitions, Thus, performance is improved. Also removes the temporary fix for #4257 that caused #4397 (The code was too eager to look for a class in the topscope) Paired-With: Paul Berry Signed-off-by: Jesse Wolfe --- spec/integration/parser/compiler_spec.rb | 43 ++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) (limited to 'spec/integration/parser') diff --git a/spec/integration/parser/compiler_spec.rb b/spec/integration/parser/compiler_spec.rb index 83bbf9500..9158ad1c2 100755 --- a/spec/integration/parser/compiler_spec.rb +++ b/spec/integration/parser/compiler_spec.rb @@ -26,4 +26,47 @@ describe Puppet::Parser::Compiler do @compiler.catalog.version.should == version end + + describe "when resolving class references" do + it "should favor local scope, even if there's an included class in topscope" do + Puppet[:code] = <<-PP + class experiment { + class baz { + } + notify {"x" : require => Class[Baz] } + } + class baz { + } + include baz + include experiment + include experiment::baz + PP + + catalog = Puppet::Parser::Compiler.compile(Puppet::Node.new("mynode")) + + notify_resource = catalog.resource( "Notify[x]" ) + + notify_resource[:require].title.should == "Experiment::Baz" + end + + it "should favor local scope, even if there's an unincluded class in topscope" do + Puppet[:code] = <<-PP + class experiment { + class baz { + } + notify {"x" : require => Class[Baz] } + } + class baz { + } + include experiment + include experiment::baz + PP + + catalog = Puppet::Parser::Compiler.compile(Puppet::Node.new("mynode")) + + notify_resource = catalog.resource( "Notify[x]" ) + + notify_resource[:require].title.should == "Experiment::Baz" + end + end end -- cgit From caca187dffbd6e628d7eda599c7f2939dd05fafc Mon Sep 17 00:00:00 2001 From: Paul Berry Date: Fri, 13 Aug 2010 15:25:17 -0700 Subject: Moved perform_initial_import from Puppet::Resource::TypeCollection to Puppet::Node::Environment. This change is part of an ongoing effort to remove functionality from TypeCollection that is not related to keeping track of a collection of types. This reduces TypeCollection's linkage to the environment, which is a step toward decoupling it from the type loading mechanism. Also, added a spec test to verify that TypeCollection.version is correctly recomputed when types are re-imported. --- spec/integration/parser/compiler_spec.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'spec/integration/parser') diff --git a/spec/integration/parser/compiler_spec.rb b/spec/integration/parser/compiler_spec.rb index 9158ad1c2..67e8e5e62 100755 --- a/spec/integration/parser/compiler_spec.rb +++ b/spec/integration/parser/compiler_spec.rb @@ -69,4 +69,15 @@ describe Puppet::Parser::Compiler do notify_resource[:require].title.should == "Experiment::Baz" end end + + it "should recompute the version after input files are re-parsed" do + Puppet[:code] = 'class foo { }' + Time.stubs(:now).returns(1) + node = Puppet::Node.new('mynode') + Puppet::Parser::Compiler.compile(node).version.should == 1 + Time.stubs(:now).returns(2) + Puppet::Parser::Compiler.compile(node).version.should == 1 # no change because files didn't change + Puppet::Resource::TypeCollection.any_instance.stubs(:stale?).returns(true).then.returns(false) # pretend change + Puppet::Parser::Compiler.compile(node).version.should == 2 + end end -- cgit From 4da88fb4cd57871f16649d50572240ac3f7420f0 Mon Sep 17 00:00:00 2001 From: Paul Berry Date: Fri, 13 Aug 2010 15:43:34 -0700 Subject: [#4496]+[#4521]+[#4522] Add structures to the AST to represent type definitions (classes, definitions, and nodes). Previously, type definitions were not represented directly in the AST. Instead, the parser would instantiate types and insert them into known_resource_types as soon as they were parsed. This made it difficult to distinguish which types had come from the file that was just parsed and which types had been loaded previously, which led to bug 4496. A side-effect of this change is that the user is no longer allowed to define types inside of conditional constructs (such as if/else). This was allowed before but had unexpected semantics (bugs 4521 and 4522). It is still possible, however, to place an "include" statement inside a conditional construct, and have that "include" statement trigger the autoloading of a file that instantiates types. --- spec/integration/parser/collector_spec.rb | 2 +- spec/integration/parser/compiler_spec.rb | 11 +++++++++++ spec/integration/parser/parser_spec.rb | 8 +++++--- 3 files changed, 17 insertions(+), 4 deletions(-) (limited to 'spec/integration/parser') diff --git a/spec/integration/parser/collector_spec.rb b/spec/integration/parser/collector_spec.rb index 73273c909..b1cfc51c7 100755 --- a/spec/integration/parser/collector_spec.rb +++ b/spec/integration/parser/collector_spec.rb @@ -17,7 +17,7 @@ describe Puppet::Parser::Collector do def query(text) code = "File <| #{text} |>" parser = Puppet::Parser::Parser.new(@scope.compiler) - parser.parse(code).hostclass("").code[0].query + return parser.parse(code).code[0].query end {true => [%{title == "/tmp/testing"}, %{(title == "/tmp/testing")}, %{group == bin}, diff --git a/spec/integration/parser/compiler_spec.rb b/spec/integration/parser/compiler_spec.rb index 67e8e5e62..266347c60 100755 --- a/spec/integration/parser/compiler_spec.rb +++ b/spec/integration/parser/compiler_spec.rb @@ -80,4 +80,15 @@ describe Puppet::Parser::Compiler do Puppet::Resource::TypeCollection.any_instance.stubs(:stale?).returns(true).then.returns(false) # pretend change Puppet::Parser::Compiler.compile(node).version.should == 2 end + + it "should not allow classes inside conditional constructs" do + Puppet[:code] = <<-PP + if true { + class foo { + } + } + PP + + lambda { Puppet::Parser::Compiler.compile(Puppet::Node.new("mynode")) }.should raise_error(Puppet::Error) + end end diff --git a/spec/integration/parser/parser_spec.rb b/spec/integration/parser/parser_spec.rb index 7b85bcacb..0d9aa51e1 100755 --- a/spec/integration/parser/parser_spec.rb +++ b/spec/integration/parser/parser_spec.rb @@ -11,7 +11,7 @@ describe Puppet::Parser::Parser do end def result_instance - @result.hostclass("").code[0] + @result.code[0] end def matches?(string) @@ -44,7 +44,7 @@ describe Puppet::Parser::Parser do end def result_instance - @result.hostclass("").code[0] + @result.code[0] end def matches?(string) @@ -85,7 +85,9 @@ describe Puppet::Parser::Parser do class test {} """) - ast.hostclass("test").doc.should == "comment\n" + ast.code[0].should be_a(Puppet::Parser::AST::Hostclass) + ast.code[0].name.should == 'test' + ast.code[0].instantiate('')[0].doc.should == "comment\n" end end -- cgit From 25048ecc40db746f7e88bb6c5e1fc4f2c0150a4f Mon Sep 17 00:00:00 2001 From: Paul Berry Date: Wed, 1 Sep 2010 14:18:41 -0700 Subject: [#4685] Classes, defines, and nodes allowed inside of non-evaluated conditionals Previously, ASTArray#evaluate() was responsible for checking whether the user had tried to declare a class, define, or node in a prohibited location (such as a conditional construct). This meant that errors would only be reported to the user if the conditional code was actually evaluated. Moved the checking into the parser, so that errors are always reported. --- spec/integration/parser/compiler_spec.rb | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) (limited to 'spec/integration/parser') diff --git a/spec/integration/parser/compiler_spec.rb b/spec/integration/parser/compiler_spec.rb index 266347c60..ffff4d845 100755 --- a/spec/integration/parser/compiler_spec.rb +++ b/spec/integration/parser/compiler_spec.rb @@ -81,7 +81,7 @@ describe Puppet::Parser::Compiler do Puppet::Parser::Compiler.compile(node).version.should == 2 end - it "should not allow classes inside conditional constructs" do + it "should not allow classes inside evaluated conditional constructs" do Puppet[:code] = <<-PP if true { class foo { @@ -91,4 +91,15 @@ describe Puppet::Parser::Compiler do lambda { Puppet::Parser::Compiler.compile(Puppet::Node.new("mynode")) }.should raise_error(Puppet::Error) end + + it "should not allow classes inside unevaluated conditional constructs" do + Puppet[:code] = <<-PP + if false { + class foo { + } + } + PP + + lambda { Puppet::Parser::Compiler.compile(Puppet::Node.new("mynode")) }.should raise_error(Puppet::Error) + end end -- cgit From ce9bf1edcaac4901de6e0a7da413d1742d216eb0 Mon Sep 17 00:00:00 2001 From: Paul Berry Date: Tue, 7 Sep 2010 18:01:42 -0700 Subject: Modified the error message that is generated when a class, definition, or node occurs in a conditional construct so that it contains the proper line number. --- spec/integration/parser/compiler_spec.rb | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) (limited to 'spec/integration/parser') diff --git a/spec/integration/parser/compiler_spec.rb b/spec/integration/parser/compiler_spec.rb index ffff4d845..df310ac07 100755 --- a/spec/integration/parser/compiler_spec.rb +++ b/spec/integration/parser/compiler_spec.rb @@ -81,15 +81,23 @@ describe Puppet::Parser::Compiler do Puppet::Parser::Compiler.compile(node).version.should == 2 end - it "should not allow classes inside evaluated conditional constructs" do - Puppet[:code] = <<-PP - if true { - class foo { + ['class', 'define', 'node'].each do |thing| + it "should not allow #{thing} inside evaluated conditional constructs" do + Puppet[:code] = <<-PP + if true { + #{thing} foo { + } + notify { decoy: } } - } - PP + PP - lambda { Puppet::Parser::Compiler.compile(Puppet::Node.new("mynode")) }.should raise_error(Puppet::Error) + begin + Puppet::Parser::Compiler.compile(Puppet::Node.new("mynode")) + raise "compilation should have raised Puppet::Error" + rescue Puppet::Error => e + e.message.should =~ /at line 2/ + end + end end it "should not allow classes inside unevaluated conditional constructs" do -- cgit