💬 TheCharlatan commented on pull request "build: use `-no_exported_symbols` on macOS":
(https://github.com/bitcoin/bitcoin/pull/29072#issuecomment-2189380466)
Guix build (aarch64):
```
544f33855573c49ff642be39c54603c9afbf64076e6575cb19c5a64fea1522e4 guix-build-81d4dc8e8739/output/aarch64-linux-gnu/SHA256SUMS.part
a8a3c29fe1a2a21f677cfbd8fadb1d65f5901b6f133531f6fce736f1e1e163e5 guix-build-81d4dc8e8739/output/aarch64-linux-gnu/bitcoin-81d4dc8e8739-aarch64-linux-gnu-debug.tar.gz
d059d19cb255b3f555eb2be6b89e0d000a9476278aa783efdc1ba0c3faac6236 guix-build-81d4dc8e8739/output/aarch64-linux-gnu/bitcoin-81d4dc8e8739-aarch64-linux-gnu.tar.gz
1ab4dac0ea
...

💬 TheCharlatan commented on pull request "depends: build libevent with CMake":
(https://github.com/bitcoin/bitcoin/pull/29835#discussion_r1653136723)
That's fine too, it is not used anyway.

💬 stickies-v commented on pull request "contrib: Fixup verify-binaries OS platform parsing":
(https://github.com/bitcoin/bitcoin/pull/30147#discussion_r1653151031)
I'd prefer limiting it to `parse_version_string` and will quickly re-ack if you force push

💬 jonatack commented on pull request "Docs improvements":
(https://github.com/bitcoin/bitcoin/pull/30337#issuecomment-2189405730)
Hi @nnsW3, several of these proposed changes are incorrect. I suggest reading through https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md and https://jonatack.github.io/articles/how-to-contribute-pull-requests-to-bitcoin-core for an idea of contributions that would be useful to this project and valued. Cheers.

💬 BenWestgate commented on pull request "contrib: Fixup verify-binaries OS platform parsing":
(https://github.com/bitcoin/bitcoin/pull/30147#discussion_r1653163359)
Done in https://github.com/bitcoin/bitcoin/compare/4a85b154d0874f7bfa848ae8d7ab341eea754607..3ab25201909bece9066ac6191670bcee09791d54

👍 stickies-v approved a pull request: "contrib: Fixup verify-binaries OS platform parsing"
(https://github.com/bitcoin/bitcoin/pull/30147#pullrequestreview-2139226506)
re-ACK 3ab25201909bece9066ac6191670bcee09791d54

💬 ryanofsky commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#issuecomment-2189439362)
> Maintainer note: `SKIP_BRANCH_PUSH=true` must be set in Cirrus for `bitcoin-core/gui` before merging this. See `https://cirrus-ci.com/github/bitcoin-core/gui` -> Settings.

I just now went to that URL and set this. There were not any other environment variables set.

I wonder if there is a good place to document this configuration, but probably it's not too important as the main effect is just to avoiding wastefully running CI twice on pushes to master (at least according to the original P
...

💬 Sjors commented on pull request "util/system: Close non-std fds when execing slave processes":
(https://github.com/bitcoin/bitcoin/pull/22417#issuecomment-2189525056)
> What is the status of this now that Boost Process is removed

For historical reference: this patch is no longer relevant, since #28981 replaced boost-process with cpp-subprocess.

@luke-jr does `cpp-subprocess` do anything similar that needs addressing? https://github.com/bitcoin/bitcoin/blob/master/src/util/subprocess.h

💬 Sjors commented on pull request "Drop -dbcache limit":
(https://github.com/bitcoin/bitcoin/pull/28358#issuecomment-2189534918)
@jlopp thanks for the testing and your article.

I assume "to reach a given block height with a 28 GB dbcache size" you did _not_ run with this patch? I'd be curious to see the result.

And if you feel like battle testing this further, you could make a custom signet, produce a similar UTXO set to mainnet (maybe #24952 can help), increase it to a few hundred gigabyte and then see if and where there's a value `-dbcache` where performance peaks and goes down.

📝 theuni opened a pull request: "RFC: Instanced logs"
(https://github.com/bitcoin/bitcoin/pull/30338)
PR'ing this early as an RFC because it's a lot of work that will require constant rebasing. This is the first logical step and is mostly mechanical.

This PR adds an "instance" param to each log function and callsite. As is, there should be no functional change here.

The intention is to allow for a specific instance to be passed in each time we use the logger, rather than the implicit global as we have it now. Eventually (one subsystem/chunk at a time), this will allow us to instantiate a p
...

💬 theuni commented on pull request "RFC: Instanced logs":
(https://github.com/bitcoin/bitcoin/pull/30338#issuecomment-2189606984)
Ping @TheCharlatan @ajtowns @maflcko for Concept (N)ACKs

💬 ryanofsky commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1653288456)
re: https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1652884524

I don't have a strong opinion, but I do think the NO_ARM commit 859e1c558fd9c8604fda361e465aa4eb4a676823 is a nice change because I don't think it fixes a purely cosmetic issue. I think it removes confusing behavior of opening a PR and seeing a job hang forever. The actual code change is very simple, just adding `skip: $NO_ARM == "true"` and I think the documentation change makes the whole "A self-hosted machine(s) can b
...

💬 ryanofsky commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1653195226)
In commit "ci: forks can opt-out of CI branch push (Cirrus only)" (46be986c2cff004a081e89433715118637bb4eb3)

Maybe add `... as a custom env variable in Cirrus repository settings, accessible from https://cirrus-ci.com/github/my-organization/my-repository` because I don't think it is obvious where this can be set.

👍 ryanofsky approved a pull request: "Support self-hosted Cirrus workers on forks"
(https://github.com/bitcoin/bitcoin/pull/29274#pullrequestreview-2139263214)
Code review ACK 0882babb102108a4351714cc3749fceba1ecbb35.

---

I think the current PR title / description are a little hard to understand. Maybe consider editing it to something like the following:

- Fix issues with CI on forks

I find myself making pull requests against my fork (mostly on top of [#28983](https://github.com/bitcoin/bitcoin/pull/28983), or asking others to do so. Currently only the Github actions are run on forks, because we use self-hosted runners for the Cirrus task
...

💬 jonatack commented on pull request "doc: clarify loadwallet path loading for wallets":
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1653307211)
Didn't verify, but could all of this be replaced by a call to `fs::path GetDefaultDataDir()`?

👍 itornaza approved a pull request: "Mining interface followups"
(https://github.com/bitcoin/bitcoin/pull/30335#pullrequestreview-2139445312)
tested ACK 22fe97a5cc612f438ec6280757487934501bf527

Manually validated all instances of `testBlockValidity` in the entire code base are having their parameter list updated with `state` as their last output parameter using good old `git grep -n testBlockValidity`. Now, the interface can be used with the `libmultiprocess` code generator!

As for the standard testing, I got the pr from upstream, built and run all unit and functional tests including the entire extended test harness and all test
...

💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1653310379)
I find this equally confusing, but was a bit reluctant to add a manual for Cirrus. Anyway, added.

💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#issuecomment-2189651581)
@ryanofsky thanks for the PR description rewrite!

💬 jonatack commented on pull request "doc: clarify loadwallet path loading for wallets":
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1653311450)
(It may also help with the Win CI errors)

💬 ryanofsky commented on pull request "RFC: Instanced logs":
(https://github.com/bitcoin/bitcoin/pull/30338#issuecomment-2189661783)
@theuni I made a similar change in #29256. That approach might be less invasive because it makes the instance argument optional. So kernel code can be changed to log to a configurable instance while other code uses a global instance.