Puppet

it’s more complex than that.

The documentation used to state that –’s didnt work, but in reality they kind of worked like they do today. I asked for the ability to be removed since the behavior is just not consistent across the language instead the docs got updated to reflect the fact that they are supported.

So at this point i think we should fix the code so it works, since everyone has been explicitly told it would work.

The docs says:

You can use the characters A-Z, a-z, 0-9, dashes (‘-‘), and underscores in variables, resources and class names. In Puppet releases prior to 0.24.6, you cannot start a class name with a number.

I use that a lot for site specific stuff: site-apache etc.

For sure it would be possible to change things and actually I even agree on the argument that in most languages this is also not possible.

However, as R.I. Pienaar said, up to now it is possible to use dashes in class names and a lot of people are using them!

Two thoughts: first, historically many languages (lisp, cobol, forth, etc.) have supported them, and second the main area where there’s a challenge, ambiguity in the parser w.r.t subtraction, has already been crossed. This remaining problems are mostly just stray regular expressions where someone (probably me in this case) was lazy or made a bad assumption.

Third, bonus thought:

diff --git a/lib/puppet/parser/lexer.rb b/lib/puppet/parser/lexer.rb
index 31d39ae..abb088d 100644
--- a/lib/puppet/parser/lexer.rb
+++ b/lib/puppet/parser/lexer.rb
@@ -240,11 +240,11 @@ class Puppet::Parser::Lexer
   end
   #:startdoc:
 
-  TOKENS.add_token :DOLLAR_VAR, %r{\$(\w*::)*\w+} do |lexer, value|
+  TOKENS.add_token :DOLLAR_VAR, %r{\$([-\w]*::)*[-\w]+} do |lexer, value|
     [TOKENS[:VARIABLE],value[1..-1]]
   end
 
-  TOKENS.add_token :VARIABLE, %r{(\w*::)*\w+}
+  TOKENS.add_token :VARIABLE, %r{([-\w]*::)*[-\w]+}
   #:stopdoc: # Issue #4161
   def (TOKENS[:VARIABLE]).acceptable?(context={})
     [:DQPRE,:DQMID].include? context[:after]
@@ -545,7 +545,7 @@ class Puppet::Parser::Lexer
     token_queue << [TOKENS[token_type[terminator]],preamble+value]
     if terminator != '$' or @scanner.scan(/\{/)
       token_queue.shift 
-    elsif var_name = @scanner.scan(%r{(\w*::)*\w+|[0-9]})
+    elsif var_name = @scanner.scan(%r{([-\w]*::)*[-\w]+|[0-9]})
       token_queue << [TOKENS[:VARIABLE],var_name]
       tokenize_interpolated_string(DQ_continuation_token_types)
     else

Markus, if you believe that we can solve the edge cases reasonably easily, I’m in favor of allowing hyphens.

Wanted to point out that I have been doing the following to work around this:

inline_template("<%= scope.lookupvar('syslog-ng::server::version') %>")

Thanks for looking into this

Just got bitten by this, also noticed it’s in the examples here:

http://docs.puppetlabs.com/guides/language_guide.html#classes

apache-ssl, etc.

Didn’t we actually commit this fix and that’s why people have been having to interpolate variables in strings with hyphens?

e.g.

foo-bar-$baz-spa used to work but now you have to do foo-bar-${baz}-spa ?

Nigel Kersten wrote:

Didn’t we actually commit this fix and that’s why people have been having to interpolate variables in strings with hyphens?

e.g.

foo-bar-$baz-spa used to work but now you have to do foo-bar-${baz}-spa ?

That’s not exactly the use case in question (though it might be related under the hood). The thing we’re getting bit by (PE1.2/2.6.9) is when the class' name is “foo-bar” and you want to use variable baz elsewhere:

$localvar = "${foo-bar::baz}" "syntax error at 'bar::baz', expected '}'"

Unquoted without curly braces, same result.

Unquoted with curly braces.

$localvar = ${foo-bar::baz} ...Could not match ${foo-bar::baz}

If you make this change it will break a lot of puppet code in the field. Enough code that refactoring could be pretty much a non-starter for certain sites, and preventing upgrading. The reality is class-name is visually a useful way to name classes, and as stated earlier.. this was “supported” and published in official examples.

I’m fine with doing the change, but I think it should issue deprecation warnings first before becoming entirely unsupported.

Best as I can tell dashes are still allowed in module names, which pretty much means that dashes in class names should be supported. I have some disagreements about some of the other changes in 3.0, but none so far that I feel strongly enough about to raise as an issue.

Brian Gupta wrote:

Best as I can tell dashes are still allowed in module names, which pretty much means that dashes in class names should be supported. I have some disagreements about some of the other changes in 3.0, but none so far that I feel strongly enough about to raise as an issue.

Ugh. We can’t be enforcing this if we’re still allowing hyphens in class names in Telly.

We need to, and intend, to enforce the stated rule that - is not acceptable in class names. That is going to end up in a Telly release, but isn’t going to make the .0 target, unlike enforcement on variable names.

This is going to mean re-writing most of my classes which is extremely frustrating (and from the copious comments in various tickets, I’m not the only one). I see a discussion in one of the linked tickets about a decision being made to kill off hyphens, but no reasoning for it. Is there a discussion around that explains why this is being made (as discussed here, solving for subtraction has been done)?

Thank you for your message.

I am currently out of office with no access to emails from July, 13th 2012 till July, 30th 2012. Your email will not be forwarded.

Kind regards, Dennis Benzinger

Paul\,

Welcome-to-the-club/.Apparently-parser-writing-is-a-bitch.(Which-in-all-fairness\,-it-probably-is,-but-it-really-shocks-me-that-the-really-smart-programmers-at-PuppetLabs-can’t-solve-this-problem.)That said,-I-leave-it-at-${Fail}– and-disappointment.(The-decision-has-been-made-by-upper-management\,-don\’t-bother-to-complain.)Do-understand,-this-probably-impacts-less-than-15\%-of-their-userbase\,-so-it\’s-\“fine\”.(I-was-told-in-IRC-to-fork-if-I-had-an-issue.)

Brian

Brian – I am sorry you’re frustrated by this. This is unfortunately a non-trivial problem to fix both technically and from the hole we dug. We messed up and allowed an inconsistent model – mea culpa – we’re annoyed and embarrassed it happened like this (I take some of the blame here – I am partially responsible for thinking this was a good idea long ago).

So now we’re trying to fix it and make it consistent. That resulting fix is probably not going to make everyone happy. We understand that and we’ll do our best to help people with it by using deprecation, publishing clear rules on what works and what doesn’t work, updating documentation and it may be feasible to provide some find-n-replace regexes to fix some of this when we have a path of action.

I will have a conversation internally with Eric, Nigel and the relevant Engineering folks and update the ticket with a clear rationale, approach and how we plan to proceed.

Thanks James. I’m looking forward to the rationale, as I do believe I am not alone in wanting this, and am still not 100% clear if there are reasons beyond it being hard to code the parser, as to why this is being disabled. I feel it should be able to be addressed by documenting the usage, and fixing any broken edge cases.

So it’s clear that the core puppet team has been struggling with this for a while, and understand all the edge cases, which make this challenging to support. I think a clear explanation to the community would be helpful. (With room for feedback, and an opportunity to see if there are alternatives to dropping support.)

I’m also like to apologize if anyone took my note personally, reading back it was more snarky than warranted, and probably expressed more frustration than was helpful.

So, #16213 was just duped here. This bug is almost 2 years old. At the moment, dashes in classes sort of work, and cause surprising failures on upgrades, where support waxes and wanes without warning.

On the theory that a bird in hand (that is, consistent behavior) is worth two in the bush (an open-ended bug), can we commit something to make classes with dashes in their names illegal, or at least issue a warning?

Relevant docs links:

http://docs.puppetlabs.com/puppet/2.7/reference/lang_reserved.html#classes-and-types http://docs.puppetlabs.com/puppet/3/reference/lang_reserved.html#classes-and-types

The documentation matches our current beliefs, but the code doesn’t enforce those beliefs. This is the case in a few other places, as well.