diff --git a/Rakefile b/Rakefile index a28b868fd18..f242afa2b5e 100644 --- a/Rakefile +++ b/Rakefile @@ -9,6 +9,8 @@ $LOAD_PATH << File.join(File.dirname(__FILE__), 'tasks') begin require 'rubygems' require 'rubygems/package_task' + require 'rspec' + require 'rspec/core/rake_task' rescue LoadError # Users of older versions of Rake (0.8.7 for example) will not necessarily # have rubygems installed, or the newer rubygems package_task for that @@ -18,8 +20,6 @@ rescue LoadError end require 'rake' -require 'rspec' -require "rspec/core/rake_task" Dir['tasks/**/*.rake'].each { |t| load t } Dir['ext/packaging/tasks/**/*'].sort.each { |t| load t } @@ -60,7 +60,9 @@ task :default do sh %{rake -T} end -RSpec::Core::RakeTask.new do |t| - t.pattern ='spec/{unit,integration}/**/*.rb' - t.fail_on_error = true +if defined?(RSpec::Core::RakeTask) + RSpec::Core::RakeTask.new do |t| + t.pattern ='spec/{unit,integration}/**/*.rb' + t.fail_on_error = true + end end diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb index b6e240b22f5..6e40ff34123 100644 --- a/lib/puppet/defaults.rb +++ b/lib/puppet/defaults.rb @@ -1479,14 +1479,27 @@ module Puppet } ) - # This doesn't actually work right now. - define_settings(:parser, :templatedir => { :default => "$vardir/templates", :type => :directory, :desc => "Where Puppet looks for template files. Can be a list of colon-separated directories.", + }, + + :allow_variables_with_dashes => { + :default => false, + :desc => <<-'EOT' +Permit hyphens (`-`) in variable names and issue deprecation warnings about +them. This setting **should always be `false`;** setting it to `true` +will cause subtle and wide-ranging bugs. It will be removed in a future version. + +Hyphenated variables caused major problems in the language, but were allowed +between Puppet 2.7.3 and 2.7.14. If you used them during this window, we +apologize for the inconvenience --- you can temporarily set this to `true` +in order to upgrade, and can rename your variables at your leisure. Please +revert it to `false` after you have renamed all affected variables. +EOT } ) define_settings(:puppetdoc, diff --git a/lib/puppet/parser/lexer.rb b/lib/puppet/parser/lexer.rb index 0934a59e7e6..302f9eed54c 100644 --- a/lib/puppet/parser/lexer.rb +++ b/lib/puppet/parser/lexer.rb @@ -100,6 +100,11 @@ def add_tokens(hash) def sort_tokens @string_tokens.sort! { |a, b| b.string.length <=> a.string.length } end + + # Yield each token name and value in turn. + def each + @tokens.each {|name, value| yield name, value } + end end TOKENS = TokenList.new @@ -232,10 +237,30 @@ def (TOKENS[:DQCONT]).acceptable?(context={}) end #:startdoc: + TOKENS.add_token :DOLLAR_VAR_WITH_DASH, %r{\$(?:::)?(?:[-\w]+::)*[-\w]+} do |lexer, value| + lexer.warn_if_variable_has_hyphen(value) + + [TOKENS[:VARIABLE], value[1..-1]] + end + def (TOKENS[:DOLLAR_VAR_WITH_DASH]).acceptable?(context = {}) + Puppet[:allow_variables_with_dashes] + end + TOKENS.add_token :DOLLAR_VAR, %r{\$(::)?(\w+::)*\w+} do |lexer, value| [TOKENS[:VARIABLE],value[1..-1]] end + TOKENS.add_token :VARIABLE_WITH_DASH, %r{(?:::)?(?:[-\w]+::)*[-\w]+} do |lexer, value| + lexer.warn_if_variable_has_hyphen(value) + + [TOKENS[:VARIABLE], value] + end + #:stopdoc: # Issue #4161 + def (TOKENS[:VARIABLE_WITH_DASH]).acceptable?(context={}) + Puppet[:allow_variables_with_dashes] and TOKENS[:VARIABLE].acceptable?(context) + end + #:startdoc: + TOKENS.add_token :VARIABLE, %r{(::)?(\w+::)*\w+} #:stopdoc: # Issue #4161 def (TOKENS[:VARIABLE]).acceptable?(context={}) @@ -514,9 +539,15 @@ def slurpstring(terminators,escapes=%w{ \\ $ ' " r n t s }+["\n"],ignore_invali def tokenize_interpolated_string(token_type,preamble='') value,terminator = slurpstring('"$') token_queue << [TOKENS[token_type[terminator]],preamble+value] + variable_regex = if Puppet[:allow_variables_with_dashes] + TOKENS[:VARIABLE_WITH_DASH].regex + else + TOKENS[:VARIABLE].regex + end if terminator != '$' or @scanner.scan(/\{/) token_queue.shift - elsif var_name = @scanner.scan(TOKENS[:VARIABLE].regex) + elsif var_name = @scanner.scan(variable_regex) + warn_if_variable_has_hyphen(var_name) token_queue << [TOKENS[:VARIABLE],var_name] tokenize_interpolated_string(DQ_continuation_token_types) else @@ -547,4 +578,10 @@ def getcomment(line = nil) def commentpush @commentstack.push(['', @line]) end + + def warn_if_variable_has_hyphen(var_name) + if var_name.include?('-') + Puppet.deprecation_warning("Using `-` in variable names is deprecated at #{file || ''}:#{line}. See http://links.puppetlabs.com/puppet-hyphenated-variable-deprecation") + end + end end diff --git a/lib/puppet/provider/augeas/augeas.rb b/lib/puppet/provider/augeas/augeas.rb index 3b7120df8e4..a301c8cd7c6 100644 --- a/lib/puppet/provider/augeas/augeas.rb +++ b/lib/puppet/provider/augeas/augeas.rb @@ -24,7 +24,7 @@ include Puppet::Util::Diff include Puppet::Util::Package - confine :true => Puppet.features.augeas? + confine :feature => :augeas has_features :parse_commands, :need_to_run?,:execute_changes diff --git a/lib/puppet/provider/group/ldap.rb b/lib/puppet/provider/group/ldap.rb index 78333f17608..19e9e1b145f 100644 --- a/lib/puppet/provider/group/ldap.rb +++ b/lib/puppet/provider/group/ldap.rb @@ -12,7 +12,7 @@ not specify one, but it is a potentially expensive operation, as it iterates across all existing groups to pick the appropriate next one." - confine :true => Puppet.features.ldap?, :false => (Puppet[:ldapuser] == "") + confine :feature => :ldap, :false => (Puppet[:ldapuser] == "") # We're mapping 'members' here because we want to make it # easy for the ldap user provider to manage groups. This diff --git a/lib/puppet/util/feature.rb b/lib/puppet/util/feature.rb index eb61dabfa9b..3dccec2fe34 100644 --- a/lib/puppet/util/feature.rb +++ b/lib/puppet/util/feature.rb @@ -23,7 +23,9 @@ def add(name, options = {}) end meta_def(method) do - @results[name] = test(name, options) unless @results.include?(name) + # Positive cache only, except blocks which are executed just once above + final = @results[name] || block_given? + @results[name] = test(name, options) unless final @results[name] end end diff --git a/spec/unit/parser/lexer_spec.rb b/spec/unit/parser/lexer_spec.rb index 492fb119914..e375bed39fd 100755 --- a/spec/unit/parser/lexer_spec.rb +++ b/spec/unit/parser/lexer_spec.rb @@ -11,6 +11,13 @@ end __ = nil +def tokens_scanned_from(s) + lexer = Puppet::Parser::Lexer.new + lexer.string = s + lexer.fullscan[0..-2] +end + + describe Puppet::Parser::Lexer do describe "when reading strings" do before { @lexer = Puppet::Parser::Lexer.new } @@ -419,7 +426,8 @@ end end -shared_examples_for "variable names in the lexer" do |prefix| + +shared_examples_for "handling `-` in standard variable names" do |prefix| # Watch out - a regex might match a *prefix* on these, not just the whole # word, so make sure you don't have false positive or negative results based # on that. @@ -436,7 +444,13 @@ illegal.each do |name| var = prefix + global_scope + name - it "should NOT accept #{var.inspect} as a valid variable name" do + it "when `variable_with_dash` is disabled it should NOT accept #{var.inspect} as a valid variable name" do + Puppet[:allow_variables_with_dashes] = false + (subject.regex.match(var) || [])[0].should_not == var + end + + it "when `variable_with_dash` is enabled it should NOT accept #{var.inspect} as a valid variable name" do + Puppet[:allow_variables_with_dashes] = true (subject.regex.match(var) || [])[0].should_not == var end end @@ -447,20 +461,118 @@ its(:skip_text) { should be_false } its(:incr_line) { should be_false } - it_should_behave_like "variable names in the lexer", '$' + it_should_behave_like "handling `-` in standard variable names", '$' end describe Puppet::Parser::Lexer::TOKENS[:VARIABLE] do its(:skip_text) { should be_false } its(:incr_line) { should be_false } - it_should_behave_like "variable names in the lexer", '' + it_should_behave_like "handling `-` in standard variable names", '' end -def tokens_scanned_from(s) - lexer = Puppet::Parser::Lexer.new - lexer.string = s - lexer.fullscan[0..-2] +describe "the horrible deprecation / compatibility variables with dashes" do + NamesWithDashes = %w{f- f-o -f f::-o f::o- f::o-o} + + { Puppet::Parser::Lexer::TOKENS[:DOLLAR_VAR_WITH_DASH] => '$', + Puppet::Parser::Lexer::TOKENS[:VARIABLE_WITH_DASH] => '' + }.each do |token, prefix| + describe token do + its(:skip_text) { should be_false } + its(:incr_line) { should be_false } + + context "when compatibly is disabled" do + before :each do Puppet[:allow_variables_with_dashes] = false end + Puppet::Parser::Lexer::TOKENS.each do |name, value| + it "should be unacceptable after #{name}" do + token.acceptable?(:after => name).should be_false + end + end + + # Yes, this should still *match*, just not be acceptable. + NamesWithDashes.each do |name| + ["", "::"].each do |global_scope| + var = prefix + global_scope + name + it "should match #{var.inspect}" do + subject.regex.match(var).to_a.should == [var] + end + end + end + end + + context "when compatibility is enabled" do + before :each do Puppet[:allow_variables_with_dashes] = true end + it "should be acceptable after DQPRE" do + token.acceptable?(:after => :DQPRE).should be_true + end + + NamesWithDashes.each do |name| + ["", "::"].each do |global_scope| + var = prefix + global_scope + name + it "should match #{var.inspect}" do + subject.regex.match(var).to_a.should == [var] + end + end + end + end + end + end + + context "deprecation warnings" do + before :each do Puppet[:allow_variables_with_dashes] = true end + + it "should match a top level variable" do + Puppet.expects(:deprecation_warning).once + + tokens_scanned_from('$foo-bar').should == [ + [:VARIABLE, { :value => 'foo-bar', :line => 1 }] + ] + end + + it "does not warn about a variable without a dash" do + Puppet.expects(:deprecation_warning).never + + tokens_scanned_from('$c').should == [ + [:VARIABLE, { :value => "c", :line => 1 }] + ] + end + + it "does not warn about referencing a class name that contains a dash" do + Puppet.expects(:deprecation_warning).never + + tokens_scanned_from('foo-bar').should == [ + [:NAME, { :value => "foo-bar", :line => 1 }] + ] + end + + it "warns about reference to variable" do + Puppet.expects(:deprecation_warning).once + + tokens_scanned_from('$::foo-bar::baz-quux').should == [ + [:VARIABLE, { :value => "::foo-bar::baz-quux", :line => 1 }] + ] + end + + it "warns about reference to variable interpolated in a string" do + Puppet.expects(:deprecation_warning).once + + tokens_scanned_from('"$::foo-bar::baz-quux"').should == [ + [:DQPRE, { :value => "", :line => 1 }], + [:VARIABLE, { :value => "::foo-bar::baz-quux", :line => 1 }], + [:DQPOST, { :value => "", :line => 1 }], + ] + end + + it "warns about reference to variable interpolated in a string as an expression" do + Puppet.expects(:deprecation_warning).once + + tokens_scanned_from('"${::foo-bar::baz-quux}"').should == [ + [:DQPRE, { :value => "", :line => 1 }], + [:VARIABLE, { :value => "::foo-bar::baz-quux", :line => 1 }], + [:DQPOST, { :value => "", :line => 1 }], + ] + end + end end describe Puppet::Parser::Lexer,"when lexing strings" do diff --git a/spec/unit/util/feature_spec.rb b/spec/unit/util/feature_spec.rb index f1439339860..d5bcafe8350 100755 --- a/spec/unit/util/feature_spec.rb +++ b/spec/unit/util/feature_spec.rb @@ -39,7 +39,7 @@ @features.should be_available end - it "should cache the results of a feature load" do + it "should cache the results of a feature load via code block" do $loaded_feature = 0 @features.add(:myfeature) { $loaded_feature += 1 } @features.myfeature? @@ -78,4 +78,15 @@ @features.should_not be_myfeature end + + it "should change the feature to be present when its libraries become available" do + @features.add(:myfeature, :libs => %w{foo bar}) + @features.expects(:require).twice().with("foo").raises(LoadError).then.returns(nil) + @features.stubs(:require).with("bar") + + Puppet.expects(:debug) + + @features.should_not be_myfeature + @features.should be_myfeature + end end