💬 josibake commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282780835)
These commands for unstalling via a venv dont work for me on macos or linux. I'm also not sure about installing via pip, since we need to be sure we are installing the package with capnproto bundled. Instead, I think `pycapnp` should be installed from source (venv or otherwise):
```
git clone https://github.com/capnproto/pycapnp.git
cd pycapnp
pip install . -C force-bundled-libcapnp=True
```
This approach worked on macos and linux for me (inside a venv on both).
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282780835)
These commands for unstalling via a venv dont work for me on macos or linux. I'm also not sure about installing via pip, since we need to be sure we are installing the package with capnproto bundled. Instead, I think `pycapnp` should be installed from source (venv or otherwise):
```
git clone https://github.com/capnproto/pycapnp.git
cd pycapnp
pip install . -C force-bundled-libcapnp=True
```
This approach worked on macos and linux for me (inside a venv on both).
💬 josibake commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282782619)
AFAICT, this only works when capnproto is bundled inside `pycapnp`.
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282782619)
AFAICT, this only works when capnproto is bundled inside `pycapnp`.
👍 josibake approved a pull request: "Add functional test for IPC interface"
(https://github.com/bitcoin/bitcoin/pull/33201#pullrequestreview-3128923018)
ACK https://github.com/bitcoin/bitcoin/pull/33201/commits/3f7ff4849e6165a6bcc4d5977c35a467fd7fc232
Thanks for picking this up! I think the docs and multiplatform support still need a bit of work but that can be tackled in a follow-up(s), considering the main benefit I see here is getting a basic functional test in place that can be extended later.
(https://github.com/bitcoin/bitcoin/pull/33201#pullrequestreview-3128923018)
ACK https://github.com/bitcoin/bitcoin/pull/33201/commits/3f7ff4849e6165a6bcc4d5977c35a467fd7fc232
Thanks for picking this up! I think the docs and multiplatform support still need a bit of work but that can be tackled in a follow-up(s), considering the main benefit I see here is getting a basic functional test in place that can be extended later.
💬 fjahr commented on pull request "index: Fix coinstats overflow":
(https://github.com/bitcoin/bitcoin/pull/30469#discussion_r2282801398)
Fixed, I missed that this had changed.
(https://github.com/bitcoin/bitcoin/pull/30469#discussion_r2282801398)
Fixed, I missed that this had changed.
💬 josibake commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#issuecomment-3197489074)
> the include paths used in the python test are a guess. It'd be nice to pass through the location of the capnp installation from the build system to the tests
I took a stab at this, ended up being harder than expected and I don't yet have something that works. But I think this is the correct longterm approach, instead of trying to guess in the functional test itself.
(https://github.com/bitcoin/bitcoin/pull/33201#issuecomment-3197489074)
> the include paths used in the python test are a guess. It'd be nice to pass through the location of the capnp installation from the build system to the tests
I took a stab at this, ended up being harder than expected and I don't yet have something that works. But I think this is the correct longterm approach, instead of trying to guess in the functional test itself.
💬 josibake commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282815035)
FWIW, this worked for me on macos: https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282780835. I'd recommend we have the docs tell users to install `pycapnp` from source, since that should "just work" everywhere due to this line: `cpp_capnp_dir = Path(capnp.__path__[0]).parent`.
Eventually, we can replace this recommendation once we have CMake automatically finding the include directory for capnproto and making the test framework aware.
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282815035)
FWIW, this worked for me on macos: https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282780835. I'd recommend we have the docs tell users to install `pycapnp` from source, since that should "just work" everywhere due to this line: `cpp_capnp_dir = Path(capnp.__path__[0]).parent`.
Eventually, we can replace this recommendation once we have CMake automatically finding the include directory for capnproto and making the test framework aware.
💬 fanquake commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282839448)
> `venv/bin/python3 test/functional/interface_ipc.py`
This should be `build/test/functional/interface_ipc.py`. Otherwise the test wont run.
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282839448)
> `venv/bin/python3 test/functional/interface_ipc.py`
This should be `build/test/functional/interface_ipc.py`. Otherwise the test wont run.
💬 ismaelsadeeq commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282841908)
@josibake
I did that initially but was having some errors
<details>
<summary>logs
</summary>
```terminal
(.venv) abubakarismail@Abubakars-MacBook-Pro pycapnp % cmake --version
cmake version 4.1.0
CMake suite maintained and supported by Kitware (kitware.com/cmake).
(.venv) abubakarismail@Abubakars-MacBook-Pro pycapnp % pip install . -C force-bundled-libcapnp=True
Processing /Users/abubakarismail/Desktop/Work/bitcoin-dev/py-bitcoin-ipc-client/pycapnp
Installing build dependen
...
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282841908)
@josibake
I did that initially but was having some errors
<details>
<summary>logs
</summary>
```terminal
(.venv) abubakarismail@Abubakars-MacBook-Pro pycapnp % cmake --version
cmake version 4.1.0
CMake suite maintained and supported by Kitware (kitware.com/cmake).
(.venv) abubakarismail@Abubakars-MacBook-Pro pycapnp % pip install . -C force-bundled-libcapnp=True
Processing /Users/abubakarismail/Desktop/Work/bitcoin-dev/py-bitcoin-ipc-client/pycapnp
Installing build dependen
...
💬 stickies-v commented on issue "ci: failure in `logging_tests`":
(https://github.com/bitcoin/bitcoin/issues/33195#issuecomment-3197554181)
> There are 1023 aaaaaa...aaaa log lines (there should be 1024) in the CI logs
It seems the 835th iteration got corrupted (scroll down a bit):
```
[09:01:31.798] aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa��������aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
...
(https://github.com/bitcoin/bitcoin/issues/33195#issuecomment-3197554181)
> There are 1023 aaaaaa...aaaa log lines (there should be 1024) in the CI logs
It seems the 835th iteration got corrupted (scroll down a bit):
```
[09:01:31.798] aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa��������aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
...
💬 ismaelsadeeq commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282849226)
When you build using depends with the autotools it will work I think?
So both options maybe?
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282849226)
When you build using depends with the autotools it will work I think?
So both options maybe?
💬 Aris-Ritz commented on something "":
(https://github.com/bitcoin/bitcoin/commit/74fc236dab438d6f970eb42e58fff8e5198a2f7a#commitcomment-164170572)
74fc236dab438d6f970eb42e58fff8e5198a2f7a
(https://github.com/bitcoin/bitcoin/commit/74fc236dab438d6f970eb42e58fff8e5198a2f7a#commitcomment-164170572)
74fc236dab438d6f970eb42e58fff8e5198a2f7a
💬 Aris-Ritz commented on something "":
(https://github.com/bitcoin/bitcoin/commit/74fc236dab438d6f970eb42e58fff8e5198a2f7a#commitcomment-164170602)
test/functional/test_framework/test_framework.py
(https://github.com/bitcoin/bitcoin/commit/74fc236dab438d6f970eb42e58fff8e5198a2f7a#commitcomment-164170602)
test/functional/test_framework/test_framework.py
💬 fanquake commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282853664)
> I did that initially but was having some errors
It looks like the `pycapnp` package doesn't currently work with Python 3.13 (https://github.com/capnproto/pycapnp/issues/372).
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282853664)
> I did that initially but was having some errors
It looks like the `pycapnp` package doesn't currently work with Python 3.13 (https://github.com/capnproto/pycapnp/issues/372).
✅ stickies-v closed a pull request: "test: ensure logs are flushed before reading them"
(https://github.com/bitcoin/bitcoin/pull/33198)
(https://github.com/bitcoin/bitcoin/pull/33198)
💬 stickies-v commented on pull request "test: ensure logs are flushed before reading them":
(https://github.com/bitcoin/bitcoin/pull/33198#issuecomment-3197570763)
This PR seems to be on the wrong track, see [this comment](https://github.com/bitcoin/bitcoin/issues/33195#issuecomment-3192883289) and further. Closing.
(https://github.com/bitcoin/bitcoin/pull/33198#issuecomment-3197570763)
This PR seems to be on the wrong track, see [this comment](https://github.com/bitcoin/bitcoin/issues/33195#issuecomment-3192883289) and further. Closing.
💬 fanquake commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282857319)
Documentation being added to master shouldn't be for Autotools.
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282857319)
Documentation being added to master shouldn't be for Autotools.
💬 fjahr commented on pull request "Embed default ASMap as binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#issuecomment-3197638496)
Updated the map to that of the latest run: https://github.com/asmap/asmap-data/pull/30
(https://github.com/bitcoin/bitcoin/pull/28792#issuecomment-3197638496)
Updated the map to that of the latest run: https://github.com/asmap/asmap-data/pull/30
💬 purpleKarrot commented on pull request "cmake: Drop python dependency for translate":
(https://github.com/bitcoin/bitcoin/pull/33209#issuecomment-3197677259)
Changes:
* xgettext is instructed to sort the output by file
* the messages are no longer sorted by cmake
* multiline strings are joined
It was tested on both arch and ubuntu 24.04 to produce the same output (Older versions of xgettext split the strings at different positions, joining the multiline strings results in the same output).
(https://github.com/bitcoin/bitcoin/pull/33209#issuecomment-3197677259)
Changes:
* xgettext is instructed to sort the output by file
* the messages are no longer sorted by cmake
* multiline strings are joined
It was tested on both arch and ubuntu 24.04 to produce the same output (Older versions of xgettext split the strings at different positions, joining the multiline strings results in the same output).
💬 ismaelsadeeq commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282939552)
> It looks like the pycapnp package doesn't currently work with Python 3.13 (https://github.com/capnproto/pycapnp/issues/372)
This issue seems unrelated.
The actual issue was that my ninja installation was not linked.
```
brew link --overwrite ninja
```
And then @josibake recommended documentation should work.
Thanks @josibake for helping me dubug.
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282939552)
> It looks like the pycapnp package doesn't currently work with Python 3.13 (https://github.com/capnproto/pycapnp/issues/372)
This issue seems unrelated.
The actual issue was that my ninja installation was not linked.
```
brew link --overwrite ninja
```
And then @josibake recommended documentation should work.
Thanks @josibake for helping me dubug.
👍 ismaelsadeeq approved a pull request: "Add functional test for IPC interface"
(https://github.com/bitcoin/bitcoin/pull/33201#pullrequestreview-3129165105)
ACK 3f7ff4849e6165a6bcc4d5977c35a467fd7fc232
(https://github.com/bitcoin/bitcoin/pull/33201#pullrequestreview-3129165105)
ACK 3f7ff4849e6165a6bcc4d5977c35a467fd7fc232