cmake: Move internal binaries from bin/ to libexec/ #31679

pull ryanofsky wants to merge 1 commits into bitcoin:master from ryanofsky:pr/libexec changing 7 files +14 −8
  1. ryanofsky commented at 5:01 pm on January 17, 2025: contributor

    Currently when “make install” or “cmake –install” are run, various internal binaries that are confusing and not typically useful are installed to ${CMAKE_INSTALL_PREFIX}/bin and can wind up on the system PATH. This PR moves internal binaries out of bin/ into libexec/ where they will still be accessible but will not be automatically placed on the PATH or be confused with more useful binaries. The PR also adds an install rule for the bitcoin-chainstate binary. After this PR binaries installed to bin/ are:

    • bitcoin-cli
    • bitcoind
    • bitcoin-qt
    • bitcoin-tx
    • bitcoin-util
    • bitcoin-wallet

    And binaries installed to libexec/ are:

    • bench_bitcoin
    • bitcoin-chainstate
    • bitcoin-gui
    • bitcoin-node
    • test_bitcoin
    • test_bitcoin-qt

    In the future if #31375 gets merged, there will be a new bitcoin wrapper executable in bin/ that can be used to call other binaries, and with that present, we could consider moving other binaries from bin/ to libexec/ and recommending that most users should use the wrapper instead of calling the different utilities directly. But this PR should make still make sense without #31375 to organize the binaries better.

    This PR is part of the process separation project.

  2. DrahtBot commented at 5:01 pm on January 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/31679.

    Reviews

    See the guideline for information on the review process. A summary of reviews will appear here.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #31349 (ci: detect outbound internet traffic generated while running tests by vasild)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  3. DrahtBot added the label Build system on Jan 17, 2025
  4. DrahtBot added the label CI failed on Jan 17, 2025
  5. DrahtBot commented at 7:52 am on January 18, 2025: contributor
    CI failed with [17:52:38.033] /ci_container_base/ci/test/03_test_script.sh: line 154: /ci_container_base/ci/scratch/out/bin/test_bitcoin: No such file or directory
  6. luke-jr commented at 3:25 pm on January 23, 2025: member
    libexec is usually used for binaries run by programs rather than people. I don’t think most of the ones you’re moving fit that, and should remain in bin.
  7. Sjors commented at 7:46 am on January 24, 2025: member

    Run by people is a higher bar than “not typically useful”, but could make sense.

    If there are no binaries for which it currently makes sense to put them in libexec, since they’re all called by people, then it seems this PR has be based on #31375.

  8. ryanofsky commented at 5:57 pm on January 28, 2025: contributor

    The idea that an executable placed in libexec/ should be called by other executables is a good rule of thumb but not a requirement of some kind. (And if it were a requirement, it would be satisfied by #31375.)

    The technical difference between binaries installed to bin/ and binaries installed to libexec/ is that binaries in bin/ get exposed on the PATH while binaries in libexec/ don’t.

    There is nothing wrong with binaries in libexec/ being documented and called used by users and packagers and other tools and scripts. Not every executable that a user could conceivably call needs to be present in PATH.

    Packages like postfix have manpages for internal executables installed in libexec/ (anvil, bounce, smtpd, etc). Git subcommands can be called manually from libexec and have manpages of their own, too. I would say postfix and git are pretty much archetypes of packages designed around unix principles and there is nothing novel about installing low-level executables in libexec/.

    With this PR, two sets of binaries are installed to libexec/ if they have been enabled in cmake:

    1. multiprocess binaries bitcoin-node and bitcoin-gui
    2. testing binaries bench_bitcoin test_bitcoin test_bitcoin-qt.

    I can’t think of a reason why multiprocess binaries should be installed on PATH, but if someone wants to make that case I’m ready to hear it. As an alternative to this PR, they could be installed in bin/ with different names like bitoin-node-multiprocess or to a completely new folder.

    For the test and bench binaries I just think it’s confusing and not useful to install these on the PATH. I think this is true today and will be especially true after #31375.

  9. ryanofsky force-pushed on Jan 30, 2025
  10. ryanofsky commented at 3:05 am on January 30, 2025: contributor
    Updated 2a952710661a1990ed1c24b0ebd16de8ac0df87c -> 47a872236e070814ad74922d3f8a653e1c6af968 (pr/libexec.1 -> pr/libexec.2, compare) to fix CI error
  11. DrahtBot removed the label CI failed on Jan 30, 2025
  12. DrahtBot added the label Needs rebase on Feb 12, 2025
  13. ryanofsky force-pushed on Feb 13, 2025
  14. ryanofsky commented at 6:48 pm on February 13, 2025: contributor
    Rebased 47a872236e070814ad74922d3f8a653e1c6af968 -> d2ceb2e0735a2c8343f8316b55fac55323aba62c (pr/libexec.2 -> pr/libexec.3, compare) due to conflict with #31834
  15. DrahtBot removed the label Needs rebase on Feb 13, 2025
  16. cmake: Move internal binaries from bin/ to libexec/ 
    Currently when "make install" or "cmake --install" are run, various internal
binaries that are confusing and not typically useful and installed to
`${CMAKE_INSTALL_PREFIX}/bin` and can wind up on the system PATH. This PR moves
internal binaries out of bin/ into libexec/ where they will still be accessible
but will not be automatically placed on the PATH or be confused with more
useful binaries. The PR also adds an install rule for the bitcoin-chainstate
binary. After this PR binaries installed to bin/ are:

- bitcoin-cli
- bitcoind
- bitcoin-qt
- bitcoin-tx
- bitcoin-util
- bitcoin-wallet

And binaries installed to libexec/ are:

- bench_bitcoin
- bitcoin-gui
- bitcoin-node
- test_bitcoin
- test_bitcoin-qt

In the future if #31375 gets merged, there will be a new `bitcoin` wrapper
executable in bin/ that can be used to call other binaries, and with that
present, we could consider moving other binaries from bin/ to libexec/ and
recommending that most users should use the wrapper instead of calling the
different utilities directly. But this PR should make sense with or without
#31375.
    45bfd97ec7
  17. DrahtBot added the label Needs rebase on Feb 14, 2025
  18. ryanofsky force-pushed on Feb 14, 2025
  19. ryanofsky commented at 3:19 pm on February 14, 2025: contributor
    Rebased d2ceb2e0735a2c8343f8316b55fac55323aba62c -> 45bfd97ec7c9991f41673d79b01277bfd940e64e (pr/libexec.3 -> pr/libexec.4, compare) due to conflict with #31844
  20. DrahtBot removed the label Needs rebase on Feb 14, 2025


ryanofsky DrahtBot luke-jr Sjors

Labels
Build system

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-02-22 06:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me