build: Add CMake-based build system (4 of N)

[hebasto]

#7 (comment):

So.. requesting a hookup to depends in the next PR please :)

Delivered :)

---

The parent PR: bitcoin#25797.
The previous PRs in the staging branch: #5, #6, #7.

---

EXAMPLES:

Cross-compiling for Windows:

make -C depends HOST=x86_64-w64-mingw32 NO_QT=1 NO_WALLET=1 NO_ZMQ=1 NO_UPNP=1 NO_NATPMP=1 NO_USDT=1
cmake -S . -B build -DCMAKE_TOOLCHAIN_FILE=depends/x86_64-w64-mingw32/share/toolchain.cmake
cmake --build build

Cross-compiling for macOS, arm64:

make -C depends HOST=arm64-apple-darwin NO_QT=1 NO_WALLET=1 NO_ZMQ=1 NO_UPNP=1 NO_NATPMP=1 NO_USDT=1
cmake -S . -B build -DCMAKE_TOOLCHAIN_FILE=depends/arm64-apple-darwin/share/toolchain.cmake
cmake --build build

[hebasto]

Rebased and ready for reviewing :)

[theuni]

High-level review comments/questions.

[theuni]

Mmm, I don't like the idea of doing CMake logic here. In this case, we're combining a set of flags without giving CMake the opportunity to do so itself.

I think we actually want to export to abstract vars instead, then set those vars to CMake vars inside the toolchain file.

So instead of:

's|@CMAKE_CXX_FLAGS_INIT@|$(strip $(host_CPPFLAGS) $(host_$(release_type)_CPPFLAGS) $(host_CXXFLAGS) $(host_$(release_type)_CXXFLAGS))|'

Have:

's|@DEPENDS_CPPFLAGS@|$(host_CPPFLAGS) $(host_$(release_type)_CPPFLAGS))|'
's|@DEPENDS_CXXFLAGS@|$(host_CXXFLAGS) $(host_$(release_type)_CXXFLAGS))|'

Then allow the CMake file to decide `CMAKE_CXX_FLAGS_INIT = DEPENDS_CPPFLAGS + DEPENDS_CXXFLAGS.

Same goes for all vars. I don't think the CMAKE_ prefix belongs here.

[hebasto]

Thanks! Updated.

[theuni]

Could you give a hint as to what's going on here? I'm a little lost :)

[hebasto]

The goal is to handle every "compiler invocation command", which we have in our build and test environment, properly. Including such cases as follows:

• make -C depends CC=clang CXX='clang++ -stdlib=libc++', see ci/test/00_setup_env_native_nowallet_libbitcoinkernel.sh
• env -u LIBRARY_PATH clang, see depends/hosts/darwin.mk, currently causes ci: ccache does not work for macOS cross-compiling builds bitcoin/bitcoin#21552

The main part is done in the split_compiler_launcher() function, which splits env -u LIBRARY_PATH clang in env -u LIBRARY_PATH and clang.

Rest of code makes it compatible with the supported range of CMake versions.

[theuni]

Won't this prohibit the use of CMAKE_CXX_COMPILER_LAUNCHER for ccache?

[hebasto]

Won't this prohibit the use of CMAKE_CXX_COMPILER_LAUNCHER for ccache?

The future use of CMAKE_{C,CXX}_COMPILER_LAUNCHER with ccache can be peeked in bitcoin@6f48c34.

[theuni]

Can't this be done in the toolchain file by conditionally setting the CMAKE_FIND_ROOT_PATH_* vars?

[hebasto]

That was my initial idea as well :)

It turns out that a toolchain file is being processed more than once (the actual number of runs depends on CMake version). Therefore, its processing must be idempotent. list(APPEND ...) command is not idempotent. That is why it has been moved out of the toolchain file.

[TheCharlatan]

I followed the build instructions and got the following warning:

CMake Warning at CMakeLists.txt:179 (message):
  PIE link options are not supported for executable targets: PIE and NO_PIE
  are not supported by linker for CXX.

Later, during the build, I get linker errors. Might this be a problem with the linker definition, or is something in my toolchain very broken? For what it's worth I can still produce cross-compiled windows binaries in the same environment using the legacy system.

[theuni]

FYI the --toolchain argument wasn't introduced until 3.21. Frustratingly, when using older CMake it's just quietly ignored and build fails.

Instead, please recommend using -DCMAKE_TOOLCHAIN_FILE.

[theuni]

I followed the build instructions and got the following warning:

CMake Warning at CMakeLists.txt:179 (message):
  PIE link options are not supported for executable targets: PIE and NO_PIE
  are not supported by linker for CXX.

Later, during the build, I get linker errors. Might this be a problem with the linker definition, or is something in my toolchain very broken? For what it's worth I can still produce cross-compiled windows binaries in the same environment using the legacy system.

Yeah, the logic for PIE isn't quite right. For some systems (win32/macos) position independent code is the default (or even the only supported method). For those platforms passing pic/pie options is either a warning or an error.

So NOT CMAKE_CXX_LINK_PIE_SUPPORTED isn't necessarily something worth warning about. It just means that we shouldn't explicitly add the options:

if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.14)
  include(CheckPIESupported)
  check_pie_supported(OUTPUT_VARIABLE check_pie_output LANGUAGES CXX)
  if(CMAKE_CXX_LINK_PIE_SUPPORTED)
    set(CMAKE_POSITION_INDEPENDENT_CODE ON)
  endif()
else()
  # Needs a compile/link test for old cmake
  if(test success)
    set(CMAKE_POSITION_INDEPENDENT_CODE ON)
  endif()
endif()

[theuni]

Also, PIE support needs CMP0083, which seems to be missing. I thought we already added that?

Edit: nm, I see that's covered by cmake_minimum_required.

[theuni]

Ping? :)

[hebasto]

FYI the --toolchain argument wasn't introduced until 3.21. Frustratingly, when using older CMake it's just quietly ignored and build fails.

Instead, please recommend using -DCMAKE_TOOLCHAIN_FILE.

Thanks! The PR description has been updated.

[hebasto]

@TheCharlatan

I followed the build instructions and got the following warning:

CMake Warning at CMakeLists.txt:179 (message):
  PIE link options are not supported for executable targets: PIE and NO_PIE
  are not supported by linker for CXX.

Later, during the build, I get linker errors. Might this be a problem with the linker definition, or is something in my toolchain very broken? For what it's worth I can still produce cross-compiled windows binaries in the same environment using the legacy system.

Hmm... I'm not able to reproduce any errors. Mind providing more details about your toolchain please?

Also, please ensure your build directory is clean before building starts.

[hebasto]

Addressed @theuni's #10 (comment).

[hebasto]

Ping? :)

@theuni @TheCharlatan

I apologize for introducing some disruption into our reviewing process.

[theuni]

I apologize for introducing some disruption into our reviewing process.

No worries! I know you were busy with secp. I just didn't want this process to lose its momentum :)

[TheCharlatan]

Hmm... I'm not able to reproduce any errors. Mind providing more details about your toolchain please?

Huh, I'm no longer getting the PIE warnings, just the linker errors now. Maybe it is missing a -lpthread somewhere? Small excerpt of errors:

/usr/bin/x86_64-w64-mingw32-ld: libbitcoin_node.a(init.cpp.obj):init.cpp:(.text+0x34): undefined reference to `std::thread::_State::~_State()'
/usr/bin/x86_64-w64-mingw32-ld: libbitcoin_node.a(init.cpp.obj):init.cpp:(.text+0x145e): undefined reference to `std::thread::_State::~_State()'

I'm running a standard install of ubuntu 22 and this mingw32 gcc:

x86_64-w64-mingw32-gcc (GCC) 10-win32 20220113
x86_64-w64-mingw32-g++-posix (GCC) 10-posix 20220113

The summary of configuration options is:

Configure summary
=================
Executables:
  bitcoind ............................ ON

Cross compiling ....................... TRUE, for Windows, x86_64
Preprocessor defined macros ........... _WIN32_WINNT=0x0601 _WIN32_IE=0x0501 WIN32_LEAN_AND_MEAN NOMINMAX WIN32 _WINDOWS _MT
C compiler ............................ /usr/bin/x86_64-w64-mingw32-gcc
CFLAGS ................................ -pipe -std=c11 -O2
C++ compiler .......................... /usr/bin/x86_64-w64-mingw32-g++-posix
CXXFLAGS .............................. -pipe -std=c++17 -O2
Common compile options ................ 
Common link options ................... -Wl,--major-subsystem-version,6 -Wl,--minor-subsystem-version,1
Build type:
 - CMAKE_BUILD_TYPE ................... 
 - CFLAGS ............................. 
 - CXXFLAGS ........................... 
 - LDFLAGS for executables ............ 
 - LDFLAGS for shared libraries ....... 
Use assembly routines ................. ON

Also, please ensure your build directory is clean before building starts.

Yes :)

[hebasto]

@TheCharlatan

I've managed to reproduce the linker error you reported. Moreover, it is reproducible even with bitcoin#25797 (

My test environment:

$ cat /etc/os-release | grep PRETTY_NAME
PRETTY_NAME="Ubuntu 20.04.5 LTS"
$ cmake --version | head -1
cmake version 3.16.3
$ /usr/bin/x86_64-w64-mingw32-g++-posix --version | head -1
x86_64-w64-mingw32-g++-posix (GCC) 9.3-posix 20200320

[hebasto]

@TheCharlatan

Later, during the build, I get linker errors.

Fixed.

Huh, I'm no longer getting the PIE warnings...

check_pie_supported is omitted now when CMAKE_CROSSCOMPILING.

[TheCharlatan]

Fixed.

Thanks, can confirm that it's building fine now.

[theuni]

@theuni

@hebasto How about addressing the feedback so far

Sorry if I missed some comments unresolved. Which ones are in your mind?

Ah, sorry, I hadn't seen your updates from today before commenting.

Will pick up review next week, hopefully we can agree on a quick merge :)

[hebasto]

... hopefully we can agree on a quick merge :)

Still expecting from fellow reviewers for a few formal (conditional?) ACKs or comments that need to be addressed here.

For your information, after merging this PR, I'm planning to rebase the staging branch (similar to #8) and squash "FIXUP" commits from this PR with the commits they are supposed to amend.

[theuni]

ACK b0351e3, with the caveat that I hope we can make significant changes to the toolchain.cmake.in. But I'm ok calling that a work-in-progress while the rest of the work is introduced to keep it from holding up review further.

[theuni]

@hebasto Want to go ahead and open the next one based on this one while we wait for another ACK or two here?

It might be helpful to get some more of the depends interactions hooked up as a next step like @TheCharlatan suggested. But if you have another path in mind that's fine by me too.

[hebasto]

Want to go ahead..?

Sure :)

For your information, after merging this PR, I'm planning to rebase the staging branch (similar to #8) and squash "FIXUP" commits from this PR with the commits they are supposed to amend.

See #11.

[hebasto]

@theuni

It might be helpful to get some more of the depends interactions hooked up as a next step like @TheCharlatan suggested. But if you have another path in mind that's fine by me too.

I've rebased the parent PR on top of the #11.

Feel free to point any commits you want to see as a next step, in addition to the dependency interactions :)

UPD. See #12

[willcl-ark]

@hebasto wondering if there are plans to add automatic depends configuration to bitcoin configure (if depends/${host}/share/config.site exists, à la bitcoin#16692 (comment)) as part of this re-write?

[hebasto]

@willcl-ark

wondering if there are plans to add automatic depends configuration to bitcoin configure (if depends/${host}/share/config.site exists, à la bitcoin#16692 (comment)) as part of this re-write?

I no longer consider this feature as a useful one due to the case, which is actually an everyday workflow for me, when the user has depends built for many hosts simultaneously.