💬 maflcko commented on pull request "test: Embed univalue json tests in binary":
(https://github.com/bitcoin/bitcoin/pull/31542#discussion_r1893820831)
Happy to review a follow-up doing this, but I'll leave this as-is for now.

💬 maflcko commented on pull request "test: Embed univalue json tests in binary":
(https://github.com/bitcoin/bitcoin/pull/31542#discussion_r1893820913)
> does this sorting (here and in unitester.cpp) serve some purpose of could we use natural ordering?

Yes, the purpose is to follow the clang-format sorting. If it was changed, clang-format will undo the change the next time.

> maybe even adding a 0 prefix for <10

Happy to review a follow-up doing this, but I'll leave this as-is for now.

💬 hebasto commented on pull request "guix: latest 2.31 glibc":
(https://github.com/bitcoin/bitcoin/pull/31529#issuecomment-2556813270)
My Guix build:
```
aarch64
8f50480a91024a77c45233e33b8151e213a08834c77493446191a6bc78d82a1f guix-build-b8710201fbd0/output/aarch64-linux-gnu/SHA256SUMS.part
6fd527ebfa0842cd3f3c743dc616d75cdcd0aea637d04e4e047f29e87c43d383 guix-build-b8710201fbd0/output/aarch64-linux-gnu/bitcoin-b8710201fbd0-aarch64-linux-gnu-debug.tar.gz
a40ec71782cc8018ffb9b8b73d99c55aece77c5779e752f1b4209894efd3c97b guix-build-b8710201fbd0/output/aarch64-linux-gnu/bitcoin-b8710201fbd0-aarch64-linux-gnu.tar.gz
74fb98d9
...

💬 maflcko commented on pull request "ci: Allow build dir on CI host":
(https://github.com/bitcoin/bitcoin/pull/31428#discussion_r1893823673)
It requires the caller to define it, which is intentional. Should I add a comment explaining it?

💬 maflcko commented on pull request "ci: Allow build dir on CI host":
(https://github.com/bitcoin/bitcoin/pull/31428#discussion_r1893825061)
Not sure what you mean, for me it is already expanded/evaluated, see https://github.com/bitcoin/bitcoin/pull/31428/checks#step:6:11

👍 hebasto approved a pull request: "guix: latest 2.31 glibc"
(https://github.com/bitcoin/bitcoin/pull/31529#pullrequestreview-2517356688)
ACK b8710201fbd01bf2cba3c3b1fd312316b1ae22af.

💬 vasild commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r1893827538)
The following works on FreeBSD. In case `dtrace` works on macOS:

```sh
dtrace -n 'proc:::exec-success { trace(curpsinfo->pr_psargs); }' -c 'bitcoin -m gui'
```

Maybe how to enable `dtrace` on macOS: https://github.com/suolapeikko/dtrace?tab=readme-ov-file#sample-dtrace-scripts-for-macos

💬 hebasto commented on pull request "ci: Allow build dir on CI host":
(https://github.com/bitcoin/bitcoin/pull/31428#discussion_r1893834634)
I mean, expanded in a log for the current step, like [this](https://github.com/hebasto/bitcoin/actions/runs/12430191280/job/34705149833#step:3:2):
```
Run echo "CCACHE_DIR=/home/runner/work/_temp/ccache_dir" >> "$GITHUB_ENV"
echo "CCACHE_DIR=/home/runner/work/_temp/ccache_dir" >> "$GITHUB_ENV"
echo "BASE_ROOT_DIR=/home/runner/work/_temp" >> "$GITHUB_ENV"
echo "DEPENDS_DIR=/home/runner/work/_temp/depends" >> "$GITHUB_ENV"
echo "BASE_BUILD_DIR=/home/runner/work/_temp/build" >> "$GITH
...

💬 hebasto commented on pull request "ci: Allow build dir on CI host":
(https://github.com/bitcoin/bitcoin/pull/31428#discussion_r1893837484)
> It requires the caller to define it, which is intentional.

Why? What happens if the default value will be used? (especially, if a CI job doesn't care about it)

> Should I add a comment explaining it?

If it helps :)

💬 maflcko commented on pull request "fuzz: Faster leak check":
(https://github.com/bitcoin/bitcoin/pull/31481#discussion_r1893840379)
Also, the message is hidden anyway by default in the fuzz runner, unless you specified `-l DEBUG`.

💬 maflcko commented on pull request "fuzz: Faster leak check":
(https://github.com/bitcoin/bitcoin/pull/31481#issuecomment-2556840485)
> ### Performance increase
>
> Changed `-max_total_time=60` beneath the line this PR is modifying and ran with & without `-detect_leaks=0`. The performance improvement is VERY stark. The number of fuzz inputs generated where either around 67 (2 tests) without, and with, it was a staggering 1937 (average from 2 runs). **Around 28x faster.**

Just for completeness, the performance increase should be mild overall, assuming that the leak detection is turned off automatically anyway for long run
...

💬 maflcko commented on pull request "ci: Allow build dir on CI host":
(https://github.com/bitcoin/bitcoin/pull/31428#discussion_r1893844905)
> > It requires the caller to define it, which is intentional.
>
> Why? What happens if the default value will be used? (especially, if a CI job doesn't care about it)

My understanding is that it would fail otherwise. I suspect the error message will be:

```
root@7b4d00222340:/# mkdir -p "${BASE_BUILD_DIR}"
mkdir: cannot create directory '': No such file or directory
root@7b4d00222340:/# mkdir -p ""
mkdir: cannot create directory '': No such file or directory

📝 hebasto converted_to_draft a pull request: "ci: Test cross-built Windows executables on Windows natively"
(https://github.com/bitcoin/bitcoin/pull/31176)
This PR enables on the CI tests of cross-compiled Windows binaries on Windows.

It is important to have such tests in CI because the release binaries for Windows are also cross-compiled.

Two functional tests, `wallet_migration.py` and `wallet_multiwallet.py`, are temporarily disabled. They require fixes, such as https://github.com/bitcoin/bitcoin/pull/31410, and adjustments for error message handling. Re-enabling these tests will be addressed in follow-up PRs.

Resolves https://github.com
...

💬 hebasto commented on pull request "qa: Use `sys.executable` when invoking other Python scripts":
(https://github.com/bitcoin/bitcoin/pull/31541#issuecomment-2556851076)
Friendly ping Python connoisseur @stickies-v :)

💬 hebasto commented on pull request "qa: Fix `wallet_multiwallet.py`":
(https://github.com/bitcoin/bitcoin/pull/31410#issuecomment-2556857324)
Friendly ping a Python connoisseur @stickies-v :)

💬 hebasto commented on pull request "ci: Allow build dir on CI host":
(https://github.com/bitcoin/bitcoin/pull/31428#discussion_r1893852851)
Right. That's why I suggest to move `BASE_BUILD_DIR=${BASE_BUILD_DIR:-$BASE_SCRATCH_DIR/build-$HOST}` before.

💬 fanquake commented on issue "depends: `capnp` package fails to build on NetBSD 10.0":
(https://github.com/bitcoin/bitcoin/issues/31499#issuecomment-2556860449)
Nothing for us to do here, so could be reopened/linked from a PR once a proper issue is filed upstream.

✅ fanquake closed an issue: "depends: `capnp` package fails to build on NetBSD 10.0"
(https://github.com/bitcoin/bitcoin/issues/31499)

💬 maflcko commented on pull request "ci: Allow build dir on CI host":
(https://github.com/bitcoin/bitcoin/pull/31428#discussion_r1893856069)
Options that start with `DANGER_` require in-depth knowledge of the inner workings of the CI system. Trying to make the existing code harder to understand for others seems suboptimal to solve a problem that doesn't really exist in practise. If there is a strong use-case, I am happy to reconsider, but absent that I don't see the value in making the code more complex.

💬 hebasto commented on pull request "ci: Allow build dir on CI host":
(https://github.com/bitcoin/bitcoin/pull/31428#discussion_r1893858493)
Let's consider this resolved.