💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2396353071)
> The PR description says you need Ninja, but `build-osx.md` does not mention this.

Ninja is required to build the `qt` package in depends. It is mentioned in `depends/README.md`. However, `doc/build-osx.md` does not cover building with depends, so I don't think it is necessary to mention Ninja there.

💬 Sjors commented on pull request "Add multiprocess binaries to release build":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2396394710)
I dropped 1ea75505d18eea8e5f00f08d1e9e5b348691c7c0 again and instead opened https://github.com/Sjors/bitcoin/pull/65 to figure out how to best enable multiprocess on non-depends CI. It seems rather non-trivial.

💬 hebasto commented on pull request "qt6: Handle different signatures of `QANEF::nativeEventFilter`":
(https://github.com/bitcoin-core/gui/pull/840#issuecomment-2396400648)
After an offline discussion with @fanquake, I'm going to proceed with merging this PR.

🚀 hebasto merged a pull request: "qt6: Handle different signatures of `QANEF::nativeEventFilter`"
(https://github.com/bitcoin-core/gui/pull/840)

💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2396412336)
Rebased.

💬 josibake commented on pull request "kernel: Move block tree db open to block manager":
(https://github.com/bitcoin/bitcoin/pull/30965#discussion_r1789888213)
Sorry, I don't think my last comment on this was very clear. Lines L109-L112 are moved up from below, and `cache_sizes.block_tree_db` is set. So I think L127 should be:

```suggestion
.block_tree_db_cache_size = cache_sizes.block_tree_db,
```

unless I'm missing something?

💬 fanquake commented on issue "assumeutxo: not syncing the snapshot chainstate":
(https://github.com/bitcoin/bitcoin/issues/30971#issuecomment-2396436910)
> But did loading of the snapshot actually complete eventually

It did. I haven't had time to (re-)test anything here further. I don't think I have the full logs to share any more, but I will try and recreate them.

💬 maflcko commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1789897798)
semi-related: I wonder if Windows can be bumped to Windows 10 either way. If long EOL, and ancient versions of Windows are documented to be supported, people may use them and open up themselves to security issues. I think it could be useful to only list supported (receiving security updates) operating systems here.

Could be done here, or in a separate pull.

⚠️ fanquake opened an issue: "lint: markdown check linting build outputs"
(https://github.com/bitcoin/bitcoin/issues/31044)
Running the linter locally I see this output:
```bash
DOCKER_BUILDKIT=1 docker build -t bitcoin-linter --file "./ci/lint_imagefile" ./ && docker run --rm -v $(pwd):/bitcoin -it bitcoin-linter
<snip>
One or more markdown links are broken.

Relative links are preferred (but not required) as jumping to file works natively within Emacs.

Markdown link errors found:
[Err ] ./depends/work/build/x86_64-apple-darwin/qt/6.7.2-b49980c3ba7/qtbase/tests/auto/corelib/serialization/qxmlstream/XML-Tes
...

💬 fanquake commented on pull request "ci: set a ctest test timeout of 1200 (20 minutes)":
(https://github.com/bitcoin/bitcoin/pull/31026#discussion_r1789919608)
> I wonder if TEST_RUNNER_TIMEOUT_FACTOR * 30 can be re-used here,

Sure, updated to use this. I haven't run a s390x build for a while, but I'd assume it would fail here. Started one in any case.

💬 maflcko commented on pull request "ci: set a ctest test timeout of 1200 (20 minutes)":
(https://github.com/bitcoin/bitcoin/pull/31026#issuecomment-2396496103)
review ACK 56aad83307e46983a397236bd0959e634207f83e

No change to the new timeout value of 1200 in this force push, but making it configurable, in case someone needs that, and also in symmetry with the functional tests.

💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2396504362)
> It'd be good if [fba4df9](https://github.com/bitcoin/bitcoin/commit/fba4df9c200af0be5e2ed4ecc98cb2c0cb0b33ab) explained why macOS `12.x` is needed, because Qt 6 claims to support macOS `11.x` through to `14.x`: https://doc.qt.io/qt-6/macos.html.

Qt 6 uses `@available` directives in Objective-C++ code, which causes unresolved `__isPlatformVersionAtLeast` symbols during cross-compilation. A [patch](https://github.com/bitcoin/bitcoin/blob/dae1d48c546c2e1daadbf982420ba3d8feda2a9f/depends/patche
...

💬 maflcko commented on issue "lint: markdown check linting build outputs":
(https://github.com/bitcoin/bitcoin/issues/31044#issuecomment-2396511868)
> simultaneously claim that things are broken, but that all checks passed?

Some lint checks print output on success, so that may be confusing. I think the only relevant information for errors is the presence of `⚠️`, which should exactly correlate with a non-zero exit status. Could you check `echo $?`?

The mlc issue is tracked in https://www.github.com/becheran/mlc/pull/96

💬 TheCharlatan commented on pull request "init: Remove retry for loop":
(https://github.com/bitcoin/bitcoin/pull/30968#issuecomment-2396514254)
Going to address the left-over comments in a follow-up.

💬 TheCharlatan commented on pull request "kernel: Move block tree db open to block manager":
(https://github.com/bitcoin/bitcoin/pull/30965#issuecomment-2396520302)
Updated 63dfc6067bd7fe06e5a440c229cca030bba6d53a -> 60ade81516d42d275c1143f49f47e142d32c45fc ([blockmanDB_1](https://github.com/TheCharlatan/bitcoin/tree/blockmanDB_1) -> [blockmanDB_2](https://github.com/TheCharlatan/bitcoin/tree/blockmanDB_2), [compare](https://github.com/TheCharlatan/bitcoin/compare/blockmanDB_1..blockmanDB_2))

* Addressed @josibake's [comment](https://github.com/bitcoin/bitcoin/pull/30965#discussion_r1789888213), reusing a option value in `bitcoin-chainstate`
* Exchanged
...

💬 maflcko commented on pull request "Enhanced error messages for invalid network prefix during address parsing.":
(https://github.com/bitcoin/bitcoin/pull/27260#discussion_r1789946568)
> @maflcko I'm puzzled why the `test each commit` job didn't catch this. No test coverage? Shouldn't it use `-Werror`?

Yeah, probably, that should be enabled. Ref: https://github.com/bitcoin/bitcoin/actions/runs/11091169274/job/30814507260#step:6:493

```
/home/runner/work/bitcoin/bitcoin/src/util/chaintype.cpp:47:13: warning: enumeration value 'TESTNET4' not handled in switch [-Wswitch]
47 | switch (chain) {
| ^~~~~
1 warning generated.

📝 maflcko opened a pull request: " ci: Add missing -DWERROR=ON to test-each-commit "
(https://github.com/bitcoin/bitcoin/pull/31045)
Found by in Sjors in https://github.com/bitcoin/bitcoin/pull/27260#discussion_r1785860610 (Thanks!)

Also, includes an unrelated commit to simplify the ci scripts, by assuming nproc exists on macos as well. (Having more than one commit is also required to trigger the `test-each-commit` task)

💬 maflcko commented on pull request "Enhanced error messages for invalid network prefix during address parsing.":
(https://github.com/bitcoin/bitcoin/pull/27260#discussion_r1789957118)
Fixed in https://github.com/bitcoin/bitcoin/pull/31045

⚠️ maflcko reopened an issue: "Intermittent timeout in tsan feature_init.py"
(https://github.com/bitcoin/bitcoin/issues/30586)
This happens intermittently with something like https://cirrus-ci.com/task/6520727065591808?logs=ci#L3416

```
Remaining jobs: [feature_init.py]
...............................................................................................................................................................................................................................................................................................................................................................
...

💬 maflcko commented on issue "Intermittent timeout in tsan feature_init.py":
(https://github.com/bitcoin/bitcoin/issues/30586#issuecomment-2396552702)
https://cirrus-ci.com/task/6689351046791168?logs=ci#L4759