git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 17/21] completion: don't use __gitdir() for git commands

From
SZEDER Gábor <szeder@ira.uka.de>
Date
Feb 25, 2016, 22:50 UTC
Message-ID
<1456440650-32623-18-git-send-email-szeder@ira.uka.de>
In-Reply-To
<1456440650-32623-1-git-send-email-szeder@ira.uka.de>

Several completion functions contain the following pattern to run git commands respecting the path to the repository specified on the command line:

  git --git-dir="$(__gitdir)" <cmd> <options>

This imposes the overhead of fork()ing a subshell for the command substitution and potentially fork()+exec()ing 'git rev-parse' inside __gitdir().

Now, if neither '--gitdir=<path>' nor '-C <path>' options are specified on the command line, then those git commands are perfectly capable to discover the repository on their own. If either one or both of those options are specified on the command line, then, again, the git commands could discover the repository, if we pass them all of those options from the command line.

This means we don't have to run __gitdir() at all for git commands and can spare its fork()+exec() overhead.

Use Bash parameter expansions to check the $__git_dir variable and $__git_C_args array and to assemble the appropriate '--git-dir=<path>' and '-C <path>' options if either one or both are present on the command line. These parameter expansions are, however, rather long, so instead of changing all git executions and make already long lines even longer, encapsulate running git with '--git-dir=<path> -C <path>' options into the new __git() wrapper function.

There's one tricky case, though: in __git_refs() local refs are listed with 'git for-each-ref', where "local" is not necessarily the repository we are currently in, but it might mean a remote repository in the filesystem (e.g. listing refs for 'git fetch /some/other/repo <TAB>'). Use one-shot variable assignment to override $__git_dir with the path of the repository where the refs should come from. Although one-shot variable assignments in front of shell functions are to be avoided in our scripts in general, in the Bash completion script we can do that safely.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash | 58 ++++++++++++++++++----------------
 1 file changed, 30 insertions(+), 28 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 9ffea9580ff7..c496f4026fc8 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -61,6 +61,14 @@ __gitdir ()
 	fi
 }
 
+# Runs git with all the options given as argument, respecting any
+# '--git-dir=<path>' and '-C <path>' options present on the command line
+__git ()
+{
+	git ${__git_C_args:+"${__git_C_args[@]}"} \
+		${__git_dir:+--git-dir="$__git_dir"} "$@"
+}
+
 # The following function is based on code from:
 #
 #   bash_completion - programmable completion functions for bash 3.2+
@@ -287,13 +295,11 @@ __gitcomp_file ()
 # argument, and using the options specified in the second argument.
 __git_ls_files_helper ()
 {
-	local dir="$(__gitdir)"
-
 	if [ "$2" == "--committable" ]; then
-		git ${__git_C_args:+"${__git_C_args[@]}"} --git-dir="$dir" -C "$1" diff-index --name-only --relative HEAD
+		__git -C "$1" diff-index --name-only --relative HEAD
 	else
 		# NOTE: $2 is not quoted in order to support multiple options
-		git ${__git_C_args:+"${__git_C_args[@]}"} --git-dir="$dir" -C "$1" ls-files --exclude-standard $2
+		__git -C "$1" ls-files --exclude-standard $2
 	fi 2>/dev/null
 }
 
@@ -323,8 +329,7 @@ __git_heads ()
 {
 	local dir="$(__gitdir)"
 	if [ -d "$dir" ]; then
-		git --git-dir="$dir" for-each-ref --format='%(refname:short)' \
-			refs/heads
+		__git for-each-ref --format='%(refname:short)' refs/heads
 		return
 	fi
 }
@@ -333,8 +338,7 @@ __git_tags ()
 {
 	local dir="$(__gitdir)"
 	if [ -d "$dir" ]; then
-		git --git-dir="$dir" for-each-ref --format='%(refname:short)' \
-			refs/tags
+		__git for-each-ref --format='%(refname:short)' refs/tags
 		return
 	fi
 }
@@ -387,14 +391,14 @@ __git_refs ()
 			refs="refs/tags refs/heads refs/remotes"
 			;;
 		esac
-		git --git-dir="$dir" for-each-ref --format="%($format)" \
+		__git_dir="$dir" __git for-each-ref --format="%($format)" \
 			$refs
 		if [ -n "$track" ]; then
 			# employ the heuristic used by git checkout
 			# Try to find a remote branch that matches the completion word
 			# but only output if the branch name is unique
 			local ref entry
-			git --git-dir="$dir" for-each-ref --shell --format="ref=%(refname:short)" \
+			__git for-each-ref --shell --format="ref=%(refname:short)" \
 				"refs/remotes/" | \
 			while read -r entry; do
 				eval "$entry"
@@ -408,7 +412,7 @@ __git_refs ()
 	fi
 	case "$cur" in
 	refs|refs/*)
-		git --git-dir="$dir" ls-remote "$remote" "$cur*" 2>/dev/null | \
+		__git ls-remote "$remote" "$cur*" 2>/dev/null | \
 		while read -r hash i; do
 			case "$i" in
 			*^{}) ;;
@@ -419,10 +423,10 @@ __git_refs ()
 	*)
 		if [ "$named_remote" = y ]; then
 			echo "HEAD"
-			git --git-dir="$dir" for-each-ref --format="%(refname:short)" -- \
+			__git for-each-ref --format="%(refname:short)" -- \
 				"refs/remotes/$remote/" 2>/dev/null | sed -e "s#^$remote/##"
 		else
-			git --git-dir="$dir" ls-remote "$remote" HEAD \
+			__git ls-remote "$remote" HEAD \
 				'refs/tags/*' 'refs/heads/*' 'refs/remotes/*' 2>/dev/null |
 			while read -r hash i; do
 				case "$i" in
@@ -449,7 +453,7 @@ __git_refs2 ()
 __git_refs_remotes ()
 {
 	local i hash
-	git --git-dir="$(__gitdir)" ls-remote "$1" 'refs/heads/*' 2>/dev/null | \
+	__git ls-remote "$1" 'refs/heads/*' 2>/dev/null | \
 	while read -r hash i; do
 		echo "$i:refs/remotes/$1/${i#refs/heads/}"
 	done
@@ -459,7 +463,7 @@ __git_remotes ()
 {
 	local d="$(__gitdir)"
 	test -d "$d/remotes" && ls -1 "$d/remotes"
-	git --git-dir="$d" remote
+	__git remote
 }
 
 # Returns true if $1 matches the name of a configured remote, false otherwise.
@@ -525,7 +529,7 @@ __git_complete_revlist_file ()
 		*)   pfx="$ref:$pfx" ;;
 		esac
 
-		__gitcomp_nl "$(git ${__git_C_args:+"${__git_C_args[@]}"} --git-dir="$(__gitdir)" ls-tree "$ls" 2>/dev/null \
+		__gitcomp_nl "$(__git ls-tree "$ls" 2>/dev/null \
 				| sed '/^100... blob /{
 				           s,^.*	,,
 				           s,$, ,
@@ -803,7 +807,7 @@ __git_compute_porcelain_commands ()
 __git_get_config_variables ()
 {
 	local section="$1" i IFS=$'\n'
-	for i in $(git --git-dir="$(__gitdir)" config --name-only --get-regexp "^$section\..*" 2>/dev/null); do
+	for i in $(__git config --name-only --get-regexp "^$section\..*" 2>/dev/null); do
 		echo "${i#$section.}"
 	done
 }
@@ -821,8 +825,7 @@ __git_aliases ()
 # __git_aliased_command requires 1 argument
 __git_aliased_command ()
 {
-	local word cmdline=$(git --git-dir="$(__gitdir)" \
-		config --get "alias.$1" 2>/dev/null)
+	local word cmdline=$(__git config --get "alias.$1" 2>/dev/null)
 	for word in $cmdline; do
 		case "$word" in
 		\!gitk|gitk)
@@ -1192,7 +1195,7 @@ _git_commit ()
 		return
 	esac
 
-	if git --git-dir="$(__gitdir)" rev-parse --verify --quiet HEAD >/dev/null; then
+	if __git rev-parse --verify --quiet HEAD >/dev/null; then
 		__git_complete_index_file "--committable"
 	else
 		# This is the first commit
@@ -1778,7 +1781,7 @@ _git_send_email ()
 	case "$prev" in
 	--to|--cc|--bcc|--from)
 		__gitcomp "
-		$(git --git-dir="$(__gitdir)" send-email --dump-aliases 2>/dev/null)
+		$(__git send-email --dump-aliases 2>/dev/null)
 		"
 		return
 		;;
@@ -1810,7 +1813,7 @@ _git_send_email ()
 		;;
 	--to=*|--cc=*|--bcc=*|--from=*)
 		__gitcomp "
-		$(git --git-dir="$(__gitdir)" send-email --dump-aliases 2>/dev/null)
+		$(__git send-email --dump-aliases 2>/dev/null)
 		" "" "${cur#--*=}"
 		return
 		;;
@@ -1855,7 +1858,7 @@ __git_config_get_set_variables ()
 		c=$((--c))
 	done
 
-	git --git-dir="$(__gitdir)" config $config_file --name-only --list 2>/dev/null
+	__git config $config_file --name-only --list 2>/dev/null
 }
 
 _git_config ()
@@ -1890,9 +1893,8 @@ _git_config ()
 	remote.*.push)
 		local remote="${prev#remote.}"
 		remote="${remote%.push}"
-		__gitcomp_nl "$(git --git-dir="$(__gitdir)" \
-			for-each-ref --format='%(refname):%(refname)' \
-			refs/heads)"
+		__gitcomp_nl "$(__git for-each-ref
+			--format='%(refname):%(refname)' refs/heads)"
 		return
 		;;
 	pull.twohead|pull.octopus)
@@ -2475,12 +2477,12 @@ _git_stash ()
 			if [ $cword -eq 3 ]; then
 				__gitcomp_nl "$(__git_refs)";
 			else
-				__gitcomp_nl "$(git --git-dir="$(__gitdir)" stash list \
+				__gitcomp_nl "$(__git stash list \
 						| sed -n -e 's/:.*//p')"
 			fi
 			;;
 		show,*|apply,*|drop,*|pop,*)
-			__gitcomp_nl "$(git --git-dir="$(__gitdir)" stash list \
+			__gitcomp_nl "$(__git stash list \
 					| sed -n -e 's/:.*//p')"
 			;;
 		*)

-- 
2.7.2.410.g92cb358
Previous: SZEDER GáborNext: SZEDER Gábor
Message 21 of 25 in “completion: __gitdir()-related improvements”
  1. 00/21 completion: __gitdir()-related improvementsSZEDER Gábor, Feb 25, 2016
  2. 01/21 completion: improve __git_refs()'s in-code documentationSZEDER Gábor, Feb 25, 2016
  3. 02/21 completion tests: don't add test cruft to the test repositorySZEDER Gábor, Feb 25, 2016
  4. 03/21 completion tests: make the $cur variable local to the test helper functionsSZEDER Gábor, Feb 25, 2016
  5. 04/21 completion tests: consolidate getting path of current working directorySZEDER Gábor, Feb 25, 2016
  6. 05/21 completion tests: check __gitdir()'s output in the error casesSZEDER Gábor, Feb 25, 2016
  7. 06/21 completion tests: add tests for the __git_refs() helper functionSZEDER Gábor, Feb 25, 2016
  8. 07/21 completion: ensure that the repository path given on the command line existsSZEDER Gábor, Feb 25, 2016
  9. 08/21 completion: fix most spots not respecting 'git --git-dir=<path>'SZEDER Gábor, Feb 25, 2016
  10. 09/21 completion: respect 'git --git-dir=<path>' when listing remote refsSZEDER Gábor, Feb 25, 2016
  11. 10/21 completion: list refs from remote when remote's name matches a directorySZEDER Gábor, Feb 25, 2016
  12. 11/21 completion: don't list 'HEAD' when trying refs completion outside of a repoSZEDER Gábor, Feb 25, 2016
  13. 12/21 completion: list short refs from a remote given as a URLSZEDER Gábor, Feb 25, 2016
  14. 13/21 rev-parse: add '--absolute-git-dir' optionSZEDER Gábor, Feb 25, 2016
  15. Mike RappazzoApr 26, 2016
  16. Jeff KingApr 26, 2016
  17. SZEDER GáborMay 18, 2016
  18. 14/21 completion: don't offer commands when 'git --opt' needs an argumentSZEDER Gábor, Feb 25, 2016
  19. 15/21 completion: fix completion after 'git -C <path>'SZEDER Gábor, Feb 25, 2016
  20. 16/21 completion: respect 'git -C <path>'SZEDER Gábor, Feb 25, 2016
  21. 17/21 completion: don't use __gitdir() for git commandsSZEDER Gábor, Feb 25, 2016
  22. 18/21 completion: consolidate silencing errors from git commandsSZEDER Gábor, Feb 25, 2016
  23. 19/21 completion: don't guard git executions with __gitdir()SZEDER Gábor, Feb 25, 2016
  24. 20/21 completion: extract repository discovery from __gitdir()SZEDER Gábor, Feb 25, 2016
  25. 21/21 completion: cache the path to the repositorySZEDER Gábor, Feb 25, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.