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

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.

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'?

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

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

 Makefile                                      |  3 +++
 extra/Makefile                                | 20 +++++++++++++++++++
 .../completion/git-completion.bash            |  0
 .../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 +-
 11 files changed, 32 insertions(+), 9 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%)
 rename {contrib => extra}/workdir/.gitattributes (100%)
 rename {contrib => extra}/workdir/git-new-workdir (100%)

-- 
2.32.0

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

-- 
2.32.0

Signed-off-by: Felipe Contreras <redacted>
---
 extra/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/extra/Makefile b/extra/Makefile
index 26d8be55b0..66a1cdcdf4 100644
--- a/extra/Makefile
+++ b/extra/Makefile

@@ -9,9 +9,12 @@ INSTALL ?= install
 
 all:
 
-install: install-completion
+install: 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-workdir:
+	$(INSTALL) -D workdir/git-new-workdir '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'/git-new-workdir

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

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

...and a large part of the Makefile is just things like this that we'd
mostly/entirely get for free.

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.

Ævar Arnfjörð Bjarmason wrote:

On Sun, Jun 13 2021, Felipe Contreras wrote:

quoted

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

All right. Fine by me.

-- 
Felipe Contreras

Ævar Arnfjörð Bjarmason wrote:

On Sun, Jun 13 2021, Felipe Contreras wrote:

quoted

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.

quoted

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.

Yeah, but then it would be less clear which components belong in extra.

I suppose if `make test` also runs the test-extra target, then the
my initial definition of extra is still maintained.

quoted

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.

extra components are not installed by default, you would have to do
install-extra.

Do you think mw-to-git and similar should be installed with
install-extra?

-- 
Felipe Contreras