💬 maflcko commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2290007939)
Ah, ok. I am not familiar with the GitHub Actions Docker Caching Buildx setup interactions with the docker containers that are run later. But given that this is documented as a buildx option (https://docs.docker.com/reference/cli/docker/buildx/create/#driver-opt) it seems plausible that it may not apply to the docker container.

💬 maflcko commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2290021504)
When running locally, the image, the container and the volumes are named after it:

```
ci/test/02_run_container.sh: docker volume create "${CONTAINER_NAME}_ccache" || true
ci/test/02_run_container.sh: docker volume create "${CONTAINER_NAME}_depends" || true
ci/test/02_run_container.sh: docker volume create "${CONTAINER_NAME}_depends_sources" || true
ci/test/02_run_container.sh: docker volume create "${CONTAINER_NAME}_previous_releases" || true
```

So I think it is fine to use the
...

🤔 maflcko reviewed a pull request: "ci: Migrate CI to hosted Cirrus Runners"
(https://github.com/bitcoin/bitcoin/pull/32989#pullrequestreview-3139298640)
only change is some env var shuffling

re-ACK c2d18db38de6d5f71d48437422b93b8a962d3935 🛄

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted commen
...

💬 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.