On Wed, Jul 14 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.
I'm very much in favor of this, i.e. both to promote git-completion.*,
and also to generally re-organize contrib/* a bit (not being done here).
> Signed-off-by: Felipe Contreras <felipe.contreras@xxxxxxxxx>
> ---
> Makefile | 10 ++++++++++
> {contrib => extra}/completion/git-completion.bash | 0
> {contrib => extra}/completion/git-completion.zsh | 0
> {contrib => extra}/completion/git-prompt.sh | 0
> t/t9902-completion.sh | 8 ++++----
> t/t9903-bash-prompt.sh | 2 +-
> 6 files changed, 15 insertions(+), 5 deletions(-)
> 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 502e0c9a81..0a13e5f077 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -532,6 +532,7 @@ sharedir = $(prefix)/share
> gitwebdir = $(sharedir)/gitweb
> perllibdir = $(sharedir)/perl5
> localedir = $(sharedir)/locale
> +bashcompdir = $(sharedir)/bash-completion/completions
> template_dir = share/git-core/templates
> htmldir = $(prefix)/share/doc/git-doc
> ETC_GITCONFIG = $(sysconfdir)/gitconfig
> @@ -2015,6 +2016,7 @@ bindir_relative_SQ = $(subst ','\'',$(bindir_relative))
> mandir_SQ = $(subst ','\'',$(mandir))
> mandir_relative_SQ = $(subst ','\'',$(mandir_relative))
> infodir_relative_SQ = $(subst ','\'',$(infodir_relative))
> +sharedir_SQ = $(subst ','\'',$(sharedir))
> perllibdir_SQ = $(subst ','\'',$(perllibdir))
> localedir_SQ = $(subst ','\'',$(localedir))
> localedir_relative_SQ = $(subst ','\'',$(localedir_relative))
> @@ -2025,6 +2027,7 @@ htmldir_relative_SQ = $(subst ','\'',$(htmldir_relative))
> prefix_SQ = $(subst ','\'',$(prefix))
> perllibdir_relative_SQ = $(subst ','\'',$(perllibdir_relative))
> gitwebdir_SQ = $(subst ','\'',$(gitwebdir))
> +bashcompdir_SQ = $(subst ','\'',$(bashcompdir))
>
> SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
> TEST_SHELL_PATH_SQ = $(subst ','\'',$(TEST_SHELL_PATH))
> @@ -3112,6 +3115,13 @@ quick-install-man:
> quick-install-html:
> $(MAKE) -C Documentation quick-install-html
>
> +install-extra: install-completion
> +
> +install-completion:
> + $(INSTALL) -D -m 644 extra/completion/git-completion.bash '$(DESTDIR_SQ)$(bashcompdir_SQ)'/git
> + $(INSTALL) -D -m 644 extra/completion/git-prompt.sh '$(DESTDIR_SQ)$(sharedir_SQ)'/git-core/git-prompt.sh
> + $(INSTALL) -D -m 644 extra/completion/git-completion.zsh '$(DESTDIR_SQ)$(sharedir_SQ)'/zsh/site-functions/_git
> +
These are missing a .PHONY target (like the other install-* targets).
The bash-completion target corresponds to what I've got in Debian's git
package, but not the prompt:
$ dpkg -L git|grep -e completion -e prompt
/etc/bash_completion.d
/etc/bash_completion.d/git-prompt
/usr/lib/git-core/git-sh-prompt
/usr/share/bash-completion
/usr/share/bash-completion/completions
/usr/share/bash-completion/completions/git
/usr/share/bash-completion/completions/gitk
I've got no idea what we should pick by default though, maybe what you
have is more standard.
Also why /git and /_git for bash and zsh (looks good) but /git-prompt
instead of /git-prompt.sh?
>
> ### Maintainer's dist rules
> 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\\]'
It's more of a "for bonus points", but a nic way to round-trip this
would be to make this work with GIT_TEST_INSTALLED.
I.e. source these relative to GIT_EXEC_PATH, not $GIT_BUILD_DIR, I think
that just sourcing them as e.g.:
. git-completion.bash
But the GIT_TEST_INSTALLED case is tricker, maybe we'd need to add a
"git --share-path" :(
