I'm almost certain that this is only needed on OSX, so if we're doing an all in one package that could work. And I would much rather contribute upstream and defer packaging to the upstream project rather than load that on ourselves.
On Wed, Aug 28, 2013 at 11:04 AM, Michael Stanhke <[email protected]>wrote: > > > On Wednesday, August 28, 2013 10:53:42 AM UTC-7, Adrien Thebo wrote: >> >> Hi folks, >> >> I've been working on some issues on how Facter handles plist libraries, >> and right now we're in a bit of a bind. The short story is that in Facter >> 1.7 we vendored a new plist library that breaks backwards compatibility, >> but there's a lot of pressure to keep the plist library in and make it >> compatible. The full discussion can be found at https://github.com/** >> puppetlabs/facter/pull/499<https://github.com/puppetlabs/facter/pull/499> >> . >> >> >> Prior to Facter 1.7, Facter had a basic plist library that could handle >> plain text plist files, but had to offload binary plist files to Apple's >> CoreFoundation libraries. My understanding is that this caused a severe >> performance hit on OSX systems. >> >> As part of #11299 >> (https://projects.puppetlabs.**com/issues/11299<https://projects.puppetlabs.com/issues/11299>) >> we vendored the CFPropertyList library to replace the old plist >> implementation. The implementation can opportunistically use libxml2, >> nokogiri, and REXML, and is significantly faster than the old plist >> library. This change was released in Facter 1.7. >> >> However, the old plist implementation monkey patched Symbol, Array, and >> Hash, and CFPropertyList monkey patched those classes as well - but with a >> different method signature. The old plist implementation defines `to_plist` >> to take an optional *boolean* value, while CFPropertyList defines >> `to_plist` to take an optional *hash*. >> >> Moreover, by our current standards CFPropertyList is not a candidate for >> vendoring. The general guideline for vendoring is that candidates must be >> stable libraries, and should only be one or two libraries. In addition the >> CFPropertyList doesn't seem particularly stable, as demonstrated by the >> comment https://github.com/**puppetlabs/facter/pull/499#** >> issuecomment-21770546<https://github.com/puppetlabs/facter/pull/499#issuecomment-21770546> >> which >> indicates that CFPropertyList may not be very stable. Having to continually >> re-vendor that library inside of Facter is a special level of torment >> that's normally reserved for people that take up two parking spaces with >> one car. >> >> But wait, there's more! >> https://github.com/**puppetlabs/facter/pull/513<https://github.com/puppetlabs/facter/pull/513>introduced >> a patch to CFPropertyList to force the string encoding type for >> Ruby compatibility. So on top of having to re-vendor the CFPropertyList to >> handle updates, we would have to carry our own patches on top of everything >> else. This means that the platform team will be forced to more or less >> maintain this library. >> >> With all of these problems in mind it seems reasonable to remove the >> CFPropertyList, but there have been some very strong arguments that >> CFPropertyList is essential, removing it would cripple the performance of >> Facter on OSX, and it's unacceptable to make CFPropertyList an external >> dependency. >> >> At this point I would like additional feedback from the community - what >> would be best. Keep CFPropertyList vendored inside of Facter and carry >> patches to ensure compatibility? Remove the library and try to load it >> opportunistically? Remove it entirely? >> >> My vote would be to unvendor it, make it a dependency, and submit fixes > upstream to the authors. Having vendored libs is generally painful for > exactly the reasons you outline above. Is this lib needed on platforms that > are not Mac? If it's only on mac there are a few other options available > since the way we package mac is kind of an all-in-one, unless you're using > gems. > > > >> >> >> -- >> Adrien Thebo | Puppet Labs >> > -- > You received this message because you are subscribed to the Google Groups > "Puppet Developers" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > To post to this group, send email to [email protected]. > Visit this group at http://groups.google.com/group/puppet-dev. > For more options, visit https://groups.google.com/groups/opt_out. > -- Adrien Thebo | Puppet Labs -- You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at http://groups.google.com/group/puppet-dev. For more options, visit https://groups.google.com/groups/opt_out.
