cmake: Install man pages for configured targets only #31765
pull hebasto wants to merge 1 commits into bitcoin:master from hebasto:250130-man-inst changing 2 files +15 −8-
hebasto commented at 1:14 pm on January 30, 2025: memberFixes #31762.
-
hebasto added the label Build system on Jan 30, 2025
-
hebasto added this to the milestone 29.0 on Jan 30, 2025
-
DrahtBot commented at 1:14 pm on January 30, 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/31765.
Reviews
See the guideline for information on the review process.
Type Reviewers Approach ACK stickies-v If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
Conflicts
Reviewers, this pull request conflicts with the following ones:
- #31802 (Add bitcoin-{node,gui} to release binaries for IPC by Sjors)
- #31741 (multiprocess: Add libmultiprocess git subtree by ryanofsky)
- #31679 (cmake: Move internal binaries from bin/ to libexec/ by ryanofsky)
- #30975 (ci: build multiprocess on most jobs by Sjors)
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.
-
hebasto renamed this:
cmake: Install man pages for built targets only
cmake: Install man pages for configured targets only
on Jan 30, 2025 -
cmake: Install man pages for configured targets only 5f6680769f
-
hebasto force-pushed on Jan 30, 2025
-
in src/CMakeLists.txt:175 in 5f6680769f
171@@ -172,6 +172,14 @@ target_link_libraries(bitcoin_common 172 173 174 set(installable_targets) 175+macro(install_manpage target)
stickies-v commented at 5:50 pm on January 31, 2025:I think we should add an optional
component
parameter. This would address the issue (see below) of installing justGUI
, and also aligns well with potentially adding more components (e.g. one per target) in the future (out of scope for this PR).0diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt 1index eb816dbacc..ea026c25be 100644 2--- a/src/CMakeLists.txt 3+++ b/src/CMakeLists.txt 4@@ -172,10 +172,11 @@ target_link_libraries(bitcoin_common 5 6 7 set(installable_targets) 8-macro(install_manpage target) 9+macro(install_manpage target component) 10 if(INSTALL_MAN) 11 install(FILES ${PROJECT_SOURCE_DIR}/doc/man/${target}.1 12 DESTINATION ${CMAKE_INSTALL_MANDIR}/man1 13+ COMPONENT ${component} 14 ) 15 endif() 16 endmacro() 17diff --git a/src/qt/CMakeLists.txt b/src/qt/CMakeLists.txt 18index 35bfd1ecf1..0e887841af 100644 19--- a/src/qt/CMakeLists.txt 20+++ b/src/qt/CMakeLists.txt 21@@ -241,7 +241,7 @@ if(WIN32) 22 set_target_properties(bitcoin-qt PROPERTIES WIN32_EXECUTABLE TRUE) 23 endif() 24 25-install_manpage(bitcoin-qt) 26+install_manpage(bitcoin-qt GUI) 27 28 if(WITH_MULTIPROCESS) 29 add_executable(bitcoin-gui
On 5f6680769f017c9e12801dfa7af7d6568e53bdbe, when installing with
--component GUI
, the manpages are ignored:0% cmake --install build --component GUI 1-- Install configuration: "RelWithDebInfo" 2-- Installing: /usr/local/bin/bitcoin-qt
With my diff:
0% cmake --install build --component GUI 1-- Install configuration: "RelWithDebInfo" 2-- Up-to-date: /usr/local/share/man/man1/bitcoin-qt.1 3-- Up-to-date: /usr/local/bin/bitcoin-qt
stickies-v commented at 5:51 pm on January 31, 2025: contributorApproach ACK 5f6680769f017c9e12801dfa7af7d6568e53bdbe, thanks for addressing my issue!
Unfamiliar enough with CMake to determine if this is the best approach, but it’s elegant and as per my testing it works well (except for my
component
comment).
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-07 18:12 UTC
More mirrored repositories can be found on mirror.b10c.me