On Fri, Sep 24, 2010 at 3:12 PM, Paul Berry <[email protected]> wrote: > 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.
Ah, but what you've said is different to the Wikipedia article. It says that the default format changed for "preference files" which are a subset of plists. A "default format" for plists doesn't really make sense. Instead we have default formats that various frameworks use when emitting plists, and the major change was Apple moving NSDefaults and a few others to default to binary, thus the reference to "preference files" which should be referring to the NSDefault preference domains such as /Library/Preferences, ~/Library/Preferences, and ~/Library/Preferences/ByHost, not the plists launchd uses. Not all System Preference changes cause the plist to change format. Many of the plists we're seeing change formats don't have a GUI control at all. > 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. Awesome. > 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}}) >> > + �[email protected](: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}}) >> > + �[email protected](: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({}) >> > + �[email protected](: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({}) >> > + �[email protected](: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({}) >> > + �[email protected](: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]. >> > 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]. >> 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. > -- 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]. For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
