💬 theuni commented on pull request "build: speed up by flattening the dependency graph":
(https://github.com/bitcoin/bitcoin/pull/30911#issuecomment-2355883704)
> Concept ACK.
>
> [a39a915](https://github.com/bitcoin/bitcoin/commit/a39a915c226775396f6505efe77a4aba4d0ae3ad) Did you consider setting `CMAKE_ADD_CUSTOM_COMMAND_DEPENDS_EXPLICIT_ONLY` globally?
>
Yes, but I didn't feel as comfortable setting this one globally due to our (for ex) phony deploy targets. I figured we may make some changes in the future that could introduce hard-to-track-down bugs. It's clear to see that the header generation is self-contained, and afaik that's the only plac
...

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

I didn't review the code in too much detail, but I like hebasto also saw a good speedup on Ubuntu 23.10 x86_64 from 14 seconds down to half a second for block413567.raw with `cmake` 3.30:

```log
will@ubuntu in ~/src/bitcoin on  master [$?⇕] via △ v3.30.3 : 🐍 (bitcoin) took 12s
$ rm -i -Rf build

will@ubuntu in ~/src/bitcoin on  master [$?⇕] via △ v3.30.3 : 🐍 (bitcoin)
$ time cmake -DRAW_SOURCE_PATH=src/bench/data/block413567.raw -DHEAD
...

💬 TheCharlatan commented on pull request "validation: fix m_best_header tracking and BLOCK_FAILED_CHILD assignment":
(https://github.com/bitcoin/bitcoin/pull/30666#discussion_r1763300138)
Ah right, I don't think this a blocker for this PR anyway, just something that came to my mind.

💬 fjahr commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#issuecomment-2355903145)
> Should I modify this PR now to be on top of yours, including only the second test? Or would it be better to wait until your PR gets merged and then modify mine?

I would recommend you wait a bit to see what the feedback on that PR is.

🚀 fanquake merged a pull request: "ci: Use macos-14 GHA image (x86_64-apple-darwin22.6.0 -> arm64-apple-darwin23.6.0)"
(https://github.com/bitcoin/bitcoin/pull/30913)

🚀 fanquake merged a pull request: "Remove Autotools packages from CI (and depends doc)"
(https://github.com/bitcoin/bitcoin/pull/30902)

🚀 fanquake merged a pull request: "build: optimize .h generation in GenerateHeaderFrom{Raw,Json}.cmake"
(https://github.com/bitcoin/bitcoin/pull/30888)

🚀 fanquake merged a pull request: "doc: update NeedsRedownload() and nStatus comment"
(https://github.com/bitcoin/bitcoin/pull/29624)

👍 hodlinator approved a pull request: "log: Use ConstevalFormatString"
(https://github.com/bitcoin/bitcoin/pull/30889#pullrequestreview-2309847632)
ACK fabd78e2e30e557e04739e29c9c221ad47245df1

Concept: Great to have compile time format string error checking on the common logging functions!

Tested changing this in *logging.h*:
```diff
- log_msg = tfm::format(fmt, params...);
+ log_msg = tfm::format("%s %s\n", 1.1f);
```
and ran with expected error output.

Agree with ryanofsky that it would be good to document rough compile time impact if any.

💬 hodlinator commented on pull request "log: Use ConstevalFormatString":
(https://github.com/bitcoin/bitcoin/pull/30889#discussion_r1763327444)
nit: Line grows from 209 to 253 chars, please consider inserting line break(s).
```suggestion
inline void LogPrintFormatInternal(std::string_view logging_function, std::string_view source_file, const int source_line,
const BCLog::LogFlags flag, const BCLog::Level level, util::ConstevalFormatString<sizeof...(Params)> fmt, const Params&... params)
```

💬 hodlinator commented on pull request "log: Use ConstevalFormatString":
(https://github.com/bitcoin/bitcoin/pull/30889#discussion_r1763307360)
nit: The `Params` name was already used in *scriptpubkeyman.h* and *wallet.h* before the PR but `Args` is used in *util/string.h*, *index/base.h/cpp*, *netbase.cpp* and used to be used here. Why switch and touch 2 additional lines?

The old way feels more consistent with the C "varargs". But maybe there's a convincing argument for "Params".

Did the change locally and it compiles without causing any non-obvious issue.

👋 hebasto's pull request is ready for review: "ci: Use `ninja` to build in macOS native CI job"
(https://github.com/bitcoin/bitcoin/pull/30915)

💬 hebasto commented on pull request "ci: Use `ninja` to build in macOS native CI job":
(https://github.com/bitcoin/bitcoin/pull/30915#issuecomment-2356083087)
cc @maflcko

💬 hebasto commented on pull request "build: drop obj/ subdirectory for generated build.h":
(https://github.com/bitcoin/bitcoin/pull/30856#issuecomment-2356089545)
@theStack

Could rebase once more please to refresh the CI?

💬 maflcko commented on issue "Increasing self-hosted runner raw performance":
(https://github.com/bitcoin/bitcoin/issues/30852#issuecomment-2356103199)
> Is this so that re-runs (of the same PR) always hit the cache? If that's it I'm not _totally_ sure it's worth it. My intuition says that if we had multiple servers, even if a re-run took place on a different machine, we'd likely get a _pretty decent_ cache hit from another run or a previous master build

Right. It is unclear whether this is worth it. I think when it comes to builds on master, the speed doesn't matter at all, because no one is waiting for the result to appear immediately. So
...

💬 maflcko commented on issue "Increasing self-hosted runner raw performance":
(https://github.com/bitcoin/bitcoin/issues/30852#issuecomment-2356181083)
> CPU-bound (and not IO-bound)

Actually, the long-running fuzz test may be IO-bound so using a ramdisk could speed them up. So this would be another idea to look at.

💬 theStack commented on pull request "build: drop obj/ subdirectory for generated build.h":
(https://github.com/bitcoin/bitcoin/pull/30856#issuecomment-2356184027)
> @theStack
>
> Could rebase once more please to refresh the CI?

Yes, done.

👍 hebasto approved a pull request: "[28.x] Further backports and rc2"
(https://github.com/bitcoin/bitcoin/pull/30827#pullrequestreview-2310065055)
ACK 06a7df70df30879e0b691d1a252636f703b8cdfb, I've backported the listed PRS locally. The only merge conflict I faced was in https://github.com/bitcoin/bitcoin/pull/30807. It was trivial to resolve.

💬 maflcko commented on pull request "build: speed up by flattening the dependency graph":
(https://github.com/bitcoin/bitcoin/pull/30911#issuecomment-2356197180)
> It's clear to see that the header generation is self-contained, and afaik that's the only place that would matter for build performance.

Could rebase on current master, now that the header generation is 30x faster, after 2a0949f0977db2dbb7ac452e8d58d413b9526756 ? This should also change the benchmark in the pull description.

💬 maflcko commented on pull request "ci: Use `ninja` to build in macOS native CI job":
(https://github.com/bitcoin/bitcoin/pull/30915#issuecomment-2356214347)
review ACK d01b85bfecbcf16ea16f90e2ade7537bf582269f

Can't hurt to test this in one task