Skip to content

cmake: Support being included with add_subdirectory #136

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
merged 2 commits into from
Jan 27, 2025
Merged

cmake: Support being included with add_subdirectory #136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1103f86
cmake: Define and use MP_INCLUDE_DIR variable
ryanofsky Jan 26, 2025
6adbb1d
cmake: Support being included with add_subdirectory
ryanofsky Jan 26, 2025
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
27 changes: 21 additions & 6 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,32 @@ include("cmake/compat_config.cmake")
include("cmake/pthread_checks.cmake")
include(GNUInstallDirs)

# Set convenience variables for subdirectories.
set(MP_INCLUDE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/include")
if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
set(MP_STANDALONE TRUE)
Copy link
Member

Choose a reason for hiding this comment

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

Once the minimum required CMake version is bumped up to 3.21+, the MP_STANDALONE variable might be replaced with PROJECT_IS_TOP_LEVEL.

else()
# Set MP_INCLUDE_DIR for parent directories too, so target_capnp_sources calls
# in parent directories can use it and not need to specify include directories
# manually or see capnproto error "error: Import failed: /mp/proxy.capnp"
Comment on lines +34 to +36
Copy link
Member

Choose a reason for hiding this comment

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

This comment does not clarify why the propagating a variable to the parent namespace is better than specifying "include directories manually".

I think that the latter is better. I do think that add_subdirectory calls should not introduce any side effects in the call sites.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I do think specifying include directories is better in general. But I don't believe libmultiprocess exporting its own include path to callers and then requiring callers to pass that include path back to it is necessarily better. For example when you use a toolchain's cc command, the cc command will allow you to override the toolchain include paths and require you to specify your own include paths, but will not force you to specify the toolchain include paths.

So I think the change in 1103f86 is reasonable and good simplification, but I'm also fine with reverting it or just updating the comment here. You can let me know what you prefer and of course a PR would be welcome.

set(MP_INCLUDE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/include" PARENT_SCOPE)
set(MP_STANDALONE FALSE)
endif()

# Prevent include directories from parent project from leaking into this one.
set_property(DIRECTORY PROPERTY INCLUDE_DIRECTORIES "")

# Generated C++ preprocessor defines
configure_file(include/mp/config.h.in "${CMAKE_CURRENT_BINARY_DIR}/include/mp/config.h")

# Generated C++ Capn'Proto schema files
capnp_generate_cpp(MP_PROXY_SRCS MP_PROXY_HDRS include/mp/proxy.capnp)

# util library
add_library(util OBJECT src/mp/util.cpp)
target_include_directories(util PRIVATE
$<BUILD_INTERFACE:${CMAKE_SOURCE_DIR}/include>
$<BUILD_INTERFACE:${CMAKE_BINARY_DIR}/include>
add_library(mputil OBJECT src/mp/util.cpp)
target_include_directories(mputil PRIVATE
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
$<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}/include>
${CAPNP_INCLUDE_DIRECTORY})

# libmultiprocess.a runtime library
Expand All @@ -50,7 +65,7 @@ add_library(multiprocess STATIC
${MP_PROXY_SRCS}
${MP_PUBLIC_HEADERS}
src/mp/proxy.cpp
$<TARGET_OBJECTS:util>)
$<TARGET_OBJECTS:mputil>)
add_library(Libmultiprocess::multiprocess ALIAS multiprocess)
target_include_directories(multiprocess PUBLIC
$<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}/include>
Expand All @@ -68,7 +83,7 @@ install(TARGETS multiprocess EXPORT LibTargets
PUBLIC_HEADER DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/mp COMPONENT lib)

# mpgen code generator
add_executable(mpgen src/mp/gen.cpp $<TARGET_OBJECTS:util>)
add_executable(mpgen src/mp/gen.cpp $<TARGET_OBJECTS:mputil>)
add_executable(Libmultiprocess::mpgen ALIAS mpgen)
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:${CMAKE_INSTALL_INCLUDEDIR}>)
Expand Down
4 changes: 2 additions & 2 deletions cmake/TargetCapnpSources.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ function(target_capnp_sources target include_prefix)
foreach(capnp_file IN LISTS TCS_UNPARSED_ARGUMENTS)
add_custom_command(
OUTPUT ${capnp_file}.c++ ${capnp_file}.h ${capnp_file}.proxy-client.c++ ${capnp_file}.proxy-types.h ${capnp_file}.proxy-server.c++ ${capnp_file}.proxy-types.c++ ${capnp_file}.proxy.h
COMMAND Libmultiprocess::mpgen ${CMAKE_CURRENT_SOURCE_DIR} ${include_prefix} ${CMAKE_CURRENT_SOURCE_DIR}/${capnp_file} ${TCS_IMPORT_PATHS}
COMMAND Libmultiprocess::mpgen ${CMAKE_CURRENT_SOURCE_DIR} ${include_prefix} ${CMAKE_CURRENT_SOURCE_DIR}/${capnp_file} ${TCS_IMPORT_PATHS} ${MP_INCLUDE_DIR}
DEPENDS ${capnp_file}
VERBATIM
)
Expand All @@ -89,7 +89,7 @@ function(target_capnp_sources target include_prefix)
if(relative_path)
string(APPEND build_include_prefix "/" "${relative_path}")
endif()
target_include_directories(${target} PUBLIC $<BUILD_INTERFACE:${build_include_prefix}>)
target_include_directories(${target} PUBLIC $<BUILD_INTERFACE:${build_include_prefix}> ${MP_INCLUDE_DIR})

if(TARGET Libmultiprocess::multiprocess)
target_link_libraries(${target} PRIVATE Libmultiprocess::multiprocess)
Expand Down
14 changes: 4 additions & 10 deletions example/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,29 +7,23 @@ include(${PROJECT_SOURCE_DIR}/cmake/TargetCapnpSources.cmake)
add_executable(mpcalculator
calculator.cpp
)
target_capnp_sources(mpcalculator ${CMAKE_CURRENT_SOURCE_DIR} init.capnp calculator.capnp printer.capnp
IMPORT_PATHS ${CMAKE_SOURCE_DIR}/include
)
target_capnp_sources(mpcalculator ${CMAKE_CURRENT_SOURCE_DIR} init.capnp calculator.capnp printer.capnp)
target_include_directories(mpcalculator PRIVATE ${CMAKE_CURRENT_SOURCE_DIR})
target_link_libraries(mpcalculator PRIVATE Threads::Threads)

add_executable(mpprinter
printer.cpp
)
target_capnp_sources(mpprinter ${CMAKE_CURRENT_SOURCE_DIR} init.capnp calculator.capnp printer.capnp
IMPORT_PATHS ${CMAKE_SOURCE_DIR}/include
)
target_capnp_sources(mpprinter ${CMAKE_CURRENT_SOURCE_DIR} init.capnp calculator.capnp printer.capnp)
target_include_directories(mpprinter PRIVATE ${CMAKE_CURRENT_SOURCE_DIR})
target_link_libraries(mpprinter PRIVATE Threads::Threads)

add_executable(mpexample
example.cpp
)
target_capnp_sources(mpexample ${CMAKE_CURRENT_SOURCE_DIR} init.capnp calculator.capnp printer.capnp
IMPORT_PATHS ${CMAKE_SOURCE_DIR}/include
)
target_capnp_sources(mpexample ${CMAKE_CURRENT_SOURCE_DIR} init.capnp calculator.capnp printer.capnp)
target_include_directories(mpexample PRIVATE ${CMAKE_CURRENT_SOURCE_DIR})
target_link_libraries(mpexample PRIVATE Threads::Threads)
target_link_libraries(mpexample PRIVATE stdc++fs)

add_custom_target(example DEPENDS mpexample mpcalculator mpprinter)
add_custom_target(mpexamples DEPENDS mpexample mpcalculator mpprinter)
17 changes: 11 additions & 6 deletions test/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,15 @@ include(CTest)
# that were previously built, without building anything itself. Define "make
# tests" here as a custom target to build all available tests and "make check"
# as a custom target to build and run them.
add_custom_target(tests)
add_custom_target(check COMMAND ${CMAKE_CTEST_COMMAND} DEPENDS tests)
add_custom_target(mptests)
add_custom_target(mpcheck COMMAND ${CMAKE_CTEST_COMMAND} DEPENDS mptests)

# Only add more convenient tests and check targets if project is being built
# standlone, to prevent clashes with external projects.
if (MP_STANDALONE)
add_custom_target(tests DEPENDS mptests)
add_custom_target(check DEPENDS mpcheck)
endif()

if(BUILD_TESTING AND TARGET CapnProto::kj-test)
set_property(SOURCE ${MP_PROXY_HDRS} PROPERTY GENERATED 1)
Expand All @@ -24,13 +31,11 @@ if(BUILD_TESTING AND TARGET CapnProto::kj-test)
mp/test/test.cpp
)
include(${PROJECT_SOURCE_DIR}/cmake/TargetCapnpSources.cmake)
target_capnp_sources(mptest ${CMAKE_CURRENT_SOURCE_DIR} mp/test/foo.capnp
IMPORT_PATHS ${CMAKE_SOURCE_DIR}/include
)
target_capnp_sources(mptest ${CMAKE_CURRENT_SOURCE_DIR} mp/test/foo.capnp)
target_include_directories(mptest PRIVATE ${CMAKE_CURRENT_SOURCE_DIR})
target_link_libraries(mptest PRIVATE CapnProto::kj-test)
target_link_libraries(mptest PRIVATE Threads::Threads)

add_dependencies(tests mptest)
add_dependencies(mptests mptest)
add_test(NAME mptest COMMAND mptest)
endif()