This PR adds configurations for the Libmultiprocess and LibmultiprocessGen packages.
The Bitcoin Core branch, with uses this PR branch, is available here.
As a suggestion for a follow-up, it seems worth disabling mpgen target for cross-compiling (see https://github.com/bitcoin/bitcoin/pull/29665#issuecomment-2002027957 and CMake docs).
Based on https://github.com/chaincodelabs/libmultiprocess/pull/95.
Looks nice! Will review this. I remembered you also posted a proof of concept branch defining packages last year in https://github.com/chaincodelabs/libmultiprocess/pull/79#issuecomment-1407430661 that I guess this supercedes?
I remembered you also posted a proof of concept branch defining packages last year in #79 (comment) that I guess this supercedes?
Yes, it does.
Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
The `<Package>Targets` pattern is widely used and does not cause
confusion.
99 | +install(EXPORT LibmultiprocessTargets 100 | NAMESPACE Libmultiprocess:: 101 | - DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake COMPONENT lib) 102 | + DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/Libmultiprocess COMPONENT lib) 103 | +include(CMakePackageConfigHelpers) 104 | +configure_package_config_file(
I think there may be need to be some additional changes to this file to make the package "relocatable" according to https://cmake.org/cmake/help/latest/guide/importing-exporting/index.html#creating-relocatable-packages.
Specifically the PUBLIC include directory $<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}> on line 84 above and ${CAPNP_INCLUDE_DIRECTORY}` on line 85 look like they will add include paths from the build system to the exported targets, and potentially cause problems in some build configurations. This might not be a problem for the bitcoin depends build, though, because it is building everything on one system.
Possible fixes to replace the $<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}> line might be:
$<INSTALL_INTERFACE:include> or $<INSTALL_INTERFACE:$<INSTALL_PREFIX>/include as suggested in https://cmake.org/cmake/help/latest/guide/importing-exporting/index.html#creating-relocatable-packagesINCLUDES DESTINATION directive described https://cmake.org/cmake/help/latest/manual/cmake-buildsystem.7.html#include-directories-and-usage-requirements which is apparently "equivalent to appending ${CMAKE_INSTALL_PREFIX}/include"Possible fix to replace the ${CAPNP_INCLUDE_DIRECTORY} line might be to change target_link_libraries(multiprocess PRIVATE CapnProto::capnp) from PRIVATE to PUBLIC, assuming that making capnproto a public dependency will cause cmake to propagate the necessary includes.
Code review ACK 4e70ad47191f85188da869400690c22f6f81e1e0. This should make it easier to depend on libmultprocess in cmake projects, using a find_package call and not having to include files directly.
I think I would like to rename a few things in the PR, to make new package names match existing component names (bin and lib). I also don't like the redundant "Libmultiprocess/Libmultiprocess" in some installed filenames, and the way one package name is a prefix of the other, since they are side-by-side dependencies. I can post a followup PR with some renames though.