-
Notifications
You must be signed in to change notification settings - Fork 38.8k
build: prune BOOST_CPPFLAGS from libbitcoin_zmq #26087
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.
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
build: prune BOOST_CPPFLAGS from libbitcoin_zmq #26087
Conversation
Rather than including validation.h, which ultimately means needing boost via txmempool.h, include primitives/block.h for CBlock, and remove validation.h, as we can get cs_main from node/blockstorage.h.
|
Nice. ACK a10df7c. As a follow-up, I wonder if we should just suck it up and move |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| #include <zmq.h> | ||
|
|
||
| #include <validation.h> | ||
| #include <primitives/block.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
iwuy reports:
zmq/zmqnotificationinterface.cpp should add these lines:
#include <assert.h> // for assert
#include <map> // for map, operator==, _Rb_tree_iterator
#include <string> // for string, char_traits, operator+
#include <utility> // for move, pair
#include <vector> // for vector
#include "logging.h" // for LogPrintf_, ZMQ, LogPrint
#include "primitives/block.h" // for CBlock
#include "zmq/zmqabstractnotifier.h" // for CZMQAbstractNotifier, CZMQNotif...
zmq/zmqnotificationinterface.cpp should remove these lines:
- #include <validation.h> // lines 11-11
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to leave this as-is for now.
| #include <rpc/server.h> | ||
| #include <streams.h> | ||
| #include <util/system.h> | ||
| #include <validation.h> // For cs_main |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: This change will confuse iwuy, as now it reports:
The full include-list for zmq/zmqpublishnotifier.cpp:
...
#include <validation.h> // for cs_main
282019c refactor: add kernel/cs_main.* (fanquake) Pull request description: One place to find / include `cs_main`. No more: > // Actually declared in validation.cpp; can't include because of circular dependency. > extern RecursiveMutex cs_main; Ultimately, no more need to include `validation.h` (which also includes (heavy/boost filled) `txmempool.h`) everywhere for `cs_main`. See bitcoin#26087 for another example of why that is useful. ACKs for top commit: ajtowns: ACK 282019c Tree-SHA512: 142835b794873e7a09c3246d6101843ae81ec0c6295e6873130c98a2abfa5f7282748d0f1a37237a779cc71c3bc0a75d03b20313ef5398c83d4814215cbc8287
Rather than including
validation.h, which ultimately means needing boost viatxmempool.h, includeprimitives/block.hforCBlock, and removevalidation.h, as we can getcs_mainfromnode/blockstorage.h.