Issue #21974 has been reported by Joshua Hoblitt.
----------------------------------------
Bug #21974: DIE :undef, DIE!
https://projects.puppetlabs.com/issues/21974
* Author: Joshua Hoblitt
* Status: Unreviewed
* Priority: Normal
* Assignee:
* Category: API
* Target version:
* Affected Puppet version: 3.2.3
* Keywords: :undef nil erb evil
* Branch:
----------------------------------------
I'm not sure if this should be flagged as a bug, a feature, or a refactor
because this issue covers all three. :)
Yesterday afternoon I discovered a template in a fairly popular module that did
not work correctly on puppet 3.2.1 due to it's use of `:undef`. The fix was to
replace `:undef` with `#nil?`.
@@ -14,10 +14,10 @@ server {
ssl_protocols <%= @ssl_protocols %>;
ssl_ciphers <%= @ssl_ciphers %>;
ssl_prefer_server_ciphers on;
-<% if @auth_basic != :undef -%>
+<% unless @auth_basic.nil? -%>
auth_basic "<%= @auth_basic %>";
<% end -%>
-<% if @auth_basic_user_file != :undef -%>
+<% unless @auth_basic_user_file.nil? -%>
auth_basic_user_file <%= @auth_basic_user_file %>;
<% end -%>
It appears that in at least some cases (perhaps limited to erb?) real `nil`s
are leaking out instead of `:undef`. I've unable to pass an `:undef` into a
template at all in testing in either 3.2.1 or 3.2.3. I don't know if this has
ever worked or is a regression.
$ cat undef_test.pp
notify{'undeclared': message => inline_template('<%= @foo == :undef %> :
<%= @foo.nil? %>')}
$empty = ''
notify{'empty': message => inline_template('<%= @empty == :undef %> : <%=
@empty.nil? %>')}
$undef = undef
notify{'undef': message => inline_template('<%= @undef == :undef %> : <%=
@undef.nil? %>')}
$false = false
notify{'false': message => inline_template('<%= @false == :undef %> : <%=
@false.nil? %>')}
$ puppet --version
3.2.3
$ puppet apply undef_test.pp
Notice: false : false
Notice: /Stage[main]//Notify[false]/message: defined 'message' as 'false :
false'
Notice: false : false
Notice: /Stage[main]//Notify[empty]/message: defined 'message' as 'false :
false'
Notice: false : true
Notice: /Stage[main]//Notify[undeclared]/message: defined 'message' as
'false : true'
Notice: false : true
Notice: /Stage[main]//Notify[undef]/message: defined 'message' as 'false :
true'
Notice: Finished catalog run in 0.11 seconds
Upon digging into the puppet source, it appears that usage and testing for
`:undef` is inconsistent in the core.
Consider `Puppet::Parser::Scope#true?` that tests for `:undef` to be used as a
boolean value.
https://github.com/puppetlabs/puppet/blob/3.2.3/lib/puppet/parser/scope.rb#L113-L122
. Elsewhere in the same class `:undef` is being checked for directly
https://github.com/puppetlabs/puppet/blob/3.2.3/lib/puppet/parser/scope.rb#L392
The case against `:undef`
* It violates the princible of 'least surprise' in that it duplicates a built
in language feature
* In at least some circumstances `nil` leaks out in place of `:undef`
* Test coverage of `:undef` is insufficent
* Usage of `:undef` appears inconsistent in the core
I'd agrue that in stead of trying to replicate/maintain externally visible
inconsistent behavior that it would be better to just remove `:undef`
completely and deal with the API change all at once, perhaps in a 3.3.0
release.
--
You have received this notification because you have either subscribed to it,
or are involved in it.
To change your notification preferences, please click here:
http://projects.puppetlabs.com/my/account
--
You received this message because you are subscribed to the Google Groups
"Puppet Bugs" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/puppet-bugs.
For more options, visit https://groups.google.com/groups/opt_out.
