Bug #2021

Returning boolean not always possible

Added by Peter Meier over 1 year ago. Updated over 1 year ago.

Status:Closed Start:02/24/2009
Priority:Normal Due date:
Assigned to:James Turnbull % Done:

0%
Category:binary
Target version:1.5.5
Keywords:boolean, return Branch:
Votes: 0

Description

assume the following code:

#cat facter/selinux.rb
Facter.add("selinux") do
    confine :kernel => :linux

    setcode do 
            result = false
        if FileTest.exists?("/selinux/enforce")
            result = true
        end
        result
    end
end

one time it is working, the other time not:

# export RUBYLIB=`pwd`
# facter selinux
false
# facter
/usr/lib/ruby/site_ruby/1.8/facter/util/confine.rb:37:in `true?': undefined method `downcase' for false:FalseClass (NoMethodError)
        from /usr/lib/ruby/site_ruby/1.8/facter/util/confine.rb:36:in `each'
        from /usr/lib/ruby/site_ruby/1.8/facter/util/confine.rb:36:in `true?'
        from /usr/lib/ruby/site_ruby/1.8/facter/util/resolution.rb:101:in `suitable?'
        from /usr/lib/ruby/site_ruby/1.8/facter/util/ip.rb:145:in `detect'
        from /usr/lib/ruby/site_ruby/1.8/facter/util/resolution.rb:101:in `each'
        from /usr/lib/ruby/site_ruby/1.8/facter/util/resolution.rb:101:in `detect'
        from /usr/lib/ruby/site_ruby/1.8/facter/util/resolution.rb:101:in `suitable?'
        from /usr/lib/ruby/site_ruby/1.8/facter/util/fact.rb:72:in `value'
         ... 10 levels...
        from /usr/lib/ruby/site_ruby/1.8/facter/util/collection.rb:103:in `to_hash'
        from /usr/lib/ruby/site_ruby/1.8/facter.rb:92:in `send'
        from /usr/lib/ruby/site_ruby/1.8/facter.rb:92:in `to_hash'
        from /usr/bin/facter:125

if I debug it I can see that value/values is one time a string and the other time a FalseClass or TrueClass

Please note that returning a string isn’t possible due to a bug in puppet I’ll report shortly.

This worked on 1.3.8.

Associated revisions

Revision 7a819454febdf34f4384e1bc64c5b2599df4bd38
Added by Peter Meier over 1 year ago

correctly compare values – fixes #2021

this ensures we can compare all kind of objects and not only instances of strings. It compares strings in a case-insensitive manner and converts symbols to strings.

introducing this behavior required that we introduce a convert util method, to ensure that we convert the value correctly. Introduced this method in other places as well.

This behavior change requires that we drop one test, which have become anyway deprecated.

History

Updated by Peter Meier over 1 year ago

Please note that returning a string isn’t possible due to a bug in puppet I’ll report shortly.

I decided to add the problem I have with puppet here, as the bugs are very correlated and if it would be possible to return booleans from facter the puppet problem would be gone:

#/tmp$ cat facter/foobar.rb 
Facter.add("foobar") do
    setcode do 
        result = false
        result
    end
end

#/tmp$ cat foo.pp 

if $foobar {
  notice("bleh $foobar")
}
#/tmp$ puppet facter
#/tmp$
[edit foobar.rb to return "false"]
#/tmp$ cat facter/foobar.rb 
Facter.add("foobar") do
    setcode do 
        result = "false"
        result
    end
end
#/tmp$ puppet foo.pp 
notice: Scope(Class[main]): bleh false

returning a boolean is working with 0.24.4 and 1.3.8 but not with latest puppet and facter.

Expected behavior: – returning booleans from facter should be possible – puppet should treat false and “false” as the same? Or will we encounter other problems with that?

If you think we should split them, please let me know.

Updated by James Turnbull over 1 year ago

  • Status changed from Unreviewed to Accepted
  • Assigned to set to Luke Kanies
  • Target version changed from 1.5.4 to 1.6.0

Updated by Peter Meier over 1 year ago

proposed patch to the list.

pushed to: http://github.com/duritong/facter/tree/ticket/2021

Updated by Peter Meier over 1 year ago

imho this is ready to merge

Updated by Peter Meier over 1 year ago

immerda wrote:

imho this is ready to merge

unfortunately we introduced a new incompatibility with this patch. Will send it to the dev list

Updated by Peter Meier over 1 year ago

immerda wrote:

immerda wrote:

imho this is ready to merge

unfortunately we introduced a new incompatibility with this patch. Will send it to the dev list

ready to merge.

Updated by Luke Kanies over 1 year ago

  • Status changed from Accepted to Ready for Checkin
  • Assigned to changed from Luke Kanies to James Turnbull

Updated by James Turnbull over 1 year ago

  • Status changed from Ready for Checkin to Closed

Fixed in commit:“7a819454febdf34f4384e1bc64c5b2599df4bd38” in branch master.

Updated by James Turnbull over 1 year ago

  • Target version changed from 1.6.0 to 1.5.5

Also available in: Atom PDF