💬 laanwj commented on pull request "fs: remove `_POSIX_C_SOURCE` defining":
(https://github.com/bitcoin/bitcoin/pull/32460#issuecomment-2882928200)
> _GNU_SOURCE isn't supposed to ever be defined by default, and I don't see us defining it anywhere...?
Not sure when it became that way, but it seems to be set by default, just compile anything using `g++` (first thought was it related to C++ standard, but it isn't):
```c++
#include <iostream>
int main()
{
std::cout << _GNU_SOURCE << std::endl;
std::cout << _POSIX_C_SOURCE << std::endl;
return 0;
}
```
```bash
$ g++ --version
g++ (Ubuntu 13.3.0-6ubuntu2~24.04) 13.
...
(https://github.com/bitcoin/bitcoin/pull/32460#issuecomment-2882928200)
> _GNU_SOURCE isn't supposed to ever be defined by default, and I don't see us defining it anywhere...?
Not sure when it became that way, but it seems to be set by default, just compile anything using `g++` (first thought was it related to C++ standard, but it isn't):
```c++
#include <iostream>
int main()
{
std::cout << _GNU_SOURCE << std::endl;
std::cout << _POSIX_C_SOURCE << std::endl;
return 0;
}
```
```bash
$ g++ --version
g++ (Ubuntu 13.3.0-6ubuntu2~24.04) 13.
...
💬 luke-jr commented on pull request "doc: Add missing top-level description to pruneblockchain RPC":
(https://github.com/bitcoin/bitcoin/pull/32333#discussion_r2090613335)
...only if certain criteria are met?
(https://github.com/bitcoin/bitcoin/pull/32333#discussion_r2090613335)
...only if certain criteria are met?
💬 luke-jr commented on pull request "doc: Add missing top-level description to pruneblockchain RPC":
(https://github.com/bitcoin/bitcoin/pull/32333#discussion_r2090613797)
`getblockfrompeer` can reverse it AFAIK
(https://github.com/bitcoin/bitcoin/pull/32333#discussion_r2090613797)
`getblockfrompeer` can reverse it AFAIK
🚀 fanquake merged a pull request: "doc: remove Carls substitute server from Guix docs"
(https://github.com/bitcoin/bitcoin/pull/32498)
(https://github.com/bitcoin/bitcoin/pull/32498)
💬 sipsorcery commented on pull request "cmake: Add application manifests when cross-compiling for Windows":
(https://github.com/bitcoin/bitcoin/pull/32396#issuecomment-2883086151)
re-tACK c0f41523973e33a8cd104a8ebf4906a4d99f3eed.
(https://github.com/bitcoin/bitcoin/pull/32396#issuecomment-2883086151)
re-tACK c0f41523973e33a8cd104a8ebf4906a4d99f3eed.
💬 hodlinator commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2090691276)
Previous thread: https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2058445224
`AlreadyConnectedToAddressPort` is significantly more verbose than master's `FindNode`. Checked for uses of `Is[A-Z]\w`-prefix and the `Already`-prefix seems to rarely work, meaning `Already` is more specific in this case. This convinced me that `Already` might be fine. :+1:
(https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2090691276)
Previous thread: https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2058445224
`AlreadyConnectedToAddressPort` is significantly more verbose than master's `FindNode`. Checked for uses of `Is[A-Z]\w`-prefix and the `Already`-prefix seems to rarely work, meaning `Already` is more specific in this case. This convinced me that `Already` might be fine. :+1:
🚀 fanquake merged a pull request: "test: add skip_if_running_under_valgrind()"
(https://github.com/bitcoin/bitcoin/pull/32492)
(https://github.com/bitcoin/bitcoin/pull/32492)
✅ fanquake closed an issue: "test: interface_usdt_net.py failure under --valgrind"
(https://github.com/bitcoin/bitcoin/issues/32374)
(https://github.com/bitcoin/bitcoin/issues/32374)
💬 fanquake commented on issue "test: interface_usdt_net.py failure under --valgrind":
(https://github.com/bitcoin/bitcoin/issues/32374#issuecomment-2883145411)
Closed via #32492.
(https://github.com/bitcoin/bitcoin/issues/32374#issuecomment-2883145411)
Closed via #32492.
💬 fanquake commented on pull request "build: document why we check for `std::system`":
(https://github.com/bitcoin/bitcoin/pull/32491#issuecomment-2883180819)
> it was observered that there's also more obscure platforms out there that don't have and/or sandbox std::system.
@laanwj you might remember / know of such systems?
(https://github.com/bitcoin/bitcoin/pull/32491#issuecomment-2883180819)
> it was observered that there's also more obscure platforms out there that don't have and/or sandbox std::system.
@laanwj you might remember / know of such systems?
💬 Sjors commented on pull request "Enhanced error messages for invalid network prefix during address parsing.":
(https://github.com/bitcoin/bitcoin/pull/27260#issuecomment-2883229046)
Any particular reason for marking this draft?
(https://github.com/bitcoin/bitcoin/pull/27260#issuecomment-2883229046)
Any particular reason for marking this draft?
💬 Sjors commented on pull request "qa: make feature_assumeutxo.py test more robust":
(https://github.com/bitcoin/bitcoin/pull/32117#issuecomment-2883235805)
I'll give this PR some thought after you rebase it...
(https://github.com/bitcoin/bitcoin/pull/32117#issuecomment-2883235805)
I'll give this PR some thought after you rebase it...
💬 polespinasa commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2090794255)
> Personally, I think it's better to think of "deprecated" as meaning just "the devs recommend you don't use this feature".
Agree on all you said, that's what I tried to argue in few words :)
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2090794255)
> Personally, I think it's better to think of "deprecated" as meaning just "the devs recommend you don't use this feature".
Agree on all you said, that's what I tried to argue in few words :)
👍 theStack approved a pull request: "psbt: add non-default sighash types to PSBTs and unify sighash type match checking"
(https://github.com/bitcoin/bitcoin/pull/31622#pullrequestreview-2843083473)
re-ACK ee045b61efc1479c1866b786661ef39a863677d0
(as per `$ git range-diff f09e1f94...ee045b61`)
(https://github.com/bitcoin/bitcoin/pull/31622#pullrequestreview-2843083473)
re-ACK ee045b61efc1479c1866b786661ef39a863677d0
(as per `$ git range-diff f09e1f94...ee045b61`)
⚠️ fanquake opened an issue: "test: feature_framework_startup_failures.py fails with `--timeout-factor=0`"
(https://github.com/bitcoin/bitcoin/issues/32506)
master @ c779ee3a4044df3a263394bbb8b104aeeaa7c727
```bash
./build/test/functional/test_runner.py feature_framework_startup_failures.py --timeout-factor=0
Temporary test directory at /tmp/test_runner_₿_🏃_20250515_101739
Remaining jobs: [feature_framework_startup_failures.py]
1/1 - feature_framework_startup_failures.py failed, Duration: 1 s
stdout:
2025-05-15T10:17:39.659000Z TestFramework (INFO): PRNG seed is: 8331063254222734517
2025-05-15T10:17:39.660000Z TestFramework (INFO): Initializing te
...
(https://github.com/bitcoin/bitcoin/issues/32506)
master @ c779ee3a4044df3a263394bbb8b104aeeaa7c727
```bash
./build/test/functional/test_runner.py feature_framework_startup_failures.py --timeout-factor=0
Temporary test directory at /tmp/test_runner_₿_🏃_20250515_101739
Remaining jobs: [feature_framework_startup_failures.py]
1/1 - feature_framework_startup_failures.py failed, Duration: 1 s
stdout:
2025-05-15T10:17:39.659000Z TestFramework (INFO): PRNG seed is: 8331063254222734517
2025-05-15T10:17:39.660000Z TestFramework (INFO): Initializing te
...
💬 fanquake commented on issue "test: feature_framework_startup_failures.py fails with `--timeout-factor=0`":
(https://github.com/bitcoin/bitcoin/issues/32506#issuecomment-2883310880)
cc @hodlinator
(https://github.com/bitcoin/bitcoin/issues/32506#issuecomment-2883310880)
cc @hodlinator
💬 laanwj commented on pull request "build: document why we check for `std::system`":
(https://github.com/bitcoin/bitcoin/pull/32491#issuecomment-2883318635)
> @laanwj you might remember / know of such systems?
Yes. At the time there was cloudabi, which was a sandboxed version of POSIX that removed all contextual access and ambient authority, so processes only get capability handles passed in to what they need. So there was no shell and hence no `std::system`. But that project was abandoned years ago.
i'm not sure any other such platforms exist right now, i'm fine with removing the check. But adding a comment makes sense nevertheless.
Aside:
...
(https://github.com/bitcoin/bitcoin/pull/32491#issuecomment-2883318635)
> @laanwj you might remember / know of such systems?
Yes. At the time there was cloudabi, which was a sandboxed version of POSIX that removed all contextual access and ambient authority, so processes only get capability handles passed in to what they need. So there was no shell and hence no `std::system`. But that project was abandoned years ago.
i'm not sure any other such platforms exist right now, i'm fine with removing the check. But adding a comment makes sense nevertheless.
Aside:
...
💬 l0rinc commented on pull request "refactor: Remove UB in prevector reverse iterators":
(https://github.com/bitcoin/bitcoin/pull/32490#issuecomment-2883326795)
This PR is an extension of https://github.com/bitcoin/bitcoin/pull/32296 (which originally just removed the unnecessary `prevector` suppression, but @maflcko bisected the cause and removed the dead code as well).
Your reviews would be appreciated there as well.
(https://github.com/bitcoin/bitcoin/pull/32490#issuecomment-2883326795)
This PR is an extension of https://github.com/bitcoin/bitcoin/pull/32296 (which originally just removed the unnecessary `prevector` suppression, but @maflcko bisected the cause and removed the dead code as well).
Your reviews would be appreciated there as well.
💬 hebasto commented on pull request "build: document why we check for `std::system`":
(https://github.com/bitcoin/bitcoin/pull/32491#issuecomment-2883331380)
> Aside: i would like to eventually convert usage of `std::system` to `subprocess`, especially now (after #32343) that it doesn't leak file descriptors anymore.
:+1:
(https://github.com/bitcoin/bitcoin/pull/32491#issuecomment-2883331380)
> Aside: i would like to eventually convert usage of `std::system` to `subprocess`, especially now (after #32343) that it doesn't leak file descriptors anymore.
:+1:
💬 hebasto commented on pull request "test: Introduce `SUPPRESS_ABORT_MESSAGE` environment variable":
(https://github.com/bitcoin/bitcoin/pull/32409#issuecomment-2883341081)
This issue seems related: https://developercommunity.visualstudio.com/t/Debug-CRT-shows-abort-message-box-instea/10904782.
(https://github.com/bitcoin/bitcoin/pull/32409#issuecomment-2883341081)
This issue seems related: https://developercommunity.visualstudio.com/t/Debug-CRT-shows-abort-message-box-instea/10904782.