💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1644123429)
Updated.

It serves the same purpose as `NO_BRANCH` on Cirrus, expect that I couldn't find a way to make that behaviour opt-in on Github CI.

💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#issuecomment-2175605000)
Rebased after #30193, which removes "noble" as a machine type. I previously already removed `NO_NOBLE`, see https://github.com/bitcoin/bitcoin/pull/29274#issuecomment-2134764402. The rebase merely impacts the documented list of persistent workers, which this PR moves.

Addressed review comments.

💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1644128922)
I moved 063ea4e0da9e1c06cc0f4aec97a03abac66ae59f to be right after 8259a13954878fd6188bb218c3b7320385516596 and expanded the commit message to explain why they don't use the same mechanism.

📝 m3dwards opened a pull request: "ci: remove unused bcc variable from workflow"
(https://github.com/bitcoin/bitcoin/pull/30299)
Fixes https://github.com/bitcoin/bitcoin/pull/30193#discussion_r1636804002

💬 m3dwards commented on pull request "ci: move ASan job to GitHub Actions from Cirrus CI":
(https://github.com/bitcoin/bitcoin/pull/30193#discussion_r1644131995)
https://github.com/bitcoin/bitcoin/pull/30299

💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1644133891)
Improved the comment, see below.

🚀 fanquake merged a pull request: "Revert "contrib: macdeploy: monkey-patch gen-sdk to be deterministic""
(https://github.com/bitcoin/bitcoin/pull/30282)

💬 maflcko commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1644151691)
Well, what problem is this trying to solve, given that it introduces new problems?

💬 maflcko commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1644154081)
An alternative would be for forks to just supply an x86 machine. The CI system should detect this itself and invoke qemu-aarch64

💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1644156898)
Is that new behavior? Let me test that again...

💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1644159505)
I guess I never tested this, because the documentation just says:

```
# - arm64: For an aarch64 machine, recommended to have 2 CPUs and 8 GB of memory.
```

If the performance difference isn't too terrible, then I can just drop the commit and mention this in the documentation.

💬 maflcko commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1644160436)
You'll have to run something like `podman run --rm --privileged docker.io/multiarch/qemu-user-static --reset -p yes` and then register the type as `arm64`, but I'd say it should work.

💬 paplorinc commented on pull request "test: Validate oversized transactions or without inputs":
(https://github.com/bitcoin/bitcoin/pull/29862#discussion_r1644161127)
We only have 3 matching lines, so extracting them would result in slightly more complicated code.
If the reviewers insist, I don't mind doing it, see https://github.com/bitcoin/bitcoin/pull/29862#discussion_r1643367751

💬 stickies-v commented on pull request "fix: typo in benchmark documentation":
(https://github.com/bitcoin/bitcoin/pull/30296#issuecomment-2175664755)
NACK, one-off typo fixes are not a good use of anyone's time

💬 maflcko commented on pull request "ci: remove unused bcc variable from workflow":
(https://github.com/bitcoin/bitcoin/pull/30299#issuecomment-2175665297)
lgtm ACK 518b06c4b889d71a3fdd61f8fe38d519ea5e4a1b

✅ glozow closed a pull request: "fix: typo in benchmark documentation"
(https://github.com/bitcoin/bitcoin/pull/30296)

💬 glozow commented on pull request "fix: typo in benchmark documentation":
(https://github.com/bitcoin/bitcoin/pull/30296#issuecomment-2175673833)
Thanks for your interest in contributing! However, as we have hundreds of pull requests, I am closing this to focus review on the others. (See [contributing guidelines on refactoring](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#refactoring)).

💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1644172953)
`qemu-aarch64` adds quite a few packages to Ubuntu, so I'd probably want to keep the `NO_ARM` option around, but document this alternative.

💬 fanquake commented on pull request "build: add `-Wundef`":
(https://github.com/bitcoin/bitcoin/pull/29876#issuecomment-2175689878)
> Maybe https://github.com/bitcoin/bitcoin/commit/a491cea18ee63e514359a5ba699e8cc159664efc could be cherry-picked here to fix the multiprocess failure https://github.com/bitcoin/bitcoin/pull/29876#issuecomment-2157739910?

That very-much looks like the correct fix.

🚀 fanquake merged a pull request: "ci: remove unused bcc variable from workflow"
(https://github.com/bitcoin/bitcoin/pull/30299)