Only #include upstream bdb headers from our cpp files.
It's generally good practice to avoid including 3rd party deps in headers as otherwise they tend to sneak into new compilation units. IMO this makes for a nice cleanup.
There's a good bit of code movement here, but each commit is small and should be obviously correct.
Note: in the future, the buildsystem can add the bdb include path for bdb.cpp and salvage.cpp only, rather than all wallet sources.
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--021abf342d371248e50ceaed478a90ca-->
See the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
ACK 2d09d0f50408f9c522b9efa4f386072502c7b3d1
Changes are primarily moves and appear otherwise obviously correct.
270 | @@ -275,6 +271,11 @@ SafeDbt::operator Dbt*() 271 | return &m_dbt; 272 | } 273 | 274 | +Span<const std::byte> SpanFromDbt(const SafeDbt& dbt)
nit in c217c542f0fd9c741b79a91419247bd44440e6d9: I think you forgot static or namespace?
Nice catch. Fixed.
lgtm
Approach ACK 2d09d0f50408f9c522b9efa4f386072502c7b3d1.
Suggesting to adjust our IWYU mapping file:
--- a/contrib/devtools/iwyu/bitcoin.core.imp
+++ b/contrib/devtools/iwyu/bitcoin.core.imp
@@ -4,4 +4,7 @@
{ include: [ "<bits/termios-struct.h>", private, "<termios.h>", public ] },
{ include: [ "<bits/termios-tcflow.h>", private, "<termios.h>", public ] },
{ include: [ "<bits/chrono.h>", private, "<chrono>", public ] },
+
+ # Berkeley DB
+ { include: [ "<db.h>", private, "<db_cxx.h>", public ] },
]
or #include <db.h> explicitly along with #include <db_cxx.h>.
No functional change, just simplifies the code move in the next commit.
Dbt requires including bdb headers.
Else some compilers/stdlibs may not be able to construct
std::unique_ptr<Db> without Db defined.
This way the dependency can't sneak into other files without being noticed.
Forward-declare bdb classes as necessary.
ACK 8b5397c00e821d7eaab22f512e9d71a1a0392ebf
@hebasto I'm ~0 on doing either/none of those changes. Happy to make a change if you or @achow101 or @MarcoFalke have a strong preference, otherwise I'll leave it as-is.
reACK 8b5397c00e821d7eaab22f512e9d71a1a0392ebf
No preference to make those changes.