π¬ hebasto commented on pull request "common: Close non-std fds before exec in RunCommandJSON":
(https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2844357436)
FWIW, I'm running the current PR branch on some other OSes: https://github.com/hebasto/bitcoin-core-nightly/pull/51.
(https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2844357436)
FWIW, I'm running the current PR branch on some other OSes: https://github.com/hebasto/bitcoin-core-nightly/pull/51.
π¬ laanwj commented on pull request "common: Close non-std fds before exec in RunCommandJSON":
(https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2844368951)
> FWIW, I'm running the current PR branch on some other OSes: https://github.com/hebasto/bitcoin-core-nightly/pull/51.
Testing is very welcome, the overall approach is going to stay the same.
(https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2844368951)
> FWIW, I'm running the current PR branch on some other OSes: https://github.com/hebasto/bitcoin-core-nightly/pull/51.
Testing is very welcome, the overall approach is going to stay the same.
π¬ laanwj commented on pull request "common: Close non-std fds before exec in RunCommandJSON":
(https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2844382194)
> (written more generally, not using bitcoin-core code)
Okay, after a bit of consideration, maybe this was a bad idea: the directory iteration and string-parsing is going to be horrible.
cpp-subprocess is C++11 only, while `std::filesystem` and `std::from_chars` are C++17. i really don't want to go back to using locale-dependent number parsing functions.
(https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2844382194)
> (written more generally, not using bitcoin-core code)
Okay, after a bit of consideration, maybe this was a bad idea: the directory iteration and string-parsing is going to be horrible.
cpp-subprocess is C++11 only, while `std::filesystem` and `std::from_chars` are C++17. i really don't want to go back to using locale-dependent number parsing functions.
π AndreaLanfranchi opened a pull request: "Update .gitignore"
(https://github.com/bitcoin/bitcoin/pull/32392)
Build process on WIndows creates the "out" folder by default which is not excluded from changes tracked by git.
Also additional directories `.vs` and `.vscode` are created with files exclusively relevant to the local development environment: commits should not be dirtied by those.
This PR
* Excludes dirs used by VS family.
* Also exclude "out" build directory (default on WIndows)
(https://github.com/bitcoin/bitcoin/pull/32392)
Build process on WIndows creates the "out" folder by default which is not excluded from changes tracked by git.
Also additional directories `.vs` and `.vscode` are created with files exclusively relevant to the local development environment: commits should not be dirtied by those.
This PR
* Excludes dirs used by VS family.
* Also exclude "out" build directory (default on WIndows)
π¬ hebasto commented on pull request "Update .gitignore":
(https://github.com/bitcoin/bitcoin/pull/32392#issuecomment-2844423790)
We generally do not accept IDE-specific entries in the projectβs `.gitignore`. Please adjust your build environment by other means.
(https://github.com/bitcoin/bitcoin/pull/32392#issuecomment-2844423790)
We generally do not accept IDE-specific entries in the projectβs `.gitignore`. Please adjust your build environment by other means.
π¬ vasild commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#issuecomment-2844423967)
`cc8f239663...8ef769cc2d`: rebase and use the naming:
```cpp
bool IsConnectedToAddrPort(const std::string& addr_port);
bool IsConnectedToAddrPort(const CService& addr_port);
bool IsConnectedToAddr(const CNetAddr& addr);
```
(https://github.com/bitcoin/bitcoin/pull/32326#issuecomment-2844423967)
`cc8f239663...8ef769cc2d`: rebase and use the naming:
```cpp
bool IsConnectedToAddrPort(const std::string& addr_port);
bool IsConnectedToAddrPort(const CService& addr_port);
bool IsConnectedToAddr(const CNetAddr& addr);
```
β
fanquake closed a pull request: "Update .gitignore"
(https://github.com/bitcoin/bitcoin/pull/32392)
(https://github.com/bitcoin/bitcoin/pull/32392)
π¬ vasild commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2070021226)
Done.
(https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2070021226)
Done.
π¬ vasild commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2070024083)
#32338 was merged and these variables are not necessary now.
(https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2070024083)
#32338 was merged and these variables are not necessary now.
π¬ vasild commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#issuecomment-2844434880)
`8ef769cc2d...4f635d100d`: make the methods `const`
(https://github.com/bitcoin/bitcoin/pull/32326#issuecomment-2844434880)
`8ef769cc2d...4f635d100d`: make the methods `const`
π vasild's pull request is ready for review: "net: improve the interface around FindNode() and avoid a recursive mutex lock"
(https://github.com/bitcoin/bitcoin/pull/32326)
(https://github.com/bitcoin/bitcoin/pull/32326)
π¬ hebasto commented on pull request "common: Close non-std fds before exec in RunCommandJSON":
(https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2844437349)
@laanwj
Before undrafting this PR. could you please rebase it to include the merged https://github.com/bitcoin/bitcoin/pull/32071. Otherwise, NetBSD [tests](https://github.com/hebasto/bitcoin-core-nightly/actions/runs/14772257138) fail.
(https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2844437349)
@laanwj
Before undrafting this PR. could you please rebase it to include the merged https://github.com/bitcoin/bitcoin/pull/32071. Otherwise, NetBSD [tests](https://github.com/hebasto/bitcoin-core-nightly/actions/runs/14772257138) fail.
π¬ laanwj commented on pull request "common: Close non-std fds before exec in RunCommandJSON":
(https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2844447695)
> Before undrafting this PR. could you please rebase it to include the merged https://github.com/bitcoin/bitcoin/pull/32071. Otherwise, NetBSD [tests](https://github.com/hebasto/bitcoin-core-nightly/actions/runs/14772257138) fail.
Sure. Rebased to master.
(https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2844447695)
> Before undrafting this PR. could you please rebase it to include the merged https://github.com/bitcoin/bitcoin/pull/32071. Otherwise, NetBSD [tests](https://github.com/hebasto/bitcoin-core-nightly/actions/runs/14772257138) fail.
Sure. Rebased to master.
π¬ AndreaLanfranchi commented on pull request "Update .gitignore":
(https://github.com/bitcoin/bitcoin/pull/32392#issuecomment-2844448468)
> We generally do not accept IDE-specific entries in the projectβs `.gitignore`. Please adjust your build environment by other means.
Thank you for the clarification.
What about instead the default "out" build dir in Windows (VS toolchain) ... that would require changes in presets and **is** IDE and OS dependant.
(https://github.com/bitcoin/bitcoin/pull/32392#issuecomment-2844448468)
> We generally do not accept IDE-specific entries in the projectβs `.gitignore`. Please adjust your build environment by other means.
Thank you for the clarification.
What about instead the default "out" build dir in Windows (VS toolchain) ... that would require changes in presets and **is** IDE and OS dependant.
π¬ vasild commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2070059886)
Done in latest push.
(https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2070059886)
Done in latest push.
π fanquake merged a pull request: "doc: Fix test_bitcoin path"
(https://github.com/bitcoin/bitcoin/pull/32389)
(https://github.com/bitcoin/bitcoin/pull/32389)
π hebasto approved a pull request: "doc: Fix test_bitcoin path"
(https://github.com/bitcoin/bitcoin/pull/32389#pullrequestreview-2809473551)
ACK 6cbc28b8dd629062950f195facc009fd8ba86310.
(https://github.com/bitcoin/bitcoin/pull/32389#pullrequestreview-2809473551)
ACK 6cbc28b8dd629062950f195facc009fd8ba86310.
π¬ hebasto commented on pull request "doc: Fix test_bitcoin path":
(https://github.com/bitcoin/bitcoin/pull/32389#issuecomment-2844458990)
Backport to 29.x?
(https://github.com/bitcoin/bitcoin/pull/32389#issuecomment-2844458990)
Backport to 29.x?
π¬ Sjors commented on pull request "policy: allow more than one OP_RETURN outputs per tx":
(https://github.com/bitcoin/bitcoin/pull/32381#issuecomment-2844569676)
@ajtowns yes, that would be even simpler than this PR, but would still run into the above objection:
> Concept NACK. At a minimum, it would need to tally up the total length
(https://github.com/bitcoin/bitcoin/pull/32381#issuecomment-2844569676)
@ajtowns yes, that would be even simpler than this PR, but would still run into the above objection:
> Concept NACK. At a minimum, it would need to tally up the total length
π¬ fanquake commented on issue "test: functional test failures under -DENABLE_WALLET=OFF":
(https://github.com/bitcoin/bitcoin/issues/32347#issuecomment-2844572481)
> could you share the build/test/config.ini file?
```bash
# Copyright (c) 2013-2016 The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
# These environment variables are set by the build process and read by
# test/*/test_runner.py and test/util/rpcauth-test.py
[environment]
CLIENT_NAME=Bitcoin Core
CLIENT_BUGREPORT=https://github.com/bitcoin/bitcoin/issues
SRCDIR=/root/bitcoin
BUIL
...
(https://github.com/bitcoin/bitcoin/issues/32347#issuecomment-2844572481)
> could you share the build/test/config.ini file?
```bash
# Copyright (c) 2013-2016 The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
# These environment variables are set by the build process and read by
# test/*/test_runner.py and test/util/rpcauth-test.py
[environment]
CLIENT_NAME=Bitcoin Core
CLIENT_BUGREPORT=https://github.com/bitcoin/bitcoin/issues
SRCDIR=/root/bitcoin
BUIL
...