cmake: Use target file name for manpage installation #35515

pull hebasto wants to merge 1 commits into bitcoin:master from hebasto:260611-cmake-install changing 1 files +1 −1
  1. hebasto commented at 3:53 PM on June 11, 2026: member

    The manpage file name is expected to coincide with the name of the installed executable file. The latter defaults to the logical target name, but diverges from it if any of the following target properties is set:

    Use the $<TARGET_FILE_BASE_NAME> generator expression, which resolves all of the properties above, instead of the logical target name.

    No change in behavior for any current target, as none of them sets any of these properties.


    Found this useful when replacing the bitcoin-qt target with the new bitcoin-qml one in the QML GUI staging branch.

  2. cmake: Use target file name for manpage installation
    The manpage file name is expected to coincide with the name of the
    installed executable file. The latter defaults to the logical target
    name, but diverges from it if any of the following target properties
    is set:
     - `OUTPUT_NAME`
     - `OUTPUT_NAME_<CONFIG>`
     - `RUNTIME_OUTPUT_NAME`
     - `RUNTIME_OUTPUT_NAME_<CONFIG>`
    
    Use the `$<TARGET_FILE_BASE_NAME>` generator expression, which resolves
    all of the properties above, instead of the logical target name.
    
    No change in behavior for any current target, as none of them sets any
    of these properties.
    410862154e
  3. hebasto added the label Build system on Jun 11, 2026
  4. DrahtBot commented at 3:53 PM on June 11, 2026: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage & Benchmarks

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

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline and AI policy for information on the review process.

    Type Reviewers
    ACK ryanofsky, BrandonOdiwuor
    Concept ACK 151henry151

    If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

  5. fanquake commented at 9:08 AM on June 12, 2026: member

    Found this useful when replacing the bitcoin-qt target with the new bitcoin-qml one in the QML GUI staging branch.

    Somewhat tangential to the change here, but are we renaming bitcoin-qt to bitcoin-qml? Couldn't the GUI just stay named bitcoin-qt, given that the new QML code is just a drop-in replacement, and we aren't going to ship/have two different qt binaries at any point?

  6. hebasto commented at 9:29 AM on June 12, 2026: member

    Found this useful when replacing the bitcoin-qt target with the new bitcoin-qml one in the QML GUI staging branch.

    Somewhat tangential to the change here, but are we renaming bitcoin-qt to bitcoin-qml?

    The current QML GUI staging branch retains src/qt/CMakeLists.txt until its final removal (which hasn't done yet). The new src/qml/CMakeLists.txt introduces new targets that require unique logical names: the bitcoin_qml library and the bitcoin-qml executable.

    Here is the relevant code snippet:

    # Use the `bitcoin-qml` target as a drop-in replacement
    # for the legacy QtWidgets-based `bitcoin-qt`.
    set_target_properties(bitcoin-qt PROPERTIES
      OUTPUT_NAME "bitcoin-qt-legacy"
      EXCLUDE_FROM_ALL TRUE
    )
    set_target_properties(bitcoin-qml PROPERTIES
      OUTPUT_NAME "bitcoin-qt"
    )
    

    ... we aren't going to ship/have two different qt binaries at any point?

    No, we are not.

  7. ryanofsky approved
  8. ryanofsky commented at 9:20 PM on June 15, 2026: contributor

    Code review ACK 410862154e58c3bfd7ec78f15d208f0742cb1f1e but concept -0.

    This change adds complexity and invites less consistent naming, and the PR description claims it is useful without explaining why it is useful. I don't know why we'd want to have cmake target names that don't match output names and filenames in doc/man/. It could help to have an explanation of what problem that solves and what other alternatives were considered. Not very important though, and the change does seem safe, just not clearly motivated.

  9. hebasto commented at 7:53 AM on June 17, 2026: member

    This change adds complexity and invites less consistent naming, and the PR description claims it is useful without explaining why it is useful.

    Please see #35515 (comment).

    I don't know why we'd want to have cmake target names that don't match output names...

    For example, this might help to avoid confusion between the bitcoin-cli and bitcoin_cli targets.

  10. ryanofsky commented at 11:35 AM on June 17, 2026: contributor

    Please see #35515 (comment).

    Thanks I skimmed that comment and didn't realize it actually showed the code using this.

    My opinion is that using having the bitcoin-qt target create a bitcoin-qt-legacy binary and having the bitcoin-qml target create a bitcoin-qt binary is pretty confusing and that it would be clearer to have the target names match the binary names, even if it results in a few extra lines of code changing in CMakeLists.txt. I think the approach described there is ok, but it seems unnecessarily confusing unless there are other benefits to using OUTPUT_NAME I'm not aware of.

    (IMO, it would also be better to not rename binaries at all but that is a separate question about packaging, not cmake implementation. I would just call the new binary bitcoin-qml and create desktop icons for it and also have the bitcoin gui command call it. Bitcoin core has good support for previous releases and should allow running bitcoin-qt from an old release and bitcoin-qml from the new release to ease difficulties like minor bugs and missing features that seem likely to arise from the transition.)

    For example, this might help to avoid confusion between the bitcoin-cli and bitcoin_cli targets.

    I'm not sure I see a way OUTPUT_NAME would help here. I guess we could rename the library targets like bitcoin_cli to libbitcoin_cli and bitcoin_node to libbitcoin_node and use OUTPUT_NAME to keep the same library filenames. But if we did, that our libraries don't have manpages anyway and I don't think the manpage logic implemented here would be helpful, so this PR wouldn't seem relevant.

  11. hebasto commented at 2:17 PM on June 17, 2026: member

    Please see #35515 (comment).

    Thanks I skimmed that comment and didn't realize it actually showed the code using this.

    My opinion is that using having the bitcoin-qt target create a bitcoin-qt-legacy binary and having the bitcoin-qml target create a bitcoin-qt binary is pretty confusing and that it would be clearer to have the target names match the binary names, even if it results in a few extra lines of code changing in CMakeLists.txt. I think the approach described there is ok, but it seems unnecessarily confusing unless there are other benefits to using OUTPUT_NAME I'm not aware of.

    (IMO, it would also be better to not rename binaries at all but that is a separate question about packaging, not cmake implementation. I would just call the new binary bitcoin-qml and create desktop icons for it and also have the bitcoin gui command call it. Bitcoin core has good support for previous releases and should allow running bitcoin-qt from an old release and bitcoin-qml from the new release to ease difficulties like minor bugs and missing features that seem likely to arise from the transition.)

    The code above is used in the current QML GUI staging branch, which is still a WIP.

    The current rough consensus is to make the new QML GUI a drop-in replacement for the legacy one. Keeping the executable names unmodified minimizes the changes in the release scripts and the documentation.

    For example, this might help to avoid confusion between the bitcoin-cli and bitcoin_cli targets.

    I'm not sure I see a way OUTPUT_NAME would help here. I guess we could rename the library targets like bitcoin_cli to libbitcoin_cli and bitcoin_node to libbitcoin_node and use OUTPUT_NAME to keep the same library filenames. But if we did, that our libraries don't have manpages anyway and I don't think the manpage logic implemented here would be helpful, so this PR wouldn't seem relevant.

    I've been thinking in opposite direction:

    --- a/src/CMakeLists.txt
    +++ b/src/CMakeLists.txt
    @@ -355,7 +355,7 @@ target_link_libraries(bitcoin_cli
     
     # Bitcoin Core RPC client
     if(BUILD_CLI)
    -  add_executable(bitcoin-cli bitcoin-cli.cpp init/basic.cpp)
    +  add_executable(bitcoin_cli_exe bitcoin-cli.cpp init/basic.cpp)
       add_windows_resources(bitcoin-cli bitcoin-cli-res.rc)
       add_windows_application_manifest(bitcoin-cli)
       target_link_libraries(bitcoin-cli
    

    I find bitcoin_cli_exe to be a much more convenient target name than bitcoin-cli when working in an IDE.

  12. ryanofsky commented at 2:43 PM on June 17, 2026: contributor

    Thanks for explanations! Your logic makes sense even if I disagree, and if you think this approach is better, I've acked the code change and don't want to get in the way.

    • re: "Keeping the executable names unmodified minimizes the changes in the release scripts and the documentation." I think this should be a non-goal. It seems to me that existing documentation about bitcoin-qt might no longer apply to the new GUI updating the name is a good way to make sure it is up to date.

    • As a command line user, not a gui user, I would find it annoying to have to build test_bitcoin_exe, for example instead of test_bitcoin. I actually think current target names are perfectly fine, but if I wanted to more clearly distinguish libraries from executables, I would change the library names not the executable names. Similarly, I also prefer unix convention of prefixing library names with lib over windows convention of suffixing executables with exe

  13. BrandonOdiwuor commented at 12:46 PM on June 26, 2026: contributor

    ACK 410862154e58c3bfd7ec78f15d208f0742cb1f1e

    Tested before/after on master vs 410862154e58c3bfd7ec78f15d208f0742cb1f1e: (Tested on Ubuntu 24.04.4 LTS with cmake version 3.28.3)

    • Applied temporary RUNTIME_OUTPUT_NAME="bitcoind-runtime" on the bitcoind target.

    <img width="997" height="927" alt="Image" src="https://github.com/user-attachments/assets/f16edd39-7cdd-4709-8946-bfab6a62e688" />

    • Confirmed that the old logic installs bitcoind.1 (mismatch with binary name).

    <img width="1365" height="208" alt="Image" src="https://github.com/user-attachments/assets/817701e1-fb3b-4ce1-8cc9-014e049522b3" />

    • New logic correctly uses $<TARGET_FILE_BASE_NAME:...> and installs the manpage matching the actual binary name.

    <img width="1340" height="333" alt="Image" src="https://github.com/user-attachments/assets/fd5c36dd-df9d-45f8-92d4-e968f91240ea" />

    The change is correct and future-proofs manpage installation for targets using OUTPUT_NAME* / RUNTIME_OUTPUT_NAME* properties.

  14. 151henry151 commented at 6:37 PM on July 28, 2026: contributor

    Tested ACK 410862154e58c3bfd7ec78f15d208f0742cb1f1e

    Tested on Debian 13 with cmake 3.31.6 / Ninja 1.12.1 (-DINSTALL_MAN=ON), and also checked the genex under cmake 3.22.0 (current cmake_minimum_required). Same-tree master vs this patch: install trees were identical (diff -r). Exercised OUTPUT_NAME (including a bitcoin-qt stand-in for the QML case), OUTPUT_NAME_<CONFIG> under Ninja Multi-Config, and the missing-manpage case — configure is fine, install fails with a clear "cannot find …/*.1". gen-manpages.py derives the page name from the binary's basename, so it stays consistent with this lookup; its hardcoded binary list means a target rename still needs a manual update there. Concept ACK as cheap future-proofing — $<TARGET_FILE_BASE_NAME> is the right tool here anyway (including that it drops the .exe suffix on Windows rather than making us hand-roll that).

    One thing the description misses: the genex also picks up <CONFIG>_POSTFIX / DEBUG_POSTFIX. Setting DEBUG_POSTFIX "d" on the bitcoind target with a Debug build makes install look for bitcoindd.1 and install bitcoindd. Fine if that's the binary you shipped, but doc/man/ won't have a postfixed page, so you get an install-time failure only in that config — easy to miss on normal Linux CI. Nothing in tree sets this today, so not blocking. Could the PR description list the postfix properties next to the four OUTPUT_NAME* ones?

    Small nit: a comment by the genex noting the manpage has to follow the installed filename (not the logical target name) would help.

    Separately, +1 to ryanofsky on naming — I'd rather see the new binary called bitcoin-qml. It's the more descriptive name once the GUI is no longer QtWidgets-based, and it keeps target, binary, manpage, and docs all lined up. But that's a QML-branch question, not a reason to hold this up.


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: 2026-08-04 00:50 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me