💬 fanquake commented on pull request "test: Introduce `SUPPRESS_ABORT_MESSAGE` environment variable":
(https://github.com/bitcoin/bitcoin/pull/32409#issuecomment-2897666760)
> This PR adds a way to suppress the abort message box when running test_bitcoin.exe and fuzz.exe built with the debug runtime library on Windows.
Given this isn't being used anywhere in our CI, it's no-longer clear why this is needed (we don't generally add unused code to the project). This also still isn't documented anywhere, so it's not clear how anyone who would need to use this, would know that it exists.
(https://github.com/bitcoin/bitcoin/pull/32409#issuecomment-2897666760)
> This PR adds a way to suppress the abort message box when running test_bitcoin.exe and fuzz.exe built with the debug runtime library on Windows.
Given this isn't being used anywhere in our CI, it's no-longer clear why this is needed (we don't generally add unused code to the project). This also still isn't documented anywhere, so it's not clear how anyone who would need to use this, would know that it exists.
💬 Sjors commented on pull request "wallet: Use `util::Error` throughout `AddWalletDescriptor` instead of returning `nullptr` for some errors":
(https://github.com/bitcoin/bitcoin/pull/32475#issuecomment-2897669129)
utACK 785e1407b0a39fef81a7b25554aab88d4cecd66b
(https://github.com/bitcoin/bitcoin/pull/32475#issuecomment-2897669129)
utACK 785e1407b0a39fef81a7b25554aab88d4cecd66b
💬 vasild commented on pull request "net: replace manual reference counting of CNode with shared_ptr":
(https://github.com/bitcoin/bitcoin/pull/32015#issuecomment-2897671621)
`20657a7c8e...cbd7bd7422`: rebase due to conflicts
(https://github.com/bitcoin/bitcoin/pull/32015#issuecomment-2897671621)
`20657a7c8e...cbd7bd7422`: rebase due to conflicts
💬 fanquake commented on pull request "fs: remove `_POSIX_C_SOURCE` defining":
(https://github.com/bitcoin/bitcoin/pull/32460#issuecomment-2897676179)
I've added another commit (that can be squashed), dropping the fallback.
(https://github.com/bitcoin/bitcoin/pull/32460#issuecomment-2897676179)
I've added another commit (that can be squashed), dropping the fallback.
👋 fanquake's pull request is ready for review: "[27.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/32479)
(https://github.com/bitcoin/bitcoin/pull/32479)
💬 fanquake commented on pull request "[27.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/32479#issuecomment-2897683814)
Added a note to the release notes as well.
(https://github.com/bitcoin/bitcoin/pull/32479#issuecomment-2897683814)
Added a note to the release notes as well.
💬 laanwj commented on pull request "subprocess: Handle quoted tokens properly":
(https://github.com/bitcoin/bitcoin/pull/32577#issuecomment-2897693239)
It's ambiguous how to split arguments. The UNIX shell does it in one way (which is pretty nice and supports quoting), Windows shell does it another way. In #32566 we want the OS shell's behavior, because the notifications have historically invoked the shell.
However, for `RunCommandParseJSON` (as changed here) we don't use a shell, so have to implement our own splitting if we want that. And i guess emulating a UNIX shell makes more sense, as it's more versatile.
Alternatively, can't we jus
...
(https://github.com/bitcoin/bitcoin/pull/32577#issuecomment-2897693239)
It's ambiguous how to split arguments. The UNIX shell does it in one way (which is pretty nice and supports quoting), Windows shell does it another way. In #32566 we want the OS shell's behavior, because the notifications have historically invoked the shell.
However, for `RunCommandParseJSON` (as changed here) we don't use a shell, so have to implement our own splitting if we want that. And i guess emulating a UNIX shell makes more sense, as it's more versatile.
Alternatively, can't we jus
...
💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-2897701739)
`503791ed57...9cf56ad085`: rebase due to conflicts
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-2897701739)
`503791ed57...9cf56ad085`: rebase due to conflicts
💬 laanwj commented on pull request "subprocess: Handle quoted tokens properly":
(https://github.com/bitcoin/bitcoin/pull/32577#discussion_r2100123797)
How would you represent a quote in this case?
(https://github.com/bitcoin/bitcoin/pull/32577#discussion_r2100123797)
How would you represent a quote in this case?
💬 Sjors commented on pull request "doc: Remove stale sections in dev notes":
(https://github.com/bitcoin/bitcoin/pull/32572#discussion_r2100124643)
It's not obvious (to me), but as long as the compiler tells me what to do... otherwise a note is helpful.
(https://github.com/bitcoin/bitcoin/pull/32572#discussion_r2100124643)
It's not obvious (to me), but as long as the compiler tells me what to do... otherwise a note is helpful.
💬 polespinasa commented on pull request "tests: Expand HTTP coverage to assert libevent behavior":
(https://github.com/bitcoin/bitcoin/pull/32408#discussion_r2100126887)
Yeah we tried that and still was not working
(https://github.com/bitcoin/bitcoin/pull/32408#discussion_r2100126887)
Yeah we tried that and still was not working
💬 Sjors commented on pull request "doc: Remove stale sections in dev notes":
(https://github.com/bitcoin/bitcoin/pull/32572#discussion_r2100127903)
Not to bash rust, but maybe suggest either...
(https://github.com/bitcoin/bitcoin/pull/32572#discussion_r2100127903)
Not to bash rust, but maybe suggest either...
💬 hebasto commented on pull request "subprocess: Handle quoted tokens properly":
(https://github.com/bitcoin/bitcoin/pull/32577#issuecomment-2897728376)
> Alternatively, can't we just pass a vector of arguments to `RunCommandParseJSON` and avoid splitting/merging?
This approach [caused](https://github.com/bitcoin/bitcoin/pull/28981#issuecomment-1837238921) quoting-related issues on Windows, though I haven’t tested it with the current master branch.
(https://github.com/bitcoin/bitcoin/pull/32577#issuecomment-2897728376)
> Alternatively, can't we just pass a vector of arguments to `RunCommandParseJSON` and avoid splitting/merging?
This approach [caused](https://github.com/bitcoin/bitcoin/pull/28981#issuecomment-1837238921) quoting-related issues on Windows, though I haven’t tested it with the current master branch.
💬 laanwj commented on pull request "subprocess: Handle quoted tokens properly":
(https://github.com/bitcoin/bitcoin/pull/32577#issuecomment-2897741176)
> This approach https://github.com/bitcoin/bitcoin/pull/28981#issuecomment-1837238921 quoting-related issues on Windows, though I haven’t tested it with the current master branch.
The thing on Windows is that `CreateProcessW` takes a single command line, instead of a list of arguments. So a "join" is unavoidable. It does do quoting on windows while joining this command line though-i don't know enough to know if it's correct: https://github.com/arun11299/cpp-subprocess/blob/master/cpp-subproce
...
(https://github.com/bitcoin/bitcoin/pull/32577#issuecomment-2897741176)
> This approach https://github.com/bitcoin/bitcoin/pull/28981#issuecomment-1837238921 quoting-related issues on Windows, though I haven’t tested it with the current master branch.
The thing on Windows is that `CreateProcessW` takes a single command line, instead of a list of arguments. So a "join" is unavoidable. It does do quoting on windows while joining this command line though-i don't know enough to know if it's correct: https://github.com/arun11299/cpp-subprocess/blob/master/cpp-subproce
...
💬 laanwj commented on pull request "doc: Remove stale sections in dev notes":
(https://github.com/bitcoin/bitcoin/pull/32572#discussion_r2100144262)
"Don't add any new bash scripts" would still be good advice imo. We're still running into bash insanity a decade later #32573.
(https://github.com/bitcoin/bitcoin/pull/32572#discussion_r2100144262)
"Don't add any new bash scripts" would still be good advice imo. We're still running into bash insanity a decade later #32573.
💬 laanwj commented on pull request "fs: remove `_POSIX_C_SOURCE` defining":
(https://github.com/bitcoin/bitcoin/pull/32460#discussion_r2100186943)
Maybe `(void)posix_fallocate(fileno(file), 0, nEndPos)` to make ignoring the return value explicit.
(https://github.com/bitcoin/bitcoin/pull/32460#discussion_r2100186943)
Maybe `(void)posix_fallocate(fileno(file), 0, nEndPos)` to make ignoring the return value explicit.
💬 vasild commented on pull request "tests: Expand HTTP coverage to assert libevent behavior":
(https://github.com/bitcoin/bitcoin/pull/32408#discussion_r2100197725)
In that discussion @maflcko [suggested](https://www.erisian.com.au/bitcoin-core-dev/log-2025-05-07.html#l-102) to start the timer before connect and send and sliv3r__ said that [before connect it worked 15 times in a row](https://www.erisian.com.au/bitcoin-core-dev/log-2025-05-07.html#l-117). Do I miss something?
(https://github.com/bitcoin/bitcoin/pull/32408#discussion_r2100197725)
In that discussion @maflcko [suggested](https://www.erisian.com.au/bitcoin-core-dev/log-2025-05-07.html#l-102) to start the timer before connect and send and sliv3r__ said that [before connect it worked 15 times in a row](https://www.erisian.com.au/bitcoin-core-dev/log-2025-05-07.html#l-117). Do I miss something?
💬 laanwj commented on pull request "fs: remove `_POSIX_C_SOURCE` defining":
(https://github.com/bitcoin/bitcoin/pull/32460#issuecomment-2897839301)
OpenBSD (as of 7.4): has no `posix_fallocate`, but does have `ftruncate` https://github.com/PointCloudLibrary/pcl/pull/5957/files
FreeBSD (as of 8.3): has `posix_fallocate`: https://man.freebsd.org/cgi/man.cgi?query=posix_fallocate&sektion=2&manpath=FreeBSD+8.3-RELEASE
NetBSD (as of 7.0): has it : https://man.netbsd.org/NetBSD-7.0/posix_fallocate.2
(https://github.com/bitcoin/bitcoin/pull/32460#issuecomment-2897839301)
OpenBSD (as of 7.4): has no `posix_fallocate`, but does have `ftruncate` https://github.com/PointCloudLibrary/pcl/pull/5957/files
FreeBSD (as of 8.3): has `posix_fallocate`: https://man.freebsd.org/cgi/man.cgi?query=posix_fallocate&sektion=2&manpath=FreeBSD+8.3-RELEASE
NetBSD (as of 7.0): has it : https://man.netbsd.org/NetBSD-7.0/posix_fallocate.2
💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2897850022)
`fb12a062b8...cedbc2cd99`: rebase due to conflicts
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2897850022)
`fb12a062b8...cedbc2cd99`: rebase due to conflicts
💬 polespinasa commented on pull request "tests: Expand HTTP coverage to assert libevent behavior":
(https://github.com/bitcoin/bitcoin/pull/32408#discussion_r2100216282)
It was "randomly" failing. The 15 times in a row was moving it before `http.clinet.HTTPConn...`.
Moving it before `conn.connect` failed the 6th time https://www.erisian.com.au/bitcoin-core-dev/log-2025-05-07.html#l-115
(https://github.com/bitcoin/bitcoin/pull/32408#discussion_r2100216282)
It was "randomly" failing. The 15 times in a row was moving it before `http.clinet.HTTPConn...`.
Moving it before `conn.connect` failed the 6th time https://www.erisian.com.au/bitcoin-core-dev/log-2025-05-07.html#l-115