meson: Add libxml2 cmake wrap #30
Conversation
|
Hi @talisein, Thanks for the CMake fallback! Looks pretty much fine to me, albeit we can try to fine-tune the libxml CMake fallback on the third-party library search in another PR if we need to. I did have a few points that I thought I should point out. With blessings, thank you! |
|
Hi, Also wondering, does Meson pass in the build type to CMake during the fallback, or do we have to specify one ourselves? This will matter because we may end up linking to different CRTs (i.e. Debug vs Release CRT), which can mean trouble. Edit: It turns out that we do need to specify it. Edit: This is how I think things will work for that regard: (Note that there is no clear direct mapping for the With blessings, thank you! |
|
Is this PR ready for merging? Or are changes or additions required due to Chun-wei Fan's comments? BTW, I tested building libxml2 as a subproject on a Linux system. I got an enormous although the function calls look perfectly fine. If this does not happen on Windows, |
|
Windows has a ton of warnings from libxml2 too; I turned off Werror for the subproject but kept it on for xml++ in my workflow. |
From meson 0.51.0, cpp_eh defaults to /EHsc on windows. Passing it to add_program_arguments() means downstream projects using cpp_eh=a will get warnings.
|
@fanc999 Are you sure it should be specified? Here is when I pass CMAKE_BUILD_TYPE= but set the meson buildtype to release and b_ndebug=false The difference is If I do not pass release to CMake, but set meson to release and set b_ndebug=true, then -DNDEBUG will appear. tl;dr: 1 more question Chen-wei! I've snuck in one more commit to this PR, removing /EHsc. Meson has cpp_eh option from meson 0.51.0 so this should be ok, but you have a lot more insight here. Let me know if its a problem. |
|
Hi @talisein, For my part, removing the This means that Did you try building with With blessings, thank you! Oh, as a side note, I think after this PR gets merged, I will move the dependency detection to use both pkg-config and CMake, instead of manually looking for the libxml2 and libraries, since CMake's built-in search mechanism actually does the manual libxml2 headers and .lib search for us.1 |
|
Are you using the ninja backend or the vs backend? I can look at how this looks on windows tomorrow, but for now here's the linux situation With debug optimized, you can see -O2 -g -DNDEBUG gets added into the compile_command but everything else is the same. Here is debug for the sake of completeness, but it is the same situation, -g is passed twice. If you'd like a patch to try yourself, talisein/libxmlplusplus@d2a270c but I don't mind poking at it some more tomorrow. |
meson: Clear CMAKE C FLAGS for subproject
|
With RelWithDebInfo, we use flags Same projects, but looking at libxml++ files there are no differences between the two as expected. It uses Looking at release, Conclusion: If we additionally add CMAKE_C_FLAGS, we are nearly at what we want. In the debug variant, CMAKE still insists on adding -D_DEBUG; meson doesn't seem to add this flag itself ever; the msvc documentation suggests the define is added automatically. I have added a commit to implement this last behavior. Ideally we could just leave CMAKE_BUILD_TYPE blank and simply set CMAKE_C_FLAGS, but that results in -D_DEBUG being passed to the non-debug library variant, and I don't want to guess if that has a side effect. So instead we need to set CMAKE_C_FLAGS_RELEASE etc |
|
About all the compiler warnings: |
Adding a cmake wrap for libxml2 turned out to be a little more involved than the wrap-db, due to the cmake project being preconfigured to requiring several dependencies.
One design choice to be made: Do we want to only use the wrap on windows?
A github action run that shows this in use (along with #29) is at: https://github.com/talisein/libxmlplusplus/actions/runs/2544884141