💬 hebasto commented on pull request "ci: Add missed configuration options to "Win64 native" job":
(https://github.com/bitcoin/bitcoin/pull/30755#discussion_r1746918973)
> For libqrencode, if it's going to cause build issues, if the user has it and Qt installed, it'd probably be good to document that as the reason it is disabled.

In suggestions about the place for such a note. Asking because the JSON format has no comments, and CMake's parser does not allow extra fields, such as `"_comment:"`.

💬 stickies-v commented on pull request "test: support std::optional in BOOST_CHECK_* and increase FromUserHex fuzz feature coverage":
(https://github.com/bitcoin/bitcoin/pull/30618#issuecomment-2333782452)
re-ACK 4ac1981d338f89ed2b814aedaebe81783293e6f5

👍 TheCharlatan approved a pull request: "init: fix init fatal error on invalid negated option value"
(https://github.com/bitcoin/bitcoin/pull/30684#pullrequestreview-2285916123)
ACK ee47ca29d6ef55650a0af63bca817c5d494f31ef

💬 fanquake commented on pull request "ci: Add missed configuration options to "Win64 native" job":
(https://github.com/bitcoin/bitcoin/pull/30755#discussion_r1746922365)
> In suggestions about the place for such a note.

I guess somewhere in the Windows build docs? Note, still wondering in regards to:

> Can you explain why some options are passed on the command line, and why some are put into CMakePresets.json; what determines where each option belongs? I'd think it'd be better to have a single source of truth for the CI config.

💬 hebasto commented on pull request "ci: Add missed configuration options to "Win64 native" job":
(https://github.com/bitcoin/bitcoin/pull/30755#discussion_r1746924747)
Presets contain options that seem reasonable in most cases.

⚠️ vostrnad opened an issue: "28.0rc1 synchronizes much slower on Windows"
(https://github.com/bitcoin/bitcoin/issues/30833)
Bitcoin Core 28.0rc1 (both pre-built and self-built binary) on Windows 10 synchronizes much slower than 27.0 after around block 120,000 when blocks stop being almost empty.

Time to synchronize to block 300,000: (measured around 10 runs each, alternating between the versions)
27.0: **695 seconds**
28.0rc1: **2760 seconds**


Running the Linux pre-built binary in WSL doesn't show the slowdown, so i
...

💬 fanquake commented on pull request "ci: Add missed configuration options to "Win64 native" job":
(https://github.com/bitcoin/bitcoin/pull/30755#discussion_r1746931123)
> Presets contain options that seem reasonable in most cases.

Sure, but that's too vauge to use as reasoning for where CI configuration should live, and doesn't solve this issue:

> For example, I can imagine someone changing CMakePresets.json, without realising that also partially controls the CI, and then we (silently) lose Qt compilation in this build.

I'm not sure the configuration of the CI, should be (unexplicitly) coupled to the CMake Preset. This isn't specific to the GUI option,
...

💬 hebasto commented on pull request "ci: Add missed configuration options to "Win64 native" job":
(https://github.com/bitcoin/bitcoin/pull/30755#discussion_r1746937151)
> If we want to use Presets for the CI, that's possibly also fine, but they should be dedicated presets, so it's clear what they are for.

You highlighted a problem that is relevant to all CI jobs and is wider than this PR goal. Can we leave it for a follow up dedicated discussion/PR?

💬 fanquake commented on pull request "ci: Add missed configuration options to "Win64 native" job":
(https://github.com/bitcoin/bitcoin/pull/30755#discussion_r1746942004)
Seems like we could just add the relevant options from the preset to the ci config file for now, which makes the config clear, and removes the possibility for external breakage? If at some point we switch to presets, they would all them be moved to a dedicated CI preset.

🤔 marcofleon reviewed a pull request: "fuzz: reduce number of iterations in `crypto_aeadchacha20poly1305` target"
(https://github.com/bitcoin/bitcoin/pull/30826#pullrequestreview-2285983094)
Tested ACK f482d0e366a84008129913b442f0c955de79ac93. Saw the same slight increase in coverage. Executed 100,000 runs several times and total time went from 30-35 sec to 20-25 sec.

💬 l0rinc commented on pull request "refactor: Generalize `CScript`'s `std::vector` push to `std::span` to accept `std::array`, too":
(https://github.com/bitcoin/bitcoin/pull/30765#discussion_r1746943429)
> But I do think it would be nice if this PR just added support for std::span and std::byte without necessarily making bigger changes.

I've added support for `const std::span<const unsigned char>` (for vector & array), but `std::byte` seems like a bigger change (as explained now in the PR's description) - unless you'd like me to add it as an extra overload - let me know if that's what you meant.

💬 fanquake commented on pull request "build: work around issue with Boost <= 1.80 and Clang >= 18":
(https://github.com/bitcoin/bitcoin/pull/30821#issuecomment-2333821161)
I had hoped we'd be able to land this workaround for `28.x` (to atleast avoid compile failures, without any other changes to other dependency or compiler requirements), and then remove it in master fairly soon, by following up with either a wholesale bump to our minimum required Boost, to 1.81.0+, or I guess a clang-18+ restriction to using Boost 1.81.0+.

I re-tested master (including this change) using Boost 1.74.0:
* clang-18 builds with no output
* clang-19 builds with no output (unrelat
...

💬 sipa commented on issue "28.0rc1 synchronizes much slower on Windows":
(https://github.com/bitcoin/bitcoin/issues/30833#issuecomment-2333833557)
Try whether #28280 had any impact.

💬 hebasto commented on pull request "ci: Add missed configuration options to "Win64 native" job":
(https://github.com/bitcoin/bitcoin/pull/30755#discussion_r1746957633)
> > In suggestions about the place for such a note.
>
> I guess somewhere in the Windows build docs?

Thanks! Done.

> Seems like we could just add the relevant options from the preset to the ci config file for now

Implemented.

💬 vostrnad commented on issue "28.0rc1 synchronizes much slower on Windows":
(https://github.com/bitcoin/bitcoin/issues/30833#issuecomment-2333846869)
I actually benchmarked #28280 right before it got merged: https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2268695679

From the results it definitely seems that PR is not to blame and the regression happened later.

👍 fanquake approved a pull request: "ci: Add missed configuration options to "Win64 native" job"
(https://github.com/bitcoin/bitcoin/pull/30755#pullrequestreview-2286047596)
ACK ee22bf55e39e4a40c31465e8ef2663eb7fefc783

💬 andrewtoth commented on issue "28.0rc1 synchronizes much slower on Windows":
(https://github.com/bitcoin/bitcoin/issues/30833#issuecomment-2333868669)
https://github.com/bitcoin/bitcoin/pull/30326?

💬 ismaelsadeeq commented on pull request "init: fix init fatal error on invalid negated option value":
(https://github.com/bitcoin/bitcoin/pull/30684#discussion_r1746980280)
adding negatives value to the values makes this test to fail.
should we also prevent passing negative integers?

💬 hebasto commented on pull request "build: Fix linking for `fuzz` target when building with MSan":
(https://github.com/bitcoin/bitcoin/pull/30778#discussion_r1746980931)
> * I'm probably missing something, but maybe a cleaner workaround instead of setting `Debug` build type and then erasing all the debug flags is just to set a custom build type -DCMAKE_BUILD_TYPE=msan that doesn't have the flags to begin with?

I agree with you, and that was my initial preference when I worked on the CMake staging branch. However, using undefined build configurations is not documented. As a result, the rough consensus among the CMake staging branch contributors was to adher
...

💬 hebasto commented on issue "cmake: passing options from depends build to main build":
(https://github.com/bitcoin/bitcoin/issues/30813#issuecomment-2333907613)
> Right now looking at `depends/toolchain.cmake.in` it seems like both of these things can happen in some cases, but neither of them happen reliably. Specifically in the current system:
>
> * The toolchain file force-writes into the cache and overwrites user settings.
>
> ...
>
> I think to implement desired behavior, `toolchain.cmake.in` could be changed to:
>
> 1. Stop setting cache variables like `BUILD_GUI` and `WITH_QRENCODE` and instead set custom non-cache variables like `
...