Issue #21974 has been updated by Charlie Sharpsteen. Status changed from Unreviewed to Needs Decision Assignee set to eric sorenson
Eric, any thoughts on the feasibility of this? ---------------------------------------- Bug #21974: DIE :undef, DIE! https://projects.puppetlabs.com/issues/21974#change-95716 * Author: Joshua Hoblitt * Status: Needs Decision * Priority: Normal * Assignee: eric sorenson * 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.
