cmake: skip missing example targets in libmultiprocess.cmake

[ryanofsky]

bitcoin-core/libmultiprocess#287 is removing the mpcalculator, mpprinter, and mpexample targets, which causes a configuration error when Bitcoin Core is compiled against the updated upstream libmultiprocess:

CMake Error at cmake/libmultiprocess.cmake:37 ... Can not find target to add properties to: mpcalculator

Fix this by iterating over the targets and only calling set_target_properties if the target exists. This is not needed for subtree builds (where upstream changes do not apply directly), but is useful for custom builds with an external libmultiprocess, and is needed to keep Bitcoin Core CI jobs working in the libmultiprocess repository.

[DrahtBot]

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35454.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	hebasto

If your review is incorrectly listed, please copy-paste <!--meta-tag:bot-skip--> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #35418 (build: exclude mptest target from compile commands by Sanjana2906)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

[hebasto]

Concept ACK on being defensive when integrating a subproject, regardless of bitcoin-core/libmultiprocess#287.

However, I believe that such issues should be handled by the subprojects themselves, as they shouldn't force parent projects to process CMake code they will never use. For example, for Libmultiprocess, this could be achieved with the following patch:

--- a/cmake/libmultiprocess.cmake
+++ b/cmake/libmultiprocess.cmake
@@ -29,10 +29,4 @@ function(add_libmultiprocess subdir)
     # Add tests to "all" target so ctest can run them
     set_target_properties(mptest PROPERTIES EXCLUDE_FROM_ALL OFF)
   endif()
-  # Exclude examples from compilation database, because the examples are not
-  # built by default, and they contain generated c++ code. Without this
-  # exclusion, tools like clang-tidy and IWYU that make use of compilation
-  # database would complain that the generated c++ source files do not exist. An
-  # alternate fix could build "mpexamples" by default like "mptests" above.
-  set_target_properties(mpcalculator mpprinter mpexample PROPERTIES EXPORT_COMPILE_COMMANDS OFF)
 endfunction()
--- a/src/ipc/libmultiprocess/CMakeLists.txt
+++ b/src/ipc/libmultiprocess/CMakeLists.txt
@@ -260,5 +260,7 @@ add_custom_target(install-lib
 add_dependencies(install-lib multiprocess)
 
 # Example and test subdirectories
-add_subdirectory(example EXCLUDE_FROM_ALL)
+if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
+  add_subdirectory(example EXCLUDE_FROM_ALL)
+endif()
 add_subdirectory(test EXCLUDE_FROM_ALL)

[ryanofsky]

 # Example and test subdirectories
-add_subdirectory(example EXCLUDE_FROM_ALL)
+if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
+  add_subdirectory(example EXCLUDE_FROM_ALL)
+endif()
 add_subdirectory(test EXCLUDE_FROM_ALL)

This doesn't seem like a great code change to me, especially without any comment explaining why example and test directories are treated differently. In general, it seems better to me to have fewer assumptions about parent projects in subprojects, not more. So I think bitcoin-core/libmultiprocess#287 is a cleaner way to avoid the fragility we are seeing here.

[hebasto]

In general, it seems better to me to have fewer assumptions about parent projects in subprojects...

Sure. As long as it does not "force parent projects to process CMake code they will never use."

The add_subdirectory() commands may be gated by dedicated build options.

[hebasto]

cc @purpleKarrot

[ryanofsky]

In general, it seems better to me to have fewer assumptions about parent projects in subprojects...

Sure. As long as it does not "force parent projects to process CMake code they will never use."

The add_subdirectory() commands may be gated by dedicated build options.

Agree a build option would be slightly better than hardcoding a "will never use" assumption, which would be a false assumption. There are plenty of reasons parent projects might want to include examples, especially parent projects created for CI or deployment which may want to test & package them.

Would probably oppose the build option, as well, though. Especially in this case when modularization seems like a better choice. Also, in other cases where defining all targets and having callers explicitly reference the targets they want to build is better than adding a level of indirection where options need to exist for every target and reconfiguring is necessary to build new targets. Whatever awkwardness currently exists around the compilation database seems like it should be fixed by code using the compilation database, because hardcoding targets to be excluded from database or adding options to exclude individual targets from compilation database add complexity that seems unnecessary if users of the database are can just use the parts they need and ignore the parts they don't need.