💬 maflcko commented on pull request "ci: Check macos-cross executables on macOS":
(https://github.com/bitcoin/bitcoin/pull/33509#issuecomment-3372189103)
> Since we don't have high hopes of this actually detecting any issues that the rest of the CI would not have found, seems to make more sense as a nightly job?
Thx, done in https://github.com/bitcoin/bitcoin/pull/33549 and https://github.com/maflcko/b-c-nightly/actions/runs/18282485376/job/52056370818
(https://github.com/bitcoin/bitcoin/pull/33509#issuecomment-3372189103)
> Since we don't have high hopes of this actually detecting any issues that the rest of the CI would not have found, seems to make more sense as a nightly job?
Thx, done in https://github.com/bitcoin/bitcoin/pull/33549 and https://github.com/maflcko/b-c-nightly/actions/runs/18282485376/job/52056370818
📝 ryanofsky opened a pull request: "Fix windows libc++ `fs::path` `fstream` compile errors"
(https://github.com/bitcoin/bitcoin/pull/33550)
As reported by hebasto in https://github.com/bitcoin/bitcoin/issues/33545, newer libc++ versions implementing https://wg21.link/lwg3430 will no longer implicitly convert `fs::path` objects to `std::filesystem::path` objects when constructing `std::ifstream` and `std::ofstream` types.
This is not a problem in Unix systems since `fs::path` objects use `std::string` as their native string type, but it causes compile errors on Windows which use `std::wstring` as their string type, since `fstream`
...
(https://github.com/bitcoin/bitcoin/pull/33550)
As reported by hebasto in https://github.com/bitcoin/bitcoin/issues/33545, newer libc++ versions implementing https://wg21.link/lwg3430 will no longer implicitly convert `fs::path` objects to `std::filesystem::path` objects when constructing `std::ifstream` and `std::ofstream` types.
This is not a problem in Unix systems since `fs::path` objects use `std::string` as their native string type, but it causes compile errors on Windows which use `std::wstring` as their string type, since `fstream`
...
💬 ryanofsky commented on issue "Implicit conversion from `fs::path` to `std::string` when constructing file streams":
(https://github.com/bitcoin/bitcoin/issues/33545#issuecomment-3372209399)
#33550 could be a good fix for this
(https://github.com/bitcoin/bitcoin/issues/33545#issuecomment-3372209399)
#33550 could be a good fix for this
🤔 hebasto reviewed a pull request: "Fix windows libc++ `fs::path` `fstream` compile errors"
(https://github.com/bitcoin/bitcoin/pull/33550#pullrequestreview-3305372031)
Concept ACK.
Maybe consider removing these lines now:https://github.com/bitcoin/bitcoin/blob/f39d3e1a2f2b9cdcf0e77942fd69eb92a58ce77f/src/util/fs.h#L79-L81 ?
(https://github.com/bitcoin/bitcoin/pull/33550#pullrequestreview-3305372031)
Concept ACK.
Maybe consider removing these lines now:https://github.com/bitcoin/bitcoin/blob/f39d3e1a2f2b9cdcf0e77942fd69eb92a58ce77f/src/util/fs.h#L79-L81 ?
💬 hebasto commented on pull request "Fix windows libc++ `fs::path` `fstream` compile errors":
(https://github.com/bitcoin/bitcoin/pull/33550#issuecomment-3372250792)
CI [fails](https://github.com/bitcoin/bitcoin/actions/runs/18285430177/job/52059106747):
```
/home/admin/actions-runner/_work/_temp/src/wallet/test/init_test_fixture.cpp:39:21: error: conversion function from 'mapped_type' (aka 'fs::path') to 'const string' (aka 'const basic_string<char>') invokes a deleted function
39 | std::ofstream f{m_walletdir_path_cases["file"]};
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/admin/actions-runner/_work/_temp/src/util/fs.h:65:
...
(https://github.com/bitcoin/bitcoin/pull/33550#issuecomment-3372250792)
CI [fails](https://github.com/bitcoin/bitcoin/actions/runs/18285430177/job/52059106747):
```
/home/admin/actions-runner/_work/_temp/src/wallet/test/init_test_fixture.cpp:39:21: error: conversion function from 'mapped_type' (aka 'fs::path') to 'const string' (aka 'const basic_string<char>') invokes a deleted function
39 | std::ofstream f{m_walletdir_path_cases["file"]};
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/admin/actions-runner/_work/_temp/src/util/fs.h:65:
...
💬 maflcko commented on pull request "Fix windows libc++ `fs::path` `fstream` compile errors":
(https://github.com/bitcoin/bitcoin/pull/33550#issuecomment-3372288022)
review ACK f39d3e1a2f2b9cdcf0e77942fd69eb92a58ce77f, but CI fails
(https://github.com/bitcoin/bitcoin/pull/33550#issuecomment-3372288022)
review ACK f39d3e1a2f2b9cdcf0e77942fd69eb92a58ce77f, but CI fails
💬 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
(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
...
(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) -
...
(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`.
(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.
(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,
...
(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.
(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
(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
(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.
(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)
(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)
(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.
(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.
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-3373034901)
`8725d10df5...dd32dfaaf3`: rebase due to conflicts and fix connman fuzz test.