Skip to content
Browse files

Merge remote-tracking branch 'zaphod42/bug/2.7.x/8174-incorrect-warni…

…ng-about-deprecated-scoping' into 2.7.x

* zaphod42/bug/2.7.x/8174-incorrect-warning-about-deprecated-scoping:
  Fixed old log test to match new autoflush behavior
  Make sure the log file writable
  Default autoflushing of log files to true
  Created test that shows enc warning problem
  Increased test coverage of scoping rules
  Use more descriptive terms for the differing scopes
  Make new scoping look through inherited scopes
  Add tests for mixed inheritence/inclusion
  Remove dynamic option for lookupvar
  Implement newlookupvar() to replace dynamic scope
  Tests for deprecation warnings for dynamic scoping
  • Loading branch information...
2 parents a30b45f + fb96747 commit 849a882d0914e305547d566471f1027102bbfba5 @jeffweiss jeffweiss committed Apr 12, 2012
View
80 acceptance/tests/language/ticket_8174_enc_causes_spurious_deprecation_warnings.rb
@@ -0,0 +1,80 @@
+test_name "#8174: incorrect warning about deprecated scoping"
+
+testdir = master.tmpdir('scoping_deprecation')
+
+create_remote_file(master, "#{testdir}/puppet.conf", <<END)
+[main]
+node_terminus = exec
+external_nodes = "#{testdir}/enc"
+manifest = "#{testdir}/site.pp"
+modulepath = "#{testdir}/modules"
+END
+
+on master, "mkdir -p #{testdir}/modules/a/manifests"
+
+create_remote_file(master, "#{testdir}/enc", <<-PP)
+#!/usr/bin/env sh
+
+cat <<END
+---
+classes:
+ a
+parameters:
+ enc_var: "Set from ENC."
+END
+exit 0
+PP
+
+create_remote_file(master, "#{testdir}/site.pp", <<-PP)
+$top_scope = "set from site.pp"
+node default {
+ $node_var = "in node"
+}
+PP
+create_remote_file(master, "#{testdir}/modules/a/manifests/init.pp", <<-PP)
+class a {
+ $locally = "locally declared"
+ $dynamic_for_b = "dynamic and declared in a"
+ notify { "fqdn from facts": message => $fqdn }
+ notify { "locally declared var": message => $locally }
+ notify { "var via enc": message => $enc_var }
+ notify { "declared top scope": message => $top_scope }
+ notify { "declared node": message => $node_var }
+
+ include a::b
+}
+PP
+create_remote_file(master, "#{testdir}/modules/a/manifests/b.pp", <<-PP)
+class a::b {
+ notify { "dynamic from elsewhere": message => $dynamic_for_b }
+}
+PP
+
+on master, "chown -R root:puppet #{testdir}"
+on master, "chmod -R g+rwX #{testdir}"
+on master, "chmod -R a+x #{testdir}/enc"
+on master, "touch #{testdir}/log"
+on master, "chown puppet #{testdir}/log"
+
+assert_log_on_master_contains = lambda do |string|
+ on master, "grep '#{string}' #{testdir}/log"
+end
+
+assert_log_on_master_does_not_contain = lambda do |string|
+ on master, "grep -v '#{string}' #{testdir}/log"
+end
+
+with_master_running_on(master, "--config #{testdir}/puppet.conf --debug --verbose --daemonize --dns_alt_names=\"puppet,$(hostname -s),$(hostname -f)\" --autosign true --logdest #{testdir}/log") do
+ agents.each do |agent|
+ run_agent_on(agent, "--no-daemonize --onetime --server #{master}")
+ end
+
+ assert_log_on_master_contains['Dynamic lookup of $dynamic_for_b']
+ assert_log_on_master_does_not_contain['Dynamic lookup of $fqdn']
+ assert_log_on_master_does_not_contain['Dynamic lookup of $locally']
+ assert_log_on_master_does_not_contain['Dynamic lookup of $enc_var']
+ assert_log_on_master_does_not_contain['Dynamic lookup of $top_scope']
+ assert_log_on_master_does_not_contain['Dynamic lookup of $node_var']
+end
+
+on master, "rm -rf #{testdir}"
View
2 lib/puppet/defaults.rb
@@ -15,7 +15,7 @@ module Puppet
setdefaults(:main,
:trace => [false, "Whether to print stack traces on some errors"],
:autoflush => {
- :default => false,
+ :default => true,
:desc => "Whether log files should always flush to disk.",
:hook => proc { |value| Log.autoflush = value }
},
View
51 lib/puppet/parser/scope.rb
@@ -21,7 +21,7 @@ class Puppet::Parser::Scope
attr_accessor :source, :resource
attr_accessor :base, :keyword
attr_accessor :top, :translated, :compiler
- attr_accessor :parent, :dynamic
+ attr_accessor :parent
attr_reader :namespaces
# thin wrapper around an ephemeral
@@ -222,27 +222,60 @@ def qualified_scope(classname)
private :qualified_scope
- # Look up a variable. The simplest value search we do.
+ # Look up a variable with traditional scoping and then with new scoping. If
+ # the answers differ then print a deprecation warning.
def lookupvar(name, options = {})
+ dynamic_value = dynamic_lookupvar(name,options)
+ twoscope_value = twoscope_lookupvar(name,options)
+ if dynamic_value != twoscope_value
+ location = (options[:file] && options[:line]) ? " at #{options[:file]}:#{options[:line]}" : ''
+ Puppet.deprecation_warning "Dynamic lookup of $#{name}#{location} is deprecated. Support will be removed in a later version of Puppet. Use a fully-qualified variable name (e.g., $classname::variable) or parameterized classes."
+ end
+ dynamic_value
+ end
+
+ # Look up a variable. The simplest value search we do.
+ def twoscope_lookupvar(name, options = {})
+ # Save the originating scope for the request
+ options[:origin] = self unless options[:origin]
+ table = ephemeral?(name) ? @ephemeral.last : @symtable
+ if name =~ /^(.*)::(.+)$/
+ begin
+ qualified_scope($1).twoscope_lookupvar($2,options.merge({:origin => nil}))
+ rescue RuntimeError => e
+ location = (options[:file] && options[:line]) ? " at #{options[:file]}:#{options[:line]}" : ''
+ warning "Could not look up qualified variable '#{name}'; #{e.message}#{location}"
+ :undefined
+ end
+ # If the value is present and either we are top/node scope or originating scope...
+ elsif (ephemeral_include?(name) or table.include?(name)) and (compiler and self == compiler.topscope or (self.resource and self.resource.type == "Node") or self == options[:origin])
+ table[name]
+ elsif resource and resource.resource_type and resource.resource_type.respond_to?("parent") and parent_type = resource.resource_type.parent
+ class_scope(parent_type).twoscope_lookupvar(name,options.merge({:origin => nil}))
+ elsif parent
+ parent.twoscope_lookupvar(name,options)
+ else
+ :undefined
+ end
+ end
+
+ # Look up a variable. The simplest value search we do.
+ def dynamic_lookupvar(name, options = {})
table = ephemeral?(name) ? @ephemeral.last : @symtable
# If the variable is qualified, then find the specified scope and look the variable up there instead.
if name =~ /^(.*)::(.+)$/
begin
- qualified_scope($1).lookupvar($2,options)
+ qualified_scope($1).dynamic_lookupvar($2,options)
rescue RuntimeError => e
location = (options[:file] && options[:line]) ? " at #{options[:file]}:#{options[:line]}" : ''
warning "Could not look up qualified variable '#{name}'; #{e.message}#{location}"
:undefined
end
elsif ephemeral_include?(name) or table.include?(name)
# We can't use "if table[name]" here because the value might be false
- if options[:dynamic] and self != compiler.topscope
- location = (options[:file] && options[:line]) ? " at #{options[:file]}:#{options[:line]}" : ''
- Puppet.deprecation_warning "Dynamic lookup of $#{name}#{location} is deprecated. Support will be removed in Puppet 2.8. Use a fully-qualified variable name (e.g., $classname::variable) or parameterized classes."
- end
table[name]
elsif parent
- parent.lookupvar(name,options.merge(:dynamic => (dynamic || options[:dynamic])))
+ parent.dynamic_lookupvar(name,options)
else
:undefined
end
@@ -374,7 +407,7 @@ def unset_ephemeral_var(level=:all)
# check if name exists in one of the ephemeral scope.
def ephemeral_include?(name)
- @ephemeral.reverse.each do |eph|
+ @ephemeral.reverse_each do |eph|
return true if eph.include?(name)
end
false
View
2 lib/puppet/resource/type.rb
@@ -66,7 +66,7 @@ def evaluate_code(resource)
static_parent = evaluate_parent_type(resource)
scope = static_parent || resource.scope
- scope = scope.newscope(:namespace => namespace, :source => self, :resource => resource, :dynamic => !static_parent) unless resource.title == :main
+ scope = scope.newscope(:namespace => namespace, :source => self, :resource => resource) unless resource.title == :main
scope.compiler.add_class(name) unless definition?
set_resource_parameters(resource, scope)
View
295 spec/unit/parser/scope_spec.rb
@@ -3,11 +3,11 @@
describe Puppet::Parser::Scope do
before :each do
- @topscope = Puppet::Parser::Scope.new
# This is necessary so we don't try to use the compiler to discover our parent.
- @topscope.parent = nil
@scope = Puppet::Parser::Scope.new
@scope.compiler = Puppet::Parser::Compiler.new(Puppet::Node.new("foo"))
+ @scope.source = Puppet::Resource::Type.new(:node, :foo)
+ @topscope = @scope.compiler.topscope
@scope.parent = @topscope
end
@@ -92,14 +92,6 @@
Puppet::Parser::Scope.new.singleton_class.ancestors.should be_include(mod)
end
-
- it "should remember if it is dynamic" do
- (!!Puppet::Parser::Scope.new(:dynamic => true).dynamic).should == true
- end
-
- it "should assume it is not dynamic" do
- (!Puppet::Parser::Scope.new.dynamic).should == true
- end
end
describe "when looking up a variable" do
@@ -128,10 +120,20 @@
@scope.lookupvar("var").should == "childval"
end
+ it "should be able to look up intermediary variables in parent scopes (DEPRECATED)" do
+ Puppet.expects(:deprecation_warning)
+ thirdscope = Puppet::Parser::Scope.new
+ thirdscope.parent = @scope
+ thirdscope.source = Puppet::Resource::Type.new(:hostclass, :foo, :module_name => "foo")
+ @scope.source = Puppet::Resource::Type.new(:hostclass, :bar, :module_name => "bar")
+
+ @topscope.setvar("var2","parentval")
+ @scope.setvar("var2","childval")
+ thirdscope.lookupvar("var2").should == "childval"
+ end
+
describe "and the variable is qualified" do
- before do
- @compiler = Puppet::Parser::Compiler.new(Puppet::Node.new("foonode"))
- @scope.compiler = @compiler
+ before :each do
@known_resource_types = @scope.known_resource_types
end
@@ -151,6 +153,7 @@ def create_class_scope(name)
end
it "should be able to look up explicitly fully qualified variables from main" do
+ Puppet.expects(:deprecation_warning).never
other_scope = create_class_scope("")
other_scope.setvar("othervar", "otherval")
@@ -159,6 +162,7 @@ def create_class_scope(name)
end
it "should be able to look up explicitly fully qualified variables from other scopes" do
+ Puppet.expects(:deprecation_warning).never
other_scope = create_class_scope("other")
other_scope.setvar("var", "otherval")
@@ -167,6 +171,7 @@ def create_class_scope(name)
end
it "should be able to look up deeply qualified variables" do
+ Puppet.expects(:deprecation_warning).never
other_scope = create_class_scope("other::deep::klass")
other_scope.setvar("var", "otherval")
@@ -175,35 +180,295 @@ def create_class_scope(name)
end
it "should return ':undefined' for qualified variables that cannot be found in other classes" do
+ Puppet.expects(:deprecation_warning).never
other_scope = create_class_scope("other::deep::klass")
@scope.lookupvar("other::deep::klass::var").should == :undefined
end
it "should warn and return ':undefined' for qualified variables whose classes have not been evaluated" do
+ Puppet.expects(:deprecation_warning).never
klass = newclass("other::deep::klass")
- @scope.expects(:warning)
+ @scope.expects(:warning).at_least_once
@scope.lookupvar("other::deep::klass::var").should == :undefined
end
it "should warn and return ':undefined' for qualified variables whose classes do not exist" do
- @scope.expects(:warning)
+ Puppet.expects(:deprecation_warning).never
+ @scope.expects(:warning).at_least_once
@scope.lookupvar("other::deep::klass::var").should == :undefined
end
it "should return ':undefined' when asked for a non-string qualified variable from a class that does not exist" do
+ Puppet.expects(:deprecation_warning).never
@scope.stubs(:warning)
@scope.lookupvar("other::deep::klass::var").should == :undefined
end
it "should return ':undefined' when asked for a non-string qualified variable from a class that has not been evaluated" do
+ Puppet.expects(:deprecation_warning).never
@scope.stubs(:warning)
klass = newclass("other::deep::klass")
@scope.lookupvar("other::deep::klass::var").should == :undefined
end
end
end
+ describe "when mixing inheritence and inclusion" do
+ let(:catalog) { Puppet::Parser::Compiler.compile(Puppet::Node.new('foonode')) }
+
+ def expect_the_message_to_be(message)
+ Puppet[:code] = yield
+ catalog = Puppet::Parser::Compiler.compile(Puppet::Node.new('foonode'))
+ catalog.resource('Notify', 'something')[:message].should == message
+ end
+
+ context "deprecated scoping" do
+ before :each do
+ Puppet.expects(:deprecation_warning)
+ end
+
+ it "prefers values in its included scope over those from the node (DEPRECATED)" do
+ expect_the_message_to_be('baz_msg') do <<-MANIFEST
+ node default {
+ $var = "node_msg"
+ include foo
+ }
+ class baz {
+ $var = "baz_msg"
+ include bar
+ }
+ class foo inherits baz {
+ }
+ class bar {
+ notify { 'something': message => $var, }
+ }
+ MANIFEST
+ end
+ end
+
+ it "finds values in its included scope (DEPRECATED)" do
+ expect_the_message_to_be('baz_msg') do <<-MANIFEST
+ node default {
+ include baz
+ }
+ class foo {
+ }
+ class bar inherits foo {
+ notify { 'something': message => $var, }
+ }
+ class baz {
+ $var = "baz_msg"
+ include bar
+ }
+ MANIFEST
+ end
+ end
+
+ it "recognizes a dynamically scoped boolean (DEPRECATED)" do
+ expect_the_message_to_be(true) do <<-MANIFEST
+ node default {
+ $var = false
+ include baz
+ }
+ class foo {
+ }
+ class bar inherits foo {
+ notify { 'something': message => $var, }
+ }
+ class baz {
+ $var = true
+ include bar
+ }
+ MANIFEST
+ end
+ end
+ end
+
+ context "supported scoping" do
+ before :each do
+ Puppet.expects(:deprecation_warning).never
+ end
+
+ it "should find values in its local scope" do
+ expect_the_message_to_be('local_msg') do <<-MANIFEST
+ node default {
+ include baz
+ }
+ class foo {
+ }
+ class bar inherits foo {
+ $var = "local_msg"
+ notify { 'something': message => $var, }
+ }
+ class baz {
+ include bar
+ }
+ MANIFEST
+ end
+ end
+
+ it "should find values in its inherited scope" do
+ expect_the_message_to_be('foo_msg') do <<-MANIFEST
+ node default {
+ include baz
+ }
+ class foo {
+ $var = "foo_msg"
+ }
+ class bar inherits foo {
+ notify { 'something': message => $var, }
+ }
+ class baz {
+ include bar
+ }
+ MANIFEST
+ end
+ end
+
+ it "prefers values in its inherited scope over those in the node (with intermediate inclusion)" do
+ expect_the_message_to_be('foo_msg') do <<-MANIFEST
+ node default {
+ $var = "node_msg"
+ include baz
+ }
+ class foo {
+ $var = "foo_msg"
+ }
+ class bar inherits foo {
+ notify { 'something': message => $var, }
+ }
+ class baz {
+ include bar
+ }
+ MANIFEST
+ end
+ end
+
+ it "prefers values in its inherited scope over those in the node (without intermediate inclusion)" do
+ expect_the_message_to_be('foo_msg') do <<-MANIFEST
+ node default {
+ $var = "node_msg"
+ include bar
+ }
+ class foo {
+ $var = "foo_msg"
+ }
+ class bar inherits foo {
+ notify { 'something': message => $var, }
+ }
+ MANIFEST
+ end
+ end
+
+ it "prefers values in its inherited scope over those from where it is included" do
+ expect_the_message_to_be('foo_msg') do <<-MANIFEST
+ node default {
+ include baz
+ }
+ class foo {
+ $var = "foo_msg"
+ }
+ class bar inherits foo {
+ notify { 'something': message => $var, }
+ }
+ class baz {
+ $var = "baz_msg"
+ include bar
+ }
+ MANIFEST
+ end
+ end
+
+ it "does not used variables from classes included in the inherited scope" do
+ expect_the_message_to_be('node_msg') do <<-MANIFEST
+ node default {
+ $var = "node_msg"
+ include bar
+ }
+ class quux {
+ $var = "quux_msg"
+ }
+ class foo inherits quux {
+ }
+ class baz {
+ include foo
+ }
+ class bar inherits baz {
+ notify { 'something': message => $var, }
+ }
+ MANIFEST
+ end
+ end
+
+ it "does not use a variable from a scope lexically enclosing it" do
+ expect_the_message_to_be('node_msg') do <<-MANIFEST
+ node default {
+ $var = "node_msg"
+ include other::bar
+ }
+ class other {
+ $var = "other_msg"
+ class bar {
+ notify { 'something': message => $var, }
+ }
+ }
+ MANIFEST
+ end
+ end
+
+ it "finds values in its node scope" do
+ expect_the_message_to_be('node_msg') do <<-MANIFEST
+ node default {
+ $var = "node_msg"
+ include baz
+ }
+ class foo {
+ }
+ class bar inherits foo {
+ notify { 'something': message => $var, }
+ }
+ class baz {
+ include bar
+ }
+ MANIFEST
+ end
+ end
+
+ it "finds values in its top scope" do
+ expect_the_message_to_be('top_msg') do <<-MANIFEST
+ $var = "top_msg"
+ node default {
+ include baz
+ }
+ class foo {
+ }
+ class bar inherits foo {
+ notify { 'something': message => $var, }
+ }
+ class baz {
+ include bar
+ }
+ MANIFEST
+ end
+ end
+
+ it "prefers variables from the node over those in the top scope" do
+ expect_the_message_to_be('node_msg') do <<-MANIFEST
+ $var = "top_msg"
+ node default {
+ $var = "node_msg"
+ include foo
+ }
+ class foo {
+ notify { 'something': message => $var, }
+ }
+ MANIFEST
+ end
+ end
+ end
+ end
+
describe "when setvar is called with append=true" do
it "should raise error if the variable is already defined in this scope" do
@scope.setvar("var","1", :append => false)
View
4 spec/unit/resource/type_spec.rb
@@ -238,7 +238,7 @@ def double_convert
describe "when setting its parameters in the scope" do
before do
- @scope = Puppet::Parser::Scope.new(:compiler => stub("compiler", :environment => Puppet::Node::Environment.new), :source => stub("source"))
+ @scope = Puppet::Parser::Scope.new(:compiler => Puppet::Parser::Compiler.new(Puppet::Node.new("foo")), :source => stub("source"))
@resource = Puppet::Parser::Resource.new(:foo, "bar", :scope => @scope)
@type = Puppet::Resource::Type.new(:hostclass, "foo")
end
@@ -435,7 +435,7 @@ def double_convert
it "should set all of its parameters in a subscope" do
subscope = stub 'subscope', :compiler => @compiler
- @scope.expects(:newscope).with(:source => @type, :dynamic => true, :namespace => 'foo', :resource => @resource).returns subscope
+ @scope.expects(:newscope).with(:source => @type, :namespace => 'foo', :resource => @resource).returns subscope
@type.expects(:set_resource_parameters).with(@resource, subscope)
@type.evaluate_code(@resource)
View
4 spec/unit/util/log/destinations_spec.rb
@@ -29,8 +29,8 @@
@class = Puppet::Util::Log.desttypes[:file]
end
- it "should default to autoflush false" do
- @class.new('/tmp/log').autoflush.should == false
+ it "should default to automatically flush log output" do
+ @class.new('/tmp/log').autoflush.should == true
end
describe "when matching" do
View
12 test/util/log.rb
@@ -178,17 +178,17 @@ def test_autoflush
Puppet::Util::Log.close(:console)
Puppet::Util::Log.newdestination(file)
Puppet.warning "A test"
- assert(File.read(file) !~ /A test/, "File defualted to autoflush")
- Puppet::Util::Log.flush
- assert(File.read(file) =~ /A test/, "File did not flush")
+ assert(File.read(file) =~ /A test/)
Puppet::Util::Log.close(file)
- # Now try one with autoflush enabled
- Puppet[:autoflush] = true
+ # Now try one with autoflush disabled
+ Puppet[:autoflush] = false
file = tempfile
Puppet::Util::Log.newdestination(file)
Puppet.warning "A test"
- assert(File.read(file) =~ /A test/, "File did not autoflush")
+ assert(File.read(file) !~ /A test/)
+ Puppet::Util::Log.flush
+ assert(File.read(file) =~ /A test/)
Puppet::Util::Log.close(file)
end

0 comments on commit 849a882

Please sign in to comment.
Something went wrong with that request. Please try again.