Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cmake: Avoid overlinking #34

Merged
merged 5 commits into from
Nov 9, 2023
Merged

cmake: Avoid overlinking #34

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0f0dcca
fixup! cmake: Build `bitcoin_util` static library
hebasto Nov 9, 2023
e4b4fdd
fixup! cmake: Add `libzmq` optional package support
hebasto Nov 9, 2023
9a0a914
fixup! cmake: Add wallet functionality
hebasto Nov 9, 2023
ab9853a
fixup! cmake: Build `bench_bitcoin` executable
hebasto Nov 9, 2023
8342981
fixup! cmake: Build `test_bitcoin` executable
hebasto Nov 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions src/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,13 +217,9 @@ add_library(bitcoin_node STATIC EXCLUDE_FROM_ALL
validation.cpp
validationinterface.cpp
versionbits.cpp
$<$<TARGET_EXISTS:bitcoin_wallet>:wallet/init.cpp>
$<$<NOT:$<TARGET_EXISTS:bitcoin_wallet>>:dummywallet.cpp>
)
if(ENABLE_WALLET)
target_sources(bitcoin_node PRIVATE wallet/init.cpp)
target_link_libraries(bitcoin_node PRIVATE bitcoin_wallet)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason not to go ahead and use TARGET_IF_NAME_EXISTS for target_sources here as well? Seems odd to leave only one behind.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Done.

else()
target_sources(bitcoin_node PRIVATE dummywallet.cpp)
endif()
target_link_libraries(bitcoin_node
PRIVATE
core
Expand All @@ -250,6 +246,7 @@ if(BUILD_DAEMON)
target_link_libraries(bitcoind
core
bitcoin_node
$<TARGET_NAME_IF_EXISTS:bitcoin_wallet>
)
list(APPEND installable_targets bitcoind)
endif()
Expand Down
7 changes: 3 additions & 4 deletions src/bench/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Copyright (c) 2023 The Bitcoin Core developers
# Copyright (c) 2023-present The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
# file COPYING or https://opensource.org/license/mit/.

include(GenerateHeaders)
generate_header_from_raw(data/block413567.raw)
Expand Down Expand Up @@ -52,8 +52,7 @@ add_executable(bench_bitcoin
target_link_libraries(bench_bitcoin
core
test_util
leveldb
univalue
bitcoin_node
Boost::headers
)

hebasto marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
9 changes: 2 additions & 7 deletions src/test/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Copyright (c) 2023 The Bitcoin Core developers
# Copyright (c) 2023-present The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
# file COPYING or https://opensource.org/license/mit/.

include(GenerateHeaders)
generate_header_from_json(data/base58_encode_decode.json)
Expand Down Expand Up @@ -140,13 +140,8 @@ target_link_libraries(test_bitcoin
test_util
bitcoin_cli
bitcoin_node
bitcoin_common
bitcoin_util
minisketch
leveldb
univalue
Boost::headers
libevent::libevent
)

if(ENABLE_WALLET)
Expand Down
21 changes: 6 additions & 15 deletions src/test/util/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Copyright (c) 2023 The Bitcoin Core developers
# Copyright (c) 2023-present The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
# file COPYING or https://opensource.org/license/mit/.

add_library(test_util STATIC EXCLUDE_FROM_ALL
blockfilter.cpp
Expand All @@ -17,22 +17,13 @@ add_library(test_util STATIC EXCLUDE_FROM_ALL
transaction_utils.cpp
txmempool.cpp
validation.cpp
$<$<BOOL:${ENABLE_WALLET}>:${PROJECT_SOURCE_DIR}/src/wallet/test/util.cpp>
)

target_link_libraries(test_util
PRIVATE
core
bitcoin_common
bitcoin_node
leveldb
univalue
Boost::headers
PUBLIC
univalue
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to comment that I'd rather see this be PRIVATE and have univalue included by the binaries as needed, but test_util pretty clearly exports univalue as part of its interface, so PUBLIC univalue actually follows the code as-is quite well I think.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Placing univalue into usage requirements, and removing it from build specifications for the bench_bitcoin and test_bitcoin targets, fixes actually, "ld: warning: ignoring duplicate libraries" on macOS.

)

if(ENABLE_WALLET)
target_sources(test_util
PRIVATE
../../wallet/test/util.cpp
)
endif()

target_link_libraries(test_util PRIVATE core)
5 changes: 2 additions & 3 deletions src/util/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Copyright (c) 2023 The Bitcoin Core developers
# Copyright (c) 2023-present The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
# file COPYING or https://opensource.org/license/mit/.

add_library(bitcoin_util STATIC EXCLUDE_FROM_ALL
asmap.cpp
Expand Down Expand Up @@ -53,7 +53,6 @@ target_link_libraries(bitcoin_util
PRIVATE
core
bitcoin_crypto
univalue
Threads::Threads
$<$<BOOL:${MINGW}>:ws2_32>
)
5 changes: 2 additions & 3 deletions src/zmq/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Copyright (c) 2023 The Bitcoin Core developers
# Copyright (c) 2023-present The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
# file COPYING or https://opensource.org/license/mit/.

add_library(bitcoin_zmq STATIC
zmqabstractnotifier.cpp
Expand All @@ -16,7 +16,6 @@ target_compile_definitions(bitcoin_zmq
target_link_libraries(bitcoin_zmq
PRIVATE
core
leveldb
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally unrelated to CMake/this PR, but univalue is an unfortunate dependency for zmq. Seems to me it'd make sense to move zmq/rpc.cpp to rpc/zmq.cpp :)

univalue
$<TARGET_NAME_IF_EXISTS:libzmq>
$<TARGET_NAME_IF_EXISTS:PkgConfig::libzmq>
Expand Down