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
Copy link
Copy Markdown
Collaborator

@ryanofsky ryanofsky commented Jan 9, 2026

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
Copy link
Copy Markdown

DrahtBot commented Jan 9, 2026

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
Copy link
Copy Markdown
Member

@hebasto hebasto left a comment

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.

Copy link
Copy Markdown
Collaborator

@sedited sedited left a comment

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
Copy link
Copy Markdown
Member

@hebasto hebasto left a comment

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