🤔 fjahr reviewed a pull request: "contrib: fix `test_deterministic_coverage.sh` script for out-of-tree builds"
(https://github.com/bitcoin/bitcoin/pull/31588#pullrequestreview-2602169248)
Hm, I tried to test this but wasn't successful so far. Aside from the fix I suggest in my inline comment I still get an error that gcov is not able to find the working dir. That seems to be because the coverage files include the build paths and not the source paths. After a little digging I tried to address this by compiling with `-DAPPEND_CXXFLAGS="-fprofile-prefix-map=${BUILDDIR}/src=${TOPDIR}/src"` but that didn't work and it doesn't seem like a good enough fix anyway.

Does this just work
...

💬 purpleKarrot commented on pull request "doc: swap CPPFLAGS for APPEND_CPPFLAGS":
(https://github.com/bitcoin/bitcoin/pull/31819#issuecomment-2643507434)
Where can I read about the decision to chose `CPPFLAGS` and `APPEND_CPPFLAGS`? I think those names are unfortunate and may cause confusion with cmake's default handling of the `CXXFLAGS` environment variable.

📝 dedyshkaPexto opened a pull request: "fixing-link-Update dependencies.md"
(https://github.com/bitcoin/bitcoin/pull/31822)
old link :
https://www.oracle.com/technetwork/database/database-technologies/berkeleydb/downloads/index.html

new link :
https://www.oracle.com/database/technologies/related/berkeleydb-downloads.html

hope it was helpful

👍 stickies-v approved a pull request: "build: consistently use `CLIENT_NAME` in libbitcoinkernel.pc.in"
(https://github.com/bitcoin/bitcoin/pull/31820#pullrequestreview-2602342964)
ACK f5b9a2f68c95182b139e8a3447b4b5888ecb4f9f

💬 ryanofsky commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#discussion_r1946890236)
>

👍 theuni approved a pull request: "build: consistently use `CLIENT_NAME` in libbitcoinkernel.pc.in"
(https://github.com/bitcoin/bitcoin/pull/31820#pullrequestreview-2602359582)
utACK f5b9a2f68c95182b139e8a3447b4b5888ecb4f9f

👍 theuni approved a pull request: "guix: remove test-security/symbol-check scripts"
(https://github.com/bitcoin/bitcoin/pull/31818#pullrequestreview-2602389053)
utACK 485de09d9edff6abd6eef9e54e70b21bf6ceb9e4.

I opened #31715 to "fix" some issues with these, but I was _really_ struggling to understand what good those tests were doing at all. I agree they've outlived their utility. At this point they're basically hard-coded things checking for hard-coded things. I proposed those fixes merely to "make tests turn green" as opposed to doing something useful. I agree that just deleting them makes more sense.

Note to other reviewers, this doesn't remove
...

✅ theuni closed a pull request: "build: CMake security checks workarounds"
(https://github.com/bitcoin/bitcoin/pull/31715)

💬 theuni commented on pull request "build: CMake security checks workarounds":
(https://github.com/bitcoin/bitcoin/pull/31715#issuecomment-2643563724)
Closing in favor of #31818.

💬 theStack commented on pull request "contrib: fix `test_deterministic_coverage.sh` script for out-of-tree builds":
(https://github.com/bitcoin/bitcoin/pull/31588#issuecomment-2643586049)
@fjahr: Executing the script with the commands mentioned in the PR description works for me on Ubuntu 22.04.03 LTS, didn't try it on any other machine yet. Which operating system are you running? (Some macOS I presume?) Some versions from my machine that would be relevant for comparison:
```
$ cat /etc/lsb-release | tail -n1
DISTRIB_DESCRIPTION="Ubuntu 22.04.3 LTS"
$ gcovr --version | head -n1
gcovr 5.0
$ gcc --version | head -n1
gcc (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0
$ bash --version
...

💬 theuni commented on pull request "build, ci: Fix linking `bitcoin-chainstate.exe` to `bitcoinkernel.dll` on Windows":
(https://github.com/bitcoin/bitcoin/pull/31158#discussion_r1946935711)
The meaning of `BITCOINKERNEL_STATIC` here is overloaded. Just to differentiate the use-cases, I'd prefer to have a 3rd define here:

`#elif defined(_WIN32) && !defined(BITCOIN_BUILD_INTERNAL) && !defined(BITCOINKERNEL_STATIC)`

`BITCOINKERNEL_STATIC`: I'm a user of the kernel building against a static kernel.
`BITCOIN_BUILD_INTERNAL`: I'm a component of Core who's not building against the kernel at all, I just happen to be seeing this header.

The former should stay as-is for bitcoin-cha
...

🤔 ryanofsky reviewed a pull request: "multiprocess: Add libmultiprocess git subtree"
(https://github.com/bitcoin/bitcoin/pull/31741#pullrequestreview-2602352724)
Rebased 99b7b857d3b8b9ce9bd7501e2480583369740c55 -> 4ca2462bf516846c5552378345e243bc819553a3 ([`pr/subtree.8`](https://github.com/ryanofsky/bitcoin/commits/pr/subtree.8) -> [`pr/subtree.9`](https://github.com/ryanofsky/bitcoin/commits/pr/subtree.9), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/subtree.8-rebase..pr/subtree.9)) to fix conflict with #31800. Also applied review suggestions from vasild, renamed Libmultiprocess_EXTERNAL_MPGEN cmake option to MPGEN_EXECUTABLE as suggested
...

💬 ryanofsky commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#discussion_r1946903444)
re: https://github.com/bitcoin/bitcoin/pull/31741#discussion_r1942670069

Thanks, updated

💬 ryanofsky commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#discussion_r1946892750)
re: https://github.com/bitcoin/bitcoin/pull/31741#discussion_r1942473099

Thanks, fixed

💬 ryanofsky commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#discussion_r1946893285)
re: https://github.com/bitcoin/bitcoin/pull/31741#discussion_r1942473641

Agree, changed to suggested name

💬 ryanofsky commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#discussion_r1946902878)
re: https://github.com/bitcoin/bitcoin/pull/31741#discussion_r1942612485

Thanks, applied both patches.

💬 ryanofsky commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#discussion_r1946901978)
re: https://github.com/bitcoin/bitcoin/pull/31741#discussion_r1942477051

Agree that's a little clearer and more consistent with the documentation, so updated.

💬 ryanofsky commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#discussion_r1946892486)
re: https://github.com/bitcoin/bitcoin/pull/31741#discussion_r1942468885

Thanks, fixed

👍 instagibbs approved a pull request: "TxOrphanage: account for size of orphans and count announcements"
(https://github.com/bitcoin/bitcoin/pull/31810#pullrequestreview-2602431093)
reACK 2a279acd89fa37fec94c66dde7408a3a9627f49a

💬 instagibbs commented on pull request "TxOrphanage: account for size of orphans and count announcements":
(https://github.com/bitcoin/bitcoin/pull/31810#discussion_r1946936618)
nit: this could be moved completely outside the loops for the same checking power

💬 Sjors commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2643619428)
Getting a bunch of CI failings in #30975:

```
error: no member named 'format' in namespace 'std'
result.append(std::format("\\{:02x}", c));
```

e.g. macOS native: https://github.com/bitcoin/bitcoin/actions/runs/13205261715/job/36866826238?pr=30975

ARM, libbitcoinkernel, previous releases jobs:
```
17:54:12.396] [ 14%] Building CXX object CMakeFiles/mputil.dir/src/mp/util.cpp.o
[17:54:12.396] /ci_container_base/depends/work/build/arm-linux-gnueabihf/native_libmulti
...