From: Felipe Contreras <hidden> Date: 2021-07-10 23:49:03
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 | 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/contrib/completion/git-completion.bash b/extra/completion/git-completion.bashsimilarity index 100%rename from contrib/completion/git-completion.bashrename to extra/completion/git-completion.bashdiff --git a/contrib/completion/git-completion.zsh b/extra/completion/git-completion.zshsimilarity index 100%rename from contrib/completion/git-completion.zshrename to extra/completion/git-completion.zshdiff --git a/contrib/completion/git-prompt.sh b/extra/completion/git-prompt.shsimilarity index 100%rename from contrib/completion/git-prompt.shrename to extra/completion/git-prompt.shdiff --git a/t/t9902-completion.sh b/t/t9902-completion.shindex 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.
diff --git a/contrib/workdir/.gitattributes b/extra/workdir/.gitattributessimilarity index 100%rename from contrib/workdir/.gitattributesrename to extra/workdir/.gitattributesdiff --git a/contrib/workdir/git-new-workdir b/extra/workdir/git-new-workdirsimilarity index 100%rename from contrib/workdir/git-new-workdirrename to extra/workdir/git-new-workdirdiff --git a/t/t1021-rerere-in-workdir.sh b/t/t1021-rerere-in-workdir.shindex 0b892894eb..035a92c0e7 100755--- a/t/t1021-rerere-in-workdir.sh+++ b/t/t1021-rerere-in-workdir.sh
@@ -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_failureSYMLINKS'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&&(cdkrow&&rm-f.git/rr-cache&&
From: Philippe Blain <hidden> Date: 2021-07-12 17:43:47
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.
From: Felipe Contreras <hidden> Date: 2021-07-12 17:55:19
Philippe Blain wrote:
Le 2021-07-10 à 19:46, Felipe Contreras a écrit :
quoted
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
From: Junio C Hamano <hidden> Date: 2021-07-13 00:17:19
Philippe Blain [off-list ref] 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.
From: Felipe Contreras <hidden> Date: 2021-07-13 01:19:08
Junio C Hamano wrote:
Philippe Blain [off-list ref] writes:
quoted
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.
From: Felipe Contreras <hidden> Date: 2021-07-14 20:23:48
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
From: Felipe Contreras <hidden> Date: 2021-07-14 20:23:50
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 | 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/contrib/completion/git-completion.bash b/extra/completion/git-completion.bashsimilarity index 100%rename from contrib/completion/git-completion.bashrename to extra/completion/git-completion.bashdiff --git a/contrib/completion/git-completion.zsh b/extra/completion/git-completion.zshsimilarity index 100%rename from contrib/completion/git-completion.zshrename to extra/completion/git-completion.zshdiff --git a/contrib/completion/git-prompt.sh b/extra/completion/git-prompt.shsimilarity index 100%rename from contrib/completion/git-prompt.shrename to extra/completion/git-prompt.shdiff --git a/t/t9902-completion.sh b/t/t9902-completion.shindex 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.
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).
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?
quoted hunk
### Maintainer's dist rules
diff --git a/contrib/completion/git-completion.bash b/extra/completion/git-completion.bashsimilarity index 100%rename from contrib/completion/git-completion.bashrename to extra/completion/git-completion.bashdiff --git a/contrib/completion/git-completion.zsh b/extra/completion/git-completion.zshsimilarity index 100%rename from contrib/completion/git-completion.zshrename to extra/completion/git-completion.zshdiff --git a/contrib/completion/git-prompt.sh b/extra/completion/git-prompt.shsimilarity index 100%rename from contrib/completion/git-prompt.shrename to extra/completion/git-prompt.shdiff --git a/t/t9902-completion.sh b/t/t9902-completion.shindex 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.
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" :(
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...
@@ -1301,7 +1301,7 @@ elif test -n "$GIT_TEST_INSTALLED"thenGIT_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:$PATHGIT_EXEC_PATH=${GIT_TEST_EXEC_PATH:-$GIT_EXEC_PATH}else# normal case, use ../bin-wrappers only unless $with_dashes:iftest-n"$no_bin_wrappers"
@@ -1322,7 +1322,7 @@ else # normal case, use ../bin-wrappers only unless $with_dashes:GIT_EXEC_PATH=$GIT_BUILD_DIRiftest-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"fifiGIT_TEMPLATE_DIR="$GIT_BUILD_DIR"/templates/blt
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'.
quoted
### Maintainer's dist rules
diff --git a/contrib/completion/git-completion.bash b/extra/completion/git-completion.bashsimilarity index 100%rename from contrib/completion/git-completion.bashrename to extra/completion/git-completion.bashdiff --git a/contrib/completion/git-completion.zsh b/extra/completion/git-completion.zshsimilarity index 100%rename from contrib/completion/git-completion.zshrename to extra/completion/git-completion.zshdiff --git a/contrib/completion/git-prompt.sh b/extra/completion/git-prompt.shsimilarity index 100%rename from contrib/completion/git-prompt.shrename to extra/completion/git-prompt.shdiff --git a/t/t9902-completion.sh b/t/t9902-completion.shindex 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.
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
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
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
?
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
From: Felipe Contreras <hidden> Date: 2021-07-16 20:14:33
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
From: Felipe Contreras <hidden> Date: 2021-07-16 20:14:34
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 <redacted>
---
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/contrib/completion/git-completion.bash b/extra/completion/git-completion.bashsimilarity index 100%rename from contrib/completion/git-completion.bashrename to extra/completion/git-completion.bashdiff --git a/contrib/completion/git-completion.zsh b/extra/completion/git-completion.zshsimilarity index 100%rename from contrib/completion/git-completion.zshrename to extra/completion/git-completion.zshdiff --git a/contrib/completion/git-prompt.sh b/extra/completion/git-prompt.shsimilarity index 100%rename from contrib/completion/git-prompt.shrename to extra/completion/git-prompt.shdiff --git a/t/t9902-completion.sh b/t/t9902-completion.shindex 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.