💬 fanquake commented on pull request "Update leveldb subtree to latest upstream":
(https://github.com/bitcoin/bitcoin/pull/31671#issuecomment-2598526399)
> Could you share more about how these changes were selected.
> might still be worth considering:
They were PR'd, with some rationale, to https://github.com/bitcoin-core/leveldb-subtree/. This PR is just updating to the current state of the subtree, but if you think there are other worthwhile changes that should be looked at, I'd suggest opening a PR upstream for discussion (that would have to be the case anyways, before they could be part of this PR).
(https://github.com/bitcoin/bitcoin/pull/31671#issuecomment-2598526399)
> Could you share more about how these changes were selected.
> might still be worth considering:
They were PR'd, with some rationale, to https://github.com/bitcoin-core/leveldb-subtree/. This PR is just updating to the current state of the subtree, but if you think there are other worthwhile changes that should be looked at, I'd suggest opening a PR upstream for discussion (that would have to be the case anyways, before they could be part of this PR).
💬 instagibbs commented on pull request "test: add coverage for immediate orphanage eviction case":
(https://github.com/bitcoin/bitcoin/pull/31628#issuecomment-2598536562)
pushed what I think the fix is: make sure we sync the node with a ping/pong before bumping the mocktime.
(https://github.com/bitcoin/bitcoin/pull/31628#issuecomment-2598536562)
pushed what I think the fix is: make sure we sync the node with a ping/pong before bumping the mocktime.
💬 ryanofsky commented on issue "multiprocess: `ipc_tests` fail on *BSD":
(https://github.com/bitcoin/bitcoin/issues/31618#issuecomment-2598540264)
Thanks, do we know yet if these errors happen reliably or are random? Do you know what the easiest way I might be able to reproduce this is? Is this a custom CI setup? Is there a way to run this CI job in a VM in on linux?
(https://github.com/bitcoin/bitcoin/issues/31618#issuecomment-2598540264)
Thanks, do we know yet if these errors happen reliably or are random? Do you know what the easiest way I might be able to reproduce this is? Is this a custom CI setup? Is there a way to run this CI job in a VM in on linux?
👋 theuni's pull request is ready for review: "init: Lock blocksdir in addition to datadir"
(https://github.com/bitcoin/bitcoin/pull/31674)
(https://github.com/bitcoin/bitcoin/pull/31674)
👍 ryanofsky approved a pull request: "cmake: Set top-level target output locations"
(https://github.com/bitcoin/bitcoin/pull/31161#pullrequestreview-2559235706)
Code review ACK 186680acef661bd139bf3d16f494365ea55993b5. Since last review just reverted CMAKE_MODULE_PATH change, removed duplicate code and changed comment as suggested. Looks good!
(https://github.com/bitcoin/bitcoin/pull/31161#pullrequestreview-2559235706)
Code review ACK 186680acef661bd139bf3d16f494365ea55993b5. Since last review just reverted CMAKE_MODULE_PATH change, removed duplicate code and changed comment as suggested. Looks good!
💬 hebasto commented on issue "multiprocess: `ipc_tests` fail on *BSD":
(https://github.com/bitcoin/bitcoin/issues/31618#issuecomment-2598589382)
> Thanks, do we know yet if these errors happen reliably or are random?
They seem intermittent. I failed to reproduce them on my local NetBSD and FreeBSD installations.
> Do you know what the easiest way I might be able to reproduce this is? Is this a custom CI setup?
Yes. It is a custom CI setup based on https://github.com/vmactions.
> Is there a way to run this CI job in a VM in on linux?
It is possible to run FreeBSD on Cirrus CI though. And @maflcko [runs](https://cirrus-ci.com/task/563
...
(https://github.com/bitcoin/bitcoin/issues/31618#issuecomment-2598589382)
> Thanks, do we know yet if these errors happen reliably or are random?
They seem intermittent. I failed to reproduce them on my local NetBSD and FreeBSD installations.
> Do you know what the easiest way I might be able to reproduce this is? Is this a custom CI setup?
Yes. It is a custom CI setup based on https://github.com/vmactions.
> Is there a way to run this CI job in a VM in on linux?
It is possible to run FreeBSD on Cirrus CI though. And @maflcko [runs](https://cirrus-ci.com/task/563
...
💬 furszy commented on pull request "Added rescan option for import descriptors":
(https://github.com/bitcoin/bitcoin/pull/31668#discussion_r1920344559)
This overrides the flag for each imported descriptor, meaning the final value depends solely on the last imported descriptor.
(https://github.com/bitcoin/bitcoin/pull/31668#discussion_r1920344559)
This overrides the flag for each imported descriptor, meaning the final value depends solely on the last imported descriptor.
💬 maflcko commented on pull request "ci: Enable DEBUG=1 for one GCC-12+ build to catch 117966 regressions":
(https://github.com/bitcoin/bitcoin/pull/31522#issuecomment-2598598297)
Ok, slapped it on to the centos task, which happens to work now that the 32-bit stuff is dropped.
(https://github.com/bitcoin/bitcoin/pull/31522#issuecomment-2598598297)
Ok, slapped it on to the centos task, which happens to work now that the 32-bit stuff is dropped.
💬 maflcko commented on issue "multiprocess: `ipc_tests` fail on *BSD":
(https://github.com/bitcoin/bitcoin/issues/31618#issuecomment-2598607148)
> It is possible to run FreeBSD on Cirrus CI though. And [@maflcko](https://github.com/maflcko) [runs](https://cirrus-ci.com/task/5637806506115072) it :)
Though, the ipc_tests are not enabled there (yet?)
(https://github.com/bitcoin/bitcoin/issues/31618#issuecomment-2598607148)
> It is possible to run FreeBSD on Cirrus CI though. And [@maflcko](https://github.com/maflcko) [runs](https://cirrus-ci.com/task/5637806506115072) it :)
Though, the ipc_tests are not enabled there (yet?)
💬 fanquake commented on pull request "Update leveldb subtree to latest upstream":
(https://github.com/bitcoin/bitcoin/pull/31671#issuecomment-2598610152)
Guix Build (aarch64):
```bash
139048cf62397ec98d6863fc1706a3c7c2afd02307873ec9a9bd707c531283a6 guix-build-910a11fa6630/output/aarch64-linux-gnu/SHA256SUMS.part
7d2de717c990cbc3f7da29291e8de257e610a5a79b6a351f00b9c9cad4e8ed65 guix-build-910a11fa6630/output/aarch64-linux-gnu/bitcoin-910a11fa6630-aarch64-linux-gnu-debug.tar.gz
c9b45a7c734d6cba6f736cbc53cea1dd207600133eaa2e82f58fa4dc96bd21c9 guix-build-910a11fa6630/output/aarch64-linux-gnu/bitcoin-910a11fa6630-aarch64-linux-gnu.tar.gz
8c11bb
...
(https://github.com/bitcoin/bitcoin/pull/31671#issuecomment-2598610152)
Guix Build (aarch64):
```bash
139048cf62397ec98d6863fc1706a3c7c2afd02307873ec9a9bd707c531283a6 guix-build-910a11fa6630/output/aarch64-linux-gnu/SHA256SUMS.part
7d2de717c990cbc3f7da29291e8de257e610a5a79b6a351f00b9c9cad4e8ed65 guix-build-910a11fa6630/output/aarch64-linux-gnu/bitcoin-910a11fa6630-aarch64-linux-gnu-debug.tar.gz
c9b45a7c734d6cba6f736cbc53cea1dd207600133eaa2e82f58fa4dc96bd21c9 guix-build-910a11fa6630/output/aarch64-linux-gnu/bitcoin-910a11fa6630-aarch64-linux-gnu.tar.gz
8c11bb
...
💬 fanquake commented on pull request "ci: Enable DEBUG=1 for one GCC-12+ build to catch 117966 regressions":
(https://github.com/bitcoin/bitcoin/pull/31522#issuecomment-2598614934)
https://cirrus-ci.com/task/5812783272427520?logs=ci#L4621:
```bash
[15:22:49.280] [ 86%] Building CXX object src/test/fuzz/CMakeFiles/fuzz.dir/float.cpp.o
[15:22:49.281] cd /ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz && /usr/bin/ccache /opt/rh/gcc-toolset-12/root/usr/bin/g++ -m64 -DABORT_ON_FAILED_ASSUME -DBOOST_MULTI_INDEX_DISABLE_SERIALIZATION -DDEBUG -DDEBUG_LOCKCONTENTION -DDEBUG_LOCKORDER -DRPC_DOC_CHECK -D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC -D_LIBCPP_HARDE
...
(https://github.com/bitcoin/bitcoin/pull/31522#issuecomment-2598614934)
https://cirrus-ci.com/task/5812783272427520?logs=ci#L4621:
```bash
[15:22:49.280] [ 86%] Building CXX object src/test/fuzz/CMakeFiles/fuzz.dir/float.cpp.o
[15:22:49.281] cd /ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz && /usr/bin/ccache /opt/rh/gcc-toolset-12/root/usr/bin/g++ -m64 -DABORT_ON_FAILED_ASSUME -DBOOST_MULTI_INDEX_DISABLE_SERIALIZATION -DDEBUG -DDEBUG_LOCKCONTENTION -DDEBUG_LOCKORDER -DRPC_DOC_CHECK -D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC -D_LIBCPP_HARDE
...
💬 furszy commented on pull request "descriptor: check whitespace in pubkeys within fragments":
(https://github.com/bitcoin/bitcoin/pull/31603#discussion_r1920362724)
nit: Because there is a `return` statement in the if block that is above, you could remove the `else` and use `str.front()` instead of `str[0]`.
(https://github.com/bitcoin/bitcoin/pull/31603#discussion_r1920362724)
nit: Because there is a `return` statement in the if block that is above, you could remove the `else` and use `str.front()` instead of `str[0]`.
💬 TheCharlatan commented on pull request "depends: Qt 5.15.16":
(https://github.com/bitcoin/bitcoin/pull/30774#issuecomment-2598620293)
Guix build (aarch64):
```
e80422975315244b3365bf9eddb9e6e5d9da08f4b1a5fba34430c6f3021c6b8a guix-build-31a0e5f0905b/output/aarch64-linux-gnu/SHA256SUMS.part
db53cd0057bf0048534e3daea2e39130cdf64a140becaa5c6ff4f98fdfa5936a guix-build-31a0e5f0905b/output/aarch64-linux-gnu/bitcoin-31a0e5f0905b-aarch64-linux-gnu-debug.tar.gz
fa061afbd1dac25c739a63a1644f2af1ad5dab36ea5e4245a7b0bb9008ca4f79 guix-build-31a0e5f0905b/output/aarch64-linux-gnu/bitcoin-31a0e5f0905b-aarch64-linux-gnu.tar.gz
f286a7442a
...
(https://github.com/bitcoin/bitcoin/pull/30774#issuecomment-2598620293)
Guix build (aarch64):
```
e80422975315244b3365bf9eddb9e6e5d9da08f4b1a5fba34430c6f3021c6b8a guix-build-31a0e5f0905b/output/aarch64-linux-gnu/SHA256SUMS.part
db53cd0057bf0048534e3daea2e39130cdf64a140becaa5c6ff4f98fdfa5936a guix-build-31a0e5f0905b/output/aarch64-linux-gnu/bitcoin-31a0e5f0905b-aarch64-linux-gnu-debug.tar.gz
fa061afbd1dac25c739a63a1644f2af1ad5dab36ea5e4245a7b0bb9008ca4f79 guix-build-31a0e5f0905b/output/aarch64-linux-gnu/bitcoin-31a0e5f0905b-aarch64-linux-gnu.tar.gz
f286a7442a
...
🤔 hebasto reviewed a pull request: "depends: Qt 5.15.16"
(https://github.com/bitcoin/bitcoin/pull/30774#pullrequestreview-2559349957)
Approach ACK 31a0e5f0905bfc6b22ceaaeca53466dfd74967ab.
I've verified changes in `patches/qt`, and they look correct.
Building...
(https://github.com/bitcoin/bitcoin/pull/30774#pullrequestreview-2559349957)
Approach ACK 31a0e5f0905bfc6b22ceaaeca53466dfd74967ab.
I've verified changes in `patches/qt`, and they look correct.
Building...
🤔 ryanofsky requested changes to a pull request: "Add multiprocess binaries to release build (except Windows)"
(https://github.com/bitcoin/bitcoin/pull/30975#pullrequestreview-2559268555)
Code review 9a90f964a3c4fe75477926d34b7f1813b144449d.
It looks like latest depends changes are not actually enabling multiprocess binaries anymore due to a referencing a bad `$(multiprocess_$(host_os)_packages)` variable that should no longer include $(host_os). Also it is concerning that there doesn't seem to be any test coverage for this because in current version of this multiprocess binaries are only used in functional tests in a non-depends build. There's no depends build using multiproc
...
(https://github.com/bitcoin/bitcoin/pull/30975#pullrequestreview-2559268555)
Code review 9a90f964a3c4fe75477926d34b7f1813b144449d.
It looks like latest depends changes are not actually enabling multiprocess binaries anymore due to a referencing a bad `$(multiprocess_$(host_os)_packages)` variable that should no longer include $(host_os). Also it is concerning that there doesn't seem to be any test coverage for this because in current version of this multiprocess binaries are only used in functional tests in a non-depends build. There's no depends build using multiproc
...
💬 ryanofsky commented on pull request "Add multiprocess binaries to release build (except Windows)":
(https://github.com/bitcoin/bitcoin/pull/30975#discussion_r1920338053)
In commit "build: depends makes libmultiprocess by default" (48268facc9bb4f3e4f98cff5c21e1fe490b232e2)
OpenBSB typo
(https://github.com/bitcoin/bitcoin/pull/30975#discussion_r1920338053)
In commit "build: depends makes libmultiprocess by default" (48268facc9bb4f3e4f98cff5c21e1fe490b232e2)
OpenBSB typo
💬 ryanofsky commented on pull request "Add multiprocess binaries to release build (except Windows)":
(https://github.com/bitcoin/bitcoin/pull/30975#discussion_r1920354544)
In commit "build: depends makes libmultiprocess by default" (48268facc9bb4f3e4f98cff5c21e1fe490b232e2)
Would suggest reverting this change to simplify. Requires also reverting the `ifeq ($(multiprocess_packages_),)` change below.
(https://github.com/bitcoin/bitcoin/pull/30975#discussion_r1920354544)
In commit "build: depends makes libmultiprocess by default" (48268facc9bb4f3e4f98cff5c21e1fe490b232e2)
Would suggest reverting this change to simplify. Requires also reverting the `ifeq ($(multiprocess_packages_),)` change below.
💬 ryanofsky commented on pull request "Add multiprocess binaries to release build (except Windows)":
(https://github.com/bitcoin/bitcoin/pull/30975#discussion_r1920355534)
In commit "build: depends makes libmultiprocess by default" (https://github.com/bitcoin/bitcoin/commit/48268facc9bb4f3e4f98cff5c21e1fe490b232e2)
Would suggest reverting this change to simplify the PR.
(https://github.com/bitcoin/bitcoin/pull/30975#discussion_r1920355534)
In commit "build: depends makes libmultiprocess by default" (https://github.com/bitcoin/bitcoin/commit/48268facc9bb4f3e4f98cff5c21e1fe490b232e2)
Would suggest reverting this change to simplify the PR.
💬 ryanofsky commented on pull request "Add multiprocess binaries to release build (except Windows)":
(https://github.com/bitcoin/bitcoin/pull/30975#discussion_r1920385465)
In commit "build: depends makes libmultiprocess by default" (48268facc9bb4f3e4f98cff5c21e1fe490b232e2)
This isn't actually doing anything because RUN_FUNCTIONAL_TESTS is false above
(https://github.com/bitcoin/bitcoin/pull/30975#discussion_r1920385465)
In commit "build: depends makes libmultiprocess by default" (48268facc9bb4f3e4f98cff5c21e1fe490b232e2)
This isn't actually doing anything because RUN_FUNCTIONAL_TESTS is false above
💬 ryanofsky commented on pull request "Add multiprocess binaries to release build (except Windows)":
(https://github.com/bitcoin/bitcoin/pull/30975#discussion_r1920342619)
In commit "build: depends makes libmultiprocess by default" (48268facc9bb4f3e4f98cff5c21e1fe490b232e2)
It doesn't seem right to include host_os in these variables anymore. I think this is is causing multiprocess packages not to be built right now. If I run `make print-multiprocess_packages_` the variable is empty and the toolchain file does not seem to have multiprocess enabled. But if I drop host_os these things are fixed:
```diff
-multiprocess_packages_$(NO_MULTIPROCESS) = $(multiproces
...
(https://github.com/bitcoin/bitcoin/pull/30975#discussion_r1920342619)
In commit "build: depends makes libmultiprocess by default" (48268facc9bb4f3e4f98cff5c21e1fe490b232e2)
It doesn't seem right to include host_os in these variables anymore. I think this is is causing multiprocess packages not to be built right now. If I run `make print-multiprocess_packages_` the variable is empty and the toolchain file does not seem to have multiprocess enabled. But if I drop host_os these things are fixed:
```diff
-multiprocess_packages_$(NO_MULTIPROCESS) = $(multiproces
...