💬 fanquake commented on pull request "ci: Build for ARM instead of Intel in macOS cross task":
(https://github.com/bitcoin/bitcoin/pull/33549#issuecomment-3380738070)
> but it doesn't seem like a good use of CI resources here.
Having at least compilation coverage of every target we are shipping a release binary for seems like something we should have? Post switchover we are much less constrained on the CI resources front.
(https://github.com/bitcoin/bitcoin/pull/33549#issuecomment-3380738070)
> but it doesn't seem like a good use of CI resources here.
Having at least compilation coverage of every target we are shipping a release binary for seems like something we should have? Post switchover we are much less constrained on the CI resources front.
💬 Sjors commented on pull request "doc: how to update a subtree":
(https://github.com/bitcoin/bitcoin/pull/33568#discussion_r2413292023)
But then I have to move the text below this too, or add even more paragraphs. So going to leave it as is for now.
(https://github.com/bitcoin/bitcoin/pull/33568#discussion_r2413292023)
But then I have to move the text below this too, or add even more paragraphs. So going to leave it as is for now.
💬 fanquake commented on pull request "ci: Check macos-cross executables on macOS":
(https://github.com/bitcoin/bitcoin/pull/33509#issuecomment-3380767887)
It's not completely clear to me why we should have cross tests for Windows, but not for macOS, regardless of how-likely we think they are to find issues (or the runtime). Putting things in nightly repos also increases the likelyhood of double-handling changes, to fix things later (also assuming they are reported).
> Since we don't have high hopes of this actually detecting any issues that the rest of the CI would not have found, seems to make more sense as a nightly job?
@hodlinator could
...
(https://github.com/bitcoin/bitcoin/pull/33509#issuecomment-3380767887)
It's not completely clear to me why we should have cross tests for Windows, but not for macOS, regardless of how-likely we think they are to find issues (or the runtime). Putting things in nightly repos also increases the likelyhood of double-handling changes, to fix things later (also assuming they are reported).
> Since we don't have high hopes of this actually detecting any issues that the rest of the CI would not have found, seems to make more sense as a nightly job?
@hodlinator could
...
💬 thewrlck commented on pull request "[30.x] Finalise v30.0":
(https://github.com/bitcoin/bitcoin/pull/33559#issuecomment-3380773895)
`-datacarriersize` is increased to 100,000 by default, which effectively uncaps 🔥🔥🔥
(https://github.com/bitcoin/bitcoin/pull/33559#issuecomment-3380773895)
`-datacarriersize` is increased to 100,000 by default, which effectively uncaps 🔥🔥🔥
💬 Sjors commented on pull request "Clear out space on GHA jobs":
(https://github.com/bitcoin/bitcoin/pull/33514#discussion_r2413331519)
Is this automatically set?
(https://github.com/bitcoin/bitcoin/pull/33514#discussion_r2413331519)
Is this automatically set?
💬 willcl-ark commented on pull request "depends: Update URL for `qrencode` package source tarball":
(https://github.com/bitcoin/bitcoin/pull/33494#issuecomment-3380802351)
Unfortunately this has inadvertently clobbered all non-master branches (e.g. https://github.com/bitcoin/bitcoin/pull/33559/checks) which don't have the required depends Makefile changes from the first 3 commits here.
The issue is that when we load a depends cache (which will come from master) we now load the (new) qrencode package using filename only in the v30 branch (without the changes here), but the hash doesn't match.
(https://github.com/bitcoin/bitcoin/pull/33494#issuecomment-3380802351)
Unfortunately this has inadvertently clobbered all non-master branches (e.g. https://github.com/bitcoin/bitcoin/pull/33559/checks) which don't have the required depends Makefile changes from the first 3 commits here.
The issue is that when we load a depends cache (which will come from master) we now load the (new) qrencode package using filename only in the v30 branch (without the changes here), but the hash doesn't match.
💬 willcl-ark commented on pull request "Clear out space on GHA jobs":
(https://github.com/bitcoin/bitcoin/pull/33514#discussion_r2413363756)
Yes, this is set by the `determine-runners` job: https://github.com/bitcoin/bitcoin/blob/b510893d00760083ac36948747aa6ebd84656192/.github/workflows/ci.yml#L33-L46
If you have [cirrus runners](https://cirrus-runners.app/) in your own repo you can set a repo variable `REPO_USE_CIRRUS_RUNNERS` to your repo name to enable cirrus runners in your fork.
(https://github.com/bitcoin/bitcoin/pull/33514#discussion_r2413363756)
Yes, this is set by the `determine-runners` job: https://github.com/bitcoin/bitcoin/blob/b510893d00760083ac36948747aa6ebd84656192/.github/workflows/ci.yml#L33-L46
If you have [cirrus runners](https://cirrus-runners.app/) in your own repo you can set a repo variable `REPO_USE_CIRRUS_RUNNERS` to your repo name to enable cirrus runners in your fork.
💬 willcl-ark commented on pull request "Clear out space on GHA jobs":
(https://github.com/bitcoin/bitcoin/pull/33514#discussion_r2413369136)
You will see the setting in the Annotations section of the Actions tab on a run:
<img width="1817" height="631" alt="image" src="https://github.com/user-attachments/assets/486d7b03-36d5-4740-89ac-0aaf8ccf6070" />
(https://github.com/bitcoin/bitcoin/pull/33514#discussion_r2413369136)
You will see the setting in the Annotations section of the Actions tab on a run:
<img width="1817" height="631" alt="image" src="https://github.com/user-attachments/assets/486d7b03-36d5-4740-89ac-0aaf8ccf6070" />
💬 fanquake commented on pull request "depends: Update URL for `qrencode` package source tarball":
(https://github.com/bitcoin/bitcoin/pull/33494#issuecomment-3380876967)
This change is causing the CI to fail on all the branches (#33559, #33551) we are about to cut releases from. I'd really prefer to not have to do another round of rcs, just to backport this.
(https://github.com/bitcoin/bitcoin/pull/33494#issuecomment-3380876967)
This change is causing the CI to fail on all the branches (#33559, #33551) we are about to cut releases from. I'd really prefer to not have to do another round of rcs, just to backport this.
💬 maflcko commented on pull request "depends: Update URL for `qrencode` package source tarball":
(https://github.com/bitcoin/bitcoin/pull/33494#issuecomment-3380917171)
Would it help to set a new filename for the cached file, like `gh_$(package)-$($(package)_version).tar.gz`?
(https://github.com/bitcoin/bitcoin/pull/33494#issuecomment-3380917171)
Would it help to set a new filename for the cached file, like `gh_$(package)-$($(package)_version).tar.gz`?
💬 willcl-ark commented on pull request "depends: Update URL for `qrencode` package source tarball":
(https://github.com/bitcoin/bitcoin/pull/33494#issuecomment-3380937238)
Other workarounds may include (from least to most hacky):
- Revert this change on master and wait for a run to complete (and a new depends cache entry to be generated, to be picked up in 33559 and 33551)
- Acquire ability to delete caches from Cirrus (I have emailed @fkorotkov to ask if he can help us with this), after which we could do "uncached runs" on those branches
(https://github.com/bitcoin/bitcoin/pull/33494#issuecomment-3380937238)
Other workarounds may include (from least to most hacky):
- Revert this change on master and wait for a run to complete (and a new depends cache entry to be generated, to be picked up in 33559 and 33551)
- Acquire ability to delete caches from Cirrus (I have emailed @fkorotkov to ask if he can help us with this), after which we could do "uncached runs" on those branches
💬 maflcko commented on pull request "ci: Check macos-cross executables on macOS":
(https://github.com/bitcoin/bitcoin/pull/33509#issuecomment-3380951117)
I just wonder if there ever was a single issue found if this was run in the CI in the past.
If this has never found an issue, the overhead doesn't seem worth it:
* It adds a 17 minute sequential spinner to the CI
* The new code requires maintenance
* The new task requires occasional manual re-runs due to https://github.com/bitcoin/bitcoin/pull/33303#discussion_r2391749840
* GHA macOS VM provisioning may fail (seen twice in the last week), which requires even more manual re-runs
I can
...
(https://github.com/bitcoin/bitcoin/pull/33509#issuecomment-3380951117)
I just wonder if there ever was a single issue found if this was run in the CI in the past.
If this has never found an issue, the overhead doesn't seem worth it:
* It adds a 17 minute sequential spinner to the CI
* The new code requires maintenance
* The new task requires occasional manual re-runs due to https://github.com/bitcoin/bitcoin/pull/33303#discussion_r2391749840
* GHA macOS VM provisioning may fail (seen twice in the last week), which requires even more manual re-runs
I can
...
💬 optout21 commented on pull request "net_processing: rename RelayTransaction to better describe what it does":
(https://github.com/bitcoin/bitcoin/pull/33565#discussion_r2413466183)
nit: "at a later time" is implied by "schedule", unnecessary to mention, plus it may suggest to the reader that you can control the later time here (which is not the case).
```suggestion
* Schedule a transaction for broadcasting to all peers.
```
(https://github.com/bitcoin/bitcoin/pull/33565#discussion_r2413466183)
nit: "at a later time" is implied by "schedule", unnecessary to mention, plus it may suggest to the reader that you can control the later time here (which is not the case).
```suggestion
* Schedule a transaction for broadcasting to all peers.
```
💬 optout21 commented on pull request "net_processing: rename RelayTransaction to better describe what it does":
(https://github.com/bitcoin/bitcoin/pull/33565#discussion_r2413467251)
nit: comma
```suggestion
* mempool, an `INV` about it is sent to the peer.
```
(https://github.com/bitcoin/bitcoin/pull/33565#discussion_r2413467251)
nit: comma
```suggestion
* mempool, an `INV` about it is sent to the peer.
```
💬 optout21 commented on pull request "net_processing: rename RelayTransaction to better describe what it does":
(https://github.com/bitcoin/bitcoin/pull/33565#issuecomment-3380980623)
ACK 44a726133a880f818228c01b55236b3cb3eb1a67
Straightforward renaming of one method, plus doc comment change.
The change is rather small for a PR, but since it's part of a larger change (#29415), it's ok.
(https://github.com/bitcoin/bitcoin/pull/33565#issuecomment-3380980623)
ACK 44a726133a880f818228c01b55236b3cb3eb1a67
Straightforward renaming of one method, plus doc comment change.
The change is rather small for a PR, but since it's part of a larger change (#29415), it's ok.
💬 hodlinator commented on pull request "Clear out space on GHA jobs":
(https://github.com/bitcoin/bitcoin/pull/33514#discussion_r2413483771)
(Would be nice to clear this up since you switched back to this commit).
(https://github.com/bitcoin/bitcoin/pull/33514#discussion_r2413483771)
(Would be nice to clear this up since you switched back to this commit).
💬 maflcko commented on pull request "ci: Add macOS cross task for arm64-apple-darwin":
(https://github.com/bitcoin/bitcoin/pull/33549#issuecomment-3380995526)
> Having at least compilation coverage of every target we are shipping a release binary for seems like something we should have? Post switchover we are much less constrained on the CI resources front.
Thx, done. Also, agree that cross-compilation is cheap and fast.
(https://github.com/bitcoin/bitcoin/pull/33549#issuecomment-3380995526)
> Having at least compilation coverage of every target we are shipping a release binary for seems like something we should have? Post switchover we are much less constrained on the CI resources front.
Thx, done. Also, agree that cross-compilation is cheap and fast.
💬 Eunovo commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2413513497)
https://github.com/bitcoin/bitcoin/pull/26966/commits/0afb05cad29e4c30a2f9eb4295e997967129dc88
I wrote a test for recursive task submission. You can add it if you consider it to be useful.
```diff
diff --git a/src/test/threadpool_tests.cpp b/src/test/threadpool_tests.cpp
index ed0af8fa3f..4797144b78 100644
--- a/src/test/threadpool_tests.cpp
+++ b/src/test/threadpool_tests.cpp
@@ -197,6 +197,21 @@ BOOST_AUTO_TEST_CASE(threadpool_basic)
blocker.set_value();
threadPool
...
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2413513497)
https://github.com/bitcoin/bitcoin/pull/26966/commits/0afb05cad29e4c30a2f9eb4295e997967129dc88
I wrote a test for recursive task submission. You can add it if you consider it to be useful.
```diff
diff --git a/src/test/threadpool_tests.cpp b/src/test/threadpool_tests.cpp
index ed0af8fa3f..4797144b78 100644
--- a/src/test/threadpool_tests.cpp
+++ b/src/test/threadpool_tests.cpp
@@ -197,6 +197,21 @@ BOOST_AUTO_TEST_CASE(threadpool_basic)
blocker.set_value();
threadPool
...
💬 Sjors commented on pull request "Clear out space on GHA jobs":
(https://github.com/bitcoin/bitcoin/pull/33514#issuecomment-3381040718)
> Happy to revert this to only apply to the CentOS job if preferred. Seems pointless to add 10 mins runtime onto like 11 jobs where it's not needed...
While testing on with `sv2-tp` https://github.com/Sjors/sv2-tp/pull/37 I also noticed the cleanup task is quite slow.
(https://github.com/bitcoin/bitcoin/pull/33514#issuecomment-3381040718)
> Happy to revert this to only apply to the CentOS job if preferred. Seems pointless to add 10 mins runtime onto like 11 jobs where it's not needed...
While testing on with `sv2-tp` https://github.com/Sjors/sv2-tp/pull/37 I also noticed the cleanup task is quite slow.
⚠️ MRHUNG369 opened an issue: "bitcoin"
(https://github.com/bitcoin/bitcoin/issues/33572)
(https://github.com/bitcoin/bitcoin/issues/33572)