[PATCH v2 0/2] extra: new concept of extra components

[PATCH v2 0/2] extra: new concept of extra components

[Felipe Contreras]

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.

This benefits everyone:

 1. Distribution packagers that just want to do `make install`
 2. People who download git's source code and just want to do
    `make install`
 3. Developers who have no idea what's production-level quality in
    contrib/ and just want to do `make install`.

For now they'll have to do `make install install-extra`. But if the
result is deemed correct, we might choose to add "install-extra" to the
"install" target.

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'? If the answer is "yes, we do run tests", then the answer is
"yes, it belongs in contrib".

We might want to move more components from contrib to extra once their
tests are being run reliably.

And we might move some components from the core which aren't really part
of the core to extra, like gitk, git-gui, git-p4, and git-svn.

For now only contrib/completion and contrib/workdir are graduated to the
new area.

Since v1 I removed extra/Makefile in favor having the targets in the
top-level Makfile as Ævar suggested.

Felipe Contreras (2):
  completion: graduate out of contrib
  git-new-workdir: graduate out of contrib

 Makefile                                          | 13 +++++++++++++
 {contrib => extra}/completion/git-completion.bash |  0
 {contrib => extra}/completion/git-completion.zsh  |  0
 {contrib => extra}/completion/git-prompt.sh       |  0
 {contrib => extra}/workdir/.gitattributes         |  0
 {contrib => extra}/workdir/git-new-workdir        |  0
 t/t1021-rerere-in-workdir.sh                      |  6 +++---
 t/t3000-ls-files-others.sh                        |  2 +-
 t/t9902-completion.sh                             |  8 ++++----
 t/t9903-bash-prompt.sh                            |  2 +-
 10 files changed, 22 insertions(+), 9 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%)
 rename {contrib => extra}/workdir/.gitattributes (100%)
 rename {contrib => extra}/workdir/git-new-workdir (100%)

Range-diff against v1:
1:  3a2c2402af ! 1:  3f44bc3253 completion: graduate out of contrib
    @@ Commit message
         Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
     
      ## Makefile ##
    +@@ Makefile: 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
    +@@ Makefile: 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))
    +@@ Makefile: 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))
     @@ Makefile: quick-install-man:
      quick-install-html:
      	$(MAKE) -C Documentation quick-install-html
      
    -+install-extra:
    -+	$(MAKE) -C extra install
    ++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
     +
      
      
      ### Maintainer's dist rules
     
    - ## extra/Makefile (new) ##
    -@@
    -+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
    -
      ## contrib/completion/git-completion.bash => extra/completion/git-completion.bash ##
     
      ## contrib/completion/git-completion.zsh => extra/completion/git-completion.zsh ##
2:  81836329cd ! 2:  af9b24eeb1 git-new-workdir: graduate out of contrib
    @@ Commit message
     
         Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
     
    - ## extra/Makefile ##
    -@@ extra/Makefile: INSTALL ?= install
    + ## Makefile ##
    +@@ Makefile: quick-install-man:
    + quick-install-html:
    + 	$(MAKE) -C Documentation quick-install-html
      
    - all:
    - 
    --install: install-completion
    -+install: install-completion install-workdir
    +-install-extra: install-completion
    ++install-extra: install-completion install-workdir
      
      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
    -+
    + 	$(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
    + 
     +install-workdir:
    -+	$(INSTALL) -D workdir/git-new-workdir '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'/git-new-workdir
    ++	$(INSTALL) -D extra/workdir/git-new-workdir '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'/git-new-workdir
    ++
    + 
    + 
    + ### Maintainer's dist rules
     
      ## contrib/workdir/.gitattributes => extra/workdir/.gitattributes ##
     
-- 
2.32.0.36.g70aac2b1aa

[PATCH v2 1/2] completion: graduate out of contrib

[Felipe Contreras]

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                                          | 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
+
 
 
 ### 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\\]'
-- 
2.32.0.36.g70aac2b1aa

[PATCH v2 2/2] git-new-workdir: graduate out of contrib

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Makefile                                   | 5 ++++-
 {contrib => extra}/workdir/.gitattributes  | 0
 {contrib => extra}/workdir/git-new-workdir | 0
 t/t1021-rerere-in-workdir.sh               | 6 +++---
 t/t3000-ls-files-others.sh                 | 2 +-
 5 files changed, 8 insertions(+), 5 deletions(-)
 rename {contrib => extra}/workdir/.gitattributes (100%)
 rename {contrib => extra}/workdir/git-new-workdir (100%)

diff --git a/Makefile b/Makefile
index 0a13e5f077..a03387f8d1 100644
--- a/Makefile
+++ b/Makefile
@@ -3115,13 +3115,16 @@ quick-install-man:
 quick-install-html:
 	$(MAKE) -C Documentation quick-install-html
 
-install-extra: install-completion
+install-extra: install-completion install-workdir
 
 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
 
+install-workdir:
+	$(INSTALL) -D extra/workdir/git-new-workdir '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'/git-new-workdir
+
 
 
 ### Maintainer's dist rules
diff --git a/contrib/workdir/.gitattributes b/extra/workdir/.gitattributes
similarity index 100%
rename from contrib/workdir/.gitattributes
rename to extra/workdir/.gitattributes
diff --git a/contrib/workdir/git-new-workdir b/extra/workdir/git-new-workdir
similarity index 100%
rename from contrib/workdir/git-new-workdir
rename to extra/workdir/git-new-workdir
diff --git a/t/t1021-rerere-in-workdir.sh b/t/t1021-rerere-in-workdir.sh
index 0b892894eb..035a92c0e7 100755
--- a/t/t1021-rerere-in-workdir.sh
+++ b/t/t1021-rerere-in-workdir.sh
@@ -27,7 +27,7 @@ test_expect_success SYMLINKS setup '
 
 test_expect_success SYMLINKS 'rerere in workdir' '
 	rm -rf .git/rr-cache &&
-	"$SHELL_PATH" "$TEST_DIRECTORY/../contrib/workdir/git-new-workdir" . work &&
+	"$SHELL_PATH" "$TEST_DIRECTORY/../extra/workdir/git-new-workdir" . work &&
 	(
 		cd work &&
 		test_must_fail git merge side &&
@@ -38,12 +38,12 @@ test_expect_success SYMLINKS 'rerere in workdir' '
 '
 
 # This fails because we don't resolve relative symlink in mkdir_in_gitdir()
-# For the purpose of helping contrib/workdir/git-new-workdir users, we do not
+# For the purpose of helping extra/workdir/git-new-workdir users, we do not
 # have to support relative symlinks, but it might be nicer to make this work
 # with a relative symbolic link someday.
 test_expect_failure SYMLINKS 'rerere in workdir (relative)' '
 	rm -rf .git/rr-cache &&
-	"$SHELL_PATH" "$TEST_DIRECTORY/../contrib/workdir/git-new-workdir" . krow &&
+	"$SHELL_PATH" "$TEST_DIRECTORY/../extra/workdir/git-new-workdir" . krow &&
 	(
 		cd krow &&
 		rm -f .git/rr-cache &&
diff --git a/t/t3000-ls-files-others.sh b/t/t3000-ls-files-others.sh
index 740ce56eab..86240a1b98 100755
--- a/t/t3000-ls-files-others.sh
+++ b/t/t3000-ls-files-others.sh
@@ -84,7 +84,7 @@ test_expect_success SYMLINKS 'ls-files --others with symlinked submodule' '
 	) &&
 	(
 		cd super &&
-		"$SHELL_PATH" "$TEST_DIRECTORY/../contrib/workdir/git-new-workdir" ../sub sub &&
+		"$SHELL_PATH" "$TEST_DIRECTORY/../extra/workdir/git-new-workdir" ../sub sub &&
 		git ls-files --others --exclude-standard >../actual
 	) &&
 	echo sub/ >expect &&
-- 
2.32.0.36.g70aac2b1aa

Re: [PATCH v2 0/2] extra: new concept of extra components

[Philippe Blain]

Hi Felipe,

Le 2021-07-10 à 19:46, Felipe Contreras a écrit :
> 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.
> 
> This benefits everyone:
> 
>   1. Distribution packagers that just want to do `make install`
>   2. People who download git's source code and just want to do
>      `make install`
>   3. Developers who have no idea what's production-level quality in
>      contrib/ and just want to do `make install`.
> 
> For now they'll have to do `make install install-extra`. But if the
> result is deemed correct, we might choose to add "install-extra" to the
> "install" target.

I agree with the end goal of this series. I myself carry a patch that adds an
'install-completion' target to the main Makefile that I apply before installing
Git from 'master'.

> 
> 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'? If the answer is "yes, we do run tests", then the answer is
> "yes, it belongs in contrib".

OK, this seems about right, it should cover prompt and completion, but...

> 
> We might want to move more components from contrib to extra once their
> tests are being run reliably.
> 
> And we might move some components from the core which aren't really part
> of the core to extra, like gitk, git-gui, git-p4, and git-svn.
> 
> For now only contrib/completion and contrib/workdir are graduated to the
> new area.

... when I read this I went "what is this workdir thing, it must date from before
'git worktree' was added". And combing through the history, it does. The latest
commit to the script is e32afab7b0 (git-new-workdir: don't fail if the target
directory is empty, 2014-11-26), which describes as v2.3.0-rc0~60^2. And
'git worktree' was shipped in Git 2.5, 2015-07-27.

Looking at the tests, I see two uses of 'git-new-workdir':
$ git grep  -p 'new-workdir'
t1021-rerere-in-workdir.sh=28=test_expect_success SYMLINKS 'rerere in workdir' '
t1021-rerere-in-workdir.sh:30:56:       "$SHELL_PATH" "$TEST_DIRECTORY/../contrib/workdir/git-new-workdir" . work &&
t1021-rerere-in-workdir.sh:41:50:# For the purpose of helping contrib/workdir/git-new-workdir users, we do not
t1021-rerere-in-workdir.sh=44=test_expect_failure SYMLINKS 'rerere in workdir (relative)' '
t1021-rerere-in-workdir.sh:46:56:       "$SHELL_PATH" "$TEST_DIRECTORY/../contrib/workdir/git-new-workdir" . krow &&
t3000-ls-files-others.sh=75=test_expect_success SYMLINKS 'ls-files --others with symlinked submodule' '
t3000-ls-files-others.sh:87:57:         "$SHELL_PATH" "$TEST_DIRECTORY/../contrib/workdir/git-new-workdir" ../sub sub &&

So they are not really testing this script per se, more like testing rerere and ls-files
in a worktree created by 'git-new-workdir'. I do not think this enough justification
to include 'git new-workdir' in 'extra/', since 'git worktree add' does the same thing
and is a builtin command. Even if its "BUGS" section in the doc says it's "in general [...]
still experimental", an experimental builtin is better than a 'contrib' script, no ?

Cheers,

Philippe.

Re: [PATCH v2 0/2] extra: new concept of extra components

[Felipe Contreras]

Philippe Blain wrote:
> Le 2021-07-10 à 19:46, Felipe Contreras a écrit :

> > We might want to move more components from contrib to extra once their
> > tests are being run reliably.
> > 
> > And we might move some components from the core which aren't really part
> > of the core to extra, like gitk, git-gui, git-p4, and git-svn.
> > 
> > For now only contrib/completion and contrib/workdir are graduated to the
> > new area.
> 
> ... when I read this I went "what is this workdir thing, it must date from before
> 'git worktree' was added". And combing through the history, it does. The latest
> commit to the script is e32afab7b0 (git-new-workdir: don't fail if the target
> directory is empty, 2014-11-26), which describes as v2.3.0-rc0~60^2. And
> 'git worktree' was shipped in Git 2.5, 2015-07-27.
> 
> Looking at the tests, I see two uses of 'git-new-workdir':
> $ git grep  -p 'new-workdir'
> t1021-rerere-in-workdir.sh=28=test_expect_success SYMLINKS 'rerere in workdir' '
> t1021-rerere-in-workdir.sh:30:56:       "$SHELL_PATH" "$TEST_DIRECTORY/../contrib/workdir/git-new-workdir" . work &&
> t1021-rerere-in-workdir.sh:41:50:# For the purpose of helping contrib/workdir/git-new-workdir users, we do not
> t1021-rerere-in-workdir.sh=44=test_expect_failure SYMLINKS 'rerere in workdir (relative)' '
> t1021-rerere-in-workdir.sh:46:56:       "$SHELL_PATH" "$TEST_DIRECTORY/../contrib/workdir/git-new-workdir" . krow &&
> t3000-ls-files-others.sh=75=test_expect_success SYMLINKS 'ls-files --others with symlinked submodule' '
> t3000-ls-files-others.sh:87:57:         "$SHELL_PATH" "$TEST_DIRECTORY/../contrib/workdir/git-new-workdir" ../sub sub &&
> 
> So they are not really testing this script per se, more like testing rerere and ls-files
> in a worktree created by 'git-new-workdir'. I do not think this enough justification
> to include 'git new-workdir' in 'extra/', since 'git worktree add' does the same thing
> and is a builtin command. Even if its "BUGS" section in the doc says it's "in general [...]
> still experimental", an experimental builtin is better than a 'contrib' script, no ?

I agree.

However, that points out to another problem: the tests should not be using `git new-workdir`.

I'm fine with dropping the last patch from the series.

-- 
Felipe Contreras

Re: [PATCH v2 0/2] extra: new concept of extra components

[Junio C Hamano]

Philippe Blain <levraiphilippeblain@gmail.com> writes:

> in a worktree created by 'git-new-workdir'. I do not think this enough justification
> to include 'git new-workdir' in 'extra/', since 'git worktree add' does the same thing
> and is a builtin command.

"git worktree" was invented primarily to properly solve what
new-workdir wanted to, so I agree that all efforts to promote,
upkeep and extend the latter should be spent to improve the former.

Re: [PATCH v2 0/2] extra: new concept of extra components

[Felipe Contreras]

Junio C Hamano wrote:
> Philippe Blain <levraiphilippeblain@gmail.com> writes:
> 
> > in a worktree created by 'git-new-workdir'. I do not think this enough justification
> > to include 'git new-workdir' in 'extra/', since 'git worktree add' does the same thing
> > and is a builtin command.
> 
> "git worktree" was invented primarily to properly solve what
> new-workdir wanted to, so I agree that all efforts to promote,
> upkeep and extend the latter should be spent to improve the former.

OK. What about $subject?

-- 
Felipe Contreras

[PATCH v3 0/1] extra: new concept of extra components

[Felipe Contreras]

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.

This benefits everyone:

 1. Distribution packagers that just want to do `make install`
 2. People who download git's source code and just want to do
    `make install`
 3. Developers who have no idea what's production-level quality in
    contrib/ and just want to do `make install`.

For now they'll have to do `make install install-extra`. But if the
result is deemed correct, we might choose to add "install-extra" to the
"install" target.

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'? If the answer is "yes, we do run tests", then the answer is
"yes, it belongs in contrib".

We might want to move more components from contrib to extra once their
tests are being run reliably.

And we might move some components from the core which aren't really part
of the core to extra, like gitk, git-gui, git-p4, and git-svn.

For now only part of contrib/completion is graduated to the new area.

Since v2 I removed workdir from the list of graduates as Philippe Blain
suggested.

Felipe Contreras (1):
  completion: graduate out of contrib

 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%)

Range-diff against v2:
1:  3f44bc3253 = 1:  3f44bc3253 completion: graduate out of contrib
2:  af9b24eeb1 < -:  ---------- git-new-workdir: graduate out of contrib
-- 
2.32.0.38.g1d70fa854e

[PATCH v3 1/1] completion: graduate out of contrib

[Felipe Contreras]

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                                          | 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
+
 
 
 ### 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\\]'
-- 
2.32.0.38.g1d70fa854e

Re: [PATCH v3 1/1] completion: graduate out of contrib

[Ævar Arnfjörð Bjarmason]

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

Re: [PATCH v3 1/1] completion: graduate out of contrib

[Ævar Arnfjörð Bjarmason]

On Thu, Jul 15 2021, Ævar Arnfjörð Bjarmason wrote:

> On Wed, Jul 14 2021, Felipe Contreras wrote:
>> [...]
>> @@ -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" :(

I forgot to include this not-working patch, i.e. I've got no idea what
the "something" should be other than the harder thing of compiling
"sharedir" into git and making "git --share-path" work.

It's not /that/ bad, see the grep for GIT_LOCALE_PATH, still a bit
painful, and maybe not worth it for this change...

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 98e20950c3..0a9fbfc253 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1301,7 +1301,7 @@ elif test -n "$GIT_TEST_INSTALLED"
 then
 	GIT_EXEC_PATH=$($GIT_TEST_INSTALLED/git --exec-path)  ||
 	error "Cannot run git from $GIT_TEST_INSTALLED."
-	PATH=$GIT_TEST_INSTALLED:$GIT_BUILD_DIR/t/helper:$PATH
+	PATH=$GIT_TEST_INSTALLED:$GIT_TEST_INSTALLED/something:$GIT_BUILD_DIR/t/helper:$PATH
 	GIT_EXEC_PATH=${GIT_TEST_EXEC_PATH:-$GIT_EXEC_PATH}
 else # normal case, use ../bin-wrappers only unless $with_dashes:
 	if test -n "$no_bin_wrappers"
@@ -1322,7 +1322,7 @@ else # normal case, use ../bin-wrappers only unless $with_dashes:
 	GIT_EXEC_PATH=$GIT_BUILD_DIR
 	if test -n "$with_dashes"
 	then
-		PATH="$GIT_BUILD_DIR:$GIT_BUILD_DIR/t/helper:$PATH"
+		PATH="$GIT_BUILD_DIR:$GIT_BUILD_DIR/t/helper:$GIT_BUILD_DIR/extra/completion:$PATH"
 	fi
 fi
 GIT_TEMPLATE_DIR="$GIT_BUILD_DIR"/templates/blt

Re: [PATCH v3 1/1] completion: graduate out of contrib

[Felipe Contreras]

Ævar Arnfjörð Bjarmason wrote:
> On Wed, Jul 14 2021, Felipe Contreras wrote:

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

All right.

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

git-prompt.sh is not really part of the completion stuff, so I don't
think the location Debian chose is correct. At some point in time they
were together, so perhaps that why they chose the same location.

Generally scripts and other shared data belongs in /usr/share.

> Also why /git and /_git for bash and zsh (looks good) but /git-prompt
> instead of /git-prompt.sh?

Probably because it was split from 'git'.

> >  ### 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" :(

Yes, we would need --share-path, and perhaps GIT_SHARE_PATH, and so on.

Might make sense for a future patch.

-- 
Felipe Contreras

Re: [PATCH v3 1/1] completion: graduate out of contrib

[Felipe Contreras]

Ævar Arnfjörð Bjarmason wrote:
> 
> On Thu, Jul 15 2021, Ævar Arnfjörð Bjarmason wrote:
> 
> > On Wed, Jul 14 2021, Felipe Contreras wrote:
> >> [...]
> >> @@ -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" :(
> 
> I forgot to include this not-working patch, i.e. I've got no idea what
> the "something" should be other than the harder thing of compiling
> "sharedir" into git and making "git --share-path" work.

I don't think there's a "something" that would make sense.

It would be something like '/opt/git/bin/completion'?

-- 
Felipe Contreras

Re: [PATCH v3 1/1] completion: graduate out of contrib

[Ævar Arnfjörð Bjarmason]

On Thu, Jul 15 2021, Felipe Contreras wrote:

> Ævar Arnfjörð Bjarmason wrote:
>> 
>> On Thu, Jul 15 2021, Ævar Arnfjörð Bjarmason wrote:
>> 
>> > On Wed, Jul 14 2021, Felipe Contreras wrote:
>> >> [...]
>> >> @@ -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" :(
>> 
>> I forgot to include this not-working patch, i.e. I've got no idea what
>> the "something" should be other than the harder thing of compiling
>> "sharedir" into git and making "git --share-path" work.
>
> I don't think there's a "something" that would make sense.
>
> It would be something like '/opt/git/bin/completion'?

I think more importantly if they're going to be "first-class" components
that we have some native way of getting them to the user.

I.e. once you install git being able to load them in your shell as:

    . git-path
    . git-completion

Or whatever, which means either putting them in $PATH (i.e. we'd drop
them in bin/ along with the non-dashed-built-ins like git-upload-pack
etc.), or something like:

    . "$(git --completion-path)"/bash
    . "$(git --completion-path)"/zsh

Or maybe:

    . "$(git --extras-path)"/completion/bash.sh
    . "$(git --extras-path)"/prompt.sh

?

Re: [PATCH v3 1/1] completion: graduate out of contrib

[Felipe Contreras]

Ævar Arnfjörð Bjarmason wrote:
> On Thu, Jul 15 2021, Felipe Contreras wrote:
> > Ævar Arnfjörð Bjarmason wrote:
> >> On Thu, Jul 15 2021, Ævar Arnfjörð Bjarmason wrote:
> >> > On Wed, Jul 14 2021, Felipe Contreras wrote:
> >> >> [...]
> >> >> @@ -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" :(
> >> 
> >> I forgot to include this not-working patch, i.e. I've got no idea what
> >> the "something" should be other than the harder thing of compiling
> >> "sharedir" into git and making "git --share-path" work.
> >
> > I don't think there's a "something" that would make sense.
> >
> > It would be something like '/opt/git/bin/completion'?
> 
> I think more importantly if they're going to be "first-class" components
> that we have some native way of getting them to the user.

Yes, I already said --share-path would make sense [1] in your other
reply.

> I.e. once you install git being able to load them in your shell as:
> 
>     . git-path
>     . git-completion
> 
> Or whatever, which means either putting them in $PATH (i.e. we'd drop
> them in bin/ along with the non-dashed-built-ins like git-upload-pack
> etc.), or something like:
> 
>     . "$(git --completion-path)"/bash
>     . "$(git --completion-path)"/zsh
> 
> Or maybe:
> 
>     . "$(git --extras-path)"/completion/bash.sh
>     . "$(git --extras-path)"/prompt.sh
> 
> ?

But that's not how completions should work.

There's a standard location for bash completions in order to be picked
by bash-completion:

  /usr/share/bash-completion/completions

This will be the case regardless of what --extras-path is.

The user *should not* be sourcing anything from there directly,
bash-completion does that automatically.

This location depends by distribution, and the standard way to figure it
out is:

  pkg-config --variable=completionsdir bash-completion

If you don't have bash-completion insalled, *then* you would need to
source the completion file directly, but you cannot use pkg-config to
figure out that location, so you would need to use the default
(/usr/share/bash-completion/completions).

We could install the completion twice (or provide a symlink):

  /usr/share/bash-completion/completions/git
  /usr/share/git-core/completion/bash.sh

So if the user doesn't have bash-completion installed, can do:

  source "$(git --extras-path)"/completion/bash.sh

Instead of the standard:

  source /usr/share/bash-completion/completions/git

But *right now* the standard location is standard, distributions are
using it, and users are using it.

Moreover, it seems a bit wasteful to have --exec-path, --html-path,
--man-path, --info-path, --extra-path. Why not have --path=extra
instead?

Once again, I think this is a good idea, but it should be done
separately, and you yourself said "for bonus points".


Not to mention my strong feeling that *even* if I implement this nice
feature, Junio will still ignore this series (as he ignores everything I
send), so I will be simply wasting my time implementing something that
will never be merged (just like he stood on the sidelines watching us
implement 6 versions of the man pager colorize patch just to say "no
thanks" [2]).

I'd be more than happy to add any other suggestions you might have, but
let's be honest: this is a patch we all will have be carrying ourselves
(I have dozens of those), so I'd rather not add more complexity for a
perfect solution that won't be applied.

I'd rather aim for something that is good enough.

Cheers.

[1] https://lore.kernel.org/git/60f0859399369_519c2083c@natae.notmuch/
[2] https://lore.kernel.org/git/xmqq4ke8pig9.fsf@gitster.g/

-- 
Felipe Contreras

[PATCH v4 0/1] extra: new concept of extra components

[Felipe Contreras]

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.

This benefits everyone:

 1. Distribution packagers that just want to do `make install`
 2. People who download git's source code and just want to do
    `make install`
 3. Developers who have no idea what's production-level quality in
    contrib/ and just want to do `make install`.

For now they'll have to do `make install install-extra`. But if the
result is deemed correct, we might choose to add "install-extra" to the
"install" target.

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'? If the answer is "yes, we do run tests", then the answer is
"yes, it belongs in extra".

We might want to move more components from contrib to extra once their
tests are being run reliably.

And we might move some components from the core which aren't really part
of the core to extra, like gitk, git-gui, git-p4, and git-svn.

For now only part of contrib/completion is graduated to the new area.

Since v3 I added .PHONY to the new targets as Ævar Arnfjörð suggetsed,
and also added a bit for the explanation of extra from the cover letter
to the commit message.

Felipe Contreras (1):
  completion: graduate out of contrib

 Makefile                                          | 11 +++++++++++
 {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, 16 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%)

Range-diff against v3:
1:  3f44bc3253 ! 1:  07eb614ef7 completion: graduate out of contrib
    @@ Commit message
         move the completions out of contrib.
     
         Let's move them out of contrib and provide an installation target
    -    install-extra.
    +    install-extra which be a convenient target for package maintainers and
    +    other people who install git themselves.
    +
    +    The measuring stick for what belongs in "extra" is simple: are we
    +    already running tests for them with 'make test'? Currently only
    +    part of completions fit that bill, but others could be added later.
     
         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.
    +    $(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.
    @@ Makefile: htmldir_relative_SQ = $(subst ','\'',$(htmldir_relative))
      
      SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
      TEST_SHELL_PATH_SQ = $(subst ','\'',$(TEST_SHELL_PATH))
    +@@ Makefile: endif
    + 	./check_bindir "z$$bindir" "z$$execdir" "$$bindir/git-add$X"
    + 
    + .PHONY: install-gitweb install-doc install-man install-man-perl install-html install-info install-pdf
    ++.PHONY: install-extra install-completion
    + .PHONY: quick-install-doc quick-install-man quick-install-html
    + install-gitweb:
    + 	$(MAKE) -C gitweb install
     @@ Makefile: quick-install-man:
      quick-install-html:
      	$(MAKE) -C Documentation quick-install-html
-- 
2.32.0.40.gb9b36f9b52

[PATCH v4 1/1] completion: graduate out of contrib

[Felipe Contreras]

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 which be a convenient target for package maintainers and
other people who install git themselves.

The measuring stick for what belongs in "extra" is simple: are we
already running tests for them with 'make test'? Currently only
part of completions fit that bill, but others could be added later.

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                                          | 11 +++++++++++
 {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, 16 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..33402cc09d 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))
@@ -3079,6 +3082,7 @@ endif
 	./check_bindir "z$$bindir" "z$$execdir" "$$bindir/git-add$X"
 
 .PHONY: install-gitweb install-doc install-man install-man-perl install-html install-info install-pdf
+.PHONY: install-extra install-completion
 .PHONY: quick-install-doc quick-install-man quick-install-html
 install-gitweb:
 	$(MAKE) -C gitweb install
@@ -3112,6 +3116,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
+
 
 
 ### 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\\]'
-- 
2.32.0.40.gb9b36f9b52