Skip to content

Conversation

@hebasto
Copy link
Member

@hebasto hebasto commented Mar 31, 2025

This PR automates handling of MP_INCLUDE_DIR in the target_capnp_sources function, removing the need to manually propagate it across directories for both this project and downstream projects.

Copy link
Collaborator

@ryanofsky ryanofsky left a comment

Choose a reason for hiding this comment

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

Code review ACK b553830. Nice change that simplifies bitcoin/bitcoin#31741 and seems to make good use of cmake's global property feature. I suggested some tweaks below but they could be followups.

This change automates handling of `MP_INCLUDE_DIR` in the
`target_capnp_sources` function, removing the need to manually propagate
it across directories for both this project and downstream projects.
@hebasto
Copy link
Member Author

hebasto commented Mar 31, 2025

@ryanofsky

Thank you for your review! Your feedback has been addressed.

Copy link
Collaborator

@ryanofsky ryanofsky left a comment

Choose a reason for hiding this comment

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

Code review ACK a77c8e1

Thanks for updates! Will test this with simplification to #31741 and merge it if everything seems like it works.

@ryanofsky ryanofsky merged commit 35944ff into bitcoin-core:master Mar 31, 2025
@hebasto hebasto deleted the 250331-mp-prop branch March 31, 2025 18:11
ryanofsky added a commit to ryanofsky/bitcoin that referenced this pull request Mar 31, 2025
Bump libmultiprocess library to include MP_INCLUDE_DIR definition from
bitcoin-core/libmultiprocess#168 which can simplify
the IPC cmake build as described
bitcoin#31741 (comment)

This update brings in the following changes:

bitcoin-core/libmultiprocess#166 doc: rename from chaincodelabs to bitcoin-core
bitcoin-core/libmultiprocess#168 Switch `MP_INCLUDE_DIR` to global property
ryanofsky added a commit to ryanofsky/bitcoin that referenced this pull request Apr 2, 2025
Bump libmultiprocess library to include MP_INCLUDE_DIR definition from
bitcoin-core/libmultiprocess#168 which can simplify
the IPC cmake build as described
bitcoin#31741 (comment)

This update brings in the following changes:

bitcoin-core/libmultiprocess#166 doc: rename from chaincodelabs to bitcoin-core
bitcoin-core/libmultiprocess#168 Switch `MP_INCLUDE_DIR` to global property
janus pushed a commit to BitgesellOfficial/bitgesell that referenced this pull request Sep 7, 2025
Bump libmultiprocess library to include MP_INCLUDE_DIR definition from
bitcoin-core/libmultiprocess#168 which can simplify
the IPC cmake build as described
bitcoin/bitcoin#31741 (comment)

This update brings in the following changes:

bitcoin-core/libmultiprocess#166 doc: rename from chaincodelabs to bitcoin-core
bitcoin-core/libmultiprocess#168 Switch `MP_INCLUDE_DIR` to global property
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.

2 participants