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.

@Brecht Van Lommel (brecht), please see my reply in D8662#216127.

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.

I landed the builder + windows libs today, I did not update this diff, since they linux maintainers may find the proposed linux changes i this diff useful.

I guarded the WITH_PUGIXML and the following lib checks with a win32 guard

set_and_warn_dependency(WITH_PUGIXML WITH_CYCLES_OSL   OFF)
set_and_warn_dependency(WITH_PUGIXML WITH_OPENIMAGEIO  OFF)

these checks can be removed once all libs have landed. This was chosen over defaulting it only to ON for windows and OFF for the other platforms, because it being a default linux/mac people will have it OFF and may run into build issues once the linux changes land.

@Sybren A. Stüvel (sybren) Are you keeping track of this? Need some help?

There is a slight inconsistency between this patch and what @Ray Molenkamp (LazyDodo) committed in rBdca9aa0053f7.
This patch uses PUGIXML_INCLUDE_DIRS (plural) whereas the commit uses PUGIXML_INCLUDE_DIR (singular).
Without feedback on this I'll assume that the commit is the correct one, because A) it prevents a change to Cycles, and B) the variable contains a single directory so singular seems to be the correct choice.

The singular "DIR" is what CMake follows in their find modules, to my knowledge.
The fact that commit is different from the patch is something we should not see happening though.

I have adjusted the patch locally so that it can be applied to master again, and things build properly. The only thing I see now is a warning from CMake:

-- Using pre-compiled LIBDIR: /home/sybren/workspace/blender-git/blender/../lib/linux_centos7_x86_64
CMake Warning (dev) at /snap/cmake/715/share/cmake-3.19/Modules/FindPackageHandleStandardArgs.cmake:424 (message):
  The package name passed to `find_package_handle_standard_args` (PUGIXML)
  does not match the name of the calling package (PugiXML).  This can lead to
  problems in calling code that expects `find_package` result variables
  (e.g., `_FOUND`) to follow a certain pattern.
Call Stack (most recent call first):
  build_files/cmake/Modules/FindPugiXML.cmake:52 (FIND_PACKAGE_HANDLE_STANDARD_ARGS)
  build_files/cmake/macros.cmake:118 (find_package)
  build_files/cmake/platform/platform_unix.cmake:82 (find_package_static)
  build_files/cmake/platform/platform_unix.cmake:354 (find_package_wrapper)
  CMakeLists.txt:901 (include)
This warning is for project developers.  Use -Wno-dev to suppress it.

This is something I've seen before, but then it would only show when a package location was not already cached. This appears every time I run CMake now.

@Brecht Van Lommel (brecht) @Ray Molenkamp (LazyDodo) how do we proceed? Committing the changed libraries to SVN will break current Blender until this patch has been committed, so I guess it's a good idea to at least sync these commits with the macOS SVN commits.

The patch was accepted with DIRS (while the find module was indeed using DIR) I caught the issue during final testing, I did not want to update the diff with the final commit code since it would have wiped out the linux changes