patch, 2 partsextra: new concept of extra components

> 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@gmail.com>
> ---
>  Makefile                                        |  3 +++
>  extra/Makefile                                  | 17 +++++++++++++++++

>  .../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

> +
> +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\\]'

> 
> 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@gmail.com>
> > ---
> >  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.

> On Sun, Jun 13 2021, Felipe Contreras wrote:
>
>> This patch series introduces the concept of extra components. These are
>> components which are not yet part of the core but are good enough for
>> distributions to ship, and in fact, they already do.
>
> I like this direction.

> I have a CI failure in one series of mine that seems to be a lack of
> updating to CMake in contrib/buildsystems, perhaps we should be adding
> that to extra/ too, i.e. extending this to the "make test" run by CI?
>
> Not something that should hinder or necessarily be included in this
> series, just a note about a related component.

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> 
> > On Sun, Jun 13 2021, Felipe Contreras wrote:
> >
> >> This patch series introduces the concept of extra components. These are
> >> components which are not yet part of the core but are good enough for
> >> distributions to ship, and in fact, they already do.
> >
> > I like this direction.
> 
> I do not mind change, but it is fuzzy to me what direction you are
> in favor of.  Is the gist of the idea to split what is in contrib/
> into two bins, ones that are closer to "official" and others?  If
> so, I see sort-of merit in such a distinction, but whom is this
> trying to help?

> 
> On Sun, Jun 13 2021, Felipe Contreras wrote:
> 
> > This patch series introduces the concept of extra components. These are
> > components which are not yet part of the core but are good enough for
> > distributions to ship, and in fact, they already do.
> 
> I like this direction.
> 
> > The measuring stick I'm using to gauge if a component in contrib belongs
> > in extra is simple: are we already running tests for them with
> > 'make test'?
> 
> I have a CI failure in one series of mine that seems to be a lack of
> updating to CMake in contrib/buildsystems, perhaps we should be adding
> that to extra/ too, i.e. extending this to the "make test" run by CI?
> 
> Not something that should hinder or necessarily be included in this
> series, just a note about a related component.

> > And we might move some components from the core which aren't realy part
> > of the core to extra, like gitk, git-gui, git-p4, and git-svn.
> 
> I'd also like to see us run the tests for the likes of mw-to-git,
> diff-highlight and subtree by default, at least under CI or some
> "extended tests" mode, even though we may not install them by default.