From 86c80e9cf296f6560f2f846bd4e7286f7b958b93 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Wed, 4 Sep 2024 15:30:58 -0400 Subject: [PATCH 1/4] contrib: make check-deps.sh script work with cmake --- contrib/devtools/check-deps.sh | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/contrib/devtools/check-deps.sh b/contrib/devtools/check-deps.sh index 9d2eebe14d8..f49e360d16a 100755 --- a/contrib/devtools/check-deps.sh +++ b/contrib/devtools/check-deps.sh @@ -8,11 +8,10 @@ declare -A LIBS LIBS[cli]="libbitcoin_cli.a" LIBS[common]="libbitcoin_common.a" LIBS[consensus]="libbitcoin_consensus.a" -LIBS[crypto]="crypto/.libs/libbitcoin_crypto_base.a crypto/.libs/libbitcoin_crypto_x86_shani.a crypto/.libs/libbitcoin_crypto_sse41.a crypto/.libs/libbitcoin_crypto_avx2.a" +LIBS[crypto]="crypto/libbitcoin_crypto.a crypto/libbitcoin_crypto_x86_shani.a crypto/libbitcoin_crypto_sse41.a crypto/libbitcoin_crypto_avx2.a" LIBS[node]="libbitcoin_node.a" -LIBS[util]="libbitcoin_util.a" -LIBS[wallet]="libbitcoin_wallet.a" -LIBS[wallet_tool]="libbitcoin_wallet_tool.a" +LIBS[util]="util/libbitcoin_util.a" +LIBS[wallet]="wallet/libbitcoin_wallet.a" # Declare allowed dependencies "X Y" where X is allowed to depend on Y. This # list is taken from doc/design/libraries.md. @@ -32,32 +31,28 @@ ALLOWED_DEPENDENCIES=( "wallet common" "wallet crypto" "wallet util" - "wallet_tool util" - "wallet_tool wallet" ) # Add minor dependencies omitted from doc/design/libraries.md to keep the # dependency diagram simple. ALLOWED_DEPENDENCIES+=( "wallet consensus" - "wallet_tool common" - "wallet_tool crypto" ) # Declare list of known errors that should be suppressed. declare -A SUPPRESS # init.cpp file currently calls Berkeley DB sanity check function on startup, so # there is an undocumented dependency of the node library on the wallet library. -SUPPRESS["libbitcoin_node_a-init.o libbitcoin_wallet_a-bdb.o _ZN6wallet27BerkeleyDatabaseSanityCheckEv"]=1 +SUPPRESS["init.cpp.o bdb.cpp.o _ZN6wallet27BerkeleyDatabaseSanityCheckEv"]=1 # init/common.cpp file calls InitError and InitWarning from interface_ui which # is currently part of the node library. interface_ui should just be part of the # common library instead, and is moved in # https://github.com/bitcoin/bitcoin/issues/10102 -SUPPRESS["libbitcoin_common_a-common.o libbitcoin_node_a-interface_ui.o _Z11InitWarningRK13bilingual_str"]=1 -SUPPRESS["libbitcoin_common_a-common.o libbitcoin_node_a-interface_ui.o _Z9InitErrorRK13bilingual_str"]=1 +SUPPRESS["common.cpp.o interface_ui.cpp.o _Z11InitWarningRK13bilingual_str"]=1 +SUPPRESS["common.cpp.o interface_ui.cpp.o _Z9InitErrorRK13bilingual_str"]=1 # rpc/external_signer.cpp adds defines node RPC methods but is built as part of the # common library. It should be moved to the node library instead. -SUPPRESS["libbitcoin_common_a-external_signer.o libbitcoin_node_a-server.o _ZN9CRPCTable13appendCommandERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEPK11CRPCCommand"]=1 +SUPPRESS["external_signer.cpp.o server.cpp.o _ZN9CRPCTable13appendCommandERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEPK11CRPCCommand"]=1 usage() { echo "Usage: $(basename "${BASH_SOURCE[0]}") [BUILD_DIR]" @@ -67,8 +62,10 @@ usage() { lib_targets() { for lib in "${!LIBS[@]}"; do for lib_path in ${LIBS[$lib]}; do - # shellcheck disable=SC2001 - sed 's:/.libs/\(.*\)\.a$:/\1.la:g' <<<"$lib_path" + local name="${lib_path##*/}" + name="${name#lib}" + name="${name%.a}" + echo "$name" done done } @@ -175,7 +172,7 @@ check_not_suppressed() { # Check arguments. if [ "$#" = 0 ]; then - BUILD_DIR="$(dirname "${BASH_SOURCE[0]}")/../../src" + BUILD_DIR="$(dirname "${BASH_SOURCE[0]}")/../../build" elif [ "$#" = 1 ]; then BUILD_DIR="$1" else @@ -190,10 +187,10 @@ if [ ! -f "$BUILD_DIR/Makefile" ]; then fi # Build libraries and run checks. -cd "$BUILD_DIR" # shellcheck disable=SC2046 -make -j"$(nproc)" $(lib_targets) +cmake --build "$BUILD_DIR" -j"$(nproc)" -t $(lib_targets) TEMP_DIR="$(mktemp -d)" +cd "$BUILD_DIR/src" extract_symbols "$TEMP_DIR" if check_libraries "$TEMP_DIR"; then echo "Success! No unexpected dependencies were detected." From 3c99f5a38a47e4e10a0daab3a114b5e476fcacfa Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Tue, 9 Jul 2024 11:29:50 -0400 Subject: [PATCH 2/4] contrib: fix check-deps.sh to check for weak symbols Fix check-deps.sh to check for weak symbols so it can detect when an exported template function is used from another library. In a previous version of this commit, this change caused an invalid dependency in the consensus library on the TryParseHex template function from the util library to be detected, and a suppression was added here. But #30377 removed the invalid dependency so the suppression is no longer needed. The invalid dependency and problem detecting weak symbol usage was originally reported by Hennadii Stepanov in https://github.com/bitcoin/bitcoin/pull/29015#issuecomment-2209258843 --- contrib/devtools/check-deps.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/devtools/check-deps.sh b/contrib/devtools/check-deps.sh index f49e360d16a..d3a76f07511 100755 --- a/contrib/devtools/check-deps.sh +++ b/contrib/devtools/check-deps.sh @@ -75,7 +75,7 @@ extract_symbols() { local temp_dir="$1" for lib in "${!LIBS[@]}"; do for lib_path in ${LIBS[$lib]}; do - nm -o "$lib_path" | grep ' T ' | awk '{print $3, $1}' >> "${temp_dir}/${lib}_exports.txt" + nm -o "$lib_path" | grep ' T \| W ' | awk '{print $3, $1}' >> "${temp_dir}/${lib}_exports.txt" nm -o "$lib_path" | grep ' U ' | awk '{print $3, $1}' >> "${temp_dir}/${lib}_imports.txt" awk '{print $1}' "${temp_dir}/${lib}_exports.txt" | sort -u > "${temp_dir}/${lib}_exported_symbols.txt" awk '{print $1}' "${temp_dir}/${lib}_imports.txt" | sort -u > "${temp_dir}/${lib}_imported_symbols.txt" From 0aaa1298a08f898318916661f2317b2e755206e6 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Thu, 25 Jul 2024 21:22:00 -0400 Subject: [PATCH 3/4] contrib: fix check-deps.sh when libraries do not import symbols Script was failing when called on libraries that do not import symbols, because bash pipefail option was specified, and grep was used in some pipelines to filter symbols, and grep returns status 1 when it doesn't match any lines. This could cause the script to fail on some systems and configurations, such as the clang-tidy CI configuration https://cirrus-ci.com/task/4801670352207872?logs=ci#L6191 where the libbitcoin_crypto_x86_shani.a library does not import symbols. --- contrib/devtools/check-deps.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contrib/devtools/check-deps.sh b/contrib/devtools/check-deps.sh index d3a76f07511..3bd48b444ab 100755 --- a/contrib/devtools/check-deps.sh +++ b/contrib/devtools/check-deps.sh @@ -75,8 +75,8 @@ extract_symbols() { local temp_dir="$1" for lib in "${!LIBS[@]}"; do for lib_path in ${LIBS[$lib]}; do - nm -o "$lib_path" | grep ' T \| W ' | awk '{print $3, $1}' >> "${temp_dir}/${lib}_exports.txt" - nm -o "$lib_path" | grep ' U ' | awk '{print $3, $1}' >> "${temp_dir}/${lib}_imports.txt" + nm -o "$lib_path" | { grep ' T \| W ' || true; } | awk '{print $3, $1}' >> "${temp_dir}/${lib}_exports.txt" + nm -o "$lib_path" | { grep ' U ' || true; } | awk '{print $3, $1}' >> "${temp_dir}/${lib}_imports.txt" awk '{print $1}' "${temp_dir}/${lib}_exports.txt" | sort -u > "${temp_dir}/${lib}_exported_symbols.txt" awk '{print $1}' "${temp_dir}/${lib}_imports.txt" | sort -u > "${temp_dir}/${lib}_imported_symbols.txt" done From 3ae35b427fe59bc9ab24d07c1adb46faa702de20 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Thu, 25 Jul 2024 09:07:20 -0400 Subject: [PATCH 4/4] ci: run check-deps.sh as part of clang-tidy job --- ci/test/00_setup_env_native_tidy.sh | 1 + ci/test/03_test_script.sh | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/ci/test/00_setup_env_native_tidy.sh b/ci/test/00_setup_env_native_tidy.sh index 581de16bed5..5105455df84 100755 --- a/ci/test/00_setup_env_native_tidy.sh +++ b/ci/test/00_setup_env_native_tidy.sh @@ -14,6 +14,7 @@ export NO_DEPENDS=1 export RUN_UNIT_TESTS=false export RUN_FUNCTIONAL_TESTS=false export RUN_FUZZ_TESTS=false +export RUN_CHECK_DEPS=true export RUN_TIDY=true export GOAL="install" export BITCOIN_CONFIG="\ diff --git a/ci/test/03_test_script.sh b/ci/test/03_test_script.sh index 7fca724a77e..4e884df17ae 100755 --- a/ci/test/03_test_script.sh +++ b/ci/test/03_test_script.sh @@ -137,6 +137,10 @@ if [ -n "$USE_VALGRIND" ]; then "${BASE_ROOT_DIR}/ci/test/wrap-valgrind.sh" fi +if [ "$RUN_CHECK_DEPS" = "true" ]; then + "${BASE_ROOT_DIR}/contrib/devtools/check-deps.sh" . +fi + if [ "$RUN_UNIT_TESTS" = "true" ]; then DIR_UNIT_TEST_DATA="${DIR_UNIT_TEST_DATA}" LD_LIBRARY_PATH="${DEPENDS_DIR}/${HOST}/lib" CTEST_OUTPUT_ON_FAILURE=ON ctest "${MAKEJOBS}" fi