build: remove threadinterrupt from libbitcoinkernel #26360

pull fanquake wants to merge 1 commits into bitcoin:master from fanquake:kernel_remove_threadinterrupt changing 1 files +0 −1

fanquake commented at 8:42 AM on October 21, 2022: member

Extracted from #26292.
build: remove threadinterrupt from libbitcoinkernel afbcd227dd
fanquake added the label Build system on Oct 21, 2022
hebasto approved

hebasto commented at 3:43 PM on October 23, 2022: member

ACK afbcd227dd0889002eeeec6148538ed2e2c9aadb, tested on Ubuntu 22.04.

FWIW, ./configure --with-experimental-kernel-lib --enable-experimental-util-chainstate && make still works even with the following diff:

--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -886,7 +886,6 @@ libbitcoinkernel_la_SOURCES = \
   chain.cpp \
   chainparamsbase.cpp \
   chainparams.cpp \
-  clientversion.cpp \
   coins.cpp \
   compressor.cpp \
   consensus/merkle.cpp \
@@ -895,11 +894,9 @@ libbitcoinkernel_la_SOURCES = \
   core_read.cpp \
   dbwrapper.cpp \
   deploymentinfo.cpp \
-  deploymentstatus.cpp \
   flatfile.cpp \
   fs.cpp \
   hash.cpp \
-  kernel/chain.cpp \
   kernel/checks.cpp \
   kernel/coinstats.cpp \
   kernel/context.cpp \
@@ -932,7 +929,6 @@ libbitcoinkernel_la_SOURCES = \
   signet.cpp \
   support/cleanse.cpp \
   support/lockedpool.cpp \
-  sync.cpp \
   txdb.cpp \
   txmempool.cpp \
   uint256.cpp \

fanquake commented at 9:23 AM on October 31, 2022: member
still works even with the following diff:

sync.cpp \
That's because you didn't test with --enable-debug. Haven't looked at the other suggestions.

hebasto commented at 10:28 AM on October 31, 2022: member

That's because you didn't test with --enable-debug.

Correct.

Haven't looked at the other suggestions.

./configure --with-experimental-kernel-lib --enable-experimental-util-chainstate --enable-debug && make still works with the following diff on Ubuntu 22.04:

--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -886,7 +886,6 @@ libbitcoinkernel_la_SOURCES = \
   chain.cpp \
   chainparamsbase.cpp \
   chainparams.cpp \
-  clientversion.cpp \
   coins.cpp \
   compressor.cpp \
   consensus/merkle.cpp \
@@ -895,11 +894,9 @@ libbitcoinkernel_la_SOURCES = \
   core_read.cpp \
   dbwrapper.cpp \
   deploymentinfo.cpp \
-  deploymentstatus.cpp \
   flatfile.cpp \
   fs.cpp \
   hash.cpp \
-  kernel/chain.cpp \
   kernel/checks.cpp \
   kernel/coinstats.cpp \
   kernel/context.cpp \

theuni commented at 4:09 PM on October 31, 2022: member

Concept ACK.

FWIW, I anticipate threadinterrupt coming back to the kernel eventually as I think we'll want to use it to get rid of shutdown.cpp.

I think it's fine to re-add it when it's actually needed though.
fanquake commented at 4:11 PM on October 31, 2022: member
still works with the following diff

deploymentstatus.cpp \
If "works" means compiles, then sure, however this also drops the compile-time deployment status sanity checks from the kernel build.
ryanofsky approved
ryanofsky commented at 5:41 PM on October 31, 2022: contributor

Code review ACK afbcd227dd0889002eeeec6148538ed2e2c9aadb
fanquake merged this on Nov 1, 2022
fanquake closed this on Nov 1, 2022
fanquake deleted the branch on Nov 1, 2022
sidhujag referenced this in commit b6455439bc on Nov 1, 2022
bitcoin locked this on Nov 1, 2023

Contributors

Labels

Linked (view graph)

#26292 util: move threadinterrupt into util/