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 .


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) 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
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/513introduced 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?



-- 
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.

Reply via email to