Just to close the communication loop: this fails spec tests due to threading problems with RubyCocoa. I've submitted an alternate patch that uses the plutil command to parse the plists from binary format into xml so we can process it.
On Thu, Sep 16, 2010 at 8:30 PM, Nigel Kersten <[email protected]>wrote: > gah. I didn't expect that subject to come out the way it did... > > Better formatted.... > > Fixes #4025. Update launchd service provider to deal with binary plists. > > * Add RubyCocoa feature to Puppet base features > * Try to use RubyCocoa/Foundation where possible to parse launchd plists > * Update launchd tests due to method call changes in provider. > * Correct error in disable test > > > > On Thu, Sep 16, 2010 at 6:49 PM, Nigel Kersten <[email protected]> > wrote: > > > > Signed-off-by: Nigel Kersten <[email protected]> > > --- > > lib/puppet/feature/base.rb | 3 ++ > > lib/puppet/provider/service/launchd.rb | 44 > ++++++++++++++++++++++++------- > > spec/unit/provider/service/launchd.rb | 22 +++++++++------ > > 3 files changed, 50 insertions(+), 19 deletions(-) > > > > diff --git a/lib/puppet/feature/base.rb b/lib/puppet/feature/base.rb > > index a6111d8..41e1f8c 100644 > > --- a/lib/puppet/feature/base.rb > > +++ b/lib/puppet/feature/base.rb > > @@ -34,3 +34,6 @@ Puppet.features.add(:openssl, :libs => ["openssl"]) > > > > # We have CouchDB > > Puppet.features.add(:couchdb, :libs => ["couchrest"]) > > + > > +# We have RubyCocoa > > +Puppet.features.add(:rubycocoa, :libs => ["osx/cocoa"]) > > diff --git a/lib/puppet/provider/service/launchd.rb > b/lib/puppet/provider/service/launchd.rb > > index 770a7b1..e2d5381 100644 > > --- a/lib/puppet/provider/service/launchd.rb > > +++ b/lib/puppet/provider/service/launchd.rb > > @@ -1,4 +1,7 @@ > > -require 'facter/util/plist' > > +unless Puppet.features.rubycocoa? > > + require 'facter/util/plist' > > +end > > + > > > > Puppet::Type.type(:service).provide :launchd, :parent => :base do > > desc "launchd service management framework. > > @@ -51,6 +54,27 @@ Puppet::Type.type(:service).provide :launchd, :parent > => :base do > > > > Launchd_Overrides = > "/var/db/launchd.db/com.apple.launchd/overrides.plist" > > > > + > > + # We try using RubyCocoa if possible as it can parse binary plists, > > + # whereas plistlib can only read xml1 plists. > > + # TODO: Stop using plistlib at all once OS X 10.4 support is > dropped. > > + def self.hash_from_plist(path) > > + if Puppet.features.rubycocoa? > > + return > OSX::NSDictionary.dictionaryWithContentsOfFile(path).to_ruby > > + else > > + return Plist::parse_xml(path) > > + end > > + end > > + > > + > > + def self.hash_to_plist(hash, path) > > + if Puppet.features.rubycocoa? > > + hash.overrides.writeToFile_atomically_(path, true) > > + else > > + Plist::Emit.save_plist(hash, path) > > + end > > + end > > + > > > > # returns a label => path map for either all jobs, or just a single > > # job if the label is specified > > @@ -62,7 +86,7 @@ Puppet::Type.type(:service).provide :launchd, :parent > => :base do > > next if f =~ /^\..*$/ > > next if FileTest.directory?(f) > > fullpath = File.join(path, f) > > - job = Plist::parse_xml(fullpath) > > + job = self.hash_from_plist(fullpath) > > if job and job.has_key?("Label") > > if job["Label"] == label > > return { label => fullpath } > > @@ -126,7 +150,7 @@ Puppet::Type.type(:service).provide :launchd, :parent > => :base do > > def plist_from_label(label) > > job = self.class.jobsearch(label) > > job_path = job[label] > > - job_plist = Plist::parse_xml(job_path) > > + job_plist = self.class.hash_from_plist(job_path) > > if not job_plist > > raise Puppet::Error.new("Unable to parse launchd plist at > path: #{job_path}") > > end > > @@ -218,7 +242,7 @@ Puppet::Type.type(:service).provide :launchd, :parent > => :base do > > end > > > > if self.class.get_macosx_version_major == "10.6": > > - overrides = Plist::parse_xml(Launchd_Overrides) > > + overrides = self.class.hash_from_plist(Launchd_Overrides) > > > > unless overrides.nil? > > if overrides.has_key?(resource[:name]) > > @@ -247,14 +271,14 @@ Puppet::Type.type(:service).provide :launchd, > :parent => :base do > > # versions this is stored in the job plist itself. > > def enable > > if self.class.get_macosx_version_major == "10.6" > > - overrides = Plist::parse_xml(Launchd_Overrides) > > + overrides = self.class.hash_from_plist(Launchd_Overrides) > > overrides[resource[:name]] = { "Disabled" => false } > > - Plist::Emit.save_plist(overrides, Launchd_Overrides) > > + self.class.hash_to_plist(overrides, Launchd_Overrides) > > else > > job_path, job_plist = plist_from_label(resource[:name]) > > if self.enabled? == :false > > job_plist.delete("Disabled") > > - Plist::Emit.save_plist(job_plist, job_path) > > + self.class.hash_to_plist(job_plist, job_path) > > end > > end > > end > > @@ -262,13 +286,13 @@ Puppet::Type.type(:service).provide :launchd, > :parent => :base do > > > > def disable > > if self.class.get_macosx_version_major == "10.6" > > - overrides = Plist::parse_xml(Launchd_Overrides) > > + overrides = self.class.hash_from_plist(Launchd_Overrides) > > overrides[resource[:name]] = { "Disabled" => true } > > - Plist::Emit.save_plist(overrides, Launchd_Overrides) > > + self.class.hash_to_plist(overrides, Launchd_Overrides) > > else > > job_path, job_plist = plist_from_label(resource[:name]) > > job_plist["Disabled"] = true > > - Plist::Emit.save_plist(job_plist, job_path) > > + self.class.hash_to_plist(job_plist, job_path) > > end > > end > > > > diff --git a/spec/unit/provider/service/launchd.rb > b/spec/unit/provider/service/launchd.rb > > index fa86a21..2d03cd1 100755 > > --- a/spec/unit/provider/service/launchd.rb > > +++ b/spec/unit/provider/service/launchd.rb > > @@ -93,24 +93,28 @@ describe provider_class do > > @provider.enabled?.should == :false > > end > > end > > - > > + > > describe "when checking whether the service is enabled on OS X 10.6" > do > > + before :each do > > + Puppet.features.stubs(:rubycocoa?).returns true > > + end > > + > > it "should return true if the job plist says disabled is true and > the global overrides says disabled is false" do > > > provider_class.stubs(:get_macosx_version_major).returns("10.6") > > @provider.stubs(:plist_from_label).returns(["foo", > {"Disabled" => true}]) > > - Plist.stubs(:parse_xml).returns({...@resource[:name] => > {"Disabled" => false}}) > > + > provider_class.stubs(:hash_from_plist).returns({...@resource[:name] => > {"Disabled" => false}}) > > @provider.enabled?.should == :true > > end > > it "should return false if the job plist says disabled is false > and the global overrides says disabled is true" do > > > provider_class.stubs(:get_macosx_version_major).returns("10.6") > > @provider.stubs(:plist_from_label).returns(["foo", > {"Disabled" => false}]) > > - Plist.stubs(:parse_xml).returns({...@resource[:name] => > {"Disabled" => true}}) > > + > provider_class.stubs(:hash_from_plist).returns({...@resource[:name] => > {"Disabled" => true}}) > > @provider.enabled?.should == :false > > end > > it "should return true if the job plist and the global overrides > have no disabled keys" do > > > provider_class.stubs(:get_macosx_version_major).returns("10.6") > > @provider.stubs(:plist_from_label).returns(["foo", {}]) > > - Plist.stubs(:parse_xml).returns({}) > > + provider_class.stubs(:hash_from_plist).returns({}) > > @provider.enabled?.should == :true > > end > > end > > @@ -182,8 +186,8 @@ describe provider_class do > > describe "when enabling the service on OS X 10.6" do > > it "should write to the global launchd overrides file once" do > > > provider_class.stubs(:get_macosx_version_major).returns("10.6") > > - Plist.stubs(:parse_xml).returns({}) > > - Plist::Emit.expects(:save_plist).once > > + provider_class.stubs(:hash_from_plist).returns({}) > > + provider_class.expects(:hash_to_plist).once > > @provider.enable > > end > > end > > @@ -191,9 +195,9 @@ describe provider_class do > > describe "when disabling the service on OS X 10.6" do > > it "should write to the global launchd overrides file once" do > > > provider_class.stubs(:get_macosx_version_major).returns("10.6") > > - Plist.stubs(:parse_xml).returns({}) > > - Plist::Emit.expects(:save_plist).once > > - @provider.enable > > + provider_class.stubs(:hash_from_plist).returns({}) > > + provider_class.expects(:hash_to_plist).once > > + @provider.disable > > end > > end > > > > -- > > 1.7.2.3 > > > > -- > > You received this message because you are subscribed to the Google Groups > "Puppet Developers" group. > > To post to this group, send email to [email protected]. > > To unsubscribe from this group, send email to > [email protected]<puppet-dev%[email protected]> > . > > For more options, visit this group at > http://groups.google.com/group/puppet-dev?hl=en. > > > > > > -- > You received this message because you are subscribed to the Google Groups > "Puppet Developers" group. > To post to this group, send email to [email protected]. > To unsubscribe from this group, send email to > [email protected]<puppet-dev%[email protected]> > . > For more options, visit this group at > http://groups.google.com/group/puppet-dev?hl=en. > > -- You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
