💬 hebasto commented on pull request "Fix windows libc++ `fs::path` `fstream` compile errors":
(https://github.com/bitcoin/bitcoin/pull/33550#discussion_r2407246802)
This might requires a bit more adjustments:
```diff
--- a/src/common/settings.cpp
+++ b/src/common/settings.cpp
@@ -78,7 +78,7 @@ bool ReadSettings(const fs::path& path, std::map<std::string, SettingsValue>& va
if (!fs::exists(path)) return true;

std::ifstream file;
- file.open(path);
+ file.open(path.std_path());
if (!file.is_open()) {
errors.emplace_back(strprintf("%s. Please check permissions.", fs::PathToString(path)));
return false;
@@ -133,
...

💬 ryanofsky commented on pull request "Fix windows libc++ `fs::path` `fstream` compile errors":
(https://github.com/bitcoin/bitcoin/pull/33550#issuecomment-3372559538)
> I assume we still need `fs::path::filename()` though.

I think we could overload it but it seems a little consistent to do that without a clear reason and without also overloading parent_path. For now just dropped some code in bitcoin.cpp which was using it.

💬 ryanofsky commented on pull request "Fix windows libc++ `fs::path` `fstream` compile errors":
(https://github.com/bitcoin/bitcoin/pull/33550#issuecomment-3372575610)
<!-- begin push-4 -->
Updated d405c76d2e32e957a72c042f1fb82fd0cfcc8e6d -> c864a4c1940d682f7eb6fdb3b91b18d638b59330 ([`pr/winstream.3`](https://github.com/ryanofsky/bitcoin/commits/pr/winstream.3) -> [`pr/winstream.4`](https://github.com/ryanofsky/bitcoin/commits/pr/winstream.4), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/winstream.3..pr/winstream.4))<!-- end --> with CI fixes for second commit

👍 instagibbs approved a pull request: "[28.x] More backports"
(https://github.com/bitcoin/bitcoin/pull/33535#pullrequestreview-3305798964)
ACK 06fe49dc88638e2ad21f1b7d0dd87661de384517

👍 hebasto approved a pull request: "Fix windows libc++ `fs::path` `fstream` compile errors"
(https://github.com/bitcoin/bitcoin/pull/33550#pullrequestreview-3305871368)
ACK c864a4c1940d682f7eb6fdb3b91b18d638b59330.

🚀 glozow merged a pull request: "net: support overriding the proxy selection in ConnectNode()"
(https://github.com/bitcoin/bitcoin/pull/33454)

🚀 glozow merged a pull request: "build: Drop support for EOL macOS 13"
(https://github.com/bitcoin/bitcoin/pull/33489)

💬 andrewtoth commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2407510165)
If we `wait()` or `get()` on the returned future, does that imply a release/acquire memory ordering or a relaxed memory ordering? I can't seem to find out what this means for other non-atomic memory that was written on the worker thread before the task is completed.

💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-3373034901)
`8725d10df5...dd32dfaaf3`: rebase due to conflicts and fix connman fuzz test.

📝 mdfaizanaquil opened a pull request: "Update README_windows.txt"
(https://github.com/bitcoin/bitcoin/pull/33552)
looks good now

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new
...

💬 hebasto commented on pull request "Modernize use of UTF-8 in Windows code":
(https://github.com/bitcoin/bitcoin/pull/32380#issuecomment-3373339683)
Rebased to resolve a conflict with the merged #33489.

✅ fanquake closed a pull request: "Update README_windows.txt"
(https://github.com/bitcoin/bitcoin/pull/33552)

💬 furszy commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2408023767)
> If we `wait()` or `get()` on the returned future, does that imply a release/acquire memory ordering or a relaxed memory ordering? I can't seem to find out what this means for other non-atomic memory that was written on the worker thread before the task is completed.

For the returned value, `wait()`/`get()` provide release/acquire semantics. All updates performed by the task should be visible after `get()` returns.
Now, if the task modifies other objects that might be accessed concurrently
...

💬 andrewtoth commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2408056506)
Sure, so I tried updating #31132 to use the thread pool here. It makes the change simpler if we were to already have this thread pool merged. However, the threads write to non-synchronized shared vectors, which the main thread will read. We need to make sure the write happens before the read. I initially stored the futures and wait on them like this https://github.com/andrewtoth/bitcoin/commit/c256f1b457cbd5b900aa34703eb5853d2449bcde#diff-3eff97d24109f473292b38a6495c5b51e8bda9f4bff9b691cf255968b
...

💬 andrewtoth commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2408106215)
> All updates performed by the task should be visible after get() returns.

Hmm so in that case, then the diff I linked above is correct? The main thread is the one waiting on the future, and the non-synchronized vector is modified only by the worker thread. So the change to the vector should be visible on the main thread.

💬 furszy commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2408379260)
> Sure, so I tried updating #31132 to use the thread pool here. It makes the change simpler if we were to already have this thread pool merged. However, the threads write to non-synchronized shared vectors, which the main thread will read. We need to make sure the write happens before the read. I initially stored the futures and wait on them like this [andrewtoth@c256f1b#diff-3eff97d24109f473292b38a6495c5b51e8bda9f4bff9b691cf255968ba80d85dR151-R164](https://github.com/andrewtoth/bitcoin/commit/c
...

💬 andrewtoth commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2408391339)
Nice!

🤔 hebasto reviewed a pull request: "Fix windows libc++ `fs::path` `fstream` compile errors"
(https://github.com/bitcoin/bitcoin/pull/33550#pullrequestreview-3307332538)
> ... even though there is not currently a CI job testing Windows libc++ builds.

Add to the nightly builds in https://github.com/hebasto/bitcoin-core-nightly/pull/74.

👍 hodlinator approved a pull request: "Clear out space on GHA jobs"
(https://github.com/bitcoin/bitcoin/pull/33514#pullrequestreview-3307376880)
ACK c01214e72b9876c59924c98e96f43906f5df3353

I'm okay with applying broadly to ease maintenance, with a very slight preference for changing it back to more surgically apply to CentOS.

👍 hodlinator approved a pull request: "Modernize use of UTF-8 in Windows code"
(https://github.com/bitcoin/bitcoin/pull/32380#pullrequestreview-3307401839)
re-ACK 53e4951a5b5b9d166d278db4240513d09b447f58