Mk/Uses: Add completions.mk
Needs ReviewPublic
Actions

Authored by lcook on Aug 8 2021, 6:34 PM.

Details

Reviewers

None

Group Reviewers

Ports Committers

Summary

To be used to help simplify ports which install programmable shell completions. While the scope of this is relatively small, feedback is appreciated (and whether it's something that actually belongs).

Further context:

I've had a few hiccups with shell completions in particular [1][2], and thought it would be an idea to make it less error-prone and canonicalize the shell completion paths (in line with [3]), additionally creating the preliminary directories (if wanted).

[1] https://cgit.freebsd.org/ports/commit/?id=36d76902abf2cb2e16da2dd31cf38e58adaf37a7
[2] https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=255384
[3] https://docs.freebsd.org/en/books/porters-handbook/special/#shell-completion

Diff Detail

Repository

R11 FreeBSD ports repository

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

lcook requested review of this revision.Aug 8 2021, 6:34 PM

lcook created this revision.

0mp added a subscriber: 0mp.Aug 8 2021, 9:43 PM

0mp added inline comments.

Mk/Uses/completions.mk
29	This probably should not be in `etc/`. See https://github.com/0mp/bash-completion-freebsd#caveats for example

lcook added inline comments.Aug 8 2021, 9:49 PM

Mk/Uses/completions.mk
29	Thanks for the note. I'm not all too much of an avid bash user myself, and followed the recommendation as listed in https://docs.freebsd.org/en/books/porters-handbook/special/#shell-completion; which might be worth a quick update.

Favor the canonical shell completion path for bash

lcook marked an inline comment as done.Aug 8 2021, 9:53 PM

lcook edited the summary of this revision. (Show Details)Aug 8 2021, 10:02 PM

tobik added a subscriber: tobik.Aug 20 2021, 6:14 AM

tobik added inline comments.

Mk/Uses/completions.mk
29	Well, if we add USES=completions we don't even need to list the actual directories in the Porter's Handbook.
29–31	Maybe the directories should also be added to PLIST_SUB.
34	This is after `do-install`. It think it should run before it.

Add shell completion directories to PLIST_SUB.
Create the shell completion directories before the do-install target.
Minor tidy up.

Thanks for the suggestions, I have addressed them now.

There is the problem that if any of the created *_COMPDIR are not used, it will leave an orphaned directory behind:

For example, with a converted devel/leiningen:

diff --git a/devel/leiningen/Makefile b/devel/leiningen/Makefile
index 7c044d4a670f..89428d5445fe 100644
--- a/devel/leiningen/Makefile
+++ b/devel/leiningen/Makefile
@@ -11,16 +11,17 @@ COMMENT=    Automate Clojure projects
 LICENSE=       EPL
 LICENSE_FILE=  ${WRKSRC}/COPYING

+USES=          completions
 USE_GITHUB=    yes
 GH_ACCOUNT=    technomancy
 USE_JAVA=      yes
 NO_ARCH=       yes
 NO_BUILD=      yes

-PLIST_FILES=   bin/lein \
-               etc/bash_completion.d/_lein.bash \
-               share/man/man1/lein.1.gz \
-               share/zsh/site-functions/_lein
+PLIST_FILES=   ${BASH_COMPDIR}/_lein.bash \
+               ${ZSH_COMPDIR}/_lein \
+               bin/lein \
+               share/man/man1/lein.1.gz
 PORTDATA=      leiningen-${PORTVERSION}-standalone.zip

 post-patch:
@@ -35,12 +36,10 @@ post-patch:
 do-install:
        ${INSTALL_SCRIPT} ${WRKSRC}/bin/lein-pkg \
                ${STAGEDIR}${PREFIX}/bin/lein
-       @${MKDIR} ${STAGEDIR}${PREFIX}/etc/bash_completion.d
        ${INSTALL_DATA} ${WRKSRC}/bash_completion.bash \
-               ${STAGEDIR}${PREFIX}/etc/bash_completion.d/_lein.bash
-       @${MKDIR} ${STAGEDIR}${PREFIX}/share/zsh/site-functions
+               ${STAGEDIR}${BASH_COMPDIR}/_lein.bash
        ${INSTALL_DATA} ${WRKSRC}/zsh_completion.zsh \
-               ${STAGEDIR}${PREFIX}/share/zsh/site-functions/_lein
+               ${STAGEDIR}${ZSH_COMPDIR}/_lein
        ${INSTALL_MAN} ${WRKSRC}/doc/lein.1 \
                ${STAGEDIR}${PREFIX}/share/man/man1
        @${MKDIR} ${STAGEDIR}${DATADIR}

$ make check-plist
====> Checking for pkg-plist issues (check-plist)
===> Parsing plist
===> Checking for items in STAGEDIR missing from pkg-plist
Error: Orphaned: @dir %%FISH_COMPDIR%%

Adding the completion directories to Templates/BSD.local.dist would probably be the cleanest solution. I don't know.

Mk/Uses/completions.mk
41	s/shells/shell/

In D31463#713322, @tobik wrote:

There is the problem that if any of the created *_COMPDIR are not used, it will leave an orphaned directory behind:

For example, with a converted devel/leiningen:

diff --git a/devel/leiningen/Makefile b/devel/leiningen/Makefile
index 7c044d4a670f..89428d5445fe 100644
--- a/devel/leiningen/Makefile
+++ b/devel/leiningen/Makefile
@@ -11,16 +11,17 @@ COMMENT=    Automate Clojure projects
 LICENSE=       EPL
 LICENSE_FILE=  ${WRKSRC}/COPYING

+USES=          completions
 USE_GITHUB=    yes
 GH_ACCOUNT=    technomancy
 USE_JAVA=      yes
 NO_ARCH=       yes
 NO_BUILD=      yes

-PLIST_FILES=   bin/lein \
-               etc/bash_completion.d/_lein.bash \
-               share/man/man1/lein.1.gz \
-               share/zsh/site-functions/_lein
+PLIST_FILES=   ${BASH_COMPDIR}/_lein.bash \
+               ${ZSH_COMPDIR}/_lein \
+               bin/lein \
+               share/man/man1/lein.1.gz
 PORTDATA=      leiningen-${PORTVERSION}-standalone.zip

 post-patch:
@@ -35,12 +36,10 @@ post-patch:
 do-install:
        ${INSTALL_SCRIPT} ${WRKSRC}/bin/lein-pkg \
                ${STAGEDIR}${PREFIX}/bin/lein
-       @${MKDIR} ${STAGEDIR}${PREFIX}/etc/bash_completion.d
        ${INSTALL_DATA} ${WRKSRC}/bash_completion.bash \
-               ${STAGEDIR}${PREFIX}/etc/bash_completion.d/_lein.bash
-       @${MKDIR} ${STAGEDIR}${PREFIX}/share/zsh/site-functions
+               ${STAGEDIR}${BASH_COMPDIR}/_lein.bash
        ${INSTALL_DATA} ${WRKSRC}/zsh_completion.zsh \
-               ${STAGEDIR}${PREFIX}/share/zsh/site-functions/_lein
+               ${STAGEDIR}${ZSH_COMPDIR}/_lein
        ${INSTALL_MAN} ${WRKSRC}/doc/lein.1 \
                ${STAGEDIR}${PREFIX}/share/man/man1
        @${MKDIR} ${STAGEDIR}${DATADIR}

$ make check-plist
====> Checking for pkg-plist issues (check-plist)
===> Parsing plist
===> Checking for items in STAGEDIR missing from pkg-plist
Error: Orphaned: @dir %%FISH_COMPDIR%%

Adding the completion directories to Templates/BSD.local.dist would probably be the cleanest solution. I don't know.

That does seem to be the cleanest way of handling this, and in doing so have created another revision accommodating your suggestion. Much appreciated.

to be honnest I don't see the point for a new keyword, it does not have enough value, I could see the point for bsd.local.dist additions, but that is all. the converted example from @tobik shows that value is minimal (note that I can be convinced otherwise)

In D31463#713343, @bapt wrote:

to be honnest I don't see the point for a new keyword, it does not have enough value, I could see the point for bsd.local.dist additions, but that is all. the converted example from @tobik shows that value is minimal (note that I can be convinced otherwise)

In short, it distinguishes between what completions are shipped by default with a shell and what completions are provided by a third-party application.

So while this is indeed a minor change, canonicalizing the preferred (oppose to haphazardly) installation paths makes it a little easier to deal with, plus avoids the situation aforementioned in the summary[1] in that case resolving an install conflict. I don't mind either way, just thought having encountered this small problem to come up with a small solution, cheers. :)

[1] https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=255384