Skip to content

Conversation

@ryanofsky
Copy link
Collaborator

Make target_capnp_sources function take ONLY_CAPNP option to only add cap'n proto-generated files to the target library and not add libmultiprocess-generated ones.

This is needed in bitcoin/bitcoin#10102 to support building with ENABLE_IPC=ON and ENABLE_WALLET=OFF because libmultiprocess-generated wallet files wallet.capnp.proxy*.c++ can't be built without causing link errors, while the wallet.capnp.c++ file is still necessary to build because it is referenced by init.capnp and node.capnp there.

Make `target_capnp_sources` function take `ONLY_CAPNP` option to only add cap'n
proto-generated files to the target library and not add
libmultiprocess-generated ones.

This is needed in bitcoin/bitcoin#10102 to support
building with ENABLE_IPC=ON and ENABLE_WALLET=OFF because libmultiprocess-generated
wallet files `wallet.capnp.proxy*.c++` can't be built without causing link
errors, while the `wallet.capnp.c++` file is still necessary to build because
it is referenced by `init.capnp` and `node.capnp` there.
@DrahtBot
Copy link

DrahtBot commented Oct 22, 2025

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

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK hebasto

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Comment on lines -114 to +119
add_custom_target("${target}_headers" DEPENDS ${generated_headers})
if(NOT TARGET "${target}_headers")
add_custom_target("${target}_headers" DEPENDS ${generated_headers})
endif()
Copy link
Member

Choose a reason for hiding this comment

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

These changes are reasonable on their own. Maybe split them into a separate commit?

nit: I don’t think quoting is needed here.

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

ACK 81c6526, I have reviewed the code and it looks OK.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants