cmake: Errors not actually errors? #33153

fanquake commented at 12:29 PM on August 7, 2025: member

Using current master (6a2bb0fd835977637e0f43db76121bbfd21565f6), if you configure with the following, CMake will "Error":

# gcc --version
# gcc (Ubuntu 14.2.0-4ubuntu2~24.04) 14.2.0

cmake -B build -DBUILD_KERNEL_LIB=ON -DCMAKE_INTERPROCEDURAL_OPTIMIZATION=ON
<snip>
Cross compiling ....................... FALSE
C++ compiler .......................... GNU 14.2.0, /usr/bin/c++
CMAKE_BUILD_TYPE ...................... RelWithDebInfo
Preprocessor defined macros ........... 
C++ compiler flags .................... -O2 -g -std=c++20 -fPIC -fno-extended-identifiers -fdebug-prefix-map=/root/ci_scratch/src=. -fmacro-prefix-map=/root/ci_scratch/src=. -fstack-reuse=none -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -Wstack-protector -fstack-protector-all -fcf-protection=full -fstack-clash-protection -Wall -Wextra -Wformat -Wformat-security -Wvla -Wredundant-decls -Wdate-time -Wduplicated-branches -Wduplicated-cond -Wlogical-op -Woverloaded-virtual -Wsuggest-override -Wimplicit-fallthrough -Wunreachable-code -Wbidi-chars=any -Wundef -Wno-unused-parameter
Linker flags .......................... -O2 -g -fstack-reuse=none -fstack-protector-all -fcf-protection=full -fstack-clash-protection -Wl,-z,relro -Wl,-z,now -Wl,-z,separate-code -fPIE -pie

NOTE: The summary above may not exactly match the final applied build flags
      if any additional CMAKE_* or environment variables have been modified.
      To see the exact flags applied, build with the --verbose option.

Treat compiler warnings as errors ..... OFF
Use ccache for compiling .............. ON


-- Configuring done (23.3s)
CMake Error at src/kernel/CMakeLists.txt:11 (add_library):
  CMake doesn't support IPO for current compiler


-- Generating done (0.1s)
CMake Generate step failed.  Build files cannot be regenerated correctly.

However you can just call cmake --build build, and compilation "works":

cmake --build build -j14 --target bitcoinkernel
[  0%] Generating bitcoin-build-info.h
[  0%] Built target secp256k1_precomputed
[  0%] Built target crc32c
[ 14%] Built target bitcoin_crypto
[ 14%] Built target secp256k1
[ 42%] Built target leveldb
[ 42%] Built target generate_build_info
[ 42%] Built target bitcoin_clientversion
[100%] Built target bitcoinkernel

# file build/lib/libbitcoinkernel.a 
build/lib/libbitcoinkernel.a: current ar archive

It would be good if when CMake errors during configuration, it's not actually possible to continue building, rather than silently ignoring errors.

Note that the presence of -Werror=dev when configuring, makes no difference here.

Also, the error string CMake is throwing, is also incorrect, as the compiler does support LTO.

fanquake added the label Build system on Aug 7, 2025

maflcko commented at 12:39 PM on August 7, 2025: member

in the Guix build, errors will just be silently ignored.

I think the guix build has set -e, so it should be fine and not proceed here.

hebasto commented at 12:44 PM on August 7, 2025: member

Using current master (6a2bb0f), if you configure with the following, CMake will "Error"

What version of CMake are you using?

fanquake commented at 1:08 PM on August 7, 2025: member

What version of CMake are you using?

Happens with 3.28.3 & 4.0.3.

fanquake commented at 1:12 PM on August 7, 2025: member

I think the guix build has set -e, so it should be fine and not proceed here.

Looks like this is the case, and tested the build does fail. So I guess it will just (potentially) silently ignore errors everywhere else.

hebasto commented at 3:22 PM on August 7, 2025: member

Also, the error string CMake is throwing, is also incorrect, as the compiler does support LTO.

CMake doesn't blame the compiler, rather itself: "CMake doesn't support IPO for current compiler".

For some reason, this occurs with the "Unix Makefiles" generator but not with the "Ninja" generator.

hebasto commented at 3:22 PM on August 7, 2025: member

cc @purpleKarrot

hebasto commented at 3:34 PM on August 7, 2025: member

It seems that the $<TARGET_OBJECTS:..> added to bitcoinkernel sources in #33077 are causing the issue.

cc @theuni

purpleKarrot commented at 2:19 PM on August 12, 2025: contributor

It is why I am against using $<TARGET_OBJECTS:..>. CMake propagates a lot of information across the edges in the build dependency graph. Extracting only the objects from a target does not create an edge and therefore a lot of necessary information is not propagated properly. Manually forwarding each individual target property is not practical either. We simply don't know the complete set of target properties, as some of them are internal.

We should not use $<TARGET_OBJECTS:..>, at all. We may link libraries with target_link_libraries(), which creates a proper edge in the build dependency graph. In the case of a static library with a private dependency, it means that this dependency is not as private as you might intend: There will be a link-only edge in the dependency graph.

But as I wrote in #33077 (comment), I would like to see a proof of concept example that linking against a prebuilt static library with a custom toolchain is even possible and safe. None of the internal symbols of the kernel library are ODR protected. What happens if a client statically links against both the kernel and secp? With the information in the dependency graph that secp is a link-only dependency, version conflicts can be detected at configure time. But without that information?

fanquake added this to the milestone 30.0 on Sep 2, 2025

fanquake commented at 3:36 PM on September 2, 2025: member

Put this on v30.0, because we shouldn't ship a build system that produces errors, and then just ignores them.

purpleKarrot commented at 10:07 AM on September 3, 2025: contributor

I would suggest the following changes:

Revert fdbade6f8ded63519b637c8fa6f39914a400ab5e
Install the kernel library iff it is built as a shared library.
Export cmake package configuration iff the kernel is built as a shared library.
Don't install a .pc file for the kernel at all.

Rationale:

.pc files are not relocatable and only work when all packages are installed to a common prefix, which is done by package managers. Since we want to distribute our own packages, we have no way to provide .pc files that are not broken. It is better to provide no .pc files at all. We may export CMake package configurations, which don't have this problem. We might also export experimental CPS files for consumers that don't (want to) use CMake.

The problem with a static kernel library is even worse, as I have explained above.

ryanofsky commented at 8:02 PM on September 3, 2025: contributor

Since we want to distribute our own packages, we have no way to provide .pc files that are not broken.

This seems like a reason not to distribute .pc files, but it doesn't seem like a reason not to generate them in local builds and generate them for package managers. There are other build systems besides cmake, and it seems like it would be nice if you could build and install the kernel from source or use a package manager, and build against the library in a meson project, or rust/zig/python/go project with pkgconfig.

Maybe I am misunderstanding "Since we want to distribute our own packages" though. I'm not sure what that means exactly. If the idea is to distribute precompiled binaries with relocatable cmake modules that seems like it could be a good approach, but probably less common as a distribution mechanism?

hebasto commented at 10:37 PM on September 4, 2025: member

I agree that this issue needs to be fixed.

However, considering:

the limited time left before branching off,
the specific conditions (e.g., -DCMAKE_INTERPROCEDURAL_OPTIMIZATION=ON) under which the issue manifests,
the lack of consensus on how to fix it, and
the absence of an open PR with a suggested fix,

I propose bumping the milestone for this PR from 30.0 to 31.0.

fanquake commented at 11:50 PM on September 4, 2025: member

the limited time left before branching off,

I don't see how that's relevant. Any fix merged into master over the next few weeks can be backported to 30.x.

fanquake closed this on Sep 15, 2025

fanquake referenced this in commit 2d6a0c4649 on Sep 15, 2025