⚠️ hebasto opened an issue: "`test_kernel` fails to build on Ubuntu 22.04"
(https://github.com/bitcoin/bitcoin/issues/33846)
```
$ sudo apt update
$ sudo apt install git build-essential cmake pkgconf python3 libevent-dev libboost-dev libsqlite3-dev
$ git clone https://github.com/bitcoin/bitcoin.git && cd bitcoin
$ cmake -B build -DENABLE_IPC=OFF -DBUILD_KERNEL_LIB=ON
$ cmake --build build -j $(nproc)
<snip>
[ 76%] Building CXX object src/test/kernel/CMakeFiles/test_kernel.dir/test_kernel.cpp.o
/bitcoin/src/test/kernel/test_kernel.cpp:17:10: fatal error: format: No such file or directory
17 | #include <format>

...

💬 maflcko commented on issue "`test_kernel` fails to build on Ubuntu 22.04":
(https://github.com/bitcoin/bitcoin/issues/33846#issuecomment-3515396269)
Please remove the include. See https://github.com/bitcoin/bitcoin/actions/runs/19176694315/job/54823183155?pr=33810#step:9:18532 :

```
/home/admin/actions-runner/_work/_temp/src/test/kernel/test_kernel.cpp should remove these lines:
- #include <boost/test/included/unit_test.hpp> // lines 9-9
- #include <cstdlib> // lines 15-15
- #include <format> // lines 17-17

💬 l0rinc commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2513989045)
do we still need `#include <pow.h>` here after the move?

💬 ryanofsky commented on pull request "cmake: Move IPC tests to `ipc/test`":
(https://github.com/bitcoin/bitcoin/pull/33774#discussion_r2524524789)
In commit "cmake, test: Improve locality of `bitcoin_ipc_test` library description" (866bbb98fd365962840ee99df0d9f7ad557cc025)

This change should be correct, but I'm confused about how it was working before. This change causes includes in generated files such `build/src/ipc/capnp/common.capnp.proxy.h` to be generated correctly relative to `src/`:

```diff
-#include <src/ipc/capnp/common.capnp.h> // IWYU pragma: keep
+#include <capnp/common.capnp.h> // IWYU pragma: keep
```

So that see
...

💬 l0rinc commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2534655978)
Continuing the thread from https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2477534828:

```patch
#include <common/system.h>
+#include <test/util/setup_common.h>
#include <util/string.h>
```

```suggestion
BOOST_CHECK_EXCEPTION(threadPool.Submit([]{ return false; }), std::runtime_error, HasReason{"No active workers; cannot accept new tasks"});
```

💬 l0rinc commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2541836752)
some of these includes seem unused, can you please check?

<details>
<summary>iwyu_tool.py</summary>

```
python3 /opt/homebrew/bin/iwyu_tool.py -p build-iwyu src/test/threadpool_tests.cpp -- -Xiwyu --cxx17ns -Xiwyu --mapping_file=$PWD/contrib/devtools/iwyu/bitcoin.core.i
mp -Xiwyu --max_line_length=160 -Xiwyu --check_also=$PWD/src/util/threadpool.h

/Users/lorinc/IdeaProjects/bitcoin/src/util/threadpool.h should add these lines:
#include <__vector/vector.h> // for vector

/Users/
...

💬 ajtowns commented on pull request "Change Parse descriptor argument to string_view":
(https://github.com/bitcoin/bitcoin/pull/33914#discussion_r2546566487)
FYI, you could also make this a `string_view` literal, and leave the function as expecting a `span<const char>`, with

```c++
#include <string_view>

using namespace std::string_view_literals;

auto descs = Parse("pk(0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798)"sv, keys, err, /*require_checksum=*/false);
```

Using `string_view` in the function declaration seems better though.

💬 diegoviola commented on pull request "Bugfix - don't allow multiple dialogs for same tx in TransactionView":
(https://github.com/bitcoin-core/gui/pull/817#issuecomment-3574067005)
@pablomartin4btc like I said, this variation when applied to your branch also makes it work:

```diff
diff --git a/src/qt/transactionview.cpp b/src/qt/transactionview.cpp
index 48e41bc352..72c9a65cd3 100644
--- a/src/qt/transactionview.cpp
+++ b/src/qt/transactionview.cpp
@@ -36,6 +36,7 @@
#include <QScrollBar>
#include <QSettings>
#include <QString>
+#include <QWindow>
#include <QTableView>
#include <QTimer>
#include <QUrl>
@@ -671,7 +672,9 @@ bool TransactionView::detailsA
...

💬 sedited commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2571742382)
It would be nice if these could stay where they are. How about the following:
<details>
<summary>Protected Method Wrapper</summary>

```diff
diff --git a/src/test/fuzz/block_index_tree.cpp b/src/test/fuzz/block_index_tree.cpp
index 037d168e0e..04fbbc5473 100644
--- a/src/test/fuzz/block_index_tree.cpp
+++ b/src/test/fuzz/block_index_tree.cpp
@@ -14,0 +15 @@
+#include <test/util/validation.h>
@@ -44 +45 @@ FUZZ_TARGET(block_index_tree, .init = initialize_block_index_tree)
- Chainst
...

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2577299367)
Looks like this needs more work, it's not as easy as I though - we could use an `std::deque` instead of a `vector` here:
```patch
Subject: [PATCH] std::deque<InputToFetch> m_inputs
---
diff --git a/src/coinsviewcacheasync.h b/src/coinsviewcacheasync.h
--- a/src/coinsviewcacheasync.h (revision a3f56354d6e3f64eaca84a16e4951e6073090f60)
+++ b/src/coinsviewcacheasync.h (revision 462408b897197de3a7067dcbdee318ad9dc1e546)
@@ -17,6 +17,7 @@
#include <atomic>
#include <barrier>
#include <cs
...

💬 laanwj commented on pull request "guix: use GCC 14.3.0 over 13.3.0":
(https://github.com/bitcoin/bitcoin/pull/33775#issuecomment-3607180028)
This is a minimum C++ file that will reproduce the issue:
```c++
#include <filesystem>
#include <string>
#include <fstream>
#include <vector>

#include <cstring> // Unused, but removing this makes it determinstic

namespace fs {

using namespace std::filesystem;

// Dummy class. Using "path = std::filesystem::path" makes it deterministic.
class path : public std::filesystem::path
{
public:
using std::filesystem::path::path;

path(std::filesystem::path path) : std::file
...

💬 hebasto commented on pull request "cmake: Move IPC tests to `ipc/test`":
(https://github.com/bitcoin/bitcoin/pull/33774#discussion_r2589020763)
> But I don't understand why the `#include <src/...>` lines were working previously?

This code is responsible for adding the required include path:https://github.com/bitcoin/bitcoin/blob/75baff98fcf987735437196a4db1919e390c4bd2/src/ipc/libmultiprocess/cmake/TargetCapnpSources.cmake#L96-L103

💬 fanquake commented on pull request "guix: use GCC 14.3.0 over 13.3.0":
(https://github.com/bitcoin/bitcoin/pull/33775#issuecomment-3612808384)
Minified a little more (https://github.com/fanquake/bitcoin/tree/repro_33775_minimal):
```cpp
#include <filesystem>
#include <string>
#include <vector>

#include <cstring> // Unused, but removing this makes it determinstic

namespace fs {

using namespace std::filesystem;

// Dummy class. Using "path = std::filesystem::path" makes it deterministic.
class path : public std::filesystem::path
{
public:
using std::filesystem::path::path;

path(std::filesystem::path path) :
...

💬 fanquake commented on pull request "guix: use GCC 14.3.0 over 13.3.0":
(https://github.com/bitcoin/bitcoin/pull/33775#issuecomment-3616532437)
Slightly more minified:
```cpp
#include <filesystem>
#include <string>
#include <vector>
#include <cstring> // Unused, but removing this makes it determinstic

namespace fs {

using namespace std::filesystem;

// Dummy class. Using "path = std::filesystem::path" makes it deterministic.
class path : public std::filesystem::path
{
public:
using std::filesystem::path::path;

path(std::filesystem::path path) : std::filesystem::path::path(std::move(path)) {}
};

}

bool
...

💬 hebasto commented on pull request "ci: Add IWYU job":
(https://github.com/bitcoin/bitcoin/pull/33810#discussion_r2592510783)
> nit: Looking at the output, I think this is already done correctly?

I don't think so. The generated diffs still [suggest](https://github.com/bitcoin/bitcoin/actions/runs/19938614368/job/57170256836) the C headers:
```diff
+#include <errno.h>
```

💬 hodlinator commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#discussion_r2592629697)
Requires `#include <cassert>`.

💬 stickies-v commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#discussion_r2599122933)
It's a larger diff, so I'm happy to keep this as-is, but I think this is actually quite a nice use-case for `util::Expected` to improve the ownership semantics of `Enqeue`. Ownership can be transferred unambiguously with:

<details>
<summary>git diff on faa23738fc</summary>

```diff
diff --git a/src/httpserver.cpp b/src/httpserver.cpp
index abfcb455e1..de6c837ed2 100644
--- a/src/httpserver.cpp
+++ b/src/httpserver.cpp
@@ -14,11 +14,13 @@
#include <rpc/protocol.h>
#include <sync.h>
...