doc: Add deps install notes for multiprocess #32293

pull TheCharlatan wants to merge 1 commits into bitcoin:master from TheCharlatan:install_multiprocess_deps changing 2 files +23 −0
  1. TheCharlatan commented at 12:48 pm on April 17, 2025: contributor
    These just mirror the content in src/ipc/libmultiprocess/doc/install.md
  2. DrahtBot commented at 12:48 pm on April 17, 2025: contributor

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32293.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK Sjors
    Stale ACK laanwj, ryanofsky

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

    LLM Linter

    compiled··with → compiled·with
    (Replace the double space between “compiled” and “with” with a single space; occurs in the apt-get line.)

    compiled··with → compiled·with
    (Replace the double space between “compiled” and “with” with a single space; occurs in the dnf line.)

  3. DrahtBot added the label Docs on Apr 17, 2025
  4. Sjors commented at 2:52 pm on April 17, 2025: member

    utACK ff4ea070aa4b1ed1ebb6dcf62e01c9dd57319f69

    If this lands before #31802 then that needs to be updated to make this the default instruction.

  5. laanwj approved
  6. laanwj commented at 2:57 pm on April 17, 2025: member

    ACK ff4ea070aa4b1ed1ebb6dcf62e01c9dd57319f69

    This needs to be mentioned. i think adding paragraph after paragraph of dependencies to the build docs like this becomes a bit of a mess for someone without context “what do i actually have to install”. Could make a table at some point what needs to be installed for exactly which build option. But that’s not something that needs to be resolved here.

  7. ryanofsky commented at 3:59 pm on April 17, 2025: contributor

    Code review ff4ea070aa4b1ed1ebb6dcf62e01c9dd57319f69

    Thanks for adding this. Somehow I thought the capnproto dependencies were already listed, but that change is sitting in a commit in #10102 (bbcbe85cef21049b5f321265264e2c4fe394d209).

    Few suggestions:

    • Would suggest changing “Multiprocess Dependencies” to “IPC Dependencies” since the only thing they are use for on master is -ipcbind feature, and multiprocess functionality is a superset of IPC functionality.
    • Might be good to state explicitly these are only needed if ENABLE_IPC is on.
    • Might be good to update doc/dependencies.md like bbcbe85cef21049b5f321265264e2c4fe394d209
    • Might be good to add For more information on IPC, see: [multiprocess.md](multiprocess.md) as seems to be done for the ZMQ above
  8. TheCharlatan force-pushed on Apr 17, 2025
  9. TheCharlatan commented at 4:41 pm on April 17, 2025: contributor

    Thanks for adding this. Somehow I thought the capnproto dependencies were already listed, but that change is sitting in a commit in #10102 (https://github.com/bitcoin/bitcoin/commit/bbcbe85cef21049b5f321265264e2c4fe394d209).

    Heh, I was looking around if either Sjors or you already did this in one of the open PRs after forgetting to install capnproto on a new machine, but forgot the check the original PR…

    Updated ff4ea070aa4b1ed1ebb6dcf62e01c9dd57319f69 -> 5ae18914805f0a78dde85cfe2a7876c4e52c0fe7 (install_multiprocess_deps_0 -> install_multiprocess_deps_1, compare)

    • Addressed @ryanofsky’s comment,
      • Calling it an IPC dependency, instead of a multiprocess dependency
      • Mention that it is only useful in combination with -DENABLE_IPC=ON
      • Add line to doc/dependencies.md
      • Link to multiprocess.md in build-osx file.
  10. Sjors commented at 4:48 pm on April 17, 2025: member
    re-ACK 5ae18914805f0a78dde85cfe2a7876c4e52c0fe7
  11. DrahtBot requested review from laanwj on Apr 17, 2025
  12. in doc/dependencies.md:39 in 5ae1891480 outdated 
    35@@ -36,3 +36,4 @@ Bitcoin Core requires one of the following compilers.
36 | [SQLite](../depends/packages/sqlite.mk) (wallet) | [link](https://sqlite.org) | [3.38.5](https://github.com/bitcoin/bitcoin/pull/25378) | [3.7.17](https://github.com/bitcoin/bitcoin/pull/19077) | No |
37 | Python (scripts, tests) | [link](https://www.python.org) | N/A | [3.10](https://github.com/bitcoin/bitcoin/pull/30527) | No |
38 | [systemtap](../depends/packages/systemtap.mk) ([tracing](tracing.md)) | [link](https://sourceware.org/systemtap/) | [4.8](https://github.com/bitcoin/bitcoin/pull/26945)| N/A | No |
39+| [Cap'n Proto](../depends/packages/capnp.mk) | [link](https://capnproto.org) | [0.6.1](https://capnproto.org/install.html) | 0.5.3 | Yes |
    ryanofsky commented at 4:56 pm on April 17, 2025:

    In commit “doc: Add deps install notes for multiprocess” (5ae18914805f0a78dde85cfe2a7876c4e52c0fe7)

    May want to update version used to 1.1.0 to match depends https://github.com/bitcoin/bitcoin/blob/bfeacc18b36132ba8ac70142133cd6c0e63b6763/depends/packages/native_capnp.mk#L2

    fanquake commented at 5:01 pm on April 17, 2025:
    Shouldn’t this be “no” for runtime? If this goes into a release, it won’t be a runtime dep.
    TheCharlatan commented at 5:29 pm on April 17, 2025:
    Mmh, shouldn’t that be flipped once the PR for adding it to the release is actually in?
    fanquake commented at 5:43 pm on April 17, 2025:
    In that case, adding this seems premature, as this document should represent the state of the release binaries.
    ryanofsky commented at 5:46 pm on April 17, 2025:
    It looks like this should be changed to “no” to be consistent with the rest of the table since qt, sqlite, etc also have “no”. I would think of these all as runtime dependences, but IIUC the distinction this is making is that all of these are statically linked but freetype and fontconfig are dynamically linked.
    ryanofsky commented at 5:49 pm on April 17, 2025:
    Does also seem ok to drop this to avoid implying it is enabled in release binaries. Could add it in #31802
    TheCharlatan commented at 6:29 pm on April 17, 2025:
    Dropped it for now.
    Sjors commented at 7:32 am on April 18, 2025:
    I made a note to add it.
  13. ryanofsky approved
  14. ryanofsky commented at 4:57 pm on April 17, 2025: contributor
    Code review ACK 5ae18914805f0a78dde85cfe2a7876c4e52c0fe7. Thanks for the change and updates!
  15. doc: Add deps install notes for multiprocess 
    These just mirror the content in src/ipc/libmultiprocess/doc/install.md
    7f5a35cf4b
  16. TheCharlatan force-pushed on Apr 17, 2025
  17. TheCharlatan commented at 6:29 pm on April 17, 2025: contributor

    Updated 5ae18914805f0a78dde85cfe2a7876c4e52c0fe7 -> 7f5a35cf4b31f176ff9138d21f3ec677ae7d1bcf (install_multiprocess_deps_1 -> install_multiprocess_deps_2, compare)

    • Dropped the line adding capnproto to doc/dependencies.md. This can be done once it is enabled in release binaries.
  18. Sjors commented at 7:33 am on April 18, 2025: member
    re-ACK 7f5a35cf4b31f176ff9138d21f3ec677ae7d1bcf
  19. DrahtBot requested review from ryanofsky on Apr 18, 2025


TheCharlatan DrahtBot Sjors laanwj ryanofsky fanquake


laanwj ryanofsky

Labels
Docs

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2025-04-19 06:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me