💬 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
💬 ryanofsky commented on pull request "ipc: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-3197715052)
Rebased e1f139bd5fc9ee737d2b08307fca2b33354c5747 -> 1e8a83247c861a9485e44e7d58668e7a9a95e61d ([`pr/mine.30`](https://github.com/ryanofsky/bitcoin/commits/pr/mine.30) -> [`pr/mine.31`](https://github.com/ryanofsky/bitcoin/commits/pr/mine.31), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/mine.30-rebase..pr/mine.31) due to conflict with #31679. Also make some framework improvements to simplify the test and avoid duplicating code with #32297 and #33201
(https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-3197715052)
Rebased e1f139bd5fc9ee737d2b08307fca2b33354c5747 -> 1e8a83247c861a9485e44e7d58668e7a9a95e61d ([`pr/mine.30`](https://github.com/ryanofsky/bitcoin/commits/pr/mine.30) -> [`pr/mine.31`](https://github.com/ryanofsky/bitcoin/commits/pr/mine.31), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/mine.30-rebase..pr/mine.31) due to conflict with #31679. Also make some framework improvements to simplify the test and avoid duplicating code with #32297 and #33201
✅ davidgumberg closed a pull request: "build: Enable -DCMAKE_EXPORT_COMPILE_COMMANDS=1"
(https://github.com/bitcoin/bitcoin/pull/33187)
(https://github.com/bitcoin/bitcoin/pull/33187)
💬 davidgumberg commented on pull request "build: Enable -DCMAKE_EXPORT_COMPILE_COMMANDS=1":
(https://github.com/bitcoin/bitcoin/pull/33187#issuecomment-3197770885)
I disagree with the reasons given for not enabling this, in particular I think what is underestimated is the number of people that use LSP tooling, and the range of development environments that support `compile_commands.json` metadata, but I'll close this for now as it lacks conceptual support.
(https://github.com/bitcoin/bitcoin/pull/33187#issuecomment-3197770885)
I disagree with the reasons given for not enabling this, in particular I think what is underestimated is the number of people that use LSP tooling, and the range of development environments that support `compile_commands.json` metadata, but I'll close this for now as it lacks conceptual support.
💬 ryanofsky commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282987892)
re: https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282752457
On nixos, I needed to change this as follows:
```diff
- cpp_capnp_dir = Path(capnp.__path__[0]).parent
+ capnp_dir = Path(shutil.which("capnp")).parent.parent / "include"
src_dir = Path(self.config['environment']['SRCDIR']) / "src"
mp_dir = src_dir / "ipc" / "libmultiprocess" / "include"
- imports = ["/usr/include", str(cpp_capnp_dir), str(src_dir), str(mp_dir)]
+ im
...
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282987892)
re: https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282752457
On nixos, I needed to change this as follows:
```diff
- cpp_capnp_dir = Path(capnp.__path__[0]).parent
+ capnp_dir = Path(shutil.which("capnp")).parent.parent / "include"
src_dir = Path(self.config['environment']['SRCDIR']) / "src"
mp_dir = src_dir / "ipc" / "libmultiprocess" / "include"
- imports = ["/usr/include", str(cpp_capnp_dir), str(src_dir), str(mp_dir)]
+ im
...
💬 ryanofsky commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282984358)
re: https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282782619
> AFAICT, this only works when capnproto is bundled inside `pycapnp`.
Can confirm at least on nixos there are no .capnp files in this directory. Seems harmless to keep this if it helps other platforms, but would be good to document where it's necessary.
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282984358)
re: https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282782619
> AFAICT, this only works when capnproto is bundled inside `pycapnp`.
Can confirm at least on nixos there are no .capnp files in this directory. Seems harmless to keep this if it helps other platforms, but would be good to document where it's necessary.
💬 ryanofsky commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282980169)
In commit "ci: add functional test for IPC interface" (3f7ff4849e6165a6bcc4d5977c35a467fd7fc232)
This seems like it leaves behind an empty directory after the test runs. In any case it should be possible drop this code and simplify this commit by cherrypicking three commits from #30437:
- c3d82ef8fa947f108502ec80c522b4bd0084dd4c test: add is_ipc_enabled() and skip_if_no_ipc() functions
- 3af4d9a50ab008aa3974c580a831095e2ba17042 test: Provide path to `bitcoin` binary
- 6d0103472f1d691e8e1
...
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282980169)
In commit "ci: add functional test for IPC interface" (3f7ff4849e6165a6bcc4d5977c35a467fd7fc232)
This seems like it leaves behind an empty directory after the test runs. In any case it should be possible drop this code and simplify this commit by cherrypicking three commits from #30437:
- c3d82ef8fa947f108502ec80c522b4bd0084dd4c test: add is_ipc_enabled() and skip_if_no_ipc() functions
- 3af4d9a50ab008aa3974c580a831095e2ba17042 test: Provide path to `bitcoin` binary
- 6d0103472f1d691e8e1
...
💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282994052)
Good idea, done.
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282994052)
Good idea, done.
💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282994684)
I added a comment to explain that Python 3.13 doesn't currently work.
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282994684)
I added a comment to explain that Python 3.13 doesn't currently work.
💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282994842)
Done.
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282994842)
Done.
💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282995076)
Thanks, added!
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282995076)
Thanks, added!
💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282995525)
Cool, used this in the instructions.
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282995525)
Cool, used this in the instructions.
💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282995874)
Fixed.
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282995874)
Fixed.
💬 jaredchapman4216-create commented on pull request "coins,refactor: Reduce `getblockstats` RPC UTXO overhead estimation":
(https://github.com/bitcoin/bitcoin/pull/31449#issuecomment-3197797741)
packages/components-shared/src/components/LoaderStakeEngine.svelteVerify your account
(https://github.com/bitcoin/bitcoin/pull/31449#issuecomment-3197797741)
packages/components-shared/src/components/LoaderStakeEngine.svelteVerify your account
💬 ryanofsky commented on pull request "build: Enable ENABLE_IPC option by default":
(https://github.com/bitcoin/bitcoin/pull/33190#issuecomment-3197824813)
Updated 732c134bfc1208377f449e5956e01dd8b78d73d9 -> 5a34156ab65ac7460c26141bb29651477e000d19 ([`pr/ipc-default.3`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-default.3) -> [`pr/ipc-default.4`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-default.4), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/ipc-default.3..pr/ipc-default.4) applying fanquake's suggestion https://github.com/bitcoin/bitcoin/pull/33190#issuecomment-3197036114 to update valgrind jobs, and sjors suggest
...
(https://github.com/bitcoin/bitcoin/pull/33190#issuecomment-3197824813)
Updated 732c134bfc1208377f449e5956e01dd8b78d73d9 -> 5a34156ab65ac7460c26141bb29651477e000d19 ([`pr/ipc-default.3`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-default.3) -> [`pr/ipc-default.4`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-default.4), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/ipc-default.3..pr/ipc-default.4) applying fanquake's suggestion https://github.com/bitcoin/bitcoin/pull/33190#issuecomment-3197036114 to update valgrind jobs, and sjors suggest
...
💬 janb84 commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2283046276)
This branch / patch works for me on Nix on macOS. Hint, do not forget to `import shutil` when applying the patch below.
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2283046276)
This branch / patch works for me on Nix on macOS. Hint, do not forget to `import shutil` when applying the patch below.
💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2283047618)
@ryanofsky Thanks! Will address later today.
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2283047618)
@ryanofsky Thanks! Will address later today.
💬 brunoerg commented on pull request "fuzz: enhance wallet_fees by mocking mempool stuff":
(https://github.com/bitcoin/bitcoin/pull/33210#discussion_r2283103326)
> Is the 256-byte value used at all? If not, seems easier to just hardcode a value (like 0 or 1 or 2), or use a deterministic random comtext?
Good catch. I don't think it's used at all, we can simply use a hardcoded value instead of spending part of the buffer with it.
(https://github.com/bitcoin/bitcoin/pull/33210#discussion_r2283103326)
> Is the 256-byte value used at all? If not, seems easier to just hardcode a value (like 0 or 1 or 2), or use a deterministic random comtext?
Good catch. I don't think it's used at all, we can simply use a hardcoded value instead of spending part of the buffer with it.