build: allow clients to provide their own pugixml#244
Conversation
webern
left a comment
There was a problem hiding this comment.
Thanks for working on this. As written it won’t quite work because I have dumb header paths for the pugi headers. There’s a small fix needed in the Cmake file and the include paths need to change at all the sites where pugi is referenced.
I’m on a mobile phone right now. Later I can add a commit to this PR or you can do it if you get there first:
git remote add upstream https://github.com/webern/mx.git
git fetch upstream pugi-headers
git cherry-pick 99dc0a9Also the CI failures are something I need to fix. It looks like my comment posting workflows for code coverage don’t work for anyone but me.
|
It was working for me, weirdly, so I never looked at the |
Change the vendored pugixml include directory from ${PRIVATE_DIR} to
${PRIVATE_DIR}/pugixml so that code includes pugixml.hpp directly
(matching upstream pugixml conventions). This lets the if(NOT TARGET
pugixml) guard work seamlessly with find_package(pugixml) or any
other standard pugixml CMake target.
99dc0a9 to
8394f86
Compare
I think it worked because your version of pugixml and the vendored version were the same, or at least close enough that the headers did not differ in any binary-affecting way. So I think when you were building mx you were reading the vendored headers but linking the cpp's you had built outside. Anyway, I had forgotten to update one thing so did a force push to that third commit (mine). And checking CI 👍 |
webern
left a comment
There was a problem hiding this comment.
Nice, thank you for fixing this!
This PR gates the inclusion of the local vendored
pugixmlon whether another already exists. It allows a client project that also usespugixmlto includemxwithout creating a conflict.Resolves #242