💬 ryanofsky commented on pull request "argsman, cli: GNU-style command-line option parsing (allows options after non-option arguments)":
(https://github.com/bitcoin/bitcoin/pull/33540#issuecomment-3371562956)
Concept ACK on the idea. I think it probably makes sense to allow options arguments to follow non-option arguments even this change is not strictly backwards compatible, as long as there is some way to escape non-option arguments beginning with `-`, such as with a `--` separator.

📝 maflcko opened a pull request: "ci: Use ARM instead of Intel in macOS cross task"
(https://github.com/bitcoin/bitcoin/pull/33549)
Cross compiling to Intel macOS seems fine, but it has to be switched at some point, see https://en.wikipedia.org/wiki/Mac_transition_to_Apple_silicon#Timeline.


Also, if the executable were to be run:

* Running the Intel executable in Rosetta 2 is slower
* It is harder to find native Intel macOS hardware (E.g. GitHub is in the process of dropping it: https://github.blog/changelog/2025-07-11-upcoming-changes-to-macos-hosted-runners-macos-latest-migration-and-xcode-support-policy-updates/#
...

💬 instagibbs commented on pull request "test: Fix reorg patterns in tests to use proper fork-based approach":
(https://github.com/bitcoin/bitcoin/pull/32587#issuecomment-3371932129)
ACK 0f1f02995b715a83f71b1b8fc92fa520625ec427

`git range-diff master a161f058089dfe5f3aa7d3b9635f0400f0147b5d 0f1f02995b715a83f71b1b8fc92fa520625ec427`

💬 ryanofsky commented on pull request "argsman, cli: GNU-style command-line option parsing (allows options after non-option arguments)":
(https://github.com/bitcoin/bitcoin/pull/33540#issuecomment-3372043307)
Assuming we are ok with losing some backwards compatibility by starting to treating arguments that begin with `-` as options instead of non-options when they follow arguments that don't begin with `-`, there could be other interesting ways to extend this PR.

For example, `bitcoin-cli` could distinguish between options that begin with single and double dashes, and treat options after the RPC method name that begin with double dashes as RPC named parameters. In other words, accept:

```bash

...

✅ maflcko closed a pull request: "ci: Check macos-cross executables on macOS"
(https://github.com/bitcoin/bitcoin/pull/33509)

💬 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

📝 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`
...

💬 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

🤔 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 ?

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

💬 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

💬 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