Skip to content

docs(README): Suggest using completionsdir as a fallback #1379

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 10, 2025
Merged

docs(README): Suggest using completionsdir as a fallback #1379

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions README.md
View file
Open in desktop
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me, but in the same answer we later refer to datarootdir instead of datadir. I've read the descriptions of the two, became confused, but was left with the feeling that datadir would actually be what we should be using (mostly because the datadir description mentions it actually being the dir for installing things into, datarootdir does not). We also use datadir for the replacements we do for the .pc file, not datarootdir.

This is not the fault of this change, but as it makes the difference so obvious, I'd like to discuss here what to do about it. I guess I'm suggesting this (to be done in another commit/PR):

--- a/README.md
+++ b/README.md
@@ -236 +236 @@ A. [ Disclaimer: Here, how to make the completion code visible to
-   bashcompdir = $(datarootdir)/bash-completion/completions
+   bashcompdir = $(datadir)/bash-completion/completions
@@ -243 +243 @@ A. [ Disclaimer: Here, how to make the completion code visible to
-   install(FILES your-completion-file DESTINATION "${CMAKE_INSTALL_DATAROOTDIR}/bash-completion/completions")
+   install(FILES your-completion-file DESTINATION "${CMAKE_INSTALL_DATADIR}/bash-completion/completions")

@akinomyoga your thoughts on this? Based on git log you were involved with the commit that added these two mentions, 530017d

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unfamiliar too, but according to the documentations, datadir seems better.

datarootdir seems to be intended as the root directory for various directories with special functions, such as share/info, share/locale, share/man, share/doc, and share itself. I agree with applying these changes. In addition, I think we need to add include(GNUInstallDirs) for the CMake example.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#1415

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading Directory Variables (GNU Coding Standards), I think datarootdir makes sense for third party projects providing completions:

This is usually the same place as 'datarootdir', but we use the two separate variables so that you can move these program-specific files without altering the location for Info files, man pages, etc.

So it allows distributor to override datadir for a third party project without installing the completion file to a path bash-completion will not be able to find it.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I read the same document a couple of days ago and came to the opposite conclusion. They could have overridden datadir for bash-completion and in that case installing 3rd party completions to datarootdir would install to a location that's not looked up from.

Anyway, anything that's not using the .pc file to figure out locations to install is doing it the fragile way, and gets to keep both pieces when it breaks :). If this would be a robust way to do it, I suppose there would be no need for the .pc file in the first place.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I read the same document a couple of days ago and came to the opposite conclusion. They could have overridden datadir for bash-completion and in that case installing 3rd party completions to datarootdir would install to a location that's not looked up from.

Right, it cuts both ways.

Though overriding datadir for bash-completion makes even less sense than doing that for programs, in my opinion:

  • It would require overriding completionsdir for every program trying to install completions without the use of pkg-config file. (I guess autotools introducing datarootdir predates pkg-config.)
  • I can imagine someone wanting to build a program with different --datadir in order allow simultaneous installations of different versions of software. I do not see a reason to do that for nexus-establishing software like bash-completion other than vanity organisation.

Anyway, anything that's not using the .pc file to figure out locations to install is doing it the fragile way, and gets to keep both pieces when it breaks :). If this would be a robust way to do it, I suppose there would be no need for the .pc file in the first place.

Right. datarootdir is precisely what prevents this mismatch when overriding datadir for programs, nexūs should be still found in datarootdir. Though we do not actually support datarootdir in our pkg-config file so we would encounter the same issue here:

bash-completion/bash-completion.pc.in

Line 6 in c55ee7f

completionsdir=${datadir}/@PACKAGE@/completions

Programs are still expected to override variables in pkg-config paths so they could use pkg-config to support datarootdir with pkg-config bash-completion --variable=completionsdir --define-variable=datadir=$datarootdir. But without guidance from us, this is pretty unlikely.

If we want to fix that, we could modify the pkg-config file, introducing datarootdir=@datarootdir@ and replacing datadir value with ${datarootdir}. This is a somewhat common pattern I have seen in pkg-config files (example) before projects started switching en masse to Meson, which does not support datarootdir.

Alternately, we could decide supporting separate datarootdir is not worth the trouble until someone comes up with an actual convincing use case.

Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ A. [ Disclaimer: Here, how to make the completion code visible to

```m4
PKG_CHECK_VAR(bashcompdir, [bash-completion], [completionsdir], ,
bashcompdir="${sysconfdir}/bash_completion.d")
bashcompdir="${datadir}/bash-completion/completions")
AC_SUBST(bashcompdir)
```

Expand All @@ -207,12 +207,13 @@ A. [ Disclaimer: Here, how to make the completion code visible to
`bash-completion-config-version.cmake` files. Example usage:

```cmake
include(GNUInstallDirs)
find_package(bash-completion)
if(BASH_COMPLETION_FOUND)
message(STATUS
"Using bash completion dir ${BASH_COMPLETION_COMPLETIONSDIR}")
else()
set (BASH_COMPLETION_COMPLETIONSDIR "/etc/bash_completion.d")
set (BASH_COMPLETION_COMPLETIONSDIR "${CMAKE_INSTALL_DATADIR}/bash-completion/completions")
message (STATUS
"Using fallback bash completion dir ${BASH_COMPLETION_COMPLETIONSDIR}")
endif()
Expand Down