ckruse / CFPropertyList

Read, write and manipulate both binary and XML property lists as defined by apple
MIT License
212 stars 47 forks source link

Error around the Enumerable::Enumerator class #26

Closed glarizza closed 10 years ago

glarizza commented 10 years ago

This may just be an error in my implementation, but I thought I'd paste it here to start the conversation.

I'm trying to require the CFPropertyList gem inside a provider that I'm using for Puppet. To do a 'safety check' for this, I created a Puppet 'feature' that will basically try to load the library and safely error-out if the lib isn't found.

The feature looks like so:

require 'puppet/util/feature'
Puppet.features.add(:cfpropertylist, :libs => ['CFPropertyList'])

In my provider code, I require the feature with:

confine :feature => :cfpropertylist

Cool, so when I try to run Puppet it complains about the feature not being available because the CFPropertyList library failed to load. Yikes. Okay, so I commented all the feature code out and just tried to require CFPropertyList at the top of my provider to see if I could expose more debugging messages. This bubbles up this stacktrace:

Notice: Compiled catalog for satori.local in environment production in 0.02 seconds
Error: Could not autoload puppet/feature/cfpropertylist: wrong number of arguments(1 for 0)
/Users/glarizza/src/puppet-property_list_key/vendor/ruby/1.9.1/gems/CFPropertyList-2.2.4/lib/rbCFPropertyList.rb:91:in `initialize'
/Users/glarizza/src/puppet-property_list_key/vendor/ruby/1.9.1/gems/CFPropertyList-2.2.4/lib/rbCFPropertyList.rb:91:in `new'
/Users/glarizza/src/puppet-property_list_key/vendor/ruby/1.9.1/gems/CFPropertyList-2.2.4/lib/rbCFPropertyList.rb:91:in `<top (required)>'
/Users/glarizza/src/puppet-property_list_key/vendor/ruby/1.9.1/gems/CFPropertyList-2.2.4/lib/CFPropertyList.rb:3:in `require'
/Users/glarizza/src/puppet-property_list_key/vendor/ruby/1.9.1/gems/CFPropertyList-2.2.4/lib/CFPropertyList.rb:3:in `<top (required)>'
/Users/glarizza/src/puppet-property_list_key/lib/puppet/feature/cfpropertylist.rb:2:in `require'
/Users/glarizza/src/puppet-property_list_key/lib/puppet/feature/cfpropertylist.rb:2:in `<top (required)>'

That's pointing back to the Enumerator::Enumerable hack that you used for the 1.8/1.9 codebase differences. So I jumped into IRB:

rirb(main):001:0> require 'puppet'
re=> true
irb(main):002:0> require 'CFPropertyList'
=> true
irb(main):015:0* begin
irb(main):016:1*   Enumerable::Enumerator.new([])
irb(main):017:1> rescue NameError => e
irb(main):018:1>   module Enumerable
irb(main):019:2>     class Enumerator
irb(main):020:3>     end
irb(main):021:2>   end
irb(main):022:1> end
ArgumentError: wrong number of arguments(1 for 0)
        from (irb):16:in `initialize'
        from (irb):16:in `new'
        from (irb):16
        from /opt/boxen/rbenv/versions/1.9.3-p392/bin/irb:12:in `<main>'

I then tried a brief hack that we'd talked about awhile back:

rirb(main):001:0> require 'puppet'
re=> true
irb(main):002:0> require 'CFPropertyList'
=> true
irb(main):003:0> begin
irb(main):004:1*   Enumerable::Enumerator.new([])
irb(main):005:1> rescue NameError => e
irb(main):006:1>   module Enumerable
irb(main):007:2>     class Enumerator
irb(main):008:3>       def initialize(*args)
irb(main):009:4>       end
irb(main):010:3>     end
irb(main):011:2>   end
irb(main):012:1> end
ArgumentError: wrong number of arguments(1 for 0)
        from (irb):4:in `initialize'
        from (irb):4:in `new'
        from (irb):4
        from /opt/boxen/rbenv/versions/1.9.3-p392/bin/irb:12:in `<main>'
irb(main):013:0>

I'm kinda stumped at the moment - it requires just fine in IRB but when I try to require the library inside the Puppet codebase I'm getting errors. I'm guessing maybe Puppet is doing some work with the Enumerable::Enumerator class, so I really need to track that down. Do you have any suggestions?

dayglojesus commented 10 years ago

Works in 1.8.7, rvm and OS X native MRI.

dayglojesus commented 10 years ago

Ooh, does not fly with Ruby 2.0! It's a bug.

dayglojesus commented 10 years ago

leela:~ itsvan$ irb -r cfpropertylist irb(main):001:0> begin irb(main):002:1* Enumerable::Enumerator.new([]) irb(main):003:1> rescue NameError => e irb(main):004:1> module Enumerable irb(main):005:2> class Enumerator irb(main):006:3> end irb(main):007:2> end irb(main):008:1> end ArgumentError: wrong number of arguments (1 for 0) from (irb):2:in initialize' from (irb):2:innew' from (irb):2 from /usr/bin/irb:12:in `

' irb(main):009:0>

dayglojesus commented 10 years ago

I think behaviour needs to be coerced based on Ruby version. In 1.9 and above, Enumerator.new wants a block…

begin
    Enumerable::Enumerator.new { [] }
rescue NameError => e
    module Enumerable
        class Enumerator
        end
    end
end
dayglojesus commented 10 years ago

The more I look at this block of code, the less I understand its purpose. Is it just scar tissue?

glarizza commented 10 years ago

@ckruse and I talked about this over email awhile back. In Ruby 1.8, Enumerator is covered in the Enumerable module while in 1.9 the class moved to the global namespace. To avoid problems, he creates the class if it doesn't exist so he can do:

when Array, Enumerator, Enumerable::Enumerator

The issue is with the default constructor that accepts no arguments

dayglojesus commented 10 years ago

I see, thanks.

ckruse commented 10 years ago

Hm, maybe we should change the check to

begin
  Enumerable::Enumerator.class
rescue NameError => e
  module Enumerable
    class Enumerator
    end
  end
end

This avoids the problem of different constructors. What do you guys think?

dayglojesus commented 10 years ago

There are may ways to quack a duck.

ckruse commented 10 years ago

I'm sorry, I don't understand?

I summarize what happend: I need to create Enumerable::Enumerator for backward compatibility if it doesn't exist. Therefore I check for existance by creating a new object. If it doesn't, it throws a NameError and I can rescue that. But since 2.0 it seems as if the constructor has changed. Therefore avoiding object creation and calling a method on the class object itself seems to be much more stable to me.

Am I missing something?

dayglojesus commented 10 years ago

Sorry, Christian, that was my awkward attempt at humor -- duck typing? ;)

What you propose will work, but if you simply want to discover if the class is defined all you need is:

begin
  Enumerable::Enumerator
rescue NameError
  module Enumerable
    class Enumerator
    end
  end
end

Perhaps more succinctly, simply:

module Enumerable
  class Enumerator
  end
end

Because it either exists or it doesn't. If it already exists, the code you see above does nothing to impact operation of the class. If it doesn't exist, the class will be defined.

ckruse commented 10 years ago

Good point! I didn't think anything of it.