My understanding was that Apple changed the default format for plists from XML to binary with release 10.4 ( http://en.wikipedia.org/wiki/Property_list#Mac_OS_X). I believe what happens now is that many plist files are still shipped in XML format, but the moment a user tries to change one of them using the GUI (e.g. "System Preferences...") it gets rewritten in binary format.
Nigel, you make a good point that there is other code in Puppet that deals with plists, and we ought to refactor so that everything that handles plists goes through the same code path, and that code path can work with both binary and XML formats. But that's a more major change, and I don't think we'll be able to slip it into the 2.6.2 release. I'll put a ticket in the system to remind us to revisit this issue when we've finished our current focus on 2.6.2. Paul On Fri, Sep 24, 2010 at 3:00 PM, Nigel Kersten <[email protected]> wrote: > We have a lot of plist parsing around the place with the Mac code. > > Is there any chance we can make a convenience method in Puppet::Util > or something so that we can start using that everywhere, and deal with > all theses issues in a single location? > > The description isn't quite right by the way, as it makes it sound > like launchd switched to binary plists with 10.4. > > NSDefaults and thus the "defaults" binary changed to modify all xml1 > plists to binary on write with 10.4, but Apple all throughout 10.5 > were only shipping xml1 format plists with launchd. > > Even when 10.6.0 came out all launchd plists were xml1, but something > has changed since then, and there are some conditions under which a > plist changes from xml1 to binary behind our back. I've yet to work > out what that is. > > It would be ideal if we could preserve the plist format when writing > to it... but it would be annoying work. > > > On Fri, Sep 24, 2010 at 2:52 PM, Paul Berry <[email protected]> wrote: > > Modified the launchd provider to use OSX's "plutil" command to read > > plists. This allows it to handle properly lists in both XML format > > (which was the default before OSX 10.4) and binary format (which is > > the default starting with OSX 10.4). > > > > Launchd continues to write out propertly lists in XML format. This is > > not a problem because the operating system is able to understand both > > formats. > > > > Signed-off-by: Paul Berry <[email protected]> > > --- > > lib/puppet/provider/service/launchd.rb | 25 > ++++++++++++++++--------- > > spec/unit/provider/service/launchd_spec.rb | 10 +++++----- > > 2 files changed, 21 insertions(+), 14 deletions(-) > > > > diff --git a/lib/puppet/provider/service/launchd.rb > b/lib/puppet/provider/service/launchd.rb > > index 9703595..b296e0a 100644 > > --- a/lib/puppet/provider/service/launchd.rb > > +++ b/lib/puppet/provider/service/launchd.rb > > @@ -38,6 +38,7 @@ Puppet::Type.type(:service).provide :launchd, :parent > => :base do > > > > commands :launchctl => "/bin/launchctl" > > commands :sw_vers => "/usr/bin/sw_vers" > > + commands :plutil => "/usr/bin/plutil" > > > > defaultfor :operatingsystem => :darwin > > confine :operatingsystem => :darwin > > @@ -52,6 +53,12 @@ Puppet::Type.type(:service).provide :launchd, :parent > => :base do > > Launchd_Overrides = > "/var/db/launchd.db/com.apple.launchd/overrides.plist" > > > > > > + # Read a plist, whether its format is XML or in Apple's "binary1" > > + # format. > > + def self.read_plist(path) > > + Plist::parse_xml(plutil('-convert', 'xml1', '-o', '-', path)) > > + end > > + > > # returns a label => path map for either all jobs, or just a single > > # job if the label is specified > > def self.jobsearch(label=nil) > > @@ -62,8 +69,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) > > - if job and job.has_key?("Label") > > + if FileTest.file?(fullpath) and job = read_plist(fullpath) and > job.has_key?("Label") > > if job["Label"] == label > > return { label => fullpath } > > else > > @@ -118,8 +124,11 @@ 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) > > - raise Puppet::Error.new("Unable to parse launchd plist at path: > #{job_path}") if not job_plist > > + if FileTest.file?(job_path) > > + job_plist = self.class.read_plist(job_path) > > + else > > + raise Puppet::Error.new("Unable to parse launchd plist at path: > #{job_path}") > > + end > > [job_path, job_plist] > > end > > > > @@ -200,9 +209,7 @@ Puppet::Type.type(:service).provide :launchd, :parent > => :base do > > job_plist_disabled = job_plist["Disabled"] if > job_plist.has_key?("Disabled") > > > > if self.class.get_macosx_version_major == "10.6": > > - overrides = Plist::parse_xml(Launchd_Overrides) > > - > > - unless overrides.nil? > > + if FileTest.file?(Launchd_Overrides) and overrides = > self.class.read_plist(Launchd_Overrides) > > if overrides.has_key?(resource[:name]) > > overrides_disabled = overrides[resource[:name]]["Disabled"] if > overrides[resource[:name]].has_key?("Disabled") > > end > > @@ -227,7 +234,7 @@ 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.read_plist(Launchd_Overrides) > > overrides[resource[:name]] = { "Disabled" => false } > > Plist::Emit.save_plist(overrides, Launchd_Overrides) > > else > > @@ -242,7 +249,7 @@ 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.read_plist(Launchd_Overrides) > > overrides[resource[:name]] = { "Disabled" => true } > > Plist::Emit.save_plist(overrides, Launchd_Overrides) > > else > > diff --git a/spec/unit/provider/service/launchd_spec.rb > b/spec/unit/provider/service/launchd_spec.rb > > index 320ee3a..116ba21 100755 > > --- a/spec/unit/provider/service/launchd_spec.rb > > +++ b/spec/unit/provider/service/launchd_spec.rb > > @@ -98,19 +98,19 @@ describe provider_class do > > 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(:read_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(:read_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(:read_plist).returns({}) > > @provider.enabled?.should == :true > > end > > end > > @@ -182,7 +182,7 @@ 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({}) > > + @provider.class.stubs(:read_plist).returns({}) > > Plist::Emit.expects(:save_plist).once > > @provider.enable > > end > > @@ -191,7 +191,7 @@ 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({}) > > + @provider.class.stubs(:read_plist).returns({}) > > Plist::Emit.expects(:save_plist).once > > @provider.enable > > end > > -- > > 1.7.2 > > > > -- > > 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. > > > > > > > > -- > nigel > > -- > 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.
