Page MenuHome

Deps: Add pugiXML as an official dependency.
Changes PlannedPublic

Authored by Ray molenkamp (LazyDodo) on Aug 19 2020, 5:03 PM.

Details

Summary

PugiXML was historically shipped hidden embedded into OIIO, the GP team had a requirement for an XML library recently so pugi seems like a natural choice since it's not really a 'new' library, we just turn an implicit dependency into an explicit one.

This diff

  1. Updates the builder to build pugiXML on all platforms (previously just windows)
  2. Disabled the embedded copy inside OIIO
  3. Adds some checks to disable components when PUGI is not available.

I tested windows and linux but given linux is not my primary platform a second look would be appreciated there.

I was unable to test on mac (or arm mac)

This diff is a prerequisite for work the GP team would like to land in 2.91

Diff Detail

Repository
rB Blender
Branch
tmp_lib_pugixml (branched from master)
Build Status
Buildable 9644
Build 9644: arc lint + arc unit

Event Timeline

Ray molenkamp (LazyDodo) requested review of this revision.Aug 19 2020, 5:03 PM

install_deps will also need some work for that...

Also, note that we already include/use libxml2 and libtinyxml currently, afaik? :/

I have no horse in that race, I offered the GP team the same alternatives you came up with as well, and they went with PUGI, bumping PUGI to fully supported vs bumping any of the others we implicitly have is about the same amount of work, so i wasn't overly concerned with that choice.

I took pugixml of the options available because it had the best documentation and fit in my needs, but no other reason. Any of the current XML libs integrated into Blender could be used, I just select one.

what is keeping this from getting reviewed ? bcon1 ends in a week, we're cutting it awfully close here.

@Sergey Sharybin (sergey), @Sebastián Barschkis (sebbas), I think this waiting on you to review this as platform maintainers.

I don't think this one is strictly required for 2.91 as it is for the SVG exporter which will be for 2.92, but might as well do it along with potrace.

I approve the inclusion of this library, but I have not tested the patch.

This revision is now accepted and ready to land.Sep 8 2020, 7:28 PM

The patch seems to work on the CentOS VM. Compiled both libraries and Blender.

Not sure what would be the best way to put all the needed bits into master, without (temporarily) breaking compilation.

Right now, (rB626201683ec0) the patch does not apply cleanly anymore (or does it depend on another one?)
Would be nice to have an updated patch so that this can be tested on macOS.

Ray molenkamp (LazyDodo) planned changes to this revision.EditedSep 22 2020, 3:52 PM

only pugi was required for the 2.91 cycle, once we get closer to or are in bcon1 for 2.92 i'll land this.