cmake: Make implicit libbitcoinkernel dependencies explicit #31884

1. 
   hebasto commented at 4:10 pm on February 16, 2025: member

   This PR fixes two regressions introduced in #30911.

   For example, on the master branch @ 28dec6c5f8bd35ef4e6cb8d7aa3f21b6879acf98:

   • first regression:

    0$ cmake -B build -G "Ninja" -DBUILD_UTIL_CHAINSTATE=ON -DBUILD_SHARED_LIBS=OFF -DCMAKE_INSTALL_PREFIX=/home/hebasto/INSTALL
    1$ cmake --build build -j $(nproc) -t libbitcoinkernel
    2$ cmake --install build --component libbitcoinkernel
    3- Install configuration: "RelWithDebInfo"
    4CMake Error at build/src/kernel/cmake_install.cmake:46 (file):
    5  file INSTALL cannot find
    6  "/home/hebasto/dev/bitcoin/build/src/crypto/libbitcoin_crypto.a": No such
    7  file or directory.
    8Call Stack (most recent call first):
    9  build/src/cmake_install.cmake:172 (include)
   10  build/cmake_install.cmake:57 (include)
   

   • second regression:

   0$ cmake -B build -G "Unix Makefiles" -DBUILD_UTIL_CHAINSTATE=ON -DBUILD_SHARED_LIBS=OFF -DCMAKE_INSTALL_PREFIX=/home/hebasto/INSTALL
   1$ cmake --build build -j $(nproc) -t libbitcoinkernel
   2...
   3gmake[3]: *** No rule to make target 'src/CMakeFiles/bitcoin_clientversion.dir/clientversion.cpp.o', needed by 'src/kernel/libbitcoinkernel.a'.  Stop.
   4gmake[2]: *** [CMakeFiles/Makefile2:1360: src/kernel/CMakeFiles/bitcoinkernel.dir/all] Error 2
   5gmake[1]: *** [CMakeFiles/Makefile2:1367: src/kernel/CMakeFiles/bitcoinkernel.dir/rule] Error 2
   6gmake: *** [Makefile:647: bitcoinkernel] Error 2
   

   With this PR:

   0$ cmake -B build -G "Ninja" -DBUILD_UTIL_CHAINSTATE=ON -DBUILD_SHARED_LIBS=OFF -DCMAKE_INSTALL_PREFIX=/home/hebasto/INSTALL
   1$ cmake --build build -j $(nproc) -t libbitcoinkernel
   2$ cmake --install build --component libbitcoinkernel
   

   and

   0$ cmake -B build -G "Unix Makefiles" -DBUILD_UTIL_CHAINSTATE=ON -DBUILD_SHARED_LIBS=OFF -DCMAKE_INSTALL_PREFIX=/home/hebasto/INSTALL
   1$ cmake --build build -j $(nproc) -t libbitcoinkernel
   2$ cmake --install build --component libbitcoinkernel
   

   ---

   A note for reviewers: An alternative approach would be to disable the OPTIMIZE_DEPENDENCIES property for the bitcoinkernel target. However, I contend that this PR is preferable because (1) it preserves parallel builds for the libbitcoinkernel target, and (2) the resulting code has one less workaround for a CMake bug.

2. 
   DrahtBot commented at 4:10 pm on February 16, 2025: contributor

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

   Code Coverage & Benchmarks

   For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31884.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	TheCharlatan, theuni
   Concept ACK	purpleKarrot

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

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #31161 (cmake: Set top-level target output locations by hebasto)
   • #30595 (kernel: Introduce initial C header API by TheCharlatan)

   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.

3. DrahtBot added the label Build system on Feb 16, 2025
4. hebasto force-pushed on Feb 17, 2025
5. cmake: Avoid using `OBJECT` libraries

   `OBJECT` libraries have historically exhibited poor support for various
   features, both in the past and now. For example, see one of the latest
   issues:
   - https://gitlab.kitware.com/cmake/cmake/-/issues/24058
   
   Furthermore, CMake maintainers have acknowledged:
   > In general, however, where there is a choice, static libraries will
   > typically be the more convenient choice in CMake projects.
   
   This change:
   1. Converts the `bitcoin_clientversion` library from an `OBJECT` library
      to a `STATIC` library.
   2. Removes an obsolete workaround.

   3fd64efb43
6. cmake: Make implicit `libbitcoinkernel` dependencies explicit

   This change fixes a regression introduced by enabling the
   `OPTIMIZE_DEPENDENCIES` property.

   3b42e05aa9
7. hebasto force-pushed on Feb 18, 2025
8. hebasto marked this as ready for review on Feb 18, 2025
9. hebasto added this to the milestone 29.0 on Feb 18, 2025
10. 
    hebasto commented at 12:34 pm on February 18, 2025: member

    cc @TheCharlatan @theuni
11. 
    purpleKarrot commented at 12:55 pm on February 18, 2025: contributor

    ACK. I agree that this approach is preferred over the mentioned alternative.
12. TheCharlatan approved
13. 
    TheCharlatan commented at 1:01 pm on February 18, 2025: contributor

    ACK 3b42e05aa9e38ba00d52b2338375b4caf032f041

    I prefer this over your mentioned alternative too.

14. fanquake requested review from theuni on Feb 18, 2025
15. theuni approved
16. 
    theuni commented at 4:20 pm on February 20, 2025: member

    utACK 3b42e05aa9e38ba00d52b2338375b4caf032f041

    Thanks for the fix!

17. fanquake merged this on Feb 20, 2025
18. fanquake closed this on Feb 20, 2025

    ---

19. hebasto deleted the branch on Feb 20, 2025

Contributors
hebasto DrahtBot purpleKarrot TheCharlatan theuni

Labels
Build system

Milestone
29.0