Skip to content

ipc: Fix mpgen capnp tool path for vcpkg/Windows builds - #317

Open
ryanofsky wants to merge 1 commit into
bitcoin-core:masterfrom
ryanofsky:pr/win-cmake
Open

ipc: Fix mpgen capnp tool path for vcpkg/Windows builds#317
ryanofsky wants to merge 1 commit into
bitcoin-core:masterfrom
ryanofsky:pr/win-cmake

Conversation

@ryanofsky

Copy link
Copy Markdown
Collaborator

It seems that in the vcpkg capnproto package, the capnp code generator can't be found in <capnp_PREFIX>/bin/capnp but is in another path that needs to be read from the cmake configuration, so this change adds cmake code to handle that.

Unfortunately the previous code using <capnp_PREFIX>/bin can't be removed because the llbcapnp-dev 1.0.1 package in Ubuntu Noble is buggy and specifies the wrong executable path in its cmake information. (If this problem is fixed, the old capnp_PREFIX code could be dropped.)

This change is needed to get MSVC builds working in bitcoin/bitcoin#32387

Error seen (MSVC native build):
  mpgen error: mp\util.cpp:332: failed: CreateProcessA(...): bitcoin-core#2 The system
  cannot find the file specified.

Problem: mpgen constructs capnp tool paths as capnp_PREFIX + "/bin/capnp"
and capnp_PREFIX + "/bin/capnpc-c++". capnp_PREFIX is derived from the
capnproto include directory parent, so on a vcpkg install it resolves to
something like vcpkg_installed/x64-windows-release. But vcpkg installs
executables under tools/capnproto/, not bin/, so the constructed path
does not exist and CreateProcessA fails with ERROR_FILE_NOT_FOUND.

Fix: pass the CAPNP_EXECUTABLE and CAPNPC_CXX_EXECUTABLE cmake variables
from CapnProtoConfig.cmake as compile definitions to mpgen, and use them
in gen.cpp instead of constructing paths from capnp_PREFIX.

Using these cmake variables rather than reading the CapnProto::capnp_tool
imported target IMPORTED_LOCATION directly is important: on Debian/Ubuntu,
IMPORTED_LOCATION is set to a non-existent path due to a multiarch cmake
packaging bug in libcapnp-dev (see
bitcoin-core#328). The cmake
variables avoid this because CapnProtoConfig.cmake hardcodes the correct
paths on Debian/Ubuntu, and on other platforms (including vcpkg) the
variables hold generator expressions that target_compile_definitions
evaluates correctly at generation time.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@DrahtBot

DrahtBot commented Jul 22, 2026

Copy link
Copy Markdown

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

Reviews

See the guideline and AI policy for information on the review process.

Type Reviewers
ACK hebasto

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

@hebasto hebasto left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I reviewed 049dae1 and played with the code on Windows using the following vcpkg.json file:

{
  "$comment": "The builtin-baseline corresponds to 2025.08.27 Release",
  "$schema": "https://raw.githubusercontent.com/microsoft/vcpkg-tool/main/docs/vcpkg.schema.json",
  "builtin-baseline": "120deac3062162151622ca4860575a33844ba10b",
  "dependencies": [
    "capnproto"
  ]
}

The code works. However, I believe the build system could do better and always define CAPNP_EXECUTABLE and CAPNPC_CXX_EXECUTABLE to verified paths.
See this branch: https://github.com/hebasto/libmultiprocess/commits/pr317/0731.alt/
Additionally, the code is organised into a function, and unnecessary quotes are removed.

Feel free to pick any part of that branch.

@ryanofsky

Copy link
Copy Markdown
Collaborator Author

Thanks for the review! I dug into the debian bug a little more and think I found a simpler workaround. The details of the bug are in #328. I will push an update to bitcoin/bitcoin#32387 including this workaround so we can be sure it still works for MSVC.


Updated 049dae1 -> a26a084 (pr/win-cmake.1 -> pr/win-cmake.2, compare) switching to simpler workaround for the debian packaging bug

@hebasto hebasto left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ACK a26a084, I've verified CAPNP_EXECUTABLE and CAPNPC_CXX_EXECUTABLE values on Ubuntu 24.04, Fedora 44 and Windows 11.

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