build: prune BOOST_CPPFLAGS from libbitcoin_zmq#26087
Merged
fanquake merged 1 commit intobitcoin:masterfrom Sep 16, 2022
Merged
build: prune BOOST_CPPFLAGS from libbitcoin_zmq#26087fanquake merged 1 commit intobitcoin:masterfrom
fanquake merged 1 commit intobitcoin:masterfrom
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.
Member
|
Nice. ACK a10df7c. As a follow-up, I wonder if we should just suck it up and move |
hebasto
approved these changes
Sep 15, 2022
| #include <zmq.h> | ||
|
|
||
| #include <validation.h> | ||
| #include <primitives/block.h> |
Member
There was a problem hiding this comment.
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
Member
Author
There was a problem hiding this comment.
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 |
Member
There was a problem hiding this comment.
nit: This change will confuse iwuy, as now it reports:
The full include-list for zmq/zmqpublishnotifier.cpp:
...
#include <validation.h> // for cs_main
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Sep 20, 2022
maflcko
pushed a commit
to maflcko/bitcoin-core
that referenced
this pull request
Jan 16, 2023
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.