π¬ maflcko commented on pull request "test: set par=2 in default config for functional test framework":
(https://github.com/bitcoin/bitcoin/pull/33485#issuecomment-3345215920)
lgtm ACK dda5228e02ca6a839bf87ae7dbd133547563816a
(https://github.com/bitcoin/bitcoin/pull/33485#issuecomment-3345215920)
lgtm ACK dda5228e02ca6a839bf87ae7dbd133547563816a
π¬ ajtowns commented on issue "[29.x] guix build failure on ppc with xcb":
(https://github.com/bitcoin/bitcoin/issues/33488#issuecomment-3345222853)
Ah, there we go; these look like relevant lines:
```
$ HOSTS=powerpc64-linux-gnu contrib/guix/guix-build
...
xcb_auth.c:74:37: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
74 | #define AUTH_PROTO_MIT_MAGIC_COOKIE "MIT-MAGIC-COOKIE-1"
| ^~~~~~~~~~~~~~~~~~~~
xcb_auth.c:80:5: note: in expansion of macro 'AUTH_PROTO_MIT_MAGIC_COOKIE'
80 | AUTH_PROTO_MIT_MAGIC_COOKIE,
| ^~~~~~~~~~~~~
...
(https://github.com/bitcoin/bitcoin/issues/33488#issuecomment-3345222853)
Ah, there we go; these look like relevant lines:
```
$ HOSTS=powerpc64-linux-gnu contrib/guix/guix-build
...
xcb_auth.c:74:37: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
74 | #define AUTH_PROTO_MIT_MAGIC_COOKIE "MIT-MAGIC-COOKIE-1"
| ^~~~~~~~~~~~~~~~~~~~
xcb_auth.c:80:5: note: in expansion of macro 'AUTH_PROTO_MIT_MAGIC_COOKIE'
80 | AUTH_PROTO_MIT_MAGIC_COOKIE,
| ^~~~~~~~~~~~~
...
π¬ maflcko commented on pull request "ci: use latest versions of lint deps":
(https://github.com/bitcoin/bitcoin/pull/33487#issuecomment-3345453011)
> Side note. I can't remember the last time one of these tools (mypy, ruff, vulture) actually caught an issue in the lint job.
The last thing mypy found was a false positive: https://github.com/bitcoin/bitcoin/pull/33201#issuecomment-3192610057 and https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2284671990
Ruff is doing basically all python checks and finds stuff frequently: E.g. https://github.com/bitcoin/bitcoin/pull/33270#issuecomment-3236372196
vulture is only doing small
...
(https://github.com/bitcoin/bitcoin/pull/33487#issuecomment-3345453011)
> Side note. I can't remember the last time one of these tools (mypy, ruff, vulture) actually caught an issue in the lint job.
The last thing mypy found was a false positive: https://github.com/bitcoin/bitcoin/pull/33201#issuecomment-3192610057 and https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2284671990
Ruff is doing basically all python checks and finds stuff frequently: E.g. https://github.com/bitcoin/bitcoin/pull/33270#issuecomment-3236372196
vulture is only doing small
...
π¬ benthecarman commented on pull request "depends: static libxcb-cursor":
(https://github.com/bitcoin/bitcoin/pull/33434#issuecomment-3345454480)
This build works for me when I don't have `libxcb-cursor0` installed
(https://github.com/bitcoin/bitcoin/pull/33434#issuecomment-3345454480)
This build works for me when I don't have `libxcb-cursor0` installed
π¬ maflcko commented on pull request "ci: use latest versions of lint deps":
(https://github.com/bitcoin/bitcoin/pull/33487#issuecomment-3345455571)
lgtm ACK aeab2c7f1d151beda3eff94fb4effc85a4d4a344
(https://github.com/bitcoin/bitcoin/pull/33487#issuecomment-3345455571)
lgtm ACK aeab2c7f1d151beda3eff94fb4effc85a4d4a344
π€ hodlinator reviewed a pull request: "ci: Drop support for EOL macOS 13"
(https://github.com/bitcoin/bitcoin/pull/33489#pullrequestreview-3278413474)
Reviewed fab9ddaf554837624fa8f388e046a30d5bf7626f
(https://github.com/bitcoin/bitcoin/pull/33489#pullrequestreview-3278413474)
Reviewed fab9ddaf554837624fa8f388e046a30d5bf7626f
π¬ hodlinator commented on pull request "ci: Drop support for EOL macOS 13":
(https://github.com/bitcoin/bitcoin/pull/33489#discussion_r2387051017)
Seems fine to push GH CI one version ahead of supported OS. Then again, it would be nice to have test coverage of minimum version as well?
(https://github.com/bitcoin/bitcoin/pull/33489#discussion_r2387051017)
Seems fine to push GH CI one version ahead of supported OS. Then again, it would be nice to have test coverage of minimum version as well?
π¬ hodlinator commented on pull request "ci: Drop support for EOL macOS 13":
(https://github.com/bitcoin/bitcoin/pull/33489#discussion_r2387053565)
nit: Noticed 15 still technically supports x86: https://en.wikipedia.org/wiki/MacOS_Sequoia - so could keep arm64.
(https://github.com/bitcoin/bitcoin/pull/33489#discussion_r2387053565)
nit: Noticed 15 still technically supports x86: https://en.wikipedia.org/wiki/MacOS_Sequoia - so could keep arm64.
π€ janb84 reviewed a pull request: "ci: use latest versions of lint deps"
(https://github.com/bitcoin/bitcoin/pull/33487#pullrequestreview-3278533531)
Concept ACK aeab2c7f1d151beda3eff94fb4effc85a4d4a344
This PR updates lint dependency packages to the latests versions.
Small NIT / question about ruff version.
(https://github.com/bitcoin/bitcoin/pull/33487#pullrequestreview-3278533531)
Concept ACK aeab2c7f1d151beda3eff94fb4effc85a4d4a344
This PR updates lint dependency packages to the latests versions.
Small NIT / question about ruff version.
π¬ janb84 commented on pull request "ci: use latest versions of lint deps":
(https://github.com/bitcoin/bitcoin/pull/33487#discussion_r2387129503)
Why ruff 0.13.1 and not 0.13.2 ? or was it updated between PR creation and now ?
(https://github.com/bitcoin/bitcoin/pull/33487#discussion_r2387129503)
Why ruff 0.13.1 and not 0.13.2 ? or was it updated between PR creation and now ?
π¬ maflcko commented on pull request "ci: Drop support for EOL macOS 13":
(https://github.com/bitcoin/bitcoin/pull/33489#discussion_r2387159193)
No strong opinion, but:
* Such test coverage did not exist before either.
* I am not aware of this ever finding an issue. The version that matters most is the xcode toolchain version, as documented in the lines above.
(https://github.com/bitcoin/bitcoin/pull/33489#discussion_r2387159193)
No strong opinion, but:
* Such test coverage did not exist before either.
* I am not aware of this ever finding an issue. The version that matters most is the xcode toolchain version, as documented in the lines above.
π¬ maflcko commented on pull request "ci: Drop support for EOL macOS 13":
(https://github.com/bitcoin/bitcoin/pull/33489#discussion_r2387159665)
I think the meaningless bloat is just frustrating when glancing at and selecting CI results, because it makes it impossible to differentiate tasks on a glance:
<img width="622" height="316" alt="Screenshot 2025-09-29 at 10-34-08 Merge bitcoin_bitcoin#33484 doc rpc fix case typo in `finalizepsbt⦠· bitcoin_bitcoin@d8fe258" src="https://github.com/user-attachments/assets/c533c42c-5df8-403a-aaeb-cd8ebbe69d08" />
Also, it is unclear what value it adds. Has the macOS version or the macOS arch
...
(https://github.com/bitcoin/bitcoin/pull/33489#discussion_r2387159665)
I think the meaningless bloat is just frustrating when glancing at and selecting CI results, because it makes it impossible to differentiate tasks on a glance:
<img width="622" height="316" alt="Screenshot 2025-09-29 at 10-34-08 Merge bitcoin_bitcoin#33484 doc rpc fix case typo in `finalizepsbt⦠· bitcoin_bitcoin@d8fe258" src="https://github.com/user-attachments/assets/c533c42c-5df8-403a-aaeb-cd8ebbe69d08" />
Also, it is unclear what value it adds. Has the macOS version or the macOS arch
...
π¬ janb84 commented on pull request "guix: documented shasum gathering command":
(https://github.com/bitcoin/bitcoin/pull/33472#issuecomment-3345772428)
Addressed in :0a97da9
- added `uname -m`
- added script for tag-based checkouts.
(https://github.com/bitcoin/bitcoin/pull/33472#issuecomment-3345772428)
Addressed in :0a97da9
- added `uname -m`
- added script for tag-based checkouts.
π¬ janb84 commented on pull request "guix: documented shasum gathering command":
(https://github.com/bitcoin/bitcoin/pull/33472#discussion_r2387191699)
Thanks Addressed in : 0a97da9
(https://github.com/bitcoin/bitcoin/pull/33472#discussion_r2387191699)
Thanks Addressed in : 0a97da9
π¬ TheCharlatan commented on pull request "multiprocess: Add capnp wrapper for Chain interface":
(https://github.com/bitcoin/bitcoin/pull/29409#issuecomment-3345899017)
Thanks for the rebase :)
I updated darosiors's [core_bdk_wallet](https://github.com/darosior/core_bdk_wallet?tab=readme-ov-file#generating-rust-ipc-interface-from-capnp-definition) to use this newest version and I ran into a few crashes again. This is the one I managed to get a reproducible backtrace for:
<details>
<summary>Crash backtrace + ipc debug logging</summary>
```
2025-09-29T09:13:03Z [ipc] {bitcoin-node-157078/b-capnp-loop-157084} IPC server recv request #1 Init.construct$P
...
(https://github.com/bitcoin/bitcoin/pull/29409#issuecomment-3345899017)
Thanks for the rebase :)
I updated darosiors's [core_bdk_wallet](https://github.com/darosior/core_bdk_wallet?tab=readme-ov-file#generating-rust-ipc-interface-from-capnp-definition) to use this newest version and I ran into a few crashes again. This is the one I managed to get a reproducible backtrace for:
<details>
<summary>Crash backtrace + ipc debug logging</summary>
```
2025-09-29T09:13:03Z [ipc] {bitcoin-node-157078/b-capnp-loop-157084} IPC server recv request #1 Init.construct$P
...
π¬ katesalazar commented on pull request "ci: Drop support for EOL macOS 13":
(https://github.com/bitcoin/bitcoin/pull/33489#issuecomment-3345907639)
ACK fab9ddaf554837624fa8f388e046a30d5bf7626f
(https://github.com/bitcoin/bitcoin/pull/33489#issuecomment-3345907639)
ACK fab9ddaf554837624fa8f388e046a30d5bf7626f
π¬ janb84 commented on pull request "CMake: Add dynamic test discovery":
(https://github.com/bitcoin/bitcoin/pull/33483#issuecomment-3346029371)
Can you explain the difference in ctest tests-cases? Is the script double discovering tests ?
Master : 150
This PR: 368
See also:
https://github.com/bitcoin/bitcoin/actions/runs/18044688187/job/51352444037?pr=33483#step:8:4795
https://github.com/bitcoin/bitcoin/actions/runs/17995985600/job/51235033214?pr=33443#step:8:4357
(https://github.com/bitcoin/bitcoin/pull/33483#issuecomment-3346029371)
Can you explain the difference in ctest tests-cases? Is the script double discovering tests ?
Master : 150
This PR: 368
See also:
https://github.com/bitcoin/bitcoin/actions/runs/18044688187/job/51352444037?pr=33483#step:8:4795
https://github.com/bitcoin/bitcoin/actions/runs/17995985600/job/51235033214?pr=33443#step:8:4357
π¬ maflcko commented on pull request "CMake: Add dynamic test discovery":
(https://github.com/bitcoin/bitcoin/pull/33483#issuecomment-3346048339)
> Can you explain the difference in ctest tests-cases? Is the script double discovering tests ?
Previously there was a single bench sanity check, now there is one bench sanity check for each bench target. (This is the goal of this pull request, but maybe it isn't obvious from the description)
(https://github.com/bitcoin/bitcoin/pull/33483#issuecomment-3346048339)
> Can you explain the difference in ctest tests-cases? Is the script double discovering tests ?
Previously there was a single bench sanity check, now there is one bench sanity check for each bench target. (This is the goal of this pull request, but maybe it isn't obvious from the description)
π¬ optout21 commented on pull request "refactor: unify container presence checks":
(https://github.com/bitcoin/bitcoin/pull/33192#issuecomment-3346064980)
reACK d870caca33ac9f0fdd76969a7341535c5722417e
([prev](https://github.com/bitcoin/bitcoin/pull/33192#pullrequestreview-3171424512))
(https://github.com/bitcoin/bitcoin/pull/33192#issuecomment-3346064980)
reACK d870caca33ac9f0fdd76969a7341535c5722417e
([prev](https://github.com/bitcoin/bitcoin/pull/33192#pullrequestreview-3171424512))
π willcl-ark approved a pull request: "ci: Turn CentOS config into Alpine musl config"
(https://github.com/bitcoin/bitcoin/pull/33480#pullrequestreview-3278870275)
ACK fa6b2e9efece2d728bdc257c36c95db03e1a7bc4
Alpine/musl feels much more useful to me than a frozen-in-time CentOS build.
Not tested locally as I'm a bit bandwidth-contrained at the moment, but the code changes and CI look good to me.
(https://github.com/bitcoin/bitcoin/pull/33480#pullrequestreview-3278870275)
ACK fa6b2e9efece2d728bdc257c36c95db03e1a7bc4
Alpine/musl feels much more useful to me than a frozen-in-time CentOS build.
Not tested locally as I'm a bit bandwidth-contrained at the moment, but the code changes and CI look good to me.