build: prune BOOST_CPPFLAGS from libbitcoin_zmq #26087

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

fanquake commented at 8:38 AM on September 14, 2022: member

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.

build: prune BOOST_CPPFLAGS from libbitcoin_zmq

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.

a10df7cf35

DrahtBot added the label Build system on Sep 14, 2022
theuni commented at 4:25 PM on September 14, 2022: member

Nice. ACK a10df7cf351e3947ba2d6cd1357d2c3348f8559a.

As a follow-up, I wonder if we should just suck it up and move cs_main to its own minimal header/cpp rather than copy/paste the declaration of it all over the place and having it defined in an arbitrary file. That's very c-like :(

in src/zmq/zmqnotificationinterface.cpp:11 in a10df7cf35

   7 | @@ -8,7 +8,7 @@
   8 |  
   9 |  #include <zmq.h>
  10 |  
  11 | -#include <validation.h>
  12 | +#include <primitives/block.h>

hebasto commented at 11:24 AM on September 15, 2022:

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

fanquake commented at 1:53 PM on September 16, 2022:

I'm going to leave this as-is for now.

in src/zmq/zmqpublishnotifier.cpp:14 in a10df7cf35

  10 | @@ -11,7 +11,6 @@
  11 |  #include <rpc/server.h>
  12 |  #include <streams.h>
  13 |  #include <util/system.h>
  14 | -#include <validation.h> // For cs_main

hebasto commented at 11:27 AM on September 15, 2022:

nit: This change will confuse iwuy, as now it reports:

The full include-list for zmq/zmqpublishnotifier.cpp:
...
#include <validation.h>              // for cs_main

hebasto approved
hebasto commented at 11:30 AM on September 15, 2022: member

ACK a10df7cf351e3947ba2d6cd1357d2c3348f8559a, tested on Linux x86_64 using theuni's patch with depends.
fanquake merged this on Sep 16, 2022
fanquake closed this on Sep 16, 2022
fanquake deleted the branch on Sep 16, 2022
sidhujag referenced this in commit 702aebcadd on Sep 20, 2022
MarcoFalke referenced this in commit 6b7ccb98a5 on Jan 16, 2023
bitcoin locked this on Sep 16, 2023

Contributors

Labels

Linked (view graph)

#26251 refactor: add kernel/cs_main.h