Bitcoin Core Github
44 subscribers
119K links
Download Telegram
💬 maflcko commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2290063942)
style nit in 5d652ca1ef996cbe7ad9e94a4c6d29b11a7f35e9: Not sure about the distinction, but `PREVIOUS_RELEASES_DIR` is also a CI dir (config). See the output in https://github.com/bitcoin/bitcoin/actions/runs/16960628990/job/48072349653?pr=32989#step:9:114:

```
+ ./ci/test/02_run_container.py
Export only allowed settings:
+ bash -c 'grep export ./ci/test/00_setup_env*.sh'
+ cat /tmp/env-admin-ci_native_previous_releases
CCACHE_TEMPDIR=/tmp/.ccache-temp
CCACHE_MAXSIZE=500M
...
PREVIOUS
...
💬 Sjors commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3209279969)
🎉 thanks for all the reviews!

I'll stay on top of followup PR's over the next few weeks.
💬 Sjors commented on pull request "multiprocess: Don't require bitcoin -m argument when IPC options are used":
(https://github.com/bitcoin/bitcoin/pull/33229#issuecomment-3209295373)
Concept ACK

This also makes it easier to document instructions for miners, they just need to drop the "d" from `bitcoind` and then `-ipcbind` behaves like any other setting.

The help text should explain that `-m` is assumed if any IPC arguments are passed.
💬 Sjors commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#issuecomment-3209298414)
@sipa let me know if you don't have to time to rebase, in which case I'll open a fresh PR next week.
💬 maflcko commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2290101120)
Also, it should be fine to re-apply once the CI intermittently fails due to this. So this pull looks good as-is and no change is needed here.

Unrelatedly, if we want to keep/use the CI setting in the future, it must be documented as a hidden option:

https://github.com/bitcoin/bitcoin/blob/7d9789401be4c91a9eb3c1112564a6524bdc4f70/ci/test/02_run_container.py#L29-L34
💬 maflcko commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3209306539)
> > [c18b093](https://github.com/bitcoin/bitcoin/commit/c18b0939da00d5851e6adcedbba5100ad791b486) also seems fine to keep, but can be cherry-picked in a follow-up.
>
> Despite this gaining quite some decent-enough runtime performance, I'd prefer to add this in a followup after having gotten TSAN failures twice in this PR.
>
> @fanquake also noted that he saw some issues when using `(2 * nproc)` in local runs, so I think safer to leave for now.

Ah, I see. I guess "some issues" refers to
...
💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#issuecomment-3209327431)
Rebased, and addressed some of the feedback above.

Note that the last commit (which enables the tests in CI) may need work after moving to be on top of #31802, so that it matches the CI configurations which have IPC enabled.
💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2290124906)
Done.
💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2290125154)
Done.
💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2290125464)
Done.
💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2290126866)
I opted to use system libcapnp in the description here because it's more likely to work. I've added a comment about it being optional now.
💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2290127079)
Done.
💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2290127497)
Done.
💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2290127873)
Leaving this for a follow-up.
💬 maflcko commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3209332133)
I've added the 30 milestone, because there seems to be conceptual agreement to do this. This is not an end-user feature, so the feature freeze does not apply, but it would be good to get some more review on this. One of the goals in the pull description is `so that "anyone" familiar with GitHub-style CI systems can work on it`. It would be nice if one or two of the GitHub-style CI persons could review this :sweat_smile:
💬 maflcko commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2290136108)
(resolving thread for now)
💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2290140668)
Done.
🤔 maflcko reviewed a pull request: "ci: Migrate CI to hosted Cirrus Runners"
(https://github.com/bitcoin/bitcoin/pull/32989#pullrequestreview-3139408721)
(just nits)
💬 maflcko commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2290149761)
can be resolved?
💬 maflcko commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2290163669)
Can be resolved?