#34721 build: install shell completions via cmake

ReviewabilityReviewsAgreementSize
#34721 willcl-ark build: install shell completions via cmakeReady2 (+2)Strong23
  • Install shell completion scripts automatically during CMake installations
  • Spares packagers and command line users from manually copying completion files into system directories
  • Ready for review
  • All requested changes have been addressed and nothing invalidates a review now
  • Strong support across multiple Linux distributions
  • Verified prefix install, staging, flag toggling, and completion behavior (151henry151)
  • Verified installation on Arch Linux with completions disabled (caesrcd)
  • Code review approval endorsing the CMake structure after configure checks were updated (ryanofsky)
  • Prior objection resolved by adding independent toggle options for each shell (luke-jr)
  • 23 lines added or modified outside tests
  • 0 lines added or modified in tests
  • 8 lines removed in total
  • 4 files, 1 commits
Build and CI#39 of 56
P3 · new feature
  • P3 because it improves packaging convenience without altering core build behavior
  • It does not affect platform build support or core release integrity
  • Ranking pass: Moved down from 23: packaging convenience with no correctness or platform consequence
Ranking notes
Reviewability: ReadyStalePaused
Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets
Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs.
Size: lines added or modified outside tests, then in tests; greener = smaller.

Goal

Adds CMake installation rules for bash and fish completion scripts alongside their corresponding binary components. Targets declare available completion scripts via new flags in `install_binary_component`, and installs can be toggled using `-DINSTALL_BASH_COMPLETIONS` and `-DINSTALL_FISH_COMPLETIONS`.

Problem: Packagers and users installing via CMake currently have to manually locate and copy shell completion scripts from `contrib/completions/` into system directories, as `cmake --install` omits them.

Evidence of importance:

  • Fixes #34714 ('Install shell completions with cmake')
  • Author states: 'Packagers currently need to copy shell completion scripts from contrib/completions/ into the appropriate system directories. Install the scripts alongside their corresponding binaries with cmake --install, including when installing a binary by component.'

Reviewability

Ready

Nothing known that would invalidate a review now.

Waiting on (from the thread read):

  • nothing: all requested changes implemented and multiple recent ACKs in place

Agreement

Strong

substantive support, no open objection (BrandonOdiwuor, BrandonOdiwuor, 151henry151, caesrcd, ryanofsky)

Thread notes: Added ryanofsky's updated code review ACK (r:5269307630). Added ryanofsky's statement as an other_reply on purpleKarrot's claim regarding install components vs options.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
purpleKarrot (member)objection, approachBuild options for completions don't affect build and 1:1 target-to-component relationship is inflexible; wrapper install_binary_component attracts responsibilitiesanswered · ryanofsky agreed in principle but supported following existing patterns · v1, not yet re-checkedansweredby others2026-03-10: 'The whole install_binary_component command should be inlined at the call sites and then removed.'
Status from: 2026-07-27: hebasto: 'Many large projects, such as LLVM and Qt, use convenience wrappers extensively. In my opinion, they improve code maintai'
luke-jr (contributor)objection, interfaceUsers unlikely to use multiple shells cannot toggle completion installation per shell individuallyfixed · fix pushed · v1, not yet re-checkedansweredyes2026-03-10: 'Users are at least somewhat unlikely to use multiple shells, so it probably makes sense to have a separate option for each one'
Status from: 2026-09-14: willcl-ark: 'I also split the install option into INSTALL_BASH_COMPLETIONS and INSTALL_FISH_COMPLETIONS, both enabled by default.'

Support:

  • kpcyrd (none) (2026-03-03) [tested]: tested patch and confirmed it works well [not substantive]
  • w0xlt (contributor) (Concept ACK, 2026-03-03): Concept ACK [not substantive]
  • hebasto (member) (Concept ACK, 2026-03-03): Concept ACK [not substantive]
  • caesrcd (contributor) (Concept ACK, 2026-03-04): Concept ACK [not substantive]
  • BrandonOdiwuor (contributor) (Concept ACK, 2026-03-11) [both]: Installing shell completions automatically with cmake --install is more convenient; tested on Ubuntu 24.04 with bash
  • ryanofsky (member) (ACK, 2026-04-06): Code review ACK [not substantive]
  • BrandonOdiwuor (contributor) (ACK, 2026-07-24) [both]: installing bash (and fish) completions is a useful feature, tested functionality
  • 151henry151 (contributor) (Tested ACK, 2026-07-27) [tested]: Tested prefix install, DESTDIR staging, flag toggling, and bash tab-completion on Debian 13
  • caesrcd (contributor) (Tested ACK, 2026-09-14) [tested]: Tested on Arch Linux with bash and fish completions disabled
  • ryanofsky (member) (ACK, 2026-09-21) [read: install_binary_component]: Code review ACK; confirmed configure-time file existence checks were replaced with explicit arguments and separate shell completion options, endorsing existing patterns

Participants:

  • fanquake (member, question, 1 statements 2026-03-03 to 2026-03-03): asked whether project will maintain completions if installed
  • hebasto (member, support, 4 statements 2026-03-03 to 2026-07-27): Concept ACK; supported avoiding if(EXISTS) and splitting shell options, while defending convenience wrappers
  • kpcyrd (none, support, 1 statements 2026-03-03 to 2026-03-03): tested and reported it works well
  • w0xlt (contributor, support, 1 statements 2026-03-03 to 2026-03-03): Concept ACK
  • caesrcd (contributor, support, 2 statements 2026-03-04 to 2026-09-14): Concept ACK and Tested ACK on Arch Linux
  • luke-jr (contributor, objection, 1 statements 2026-03-10 to 2026-03-10): suggested separate toggle options per shell
  • purpleKarrot (member, objection, 1 statements 2026-03-10 to 2026-03-10): objected to if(EXISTS) file sniffing, monolithic build option, and wrapper function install_binary_component
  • BrandonOdiwuor (contributor, support, 2 statements 2026-03-11 to 2026-07-24): Concept ACK and ACK, noted inconsistency of if(EXISTS) with HAS_MANPAGE pattern
  • ryanofsky (member, support, 4 statements 2026-04-06 to 2026-09-21): ACKed latest commit; noted if(EXISTS) removal and agreed with purpleKarrot in principle but endorsed following existing patterns
  • sedited (member, neutral, 1 statements 2026-07-21 to 2026-07-21): asked BrandonOdiwuor for re-review
  • 151henry151 (contributor, support, 1 statements 2026-07-27 to 2026-07-27): Tested ACK on Debian 13, confirmed if(EXISTS) flakiness, addressed maintenance question

Files

0 lines under test/bench/ci.

  • cmake/module/InstallBinaryComponent.cmake +14/-1
  • src/CMakeLists.txt +6/-6
  • CMakeLists.txt +2/-0
  • src/qt/CMakeLists.txt +1/-1

Card

The short text the ranking pass compares across PRs.

Adds CMake install rules and configuration options to automatically install bash and fish shell completions alongside binary components during cmake --install. Currently, packagers and users must manually copy completion scripts out of contrib/completions into system directories. The PR resolves issue #34714 by tying script installation to individual binary components following the existing manpage installation pattern. It has no dependencies.

Data

dossier JSON · extract JSON · model ledger, generated 2026-09-22T01:14, confidence high, input hash b2e657c32948041f
thread read through 2026-09-21 (21 statements), head 91b96bef34
Discussion read under schema 1 rules: suggestions and questions may be missing, and an objection may combine several harms (schema history).
labels: Build system

Categories

Build and CI

P3 · new feature (ranking pass: Moved down from 23: packaging convenience with no correctness or platform consequence)

Adds CMake installation rules for shell completions so packagers and users do not need to manually copy scripts from contrib/completions/. This provides clear convenience for packaging workflows without impacting core release integrity or platform build support.

Membership: Matches 'Build system' label and directly modifies CMake build scripts (CMakeLists.txt and cmake/module/InstallBinaryComponent.cmake) to add install targets.

Factors: security/stability 0, bug 0, performance 0, user value 1, leverage 0