cmake: check for Cap'n Proto / Clang / C++20 incompatibility #205

Cap'n Proto 0.9.x and 0.10.x are incompatible with Clang 16+ when using C++20 due to P2468R2 implementation. This was fixed in Cap'n Proto 1.0+.

Generate a helpful error when these combinations are detected.

See: #199 for more context

<!--e57a25ab6845829454e8d69fc972939a-->

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

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

See the guideline for information on the review process.

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

<!--5faf32d7da4f0f540f40219e4f7537a3-->

I've left direct reference to Bitcoin Core in the error message as, although this library is pretty generic, it seems that Bitcoin Core is the primary (or perhaps only) user, and therefore worth including for the tip on disabling IPC to bypass.

Code review ACK ee5e284cca563eb43ec3587d114586865db70b43. This seems like it should be a helpful change.

The approach suggested https://github.com/bitcoin/bitcoin/pull/33290#issuecomment-3249470528 could potentially be used to avoid needing to mention the bitcoin core option here, and could be a followup

LLM linter also suggested some reasonable edits #205 (comment)

OK, I am a bit of a cmake language noob, so apologies in advance if I've butchered this, but I've tried to share the common error message in 197aef2e2b4a4dd35ffa2bf5783ba6fe636ea48d to along with @fanquake's suggestion of not recommending downgrading (which seems sensible).

To avoid also the situation where we have two seperate checks which a user could fail sequentially (but only after installing a second version of capnproto!), I have tried to merge them into a single check, with logic for each check type.

This way a user should only ever fail on this version check once, and should be shown appropriate options.

@willcl-ark I started to look at 197aef2e2b4a4dd35ffa2bf5783ba6fe636ea48d which seems like a nice approach.

Any interested in extending it to replace https://github.com/bitcoin/bitcoin/pull/33290? It would be great to show a better error message when find_package(CapnProto) fails here and it looks like the new code you've written could be easily extended to do it.

I think you would just need to replace

find_package(CapnProto 0.7 REQUIRED NO_MODULE)

with

find_package(CapnProto 0.7 QUIET NO_MODULE)
if (NOT CapnProto_FOUND)
   # ... custom handling here ...
endif()

Code review ACK ee5e284cca563eb43ec3587d114586865db70b43. New code is clean and pretty straightforward and I really like the approach because it clearly spells out the solutions instead of requiring users and developers to look for them.

@ryanofsky I added the suggested check here.

I did look at having a single giant "check package, then version compatibility" section (i.e. extending what is there currently), but the code is much cleaner using a specific

find_package(CapnProto 0.7 QUIET NO_MODULE)
 if(NOT CapnProto_FOUND)
   message(FATAL_ERROR

...even if that means we repeat a string or two.

I tested 657d80622f81ba3258780b2157a2771195ee4988 by updating the subtree in Bitcoin Core, and uninstalling capnp:

CMake Error at src/ipc/libmultiprocess/CMakeLists.txt:18 (message):
  Cap'n Proto is required but was not found.

  To resolve, choose one of the following:

    - Install Cap'n Proto (version 1.0+ recommended)
    - For Bitcoin Core compilation build with -DENABLE_IPC=OFF to disable multiprocess support

That's quite nice.

Might make sense to update PR title and description to reflect broader scope of this change now

Testing 657d80622f81ba3258780b2157a2771195ee4988 with different settings in olddeps.bash and it seems to work well.

<details><summary>setting 0.9.0 in olddeps.bash warns about CVE and suggests upgrading to major or minor version or disabling</summary> <p>

$ rm -rvf build-olddeps; CI_CONFIG=ci/configs/olddeps.bash ci/scripts/run.sh
[...]
CMake Error at CMakeLists.txt:76 (message):
  The version of Cap'n Proto detected: 0.9.0 has known compatibility issues:

  - CVE-2022-46149 security vulnerability (details:
  https://github.com/advisories/GHSA-qqff-4vw4-f6hx)

  To resolve, choose one of the following:

    - Upgrade to Cap'n Proto version 1.0 or newer (recommended)
    - Upgrade to a patched minor version (0.7.1, 0.8.1, 0.9.2, 0.10.3, or later)
    - For Bitcoin Core compilation build with -DENABLE_IPC=OFF to disable multiprocess support

</p> </details>

<details><summary>setting 0.9.0 in and export CXX=clang++ in olddeps.bash suggests upgrading to major version or disabling</summary> <p>

$ rm -rf build-olddeps; CI_CONFIG=ci/configs/olddeps.bash ci/scripts/run.sh
[...]
CMake Error at CMakeLists.txt:76 (message):
  The version of Cap'n Proto detected: 0.9.0 has known compatibility issues:

  - CVE-2022-46149 security vulnerability (details:
  https://github.com/advisories/GHSA-qqff-4vw4-f6hx)

  - Incompatible with Clang 20.1.5 when using C++20

  To resolve, choose one of the following:

    - Upgrade to Cap'n Proto version 1.0 or newer (recommended)
    - For Bitcoin Core compilation build with -DENABLE_IPC=OFF to disable multiprocess support

</p> </details>

<details><summary>setting 0.9.2 in and export CXX=clang++ in olddeps.bash suggests using gcc or updating to major version or disabling</summary> <p>

$ rm -rf build-olddeps; CI_CONFIG=ci/configs/olddeps.bash ci/scripts/run.sh
[...]
CMake Error at CMakeLists.txt:76 (message):
  The version of Cap'n Proto detected: 0.9.2 has known compatibility issues:

  - Incompatible with Clang 20.1.5 when using C++20

  To resolve, choose one of the following:

    - Upgrade to Cap'n Proto version 1.0 or newer (recommended)
    - Use GCC instead of Clang
    - For Bitcoin Core compilation build with -DENABLE_IPC=OFF to disable multiprocess support

</p> </details>

<details><summary>commenting out capnproto input in shell.nix suggests installing cap'n proto or disabling</summary> <p>

$ rm -rf build-olddeps; CI_CONFIG=ci/configs/olddeps.bash ci/scripts/run.sh
[...]
CMake Error at CMakeLists.txt:18 (message):
  Cap'n Proto is required but was not found.

  To resolve, choose one of the following:

    - Install Cap'n Proto (version 1.0+ recommended)
    - For Bitcoin Core compilation build with -DENABLE_IPC=OFF to disable multiprocess support

</p> </details>

Code review ACK 657d80622f81ba3258780b2157a2771195ee4988. Thanks for these changes! I think they should very helpful and prevent unnecessary confusion.

Concept ACK.

As a follow-up, all code related to handling the CapnProto package could be encapsulated in a cmake/FindCapnProto.cmake module.

I think I'll go ahead and merge this now, and we can iterate and improve in future PRs. There are good ideas above for fixing up an indent, changing wording, and moving code to a module. There is also the MP_SUBPROJECT_ERROR idea from https://github.com/bitcoin/bitcoin/pull/33290#issuecomment-3249470528 and possibility of extending this to detect an incompatible netbsd package from #196 (comment)