fix(Makefile): Make bash completion honor PREFIX #155
Conversation
The current version of the build system uses `pkgconfig` to discover the location of the bash completion scripts. Typically, this script outputs a directory under the `/usr` prefix. Unfortunately, if the user chooses to override the build prefix with PREFIX, the BASHCOMPLETION variable is not also updated. In this commit, I propose to correct this issue by simply looking for the `/usr` prefix and substituting it with the contents of `PREFIX`. I've tested this approach using my `stow`-based "package" collection, where I build commands with custom prefixes, and do not grant root or sudo privileges when doing so. It appears to function normally for me.
|
Hm. This would result in a default In particular, we really need to make sure that we agree with bash about where we're putting files, in order for it to find them, and so "making up" paths isn't necessarily okay. That said, reading the bash_completion documentation, I notice this:
v2.12 is new this year so that's not necessarily a safe assumption, but at least that's the direction things are moving, and also seems much more sensible behavior. (I also see that FreeBSD already patches us to use that behavior) I'll amend this to just use |
Per [the bash_completion documentation](https://github.com/scop/bash-completion/blob/main/README.md), this is the preferred style as of new versions. It's also generally much simpler and more consistent with other options, so let's see if we can just switch over.
|
That should work just fine for me. Thanks! |
| install -d -m 755 $(DESTDIR)$(BASHCOMPDIR) ; \ | ||
| install -m 644 reptyr.bash $(DESTDIR)$(BASHCOMPDIR)/reptyr |
There was a problem hiding this comment.
Where does $(DESTDIR) come from? I know that autoconf works with PREFIX and DESTDIR, but I didn't see an autoconf setup in your build system, just the Makefile. What happens if someone sets the DESTDIR to e.g. foobar? Does line 54 expand to install -m 644 reptyr.bash foobar/usr/local/share/bash-completions/reptyr? That seems a little odd to me.
There was a problem hiding this comment.
Make allows referencing undefined variables, and they'll just expand to an empty string. I just cargo-culted the existing pattern we have, which seems to match the convention recommended by GNU Make.
2 participants
The current version of the build system uses
pkgconfigto discover the location of the bash completion scripts. Typically, this script outputs a directory under the/usrprefix. Unfortunately, if the user chooses to override the build prefix with PREFIX, the BASHCOMPLETION variable is not also updated.In this commit, I propose to correct this issue by simply looking for the
/usrprefix and substituting it with the contents ofPREFIX. I've tested this approach using mystow-based "package" collection, where I build commands with custom prefixes, and do not grant root or sudo privileges when doing so. It appears to function normally for me.