Issue #21974 has been updated by eric sorenson.

Status changed from Needs Decision to Needs More Information
Assignee changed from eric sorenson to Joshua Hoblitt

josh I'm not sure what you're proposing to do/change here. Is there something 
that's not covered by the über-undef bug, #15329?  We need some way to express 
/ pass around undefined so it's not like we can straight up remove it... i 
think. What should we do instead?

----------------------------------------
Bug #21974: DIE :undef, DIE!
https://projects.puppetlabs.com/issues/21974#change-95830

* Author: Joshua Hoblitt
* Status: Needs More Information
* Priority: Normal
* Assignee: Joshua Hoblitt
* 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.


Reply via email to