💬 maflcko commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2212606651)
nit in 2d0ac71899eadcbff8252b1704d2affb73f0160d: Should be named job-id, not the name?

💬 maflcko commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2212658179)
nit in https://github.com/bitcoin/bitcoin/commit/2d0ac71899eadcbff8252b1704d2affb73f0160d: why hardcode `master`. This makes it impossible to cache in pull requests, if this is ever done in the future. Also, only `master` caches are created, so this doesn't guard against anything.

Also, the others don't have it.

💬 maflcko commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2212642449)
nit in https://github.com/bitcoin/bitcoin/commit/2d0ac71899eadcbff8252b1704d2affb73f0160d: Comment seems wrong? It is a cache for each job-id + ref_name + run_id. Could just remove the comment?

💬 maflcko commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2212693443)
same commit: Unused?

💬 maflcko commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2212685421)
same commit: This seems broken? A job-id with NO_QT=1 populating this would corrupt the cache for other job-id's that build qt?

💬 maflcko commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2212715537)
nit in b1a4adc814369aff21b8c12775270cfe648db486: Could add a comment that this is only used to extract CONTAINER_NAME.

💬 maflcko commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2212722450)
9002faff0d7518ebafe05d8d43d6558d238c2659: adjust for https://quay.io/repository/bitcoincore/bitcoin-ci ?

💬 fanquake commented on pull request "ci: Use APT_LLVM_V in msan task":
(https://github.com/bitcoin/bitcoin/pull/32999#discussion_r2212750361)
`.8` while touching?

💬 fanquake commented on pull request "ci: switch MSAN to use prebuilt Clang binaries":
(https://github.com/bitcoin/bitcoin/pull/31850#issuecomment-3083228052)
PIcked up in #32999.

💬 fanquake commented on pull request "ci: Use APT_LLVM_V in msan task":
(https://github.com/bitcoin/bitcoin/pull/32999#discussion_r2212772055)
Actually, I'm wondering why these can't both be 20.1.8? If the checkout needs to match the apt version, then its going to break whenever the apt is updated but this isn't bumped.

💬 maflcko commented on pull request "ci: Use APT_LLVM_V in msan task":
(https://github.com/bitcoin/bitcoin/pull/32999#discussion_r2212817198)
There shouldn't be any breaking changes in minor versions, so anything is probably fine here. However, if msan fuzz wants to use a different major version, it seems better to keep it separate?

Maybe a follow-up can investigate if msan fuzz can bootstrap `compiler-rt` (libfuzzer) directly, and remove all of this code here.

however, my guess is that you still need to compile at least libcxx(abi) in this this if-branch.

👋 maflcko's pull request is ready for review: "ci: Use APT_LLVM_V in msan task"
(https://github.com/bitcoin/bitcoin/pull/32999)

💬 fjahr commented on pull request "index: Deduplicate HashKey / HeightKey handling":
(https://github.com/bitcoin/bitcoin/pull/32997#discussion_r2212838453)
Yes, the return false would prevent potentially writing false stats because the member variables are not up-to-date with what the prev block should be. I don't remember what the scenarios were but I think there was also fear of some edge cases with reorgs but from today's perspective I am not sure anymore how that would happen. Maybe the refactorings in the index have made this scenario impossible but I'll have to think about it some more.

💬 fjahr commented on pull request "index: Deduplicate HashKey / HeightKey handling":
(https://github.com/bitcoin/bitcoin/pull/32997#issuecomment-3083380164)
Concept ACK on the deduplication parts

📝 maflcko opened a pull request: "ci: Run unit test parallel with functional tests"
(https://github.com/bitcoin/bitcoin/pull/33000)
Fixes https://github.com/bitcoin/bitcoin/issues/32770

📝 maflcko opened a pull request: "test: Do not pass tests on unhandled exceptions"
(https://github.com/bitcoin/bitcoin/pull/33001)
Currently the functional tests are problematic, because they pass, even if they encounter an unhanded exception.

Fix this by handling all exceptions: Catch `BaseException` as fallback and mark it as failure.

Can be tested via:

```diff
diff --git a/test/functional/wallet_disable.py b/test/functional/wallet_disable.py
index da6e5d408f..ecc41fb041 100755
--- a/test/functional/wallet_disable.py
+++ b/test/functional/wallet_disable.py
@@ -19,6 +19,7 @@ class DisableWalletTest (BitcoinTe
...

👍 hebasto approved a pull request: "[29.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/32863#pullrequestreview-3028987019)
ACK 5300295083f2e199c22a7ad55e62a8dc7549a76e, I've backported all listed PRs locally (had 3 conflict to resolve), and got zero diff with this PR.

The last commit also addresses https://github.com/bitcoin/bitcoin/pull/32810#pullrequestreview-2982682093.

💬 theStack commented on pull request "test: refactor: overhaul block hash determination for `CBlock{,Header}` objects":
(https://github.com/bitcoin/bitcoin/pull/32868#issuecomment-3083682118)
Thanks for the reviews! Rebased on master as suggested (to fix the silent merge conflict related to #32968).

💬 hebasto commented on pull request "doc: add alpine build instructions":
(https://github.com/bitcoin/bitcoin/pull/32559#discussion_r2213081726)
On Alpine Linux v3.22:
```
$ apk info -e xz | wc -l
0
$ gmake -C depends freetype_extracted # Succeeds, just like for any other package with a .tar.xz tarball.
```

🤔 hebasto reviewed a pull request: "doc: add alpine build instructions"
(https://github.com/bitcoin/bitcoin/pull/32559#pullrequestreview-3029112960)
ACK 4f502baf8f649e30d9495760b54080c272882213.