💬 hodlinator commented on pull request "Clear out space on centos job":
(https://github.com/bitcoin/bitcoin/pull/33514#discussion_r2405228550)
Don't see a problem with the current approach. Would just prefer:
```suggestion
if: ${{ matrix.freeup-space }}
```
(https://github.com/bitcoin/bitcoin/pull/33514#discussion_r2405228550)
Don't see a problem with the current approach. Would just prefer:
```suggestion
if: ${{ matrix.freeup-space }}
```
💬 maflcko commented on pull request "Release: 30.0rc3 translations update":
(https://github.com/bitcoin/bitcoin/pull/33541#issuecomment-3370423916)
Just as a note: It will likely never be possible to fully please the LLM. It is still experimental and WIP, so not all issues have to be addressed.
Though, this lgtm ACK 71ee0163dedd28327993415120e864253b127f8e
(https://github.com/bitcoin/bitcoin/pull/33541#issuecomment-3370423916)
Just as a note: It will likely never be possible to fully please the LLM. It is still experimental and WIP, so not all issues have to be addressed.
Though, this lgtm ACK 71ee0163dedd28327993415120e864253b127f8e
💬 theStack commented on pull request "test: add functional test for `TestShell` (matching doc example)":
(https://github.com/bitcoin/bitcoin/pull/33546#issuecomment-3370425877)
Force-pushed with a fix to make the test also work for non-wallet builds. With this, all CI jobs should pass now.
> Looks like the new test times out in CI?
Hmm, seems like the test just hangs whenever the `TestShell` instance is not shut down at the end. Wrapped it in an `try: ... finally: test.shutdown()` block in order to prevent time-outs if any exception would be thrown there.
(https://github.com/bitcoin/bitcoin/pull/33546#issuecomment-3370425877)
Force-pushed with a fix to make the test also work for non-wallet builds. With this, all CI jobs should pass now.
> Looks like the new test times out in CI?
Hmm, seems like the test just hangs whenever the `TestShell` instance is not shut down at the end. Wrapped it in an `try: ... finally: test.shutdown()` block in order to prevent time-outs if any exception would be thrown there.
💬 maflcko commented on pull request "Clear out space on centos job":
(https://github.com/bitcoin/bitcoin/pull/33514#discussion_r2405299263)
> Don't see a problem with the current approach.
The problem is that GHA does not run in this repo, so there likely won't be any indication when this setting is needed. It can practically most likely only be removed/added in a follow-up, which increases the churn a bit.
If the overhead is too large, the step could be run in the background, albeit that would be dropping the reporting feature.
No strong opinion, though, just wanted to mention it.
(https://github.com/bitcoin/bitcoin/pull/33514#discussion_r2405299263)
> Don't see a problem with the current approach.
The problem is that GHA does not run in this repo, so there likely won't be any indication when this setting is needed. It can practically most likely only be removed/added in a follow-up, which increases the churn a bit.
If the overhead is too large, the step could be run in the background, albeit that would be dropping the reporting feature.
No strong opinion, though, just wanted to mention it.
💬 maflcko commented on pull request "Clear out space on centos job":
(https://github.com/bitcoin/bitcoin/pull/33514#issuecomment-3370449972)
> Was curious if it would take less time to switch to the _stream10-minimal container_,
Just for reference, the added step here runs before any of the CI logic is run, so changing the CI config should not have any effect on this step here.
(https://github.com/bitcoin/bitcoin/pull/33514#issuecomment-3370449972)
> Was curious if it would take less time to switch to the _stream10-minimal container_,
Just for reference, the added step here runs before any of the CI logic is run, so changing the CI config should not have any effect on this step here.
💬 hodlinator commented on pull request "Clear out space on centos job":
(https://github.com/bitcoin/bitcoin/pull/33514#issuecomment-3370455740)
> > Was curious if it would take less time to switch to the _stream10-minimal container_,
>
> Just for reference, the added step here runs before any of the CI logic is run, so changing the CI config should not have any effect on this step here.
Yeah, just realized `/usr/local/lib/android` was being removed on the host OS, not inside the container.
(https://github.com/bitcoin/bitcoin/pull/33514#issuecomment-3370455740)
> > Was curious if it would take less time to switch to the _stream10-minimal container_,
>
> Just for reference, the added step here runs before any of the CI logic is run, so changing the CI config should not have any effect on this step here.
Yeah, just realized `/usr/local/lib/android` was being removed on the host OS, not inside the container.
💬 willcl-ark commented on pull request "Clear out space on centos job":
(https://github.com/bitcoin/bitcoin/pull/33514#discussion_r2405313117)
Yeah it's a fair shout.
The step in my run in OP took 4 mins, but I have seen it also take 2 minutes. Either way that's "wasted time" on the run, although this doesn't particularly matter on a fork run IMO, as they're way less time-sensitive, less likely to be queued up with a backlog (and using free compute).
When each job is takining in the order of 1-2 hours, adding 4 minutes on doesnt' feel that problematic to me.
(https://github.com/bitcoin/bitcoin/pull/33514#discussion_r2405313117)
Yeah it's a fair shout.
The step in my run in OP took 4 mins, but I have seen it also take 2 minutes. Either way that's "wasted time" on the run, although this doesn't particularly matter on a fork run IMO, as they're way less time-sensitive, less likely to be queued up with a backlog (and using free compute).
When each job is takining in the order of 1-2 hours, adding 4 minutes on doesnt' feel that problematic to me.
💬 janb84 commented on pull request "doc: Add `INSTALL.md` to Linux release tarballs":
(https://github.com/bitcoin/bitcoin/pull/33451#issuecomment-3370466568)
> [Feedback](https://github.com/bitcoin/bitcoin/pull/33451#discussion_r2402371308) from @fanquake has been addressed.
I re-tested this but without `libxcb-cursor` I cannot start bitcoin-qt on Xcfe / Debian 13
```sh
./bitcoin-qt
./bitcoin-qt: error while loading shared libraries: libxcb-cursor.so.0: cannot open shared object file: No such file or directory
```
(https://github.com/bitcoin/bitcoin/pull/33451#issuecomment-3370466568)
> [Feedback](https://github.com/bitcoin/bitcoin/pull/33451#discussion_r2402371308) from @fanquake has been addressed.
I re-tested this but without `libxcb-cursor` I cannot start bitcoin-qt on Xcfe / Debian 13
```sh
./bitcoin-qt
./bitcoin-qt: error while loading shared libraries: libxcb-cursor.so.0: cannot open shared object file: No such file or directory
```
💬 rkrux commented on pull request "doc: update multisig tutorial to use multipath descriptors":
(https://github.com/bitcoin/bitcoin/pull/33286#discussion_r2405318692)
Yes, this handles it. Thank you.
(https://github.com/bitcoin/bitcoin/pull/33286#discussion_r2405318692)
Yes, this handles it. Thank you.
👍 rkrux approved a pull request: "doc: update multisig tutorial to use multipath descriptors"
(https://github.com/bitcoin/bitcoin/pull/33286#pullrequestreview-3303358609)
crACK 2a46e94
(https://github.com/bitcoin/bitcoin/pull/33286#pullrequestreview-3303358609)
crACK 2a46e94
💬 willcl-ark commented on pull request "doc: Add `INSTALL.md` to Linux release tarballs":
(https://github.com/bitcoin/bitcoin/pull/33451#issuecomment-3370478414)
@janb84 you'll need to guix build a binary from a commit post #33434 (or for v30 with #33473 checked out)
(https://github.com/bitcoin/bitcoin/pull/33451#issuecomment-3370478414)
@janb84 you'll need to guix build a binary from a commit post #33434 (or for v30 with #33473 checked out)
💬 willcl-ark commented on pull request "Clear out space on GHA jobs":
(https://github.com/bitcoin/bitcoin/pull/33514#discussion_r2405334760)
Yeah I don't have much of an opinion on whether
> nit: Could run it for all tasks, to avoid having to select them individually?
OK I took this suggestion in 825f2e4a152301d757be8b02820d8679b69e134b as the future-proofing/churn-reduction seems worth it.
(https://github.com/bitcoin/bitcoin/pull/33514#discussion_r2405334760)
Yeah I don't have much of an opinion on whether
> nit: Could run it for all tasks, to avoid having to select them individually?
OK I took this suggestion in 825f2e4a152301d757be8b02820d8679b69e134b as the future-proofing/churn-reduction seems worth it.
💬 willcl-ark commented on pull request "Clear out space on GHA jobs":
(https://github.com/bitcoin/bitcoin/pull/33514#discussion_r2405343162)
Removed line altogether as we now run on all jobs on GHA
(https://github.com/bitcoin/bitcoin/pull/33514#discussion_r2405343162)
Removed line altogether as we now run on all jobs on GHA
🤔 janb84 reviewed a pull request: "doc: Add `INSTALL.md` to Linux release tarballs"
(https://github.com/bitcoin/bitcoin/pull/33451#pullrequestreview-3303526064)
(re) ACK 0fe7d552ab213065b8d5807c3dd9f4e976717529
retested on Debian 13 / Xfce with recent guix build of master (a33bd767a37d)
(No need for libxcb-cursor anymore)
(https://github.com/bitcoin/bitcoin/pull/33451#pullrequestreview-3303526064)
(re) ACK 0fe7d552ab213065b8d5807c3dd9f4e976717529
retested on Debian 13 / Xfce with recent guix build of master (a33bd767a37d)
(No need for libxcb-cursor anymore)
🤔 janb84 reviewed a pull request: "Release: 30.0rc3 translations update"
(https://github.com/bitcoin/bitcoin/pull/33541#pullrequestreview-3303538177)
ACK 71ee0163dedd28327993415120e864253b127f8e
Did some spot checks with a translator/llm. All checked-out ✅
(https://github.com/bitcoin/bitcoin/pull/33541#pullrequestreview-3303538177)
ACK 71ee0163dedd28327993415120e864253b127f8e
Did some spot checks with a translator/llm. All checked-out ✅
💬 Sjors commented on pull request "doc: update multisig tutorial to use multipath descriptors":
(https://github.com/bitcoin/bitcoin/pull/33286#issuecomment-3370664740)
utACK 2a46e94a1600a4f28e01db23a89f039acaa2c45e
(https://github.com/bitcoin/bitcoin/pull/33286#issuecomment-3370664740)
utACK 2a46e94a1600a4f28e01db23a89f039acaa2c45e
🚀 fanquake merged a pull request: "Release: 30.0rc3 translations update"
(https://github.com/bitcoin/bitcoin/pull/33541)
(https://github.com/bitcoin/bitcoin/pull/33541)
💬 fanquake commented on pull request "[30.x] Backports & rc3":
(https://github.com/bitcoin/bitcoin/pull/33473#issuecomment-3370701596)
> Can you add #33229?
I'm not going to add that here, `rc3` is already late. There might be more multiprocess backporting done, so it could be included there, but that's also blocked on at least https://github.com/bitcoin-core/libmultiprocess/pull/222.
(https://github.com/bitcoin/bitcoin/pull/33473#issuecomment-3370701596)
> Can you add #33229?
I'm not going to add that here, `rc3` is already late. There might be more multiprocess backporting done, so it could be included there, but that's also blocked on at least https://github.com/bitcoin-core/libmultiprocess/pull/222.
⚠️ hodlinator opened an issue: "HeadersSync tracking issue"
(https://github.com/bitcoin/bitcoin/issues/33547)
End Goal: Make the headers sync phase quicker and less prone to fail, while improving implementation and test code.
The Bitcoin Optech Podcast episode for Newsletter 322 (https://bitcoinops.org/en/podcast/2024/10/01/) among other things prompted the (re)discovery of the possible improvement of caching headers so they only needed to be downloaded once. To avoid adding unnecessary resource demands on nodes, the number of `HeaderSyncState` instances created over time should ideally be made more pr
...
(https://github.com/bitcoin/bitcoin/issues/33547)
End Goal: Make the headers sync phase quicker and less prone to fail, while improving implementation and test code.
The Bitcoin Optech Podcast episode for Newsletter 322 (https://bitcoinops.org/en/podcast/2024/10/01/) among other things prompted the (re)discovery of the possible improvement of caching headers so they only needed to be downloaded once. To avoid adding unnecessary resource demands on nodes, the number of `HeaderSyncState` instances created over time should ideally be made more pr
...
🚀 fanquake merged a pull request: "[30.x] Backports & rc3"
(https://github.com/bitcoin/bitcoin/pull/33473)
(https://github.com/bitcoin/bitcoin/pull/33473)