🤔 hebasto reviewed a pull request: "build: optimize .h generation in GenerateHeaderFrom{Raw,Json}.cmake"
(https://github.com/bitcoin/bitcoin/pull/30888#pullrequestreview-2302930242)
The minimum supported CMake version, 3.22, and the most recent version, 3.30, show similar performance on Linux as in the PR description.

💬 l0rinc commented on pull request "build: optimize .h generation in GenerateHeaderFrom{Raw,Json}.cmake":
(https://github.com/bitcoin/bitcoin/pull/30888#discussion_r1758737293)
would it make sense to change the return type to `std::span<const std::byte>` instead?

💬 l0rinc commented on pull request "build: optimize .h generation in GenerateHeaderFrom{Raw,Json}.cmake":
(https://github.com/bitcoin/bitcoin/pull/30888#issuecomment-2348787319)
Thanks a lot for checking @hebasto!

💬 maflcko commented on pull request "log: Use ConstevalFormatString":
(https://github.com/bitcoin/bitcoin/pull/30889#issuecomment-2348789890)
> Regarding the abort on CI specifically would be very useful as well in order to catch the issues earlier, not sure about the cons on test-only builds

It should be trivial to extend the check to reject `%n`, but I still don't think it matters much either way, and it would be better in a separate pull request anyway.

💬 hebasto commented on pull request "build: add `standard branch-protection` to hardening flags for aarch64-linux":
(https://github.com/bitcoin/bitcoin/pull/30433#issuecomment-2348791388)
My Guix build:
```
aarch64
86d20fcaf2331084035fba29305dbcd22665a7a0f910ffbe1667541a338129ed guix-build-001b1cf01045/output/aarch64-linux-gnu/SHA256SUMS.part
06ec1b29ed2ac24733fa24083460298e4c9a2cbba8845164f849aebb50c94e91 guix-build-001b1cf01045/output/aarch64-linux-gnu/bitcoin-001b1cf01045-aarch64-linux-gnu-debug.tar.gz
16b21d25dab9222dc69774e0e788c10bac4f15783c4ccb46c11e0608d132021d guix-build-001b1cf01045/output/aarch64-linux-gnu/bitcoin-001b1cf01045-aarch64-linux-gnu.tar.gz
3cdd10a2
...

💬 fanquake commented on pull request "guix: (explicitly) build Linux GCC with `--enable-cet`":
(https://github.com/bitcoin/bitcoin/pull/30438#issuecomment-2348796541)
Guix Build:
```bash
94142f4399e6b57ae5d95364685cf545a20e1974eb3e6061e62b77af57e59a6b guix-build-89bf11b80725/output/aarch64-linux-gnu/SHA256SUMS.part
1bbbbc9c2818eb6c87a7d7164afc61eee299d8417d986832eb0d503484994f4b guix-build-89bf11b80725/output/aarch64-linux-gnu/bitcoin-89bf11b80725-aarch64-linux-gnu-debug.tar.gz
0601f57454694181473f03bc8ff6c6f23f6ff50f38190a9ff42bd351ff739460 guix-build-89bf11b80725/output/aarch64-linux-gnu/bitcoin-89bf11b80725-aarch64-linux-gnu.tar.gz
10ce23a32f6a21e4
...

💬 maflcko commented on pull request "build: optimize .h generation in GenerateHeaderFrom{Raw,Json}.cmake":
(https://github.com/bitcoin/bitcoin/pull/30888#issuecomment-2348809433)
review ACK 2a581144f28bad44de40122864f2f7b9fc5000de 👒

<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 comment: review ACK 2a581144f28b
...

💬 maflcko commented on pull request "build: optimize .h generation in GenerateHeaderFrom{Raw,Json}.cmake":
(https://github.com/bitcoin/bitcoin/pull/30888#issuecomment-2348832162)
(Also tested on a fresh Ubuntu 24.04 VM and saw a 30x speedup, which is useful, because this part isn't cached by ccache between builds)

💬 maflcko commented on pull request "test: re-bucket p2p_node_network_limited":
(https://github.com/bitcoin/bitcoin/pull/30879#issuecomment-2348843336)
> Locally these tests take about 20 seconds and appear in the correct bucket.

I think the sorting isn't really based on the "correct bucket", but rather the inverse overall duration, the buckets are just a guideline/reminder. That is, as long as the longest running test is started first, everything is fine.


Did you measure if this actually makes a meaningful difference?

👍 hebasto approved a pull request: "build: add `standard branch-protection` to hardening flags for aarch64-linux"
(https://github.com/bitcoin/bitcoin/pull/30433#pullrequestreview-2303002464)
ACK 001b1cf010453adbb1316a6fa8911398953afe61.

💬 maflcko commented on pull request "test: re-bucket p2p_node_network_limited":
(https://github.com/bitcoin/bitcoin/pull/30879#issuecomment-2348847390)
Also, could rebase for a fresh CI? At least for me the CI timeouts went away earlier this night when trying to reproduce, but I don't know if they are really gone, and whether the issue was inside this repo, or on the CI runners, or on the CI backend software, or on the Cirrus/GHA integration.

💬 maflcko commented on pull request "doc: unit test runner help fixup":
(https://github.com/bitcoin/bitcoin/pull/30890#issuecomment-2348854269)
review ACK 0024d2c6ea0fafe9b9949af4bbcd0c583e580746

💬 kevkevinpal commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1758844489)
thanks for the feedback!

I updated this to
```
cd <BITCOIN_SOURCE_DIRECTORY>
make -C depends NO_QT=1 MULTIPROCESS=1
cmake -B build --toolchain=depends/x86_64-pc-linux-gnu/toolchain.cmake
cmake --build build
build/src/bitcoind -regtest -printtoconsole -debug=ipc
BITCOIND=$(pwd)/build/src/bitcoind build/test/functional/test_runner.py
```

Which does start running the tests please let me know if this looks ok

👍 hebasto approved a pull request: "build: optimize .h generation in GenerateHeaderFrom{Raw,Json}.cmake"
(https://github.com/bitcoin/bitcoin/pull/30888#pullrequestreview-2303103922)
ACK 2a581144f28bad44de40122864f2f7b9fc5000de.

The new scripts introduce trailing spaces in the generated header, but these are not essential.

💬 kevkevinpal commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1758847279)
thanks! I removed that in [5a85022](https://github.com/bitcoin/bitcoin/pull/30875/commits/5a85022cc6c315e9c756bbbb63a7ed992e7f23d3)

💬 kevkevinpal commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1758851197)
thanks! I updated this to `cmake build` instead of `configure script` in [5a85022](https://github.com/bitcoin/bitcoin/pull/30875/commits/5a85022cc6c315e9c756bbbb63a7ed992e7f23d3)

let me know if you'd prefer different wording!

💬 kevkevinpal commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1758851652)
thanks! I updated this to `cmake build` instead of `configure script` in [5a85022](https://github.com/bitcoin/bitcoin/pull/30875/commits/5a85022cc6c315e9c756bbbb63a7ed992e7f23d3)

let me know if you'd prefer different wording!

💬 laanwj commented on pull request "streams: cache file position within AutoFile":
(https://github.com/bitcoin/bitcoin/pull/30884#issuecomment-2348948414)
> I made a significant change here, making m_position an std::optional<int64_t>, which is std::nullopt when the position is unknown (due to ftell failing, and no successful fseek afterwards).

i first couldn't think of a valid scenario where ftell would fail but we'd want to continue, but i guess this maks sense when reading a FIFO or device, when there isn't really an offset.

💬 kevkevinpal commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#issuecomment-2348968829)
> It would be nice to fix all remaining instances in one go. See also [#30664 (comment)](https://github.com/bitcoin/bitcoin/pull/30664#issuecomment-2320892444) and [#30664 (comment)](https://github.com/bitcoin/bitcoin/pull/30664#issuecomment-2321610186)

There are still some changes I need to address from these comments

⚠️ fanquake opened an issue: "translations: `28.x` update pulled in random strings?"
(https://github.com/bitcoin/bitcoin/issues/30897)
Transations were updated for 28.x in #30715.

Looks like that update in things that are not translations. i.e:

https://github.com/bitcoin/bitcoin/blob/e43ce250c6fb65cc0c8903296c8ab228539d2204/src/qt/locale/bitcoin_gl_ES.ts#L6

?