π€ 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.
π¬ willcl-ark commented on pull request "ci: Turn CentOS config into Alpine musl config":
(https://github.com/bitcoin/bitcoin/pull/33480#discussion_r2387353020)
What do you think about setting this to `alpine:latest` to automatically use new versions? Might give us the occasionaly false positive when the alpine version is auto-bumped, but feels useful to catch musl changes as early as possible?
Perhaps that's a jobs for a nightly though...
(https://github.com/bitcoin/bitcoin/pull/33480#discussion_r2387353020)
What do you think about setting this to `alpine:latest` to automatically use new versions? Might give us the occasionaly false positive when the alpine version is auto-bumped, but feels useful to catch musl changes as early as possible?
Perhaps that's a jobs for a nightly though...
π¬ willcl-ark commented on pull request "ci: Turn CentOS config into Alpine musl config":
(https://github.com/bitcoin/bitcoin/pull/33480#discussion_r2387381492)
I wonder if this needs to be `lg` any more? Could probably be `md` I think?
(https://github.com/bitcoin/bitcoin/pull/33480#discussion_r2387381492)
I wonder if this needs to be `lg` any more? Could probably be `md` I think?
π¬ maflcko commented on issue "How to backport libmultiprocess changes?":
(https://github.com/bitcoin/bitcoin/issues/33439#issuecomment-3346073946)
> (1) should be either _all_ of the changes or _none_ of them. At least for the next few weeks I don't think there will be any libmultiprocess changes that aren't either safe or desirable to backport, like bugfixes, documentation improvements, CI improvements, and error handling improvements. I think the only changes to avoid backporting might be PRs that add new features like [#32387](https://github.com/bitcoin/bitcoin/pull/32387) (adding windows support) or [bitcoin-core/libmultiprocess#209](h
...
(https://github.com/bitcoin/bitcoin/issues/33439#issuecomment-3346073946)
> (1) should be either _all_ of the changes or _none_ of them. At least for the next few weeks I don't think there will be any libmultiprocess changes that aren't either safe or desirable to backport, like bugfixes, documentation improvements, CI improvements, and error handling improvements. I think the only changes to avoid backporting might be PRs that add new features like [#32387](https://github.com/bitcoin/bitcoin/pull/32387) (adding windows support) or [bitcoin-core/libmultiprocess#209](h
...
π¬ willcl-ark commented on issue "ci: GHA fallback centos task runs out of space":
(https://github.com/bitcoin/bitcoin/issues/33293#issuecomment-3346080573)
This will be "fixed" by #33480 (as a side-effect). If we decide not to merge that, I can open a PR with something like https://github.com/willcl-ark/bitcoin/commit/f5885d05160fe0ac167976f2b36b6d5bec373e08 in to fix the space issue on CentOS.
(https://github.com/bitcoin/bitcoin/issues/33293#issuecomment-3346080573)
This will be "fixed" by #33480 (as a side-effect). If we decide not to merge that, I can open a PR with something like https://github.com/willcl-ark/bitcoin/commit/f5885d05160fe0ac167976f2b36b6d5bec373e08 in to fix the space issue on CentOS.
π¬ vasild commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-3346117320)
Updated the PR description. It referred to a older version of the PR.
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-3346117320)
Updated the PR description. It referred to a older version of the PR.