💬 i-am-yuvi commented on pull request "init: drop `-upnp`":
(https://github.com/bitcoin/bitcoin/pull/32500#issuecomment-2882719488)
ACK 301993ebf7f8ec23050e91377e0fd05823bb372a
(https://github.com/bitcoin/bitcoin/pull/32500#issuecomment-2882719488)
ACK 301993ebf7f8ec23050e91377e0fd05823bb372a
🤔 i-am-yuvi reviewed a pull request: "build: Fix `macdeployqtplus` after switching to Qt 6"
(https://github.com/bitcoin/bitcoin/pull/32287#pullrequestreview-2842440076)
re-ACK 84de8c93e7d4979575161a2bb8f7eb64e1317b89
(https://github.com/bitcoin/bitcoin/pull/32287#pullrequestreview-2842440076)
re-ACK 84de8c93e7d4979575161a2bb8f7eb64e1317b89
💬 hebasto commented on pull request "test: Introduce `SUPPRESS_ABORT_MESSAGE` environment variable":
(https://github.com/bitcoin/bitcoin/pull/32409#issuecomment-2882767896)
> > It is documented, and it is pre-defined by CI itself:
>
> It's not in our docs, or in our CI code, which means our CI would "work", because a third party is putting something into the environment, and the fact that this is even happening, is only discoverable if you happen to read a `.cpp` file.
>
> Would someone running these binaries locally also need/want to set this env var to get the same behaviour? If so, how would they figure that out?
Reworked to avoid relying on third-party
...
(https://github.com/bitcoin/bitcoin/pull/32409#issuecomment-2882767896)
> > It is documented, and it is pre-defined by CI itself:
>
> It's not in our docs, or in our CI code, which means our CI would "work", because a third party is putting something into the environment, and the fact that this is even happening, is only discoverable if you happen to read a `.cpp` file.
>
> Would someone running these binaries locally also need/want to set this env var to get the same behaviour? If so, how would they figure that out?
Reworked to avoid relying on third-party
...
💬 Sjors commented on pull request "build: document why we check for `std::system`":
(https://github.com/bitcoin/bitcoin/pull/32491#issuecomment-2882769993)
ACK 8f4ba90b8ff47c7f90fe65d3ed37f486f9fe3a74
My understanding is that QML based QT will run fine on iOs, though I haven't tried myself.
Back when this was introduced https://github.com/bitcoin/bitcoin/pull/15457#issuecomment-466315628 it was observered that there's also more obscure platforms out there that don't have and/or sandbox `std::system`. Though nowadays we would probably direct them towards the kernel project.
(https://github.com/bitcoin/bitcoin/pull/32491#issuecomment-2882769993)
ACK 8f4ba90b8ff47c7f90fe65d3ed37f486f9fe3a74
My understanding is that QML based QT will run fine on iOs, though I haven't tried myself.
Back when this was introduced https://github.com/bitcoin/bitcoin/pull/15457#issuecomment-466315628 it was observered that there's also more obscure platforms out there that don't have and/or sandbox `std::system`. Though nowadays we would probably direct them towards the kernel project.
💬 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