On Sun, Jun 13 2021, Felipe Contreras wrote: > These have been stable and widely used for quite a long time, they even > have tests outside of the contrib area, and most distributions ship > them, so they can be considered part of the core already. > > We should be consistent and either we move the tests to contrib, or we > move the completions out of contrib. > > Let's move them out of contrib and provide an installation target > install-extra. > > By default bash-completion installs the completions to > $(pkgdatadir)/completions, which is > $(prefix)/share/bash-completion/completions. And since most distributions do > not change this, it is obviously the right default that distributions > can override with bashcompdir. > > By default zsh looks for completions in > $(prefix)/share/zsh/site-functions. > > Signed-off-by: Felipe Contreras <felipe.contreras@xxxxxxxxx> > --- > Makefile | 3 +++ > extra/Makefile | 17 +++++++++++++++++ Please let's not continue following the IMO anti-pattern of having these sub-Makefiles. Let's just add the target to the top-level Makefile. See e.g. the recent discussion starting at https://lore.kernel.org/git/87pmz4ig4o.fsf@xxxxxxxxxxxxxxxxxxx/ I also have some WIP work to un-split most of this to e.g. make "install" follow the normal quiet rules, if we're invoking those in a sub-Makefile that becomes much more difficult.... > .../completion/git-completion.bash | 0 > .../completion/git-completion.zsh | 0 > {contrib => extra}/completion/git-prompt.sh | 0 > t/t9902-completion.sh | 8 ++++---- > t/t9903-bash-prompt.sh | 2 +- > 7 files changed, 25 insertions(+), 5 deletions(-) > create mode 100644 extra/Makefile > rename {contrib => extra}/completion/git-completion.bash (100%) > rename {contrib => extra}/completion/git-completion.zsh (100%) > rename {contrib => extra}/completion/git-prompt.sh (100%) > > diff --git a/Makefile b/Makefile > index c3565fc0f8..5bb03808b3 100644 > --- a/Makefile > +++ b/Makefile > @@ -3105,6 +3105,9 @@ quick-install-man: > quick-install-html: > $(MAKE) -C Documentation quick-install-html > > +install-extra: > + $(MAKE) -C extra install > + > > > ### Maintainer's dist rules > diff --git a/extra/Makefile b/extra/Makefile > new file mode 100644 > index 0000000000..26d8be55b0 > --- /dev/null > +++ b/extra/Makefile > @@ -0,0 +1,17 @@ > +bashcompdir = $(sharedir)/bash-completion/completions > + > +DESTDIR_SQ = $(subst ','\'',$(DESTDIR)) > +sharedir_SQ = $(subst ','\'',$(sharedir)) > +bashcompdir_SQ = $(subst ','\'',$(bashcompdir)) > +gitexec_instdir_SQ = $(subst ','\'',$(gitexec_instdir)) > + > +INSTALL ?= install ...and a large part of the Makefile is just things like this that we'd mostly/entirely get for free. > + > +all: > + > +install: install-completion > + > +install-completion: > + $(INSTALL) -D -m 644 completion/git-completion.bash '$(DESTDIR_SQ)$(bashcompdir_SQ)'/git > + $(INSTALL) -D -m 644 completion/git-prompt.sh '$(DESTDIR_SQ)$(sharedir_SQ)'/git-core/git-prompt.sh > + $(INSTALL) -D -m 644 completion/git-completion.zsh '$(DESTDIR_SQ)$(sharedir_SQ)'/zsh/site-functions/_git > diff --git a/contrib/completion/git-completion.bash b/extra/completion/git-completion.bash > similarity index 100% > rename from contrib/completion/git-completion.bash > rename to extra/completion/git-completion.bash > diff --git a/contrib/completion/git-completion.zsh b/extra/completion/git-completion.zsh > similarity index 100% > rename from contrib/completion/git-completion.zsh > rename to extra/completion/git-completion.zsh > diff --git a/contrib/completion/git-prompt.sh b/extra/completion/git-prompt.sh > similarity index 100% > rename from contrib/completion/git-prompt.sh > rename to extra/completion/git-prompt.sh > diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh > index cb057ef161..32601b755d 100755 > --- a/t/t9902-completion.sh > +++ b/t/t9902-completion.sh > @@ -36,7 +36,7 @@ complete () > GIT_TESTING_ALL_COMMAND_LIST='add checkout check-attr rebase ls-files' > GIT_TESTING_PORCELAIN_COMMAND_LIST='add checkout rebase' > > -. "$GIT_BUILD_DIR/contrib/completion/git-completion.bash" > +. "$GIT_BUILD_DIR/extra/completion/git-completion.bash" > > # We don't need this function to actually join words or do anything special. > # Also, it's cleaner to avoid touching bash's internal completion variables. > @@ -2383,14 +2383,14 @@ test_expect_success 'git clone --config= - value' ' > test_expect_success 'sourcing the completion script clears cached commands' ' > __git_compute_all_commands && > verbose test -n "$__git_all_commands" && > - . "$GIT_BUILD_DIR/contrib/completion/git-completion.bash" && > + . "$GIT_BUILD_DIR/extra/completion/git-completion.bash" && > verbose test -z "$__git_all_commands" > ' > > test_expect_success 'sourcing the completion script clears cached merge strategies' ' > __git_compute_merge_strategies && > verbose test -n "$__git_merge_strategies" && > - . "$GIT_BUILD_DIR/contrib/completion/git-completion.bash" && > + . "$GIT_BUILD_DIR/extra/completion/git-completion.bash" && > verbose test -z "$__git_merge_strategies" > ' > > @@ -2399,7 +2399,7 @@ test_expect_success 'sourcing the completion script clears cached --options' ' > verbose test -n "$__gitcomp_builtin_checkout" && > __gitcomp_builtin notes_edit && > verbose test -n "$__gitcomp_builtin_notes_edit" && > - . "$GIT_BUILD_DIR/contrib/completion/git-completion.bash" && > + . "$GIT_BUILD_DIR/extra/completion/git-completion.bash" && > verbose test -z "$__gitcomp_builtin_checkout" && > verbose test -z "$__gitcomp_builtin_notes_edit" > ' > diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh > index bbd513bab0..784e523fd4 100755 > --- a/t/t9903-bash-prompt.sh > +++ b/t/t9903-bash-prompt.sh > @@ -10,7 +10,7 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME > > . ./lib-bash.sh > > -. "$GIT_BUILD_DIR/contrib/completion/git-prompt.sh" > +. "$GIT_BUILD_DIR/extra/completion/git-prompt.sh" > > actual="$TRASH_DIRECTORY/actual" > c_red='\\[\\e[31m\\]'