build: pass bdb cppflags only where needed #25244

pull fanquake wants to merge 1 commits into bitcoin:master from fanquake:use_bdb_cppflags_needed changing 2 files +4 −2

fanquake commented at 10:32 AM on May 30, 2022: member

Move bdb cppflags out of the catch-all BITCOIN_INCLUDES, and pass them only where they are needed, which is in libbitcoin_node/wallet and the tests.
fanquake added the label Build system on May 30, 2022
hebasto commented at 12:45 PM on May 30, 2022: member

Concept ACK.

Is guarding with USE_BDB condition really required? If it fails, the BDB_CPPFLAGS is just unset, no?
laanwj commented at 5:07 PM on May 30, 2022: member

Concept ACK

build: pass bdb cppflags only where needed

Move bdb cppflags out of the catch-all BITCOIN_INCLUDES, and pass them
only where they are needed, which is in libbitcoin_node/wallet and the
tests.

46a890960e

fanquake force-pushed on May 31, 2022
fanquake commented at 5:40 AM on May 31, 2022: member

Is guarding with USE_BDB condition really required?

dropped
fanquake requested review from hebasto on Jun 1, 2022
hebasto approved

hebasto commented at 10:34 AM on June 1, 2022: member

ACK 46a890960e4b07e5aec479aa8e07e9c34ce68aee

The <db_cxx.h> included in the wallet/bdb.h only. Therefore, BDB_CPPFLAGS are required only for libraries which sources have #include <wallet/bdb.h>.

nit, the following diff

--- a/src/Makefile.test.include
+++ b/src/Makefile.test.include
@@ -177,6 +177,7 @@ FUZZ_SUITE_LD_COMMON +=\
 
 if USE_BDB
 BITCOIN_TESTS += wallet/test/db_tests.cpp
+test_test_bitcoin_CPPFLAGS += $(BDB_CPPFLAGS)
 endif
 
 FUZZ_WALLET_SRC = \
@@ -201,7 +202,6 @@ test_test_bitcoin_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(TESTDEFS) $(EV
 test_test_bitcoin_LDADD = $(LIBTEST_UTIL)
 if ENABLE_WALLET
 test_test_bitcoin_LDADD += $(LIBBITCOIN_WALLET)
-test_test_bitcoin_CPPFLAGS += $(BDB_CPPFLAGS)
 endif
 
 test_test_bitcoin_LDADD += $(LIBBITCOIN_NODE) $(LIBBITCOIN_CLI) $(LIBBITCOIN_COMMON) $(LIBBITCOIN_UTIL) $(LIBBITCOIN_CONSENSUS) $(LIBBITCOIN_CRYPTO) $(LIBUNIVALUE) \

looks slightly more correct / clear?

fanquake commented at 11:34 AM on June 1, 2022: member

looks slightly more correct / clear?

Probably not, given that diff wouldn't compile
fanquake merged this on Jun 1, 2022
fanquake closed this on Jun 1, 2022
fanquake deleted the branch on Jun 1, 2022
sidhujag referenced this in commit c3ac3f1390 on Jun 1, 2022
DrahtBot locked this on Jun 1, 2023

Contributors

Labels

Linked (view graph)

#25263 build: pass minisketch cppflags only where needed #25266 ./wallet/bdb.h:27:10: fatal error: db_cxx.h: No such file or directory