Merge remote-tracking branch 'upstream/2.7.x' into 3.0.x

* upstream/2.7.x:
  (#17260) Include link to information about deprecation
  Update Rakefile to make rspec optional
  Remove the asc file from the source of spec file
  (#17260) Warn when variables contain hyphens
  Edit description of hyphenated variables compatibility setting
  (#10146) `-` in variable names should be deprecated!
  (#14822) Use feature confine for feature tests during run
  (#14822) Re-evaluate features if they previously were false

Conflicts:
	ext/build_defaults.yaml
	ext/redhat/puppet.spec.erb
	lib/puppet/defaults.rb

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

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,

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 || '<string>'}:#{line}. See http://links.puppetlabs.com/puppet-hyphenated-variable-deprecation")
+    end
+  end
 end

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
 

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

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

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

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