Skip to content

Commit

Permalink
Merge bitcoin#31884: cmake: Make implicit libbitcoinkernel dependen…
Browse files Browse the repository at this point in the history
…cies explicit

3b42e05 cmake: Make implicit `libbitcoinkernel` dependencies explicit (Hennadii Stepanov)
3fd64ef cmake: Avoid using `OBJECT` libraries (Hennadii Stepanov)

Pull request description:

  This PR fixes two regressions introduced in bitcoin#30911.

  For example, on the master branch @ 28dec6c:
  - first regression:
  ```
  $ cmake -B build -G "Ninja" -DBUILD_UTIL_CHAINSTATE=ON -DBUILD_SHARED_LIBS=OFF -DCMAKE_INSTALL_PREFIX=/home/hebasto/INSTALL
  $ cmake --build build -j $(nproc) -t libbitcoinkernel
  $ cmake --install build --component libbitcoinkernel
  - Install configuration: "RelWithDebInfo"
  CMake Error at build/src/kernel/cmake_install.cmake:46 (file):
    file INSTALL cannot find
    "/home/hebasto/dev/bitcoin/build/src/crypto/libbitcoin_crypto.a": No such
    file or directory.
  Call Stack (most recent call first):
    build/src/cmake_install.cmake:172 (include)
    build/cmake_install.cmake:57 (include)

  ```

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

  With this PR:
  ```
  $ cmake -B build -G "Ninja" -DBUILD_UTIL_CHAINSTATE=ON -DBUILD_SHARED_LIBS=OFF -DCMAKE_INSTALL_PREFIX=/home/hebasto/INSTALL
  $ cmake --build build -j $(nproc) -t libbitcoinkernel
  $ cmake --install build --component libbitcoinkernel
  ```
  and
  ```
  $ cmake -B build -G "Unix Makefiles" -DBUILD_UTIL_CHAINSTATE=ON -DBUILD_SHARED_LIBS=OFF -DCMAKE_INSTALL_PREFIX=/home/hebasto/INSTALL
  $ cmake --build build -j $(nproc) -t libbitcoinkernel
  $ 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.

ACKs for top commit:
  TheCharlatan:
    ACK 3b42e05
  theuni:
    utACK 3b42e05

Tree-SHA512: 73e9da845688a02e5d61770b7cfd5e1a17440182eb524c7329a47df8f1daa6fe0f9cbde5274832bf43f52e17de86473881dc876dee4276c9c06b173b1b78b7a2
  • Loading branch information
fanquake committed Feb 20, 2025
2 parents 58f15d4 + 3b42e05 commit eb51963
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 8 deletions.
2 changes: 1 addition & 1 deletion src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ add_custom_target(generate_build_info
COMMENT "Generating bitcoin-build-info.h"
VERBATIM
)
add_library(bitcoin_clientversion OBJECT EXCLUDE_FROM_ALL
add_library(bitcoin_clientversion STATIC EXCLUDE_FROM_ALL
clientversion.cpp
)
target_link_libraries(bitcoin_clientversion
Expand Down
9 changes: 5 additions & 4 deletions src/kernel/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,10 @@ set_target_properties(bitcoinkernel PROPERTIES
CXX_VISIBILITY_PRESET default
)

# Add a convenience libbitcoinkernel target as a synonym for bitcoinkernel.
add_custom_target(libbitcoinkernel)
add_dependencies(libbitcoinkernel bitcoinkernel)

# When building the static library, install all static libraries the
# bitcoinkernel depends on.
if(NOT BUILD_SHARED_LIBS)
Expand All @@ -110,6 +114,7 @@ if(NOT BUILD_SHARED_LIBS)
get_target_property(linked_libraries ${target} LINK_LIBRARIES)
foreach(dep ${linked_libraries})
if(TARGET ${dep})
add_dependencies(libbitcoinkernel ${dep})
get_target_property(dep_type ${dep} TYPE)
if(dep_type STREQUAL "STATIC_LIBRARY")
list(APPEND ${libs_out} ${dep})
Expand All @@ -132,10 +137,6 @@ endif()
configure_file(${PROJECT_SOURCE_DIR}/libbitcoinkernel.pc.in ${PROJECT_BINARY_DIR}/libbitcoinkernel.pc @ONLY)
install(FILES ${PROJECT_BINARY_DIR}/libbitcoinkernel.pc DESTINATION "${CMAKE_INSTALL_LIBDIR}/pkgconfig" COMPONENT libbitcoinkernel)

# Add a convenience libbitcoinkernel target as a synonym for bitcoinkernel.
add_custom_target(libbitcoinkernel)
add_dependencies(libbitcoinkernel bitcoinkernel)

install(TARGETS bitcoinkernel
RUNTIME
DESTINATION ${CMAKE_INSTALL_BINDIR}
Expand Down
3 changes: 0 additions & 3 deletions src/util/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,6 @@ add_library(bitcoin_util STATIC EXCLUDE_FROM_ALL
../sync.cpp
)

# Workaround for https://gitlab.kitware.com/cmake/cmake/-/issues/24058
set_target_properties(bitcoin_util PROPERTIES OPTIMIZE_DEPENDENCIES OFF)

target_link_libraries(bitcoin_util
PRIVATE
core_interface
Expand Down

0 comments on commit eb51963

Please sign in to comment.