Skip to content

cmake, ci: updates for recent nixpkgs#238

Merged
ryanofsky merged 2 commits into
bitcoin-core:masterfrom
ryanofsky:pr/iwyufu
Jun 2, 2026
Merged

cmake, ci: updates for recent nixpkgs#238
ryanofsky merged 2 commits into
bitcoin-core:masterfrom
ryanofsky:pr/iwyufu

Conversation

@ryanofsky

@ryanofsky ryanofsky commented Jan 9, 2026

Copy link
Copy Markdown
Collaborator

This PR fixes two problems that happen with recent nixpkgs. The first commit works around compilation errors that happen when IWYU is enabled with (-DMP_ENABLE_IWYU=ON) that were caused by NixOS/nixpkgs#445095. The second commit fixes problems building old versions of Cap'n Proto with CMake 4.0 in shell.nix.

Neither problem has showed up so far running in github actions, only running locally with unstable nixpkgs. More details about the problems and fixes are in the commit messages.

Without this change, building with -DMP_ENABLE_IWYU=ON and newer versions of
nixpkgs (after 2025-11-12) fails with mysterious errors that look like:

make[2]: *** [CMakeFiles/mputil.dir/build.make:79: CMakeFiles/mputil.dir/src/mp/util.cpp.o] Error 1

with no other error messages. The are no messages because cmake -E
__run_co_compile hides the output from IWYU and does not display anything itself.

The actual problem is missing include directories on the compiler command line,
and the fix here is to extend a workaround previously added for clang-tidy to
be used for IWYU as well.

Breakage seems to have been caused by
NixOS/nixpkgs@76a8ffa
from NixOS/nixpkgs#445095, which I found by bisecting
with --first-parent to a change between the following merge commit:

https://github.com/NixOS/nixpkgs/commits/de21549a840be528ef4763dd7dca2006caf9c11f (bad)
https://github.com/NixOS/nixpkgs/commits/c4d1151093af5d483d1900d9214a65e68bc21560 (good)

If you check out the bad commit above (or any later commit) and revert
NixOS/nixpkgs@76a8ffa
the problem disappears.

I suspect the problem happens because cmake is not handling -cxx-isystem
properly, but did not debug further. The later change
NixOS/nixpkgs#462747 which followed up on
NixOS/nixpkgs#445095 did not seem to have any effect on
this issue in my testing.
@DrahtBot

DrahtBot commented Jan 9, 2026

Copy link
Copy Markdown

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 sedited, hebasto

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #276 (build: prepare for subtree split by Sjors)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

CMake 4.0 dropped support for older policies, so versions of Cap'n Proto <=
v1.1.0 (before capnproto/capnproto#2272) require an
additional option to build with CMake 4.0. Without this change, there are
errors like:

> CMake Error at CMakeLists.txt:1 (cmake_minimum_required):
>   Compatibility with CMake < 3.5 has been removed from CMake.
>
>   Update the VERSION argument <min> value.  Or, use the <min>...<max> syntax
>   to tell CMake that the project requires at least <min> but has been updated
>   to work with policies introduced by <max> or earlier.
>
>   Or, add -DCMAKE_POLICY_VERSION_MINIMUM=3.5 to try configuring anyway.

This change fixes the problem by specifying -DCMAKE_POLICY_VERSION_MINIMUM=3.5
when needed.
@ryanofsky ryanofsky changed the title cmake: Fix IWYU in nix by adding CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES cmake, ci: updates for recent nixpkgs Jan 9, 2026

@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 b66a6d3, I have reviewed the code and it looks OK.

@ryanofsky

Copy link
Copy Markdown
Collaborator Author

Updated b66a6d3 -> db7acb3 (pr/iwyufu.2 -> pr/iwyufu.3, compare) to remove an incorrect comment in the b66a6d3 commit message about the cmake 4.0 problem not affecting the olddeps CI job. The problem does affect the olddeps job when I run it locally. It doesn't affect github actions CI just because it is currently using CMake 3.31.6.

@sedited sedited left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

ACK db7acb3

@DrahtBot DrahtBot requested a review from hebasto February 2, 2026 10:18

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

re-ACK db7acb3.

Comment thread CMakeLists.txt
# CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES and omits them from the compile
# database, so clang-tidy, which ignores $NIX_CFLAGS_COMPILE, can't find capnp
# headers. Setting them as standard passes them to clang-tidy.
set(CMAKE_CXX_STANDARD_INCLUDE_DIRECTORIES ${CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES})

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

In commit "cmake: Fix IWYU in nix by adding CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES" (91a7759)

Note: I might merge this PR anyway since it has two acks, but this comment isn't exactly right since newer versions of clang-tidy in nixpkgs fix the issue and make this workaround unnecessary. The fix was NixOS/nixpkgs@d46fa26 (NixOS/nixpkgs#462747).

The problem with IWYU still exists but can be fixed with a similar change, which should probably be a PR:

--- a/pkgs/development/tools/analysis/include-what-you-use/wrapper
+++ b/pkgs/development/tools/analysis/include-what-you-use/wrapper
@@ -4,7 +4,7 @@ buildcpath() {
   local path after
   while (( $# )); do
     case $1 in
-        -isystem)
+        -isystem|-cxx-isystem)
             shift
             path=$path${path:+':'}$1
             ;;

@ryanofsky ryanofsky merged commit 4aaff11 into bitcoin-core:master Jun 2, 2026
10 checks passed
enirox001 added a commit to enirox001/sv2-tp that referenced this pull request Jun 3, 2026
61de697 Merge bitcoin-core/libmultiprocess#273: proxy-client: tolerate exceptions from remote destroy during cleanup
9cec9d6 Merge bitcoin-core/libmultiprocess#243: mpgen: support primitive std::optional struct fields
4aaff11 Merge bitcoin-core/libmultiprocess#238: cmake, ci: updates for recent nixpkgs
2ac55a5 Merge bitcoin-core/libmultiprocess#218: Better error and log messages
6de92e1 proxy-client: tolerate exceptions from remote destroy during cleanup
90be835 test: regression for ~ProxyClient destroy after peer disconnect
3c69d12 Merge bitcoin-core/libmultiprocess#260: event loop: tolerate unexpected exceptions in `post()` callbacks
b8a48c6 event loop: tolerate unexpected exceptions in `post()` callbacks
f787863 Merge bitcoin-core/libmultiprocess#270: doc: Bump version 10 > 11
a22f602 doc: Bump version 10 > 11
4eae445 debug: Add TypeName() function and log statements for Proxy objects being created and destroyed
f326c5b logging: Add better logging on IPC server-side failures
6dbfa56 mpgen: support primitive std::optional struct fields
8d1277d mpgen refactor: add AccessorType function
db716bb mpgen refactor: Move field handling code to FieldList class
db7acb3 ci: Fix shell.nix compatibility with CMake 4.0
91a7759 cmake: Fix IWYU in nix by adding CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES

git-subtree-dir: src/ipc/libmultiprocess
git-subtree-split: 61de6975362a7070276da47cc2aa2c2b8909ed49
Sjors added a commit to Sjors/sv2-tp that referenced this pull request Jun 17, 2026
8412fcdc65 Merge bitcoin-core/libmultiprocess#295: Mark Waiter m_cv as guarded by m_mutex
1593ee2d18 Merge bitcoin-core/libmultiprocess#294: test: Add passDouble smoke test
9885d7dd33 Merge bitcoin-core/libmultiprocess#286: proxy-client: fix TSan data race in clientDestroy
fa35501c4f Mark Waiter m_cv as guarded by m_mutex
faaedb11f8 test: Add passDouble smoke test
733c64318d Merge bitcoin-core/libmultiprocess#292: type-number: fix clang-tidy modernize-use-nullptr
9cc3479ab3 Merge bitcoin-core/libmultiprocess#291: cmake: Add `mp_headers` custom target
201abd9e3a Merge bitcoin-core/libmultiprocess#289: cmake: make target_capnp_sources use CURRENT dirs
99820c8aec Merge bitcoin-core/libmultiprocess#279: doc: Add comments to FIELD_* constants in proxy.h
73b985540c Merge bitcoin-core/libmultiprocess#278: doc: Fix and expand design.md
e7e91b2e23 Merge bitcoin-core/libmultiprocess#277: Add std::unordered_set support and a helper BuildList to dedup list build handlers
91a951f59a tidy fix: modernize-use-nullptr
16362f42d0 cmake: Add `mp_headers` custom target
615a94fe3a cmake: document ONLY_CAPNP option in target_capnp_sources
90982f75c6 mpgen: iwyu changes required by previous commit
25bb3e67f3 proxy-client: fix TSan data race in clientDestroy
620f297f31 cmake: make target_capnp_sources use CURRENT dirs
9de4b885aa test: use camelCase + $Proxy.name for FooStruct fields
011b91793d type: add std::unordered_set support
20d19b9644 proxy: add BuildList helper and dedup map/set/vector build handlers
e863c6cdf6 doc: Add comments to FIELD_* constants in proxy.h
18db0ab957 doc: Fix and expand design.md
61de697536 Merge bitcoin-core/libmultiprocess#273: proxy-client: tolerate exceptions from remote destroy during cleanup
9cec9d6ca5 Merge bitcoin-core/libmultiprocess#243: mpgen: support primitive std::optional struct fields
4aaff11374 Merge bitcoin-core/libmultiprocess#238: cmake, ci: updates for recent nixpkgs
2ac55a56b5 Merge bitcoin-core/libmultiprocess#218: Better error and log messages
6de92e1c73 proxy-client: tolerate exceptions from remote destroy during cleanup
90be8354d4 test: regression for ~ProxyClient destroy after peer disconnect
3c69d125a1 Merge bitcoin-core/libmultiprocess#260: event loop: tolerate unexpected exceptions in `post()` callbacks
b8a48c65e6 event loop: tolerate unexpected exceptions in `post()` callbacks
f787863d2c Merge bitcoin-core/libmultiprocess#270: doc: Bump version 10 > 11
a22f602910 doc: Bump version 10 > 11
4eae445d6d debug: Add TypeName() function and log statements for Proxy objects being created and destroyed
f326c5b1b7 logging: Add better logging on IPC server-side failures
6dbfa56a04 mpgen: support primitive std::optional struct fields
8d1277deb5 mpgen refactor: add AccessorType function
db716bbcba mpgen refactor: Move field handling code to FieldList class
db7acb3ce2 ci: Fix shell.nix compatibility with CMake 4.0
91a7759a9a cmake: Fix IWYU in nix by adding CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES

git-subtree-dir: src/ipc/libmultiprocess
git-subtree-split: 8412fcdc659e1379f9b4dea896c26bc1c5f3afa8
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.

4 participants