💬 ryanofsky commented on pull request "Fix windows libc++ `fs::path` `fstream` compile errors":
(https://github.com/bitcoin/bitcoin/pull/33550#issuecomment-3372301217)
<!-- begin push-2 -->
Updated f39d3e1a2f2b9cdcf0e77942fd69eb92a58ce77f -> b0113afd44b4c7c0d0da9883424bd2978de3d18c ([`pr/winstream.1`](https://github.com/ryanofsky/bitcoin/commits/pr/winstream.1) -> [`pr/winstream.2`](https://github.com/ryanofsky/bitcoin/commits/pr/winstream.2), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/winstream.1..pr/winstream.2))<!-- end --> to fix CI failures caused by missing std_path replacement

💬 ryanofsky commented on pull request "Fix windows libc++ `fs::path` `fstream` compile errors":
(https://github.com/bitcoin/bitcoin/pull/33550#issuecomment-3372384215)
re: https://github.com/bitcoin/bitcoin/pull/33550#pullrequestreview-3305372031

> Maybe consider removing these lines now:

Nice suggestion. Attempted this in new commit.

<!-- begin push-3 -->
Added 1 commits b0113afd44b4c7c0d0da9883424bd2978de3d18c -> d405c76d2e32e957a72c042f1fb82fd0cfcc8e6d ([`pr/winstream.2`](https://github.com/ryanofsky/bitcoin/commits/pr/winstream.2) -> [`pr/winstream.3`](https://github.com/ryanofsky/bitcoin/commits/pr/winstream.3), [compare](https://github.com/ryan
...

💬 hebasto commented on pull request "Fix windows libc++ `fs::path` `fstream` compile errors":
(https://github.com/bitcoin/bitcoin/pull/33550#issuecomment-3372389642)
> re: [#33550 (review)](https://github.com/bitcoin/bitcoin/pull/33550#pullrequestreview-3305372031)
>
> > Maybe consider removing these lines now:
>
> Nice suggestion. Attempted this in new commit.
>
> Added 1 commits [b0113af](https://github.com/bitcoin/bitcoin/commit/b0113afd44b4c7c0d0da9883424bd2978de3d18c) -> [d405c76](https://github.com/bitcoin/bitcoin/commit/d405c76d2e32e957a72c042f1fb82fd0cfcc8e6d) ([`pr/winstream.2`](https://github.com/ryanofsky/bitcoin/commits/pr/winstream.2) -
...

📝 fanquake opened a pull request: "[29.x] Finalise 29.2"
(https://github.com/bitcoin/bitcoin/pull/33551)
I'm optimistic that 29.2 wont need an `rc3`.

🤔 hebasto reviewed a pull request: "Fix windows libc++ `fs::path` `fstream` compile errors"
(https://github.com/bitcoin/bitcoin/pull/33550#pullrequestreview-3305683677)
Tested b0113afd44b4c7c0d0da9883424bd2978de3d18c by cross-compiling for `x86_64-w64-mingw32` and `aarch64-w64-mingw32` targets using the [LLVM MinGW](https://github.com/mstorsjo/llvm-mingw) toolchain.

💬 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.