Skip to content

Install Exports for custom install-{lib,bin} targets #79

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ryanofsky merged 2 commits into from
Feb 14, 2023
Merged

Install Exports for custom install-{lib,bin} targets #79

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
54bd57f
Use `GNUInstallDirs` module
hebasto Jan 27, 2023
2dda753
Install Exports for custom `install-{lib,bin}` targets
hebasto Jan 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 17 additions & 12 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ set(CMAKE_CXX_STANDARD_REQUIRED YES)

include(CMakePushCheckState)
include(CheckCXXSourceCompiles)
include(GNUInstallDirs)
find_package(CapnProto REQUIRED)
find_package(Threads REQUIRED)

Expand Down Expand Up @@ -70,27 +71,30 @@ add_library(multiprocess STATIC
target_include_directories(multiprocess PUBLIC
$<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}/include>
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
$<INSTALL_INTERFACE:include>
$<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}>
${CAPNP_INCLUDE_DIRECTORY})
target_link_libraries(multiprocess PRIVATE CapnProto::capnp)
target_link_libraries(multiprocess PRIVATE CapnProto::capnp-rpc)
target_link_libraries(multiprocess PRIVATE CapnProto::kj)
target_link_libraries(multiprocess PRIVATE CapnProto::kj-async)
set_target_properties(multiprocess PROPERTIES
PUBLIC_HEADER "${MP_PUBLIC_HEADERS}")
install(TARGETS multiprocess EXPORT Multiprocess
ARCHIVE DESTINATION lib COMPONENT lib
PUBLIC_HEADER DESTINATION include/mp COMPONENT lib)
install(TARGETS multiprocess EXPORT libmultiprocess-lib
ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR} COMPONENT lib
PUBLIC_HEADER DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/mp COMPONENT lib)
install(FILES "${CMAKE_CURRENT_BINARY_DIR}/pkgconfig/libmultiprocess.pc"
DESTINATION "lib/pkgconfig" COMPONENT lib)
DESTINATION ${CMAKE_INSTALL_LIBDIR}/pkgconfig COMPONENT lib)
Copy link
Collaborator

@ryanofsky ryanofsky Nov 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In commit "Use GNUInstallDirs module" (54bd57f)

I think this change is right, but it did seem to cause a problem with a depends build that fanquake is fixing in bitcoin/bitcoin#28846 commit bitcoin/bitcoin@156366f

On fedora, replacing lib with ${CMAKE_INSTALL_LIBDIR} here causes the pkgconfig file to be installed in lib64/ instead of lib/, which seems right and makes sense according to explanations given https://cmake.org/cmake/help/latest/module/GNUInstallDirs.html and https://github.com/Kitware/CMake/blob/master/Modules/GNUInstallDirs.cmake

But this is incompatible with the depends build that hardcodes the lib/ directory: https://github.com/bitcoin/bitcoin/blob/d752349029ec7a76f1fd440db2ec2e458d0f3c99/depends/config.site.in#L91, so hardcoding lib again in bitcoin/bitcoin#28846 is needed after this change.

Copy link
Member Author

@hebasto hebasto Nov 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. The current Bitcoin Core depends build system will handle any other package built and installed with CMake in this flawed way. A generic fix for it looks quite reasonable.

Copy link
Collaborator

@ryanofsky ryanofsky Nov 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. The current Bitcoin Core depends build system will handle any other package built and installed with CMake in this flawed way.

The depends system definitely has a bug because it will install the library in "lib64" on some platforms, but look for it in "lib" regardless of the platform. But the approach isn't really flawed, is it? It seems reasonable for the depends system to just always install the library in "lib" and always look for it there.

Copy link
Member Author

@hebasto hebasto Nov 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with either approach. But in the light of the upcoming migration from Autotools to CMake, the forced installation to the lib subdirectory looks useless.

install(EXPORT libmultiprocess-lib
NAMESPACE Libmultiprocess::
DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake COMPONENT lib)
add_custom_target(install-lib
COMMAND ${CMAKE_COMMAND} -DCOMPONENT=lib -P ${CMAKE_CURRENT_BINARY_DIR}/cmake_install.cmake
VERBATIM)
add_dependencies(install-lib multiprocess)

add_executable(mpgen src/mp/gen.cpp $<TARGET_OBJECTS:util>)
target_include_directories(mpgen PRIVATE $<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}/include>)
target_include_directories(mpgen PUBLIC $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include> $<INSTALL_INTERFACE:include>)
target_include_directories(mpgen PUBLIC $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include> $<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}>)
target_link_libraries(mpgen PRIVATE CapnProto::capnp)
target_link_libraries(mpgen PRIVATE CapnProto::capnp-rpc)
target_link_libraries(mpgen PRIVATE CapnProto::capnpc)
Expand All @@ -100,11 +104,14 @@ set_target_properties(mpgen PROPERTIES
INSTALL_RPATH_USE_LINK_PATH TRUE)
set_target_properties(mpgen PROPERTIES
PUBLIC_HEADER include/mp/proxy.capnp)
install(TARGETS mpgen EXPORT Multiprocess
RUNTIME DESTINATION bin COMPONENT bin
PUBLIC_HEADER DESTINATION include/mp COMPONENT bin)
install(TARGETS mpgen EXPORT libmultiprocess-bin
RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR} COMPONENT bin
PUBLIC_HEADER DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/mp COMPONENT bin)
install(FILES "include/mpgen.mk"
DESTINATION "include" COMPONENT bin)
DESTINATION ${CMAKE_INSTALL_INCLUDEDIR} COMPONENT bin)
install(EXPORT libmultiprocess-bin
NAMESPACE Libmultiprocess::
DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake COMPONENT bin)
add_custom_target(install-bin
COMMAND ${CMAKE_COMMAND} -DCOMPONENT=bin -P ${CMAKE_CURRENT_BINARY_DIR}/cmake_install.cmake
VERBATIM)
Expand All @@ -113,7 +120,5 @@ add_dependencies(install-bin mpgen)
configure_file(include/mp/config.h.in "${CMAKE_CURRENT_BINARY_DIR}/include/mp/config.h")
configure_file(pkgconfig/libmultiprocess.pc.in "${CMAKE_CURRENT_BINARY_DIR}/pkgconfig/libmultiprocess.pc" @ONLY)

install(EXPORT Multiprocess DESTINATION lib/cmake/Multiprocess)

add_subdirectory(example EXCLUDE_FROM_ALL)
add_subdirectory(test EXCLUDE_FROM_ALL)