Skip to content

Switch MP_INCLUDE_DIR to global property #168

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.

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 1 commit into from
Mar 31, 2025

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
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