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

[PATCH 07/19] completion: make __gitdir() store repository path in $__git_dir

From
SZEDER Gábor <szeder@ira.uka.de>
Date
May 9, 2012, 00:44 UTC
Message-ID
<1336524290-30023-8-git-send-email-szeder@ira.uka.de>
In-Reply-To
<1336524290-30023-1-git-send-email-szeder@ira.uka.de>

The __gitdir() helper function is responsible for finding out the path to the repository, and it's invoked from several completion (helper) functions and from __git_ps1(). The path of the repository is printed to stdout, therefore it's invoked in a command substitution. This has the following drawbacks:

  1. The command substitution involves the forking of a subshell,
     which has a considerable overhead.
  2. There are a few cases, where __gitdir() is called more than once
     during a single completion, which means multiple command
     substitutions and eventual multiple 'git rev-parse --git-dir'
     executions.  For example, __gitdir() is invoked twice during the
     completion of options for 'gitk', 'git log', 'git rebase', but
     for certain aliases it might be invoked three times.

Now, _git(), the top-level git completion function declares the local $__git_dir variable for storing the repository path given as parameter to the '--git-dir=' option. Since $__git_dir is declared at the top-level, it is already available in all completion functions, but it's only used in __gitdir() (if set then there's no need to autodetect the path to the repository). This means that we can use $__git_dir to always store the path to the repository found by __gitdir().

This way we can address both of the above drawbacks. We won't need the command substitution when invoking __gitdir(), because won't need __gitdir()'s output anymore: the path to the repository will be available in $__git_dir. Furthermore, onlyt the first __gitdir() invocation will perform the search for the repository, and all subsequent calls will see the path already stored in $__git_dir by the first call.

So change __gitdir() to store the path to the repository in $__git_dir. However, still print the found path, because there might be user-supplied completion functions relying on it.

Also declare $__git_dir as local in __git_ps1() and _gitk() to prevent the variable from leaking into the environment when they call __gitdir() (that would break completion and bash prompt when the user moves to a different git repository).

Finally, extend the __gitdir() tests to check the value of $__git_dir, too.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash | 20 ++++----
 t/t9903-bash-prompt.sh                 | 85 +++++++++++++++++++++++-----------
 2 files changed, 71 insertions(+), 34 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index eaa3df9d..85b933f2 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -71,25 +71,27 @@ case "$COMP_WORDBREAKS" in
 esac
 
 # __gitdir accepts 0 or 1 arguments (i.e., location)
-# returns location of .git repo
+# Prints the path to the .git directory, and stores it in $__git_dir as well.
 __gitdir ()
 {
 	if [ -z "${1-}" ]; then
 		if [ -n "${__git_dir-}" ]; then
-			echo "$__git_dir"
+			:
 		elif [ -n "${GIT_DIR-}" ]; then
 			test -d "${GIT_DIR-}" || return 1
-			echo "$GIT_DIR"
+			__git_dir="$GIT_DIR"
 		elif [ -d .git ]; then
-			echo .git
+			__git_dir=.git
 		else
-			git rev-parse --git-dir 2>/dev/null
+			__git_dir="$(git rev-parse --git-dir 2>/dev/null)" || return 1
 		fi
 	elif [ -d "$1/.git" ]; then
-		echo "$1/.git"
+		__git_dir="$1/.git"
 	else
-		echo "$1"
+		__git_dir="$1"
 	fi
+
+	echo "$__git_dir"
 }
 
 # stores the divergence from upstream in $p
@@ -216,6 +218,7 @@ __git_ps1_show_upstream ()
 # returns text to add to bash PS1 prompt (includes branch name)
 __git_ps1 ()
 {
+	local __git_dir=""
 	local g="$(__gitdir)"
 	if [ -z "$g" ]; then
 		return
@@ -2606,7 +2609,7 @@ _git_whatchanged ()
 
 _git ()
 {
-	local i c=1 command __git_dir
+	local i c=1 command __git_dir=""
 
 	if [[ -n ${ZSH_VERSION-} ]]; then
 		emulate -L bash
@@ -2690,6 +2693,7 @@ _gitk ()
 
 	__git_has_doubledash && return
 
+	local __git_dir=""
 	local g="$(__gitdir)"
 	local merge=""
 	if [ -f "$g/MERGE_HEAD" ]; then
diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index 96468ceb..496e04ad 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh
@@ -10,6 +10,7 @@ test_description='test git-specific bash prompt functions'
 . "$GIT_BUILD_DIR/contrib/completion/git-completion.bash"
 
 actual="$TRASH_DIRECTORY/actual"
+actual_var="$TRASH_DIRECTORY/actual_var"
 
 test_expect_success 'setup for prompt tests' '
 	mkdir -p subdir/subsubdir &&
@@ -35,54 +36,74 @@ test_expect_success 'gitdir - from command line (through $__git_dir)' '
 	echo "$TRASH_DIRECTORY/otherrepo/.git" > expected &&
 	(
 		__git_dir="$TRASH_DIRECTORY/otherrepo/.git" &&
-		__gitdir > "$actual"
+		__gitdir > "$actual" &&
+		echo "$__git_dir" > "$actual_var"
 	) &&
-	test_cmp expected "$actual"
+	test_cmp expected "$actual" &&
+	test_cmp expected "$actual_var"
 '
 
 test_expect_success 'gitdir - repo as argument' '
 	echo "otherrepo/.git" > expected &&
-	__gitdir "otherrepo" > "$actual" &&
-	test_cmp expected "$actual"
+	(
+		__gitdir "otherrepo" > "$actual" &&
+		echo "$__git_dir" > "$actual_var"
+	) &&
+	test_cmp expected "$actual" &&
+	test_cmp expected "$actual_var"
 '
 
 test_expect_success 'gitdir - remote as argument' '
 	echo "remote" > expected &&
-	__gitdir "remote" > "$actual" &&
-	test_cmp expected "$actual"
+	(
+		__gitdir "remote" > "$actual" &&
+		echo "$__git_dir" > "$actual_var"
+	) &&
+	test_cmp expected "$actual" &&
+	test_cmp expected "$actual_var"
 '
 
 test_expect_success 'gitdir - .git directory in cwd' '
 	echo ".git" > expected &&
-	__gitdir > "$actual" &&
-	test_cmp expected "$actual"
+	(
+		__gitdir > "$actual" &&
+		echo "$__git_dir" > "$actual_var"
+	) &&
+	test_cmp expected "$actual" &&
+	test_cmp expected "$actual_var"
 '
 
 test_expect_success 'gitdir - .git directory in parent' '
 	echo "$TRASH_DIRECTORY/.git" > expected &&
 	(
 		cd subdir/subsubdir &&
-		__gitdir > "$actual"
+		__gitdir > "$actual" &&
+		echo "$__git_dir" > "$actual_var"
 	) &&
-	test_cmp expected "$actual"
+	test_cmp expected "$actual" &&
+	test_cmp expected "$actual_var"
 '
 
 test_expect_success 'gitdir - cwd is a .git directory' '
 	echo "." > expected &&
 	(
 		cd .git &&
-		__gitdir > "$actual"
+		__gitdir > "$actual" &&
+		echo "$__git_dir" > "$actual_var"
 	) &&
-	test_cmp expected "$actual"
+	test_cmp expected "$actual" &&
+	test_cmp expected "$actual_var"
 '
 
 test_expect_success 'gitdir - parent is a .git directory' '
 	echo "$TRASH_DIRECTORY/.git" > expected &&
 	(
 		cd .git/refs/heads &&
-		__gitdir > "$actual"
+		__gitdir > "$actual" &&
+		echo "$__git_dir" > "$actual_var"
 	) &&
-	test_cmp expected "$actual"
+	test_cmp expected "$actual" &&
+	test_cmp expected "$actual_var"
 '
 
 test_expect_success 'gitdir - $GIT_DIR set while .git directory in cwd' '
@@ -90,9 +111,11 @@ test_expect_success 'gitdir - $GIT_DIR set while .git directory in cwd' '
 	(
 		GIT_DIR="$TRASH_DIRECTORY/otherrepo/.git" &&
 		export GIT_DIR &&
-		__gitdir > "$actual"
+		__gitdir > "$actual" &&
+		echo "$__git_dir" > "$actual_var"
 	) &&
-	test_cmp expected "$actual"
+	test_cmp expected "$actual" &&
+	test_cmp expected "$actual_var"
 '
 
 test_expect_success 'gitdir - $GIT_DIR set while .git directory in parent' '
@@ -101,16 +124,19 @@ test_expect_success 'gitdir - $GIT_DIR set while .git directory in parent' '
 		GIT_DIR="$TRASH_DIRECTORY/otherrepo/.git" &&
 		export GIT_DIR &&
 		cd subdir &&
-		__gitdir > "$actual"
+		__gitdir > "$actual" &&
+		echo "$__git_dir" > "$actual_var"
 	) &&
-	test_cmp expected "$actual"
+	test_cmp expected "$actual" &&
+	test_cmp expected "$actual_var"
 '
 
 test_expect_success 'gitdir - non-existing $GIT_DIR' '
 	(
 		GIT_DIR="$TRASH_DIRECTORY/non-existing" &&
 		export GIT_DIR &&
-		test_must_fail __gitdir
+		test_must_fail __gitdir &&
+		test -z "$__git_dir"
 	)
 '
 
@@ -120,9 +146,11 @@ test_expect_success 'gitdir - gitfile in cwd' '
 	test_when_finished "rm -f subdir/.git" &&
 	(
 		cd subdir &&
-		__gitdir > "$actual"
+		__gitdir > "$actual" &&
+		echo "$__git_dir" > "$actual_var"
 	) &&
-	test_cmp expected "$actual"
+	test_cmp expected "$actual" &&
+	test_cmp expected "$actual_var"
 '
 
 test_expect_success 'gitdir - gitfile in parent' '
@@ -131,9 +159,11 @@ test_expect_success 'gitdir - gitfile in parent' '
 	test_when_finished "rm -f subdir/.git" &&
 	(
 		cd subdir/subsubdir &&
-		__gitdir > "$actual"
+		__gitdir > "$actual" &&
+		echo "$__git_dir" > "$actual_var"
 	) &&
-	test_cmp expected "$actual"
+	test_cmp expected "$actual" &&
+	test_cmp expected "$actual_var"
 '
 
 test_expect_success SYMLINKS 'gitdir - resulting path avoids symlinks' '
@@ -144,9 +174,11 @@ test_expect_success SYMLINKS 'gitdir - resulting path avoids symlinks' '
 	test_when_finished "rm -f link" &&
 	(
 		cd link &&
-		__gitdir > "$actual"
+		__gitdir > "$actual" &&
+		echo "$__git_dir" > "$actual_var"
 	) &&
-	test_cmp expected "$actual"
+	test_cmp expected "$actual" &&
+	test_cmp expected "$actual_var"
 '
 
 test_expect_success 'gitdir - not a git repository' '
@@ -154,7 +186,8 @@ test_expect_success 'gitdir - not a git repository' '
 		cd subdir/subsubdir &&
 		GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY" &&
 		export GIT_CEILING_DIRECTORIES &&
-		test_must_fail __gitdir
+		test_must_fail __gitdir &&
+		test -z "$__git_dir"
 	)
 '
 

-- 
1.7.10.1.541.gb1be298
Previous: SZEDER GáborNext: Junio C Hamano
Message 19 of 50 in “Bash prompt speedup”
  1. 00/19 Bash prompt speedupSZEDER Gábor, May 9, 2012
  2. 01/19 tests: move code to run tests under bash into a helper librarySZEDER Gábor, May 9, 2012
  3. 02/19 tests: add tests for the bash prompt functions in the completion scriptSZEDER Gábor, May 9, 2012
  4. Johannes SixtMay 9, 2012
  5. Junio C HamanoMay 9, 2012
  6. Johannes SixtMay 10, 2012
  7. Junio C HamanoMay 9, 2012
  8. SZEDER GáborMay 9, 2012
  9. 03/19 completion: use __gitdir() in _git_log()SZEDER Gábor, May 9, 2012
  10. Junio C HamanoMay 9, 2012
  11. SZEDER GáborMay 9, 2012
  12. 04/19 completion: respect $GIT_DIRSZEDER Gábor, May 9, 2012
  13. Johannes SixtMay 9, 2012
  14. Junio C HamanoMay 9, 2012
  15. 05/19 bash prompt: don't show the prompt when .git/HEAD is unreadableSZEDER Gábor, May 9, 2012
  16. Junio C HamanoMay 9, 2012
  17. SZEDER GáborMay 9, 2012
  18. 06/19 bash prompt: return early from __git_ps1() when not in a git repositorySZEDER Gábor, May 9, 2012
  19. 07/19 completion: make __gitdir() store repository path in $__git_dirSZEDER Gábor, May 9, 2012
  20. Junio C HamanoMay 9, 2012
  21. 08/19 completion: use $__git_dir instead of $(__gitdir)SZEDER Gábor, May 9, 2012
  22. Junio C HamanoMay 9, 2012
  23. SZEDER GáborMay 9, 2012
  24. Junio C HamanoMay 9, 2012
  25. SZEDER GáborMay 9, 2012
  26. 09/19 completion: platform-specific helper function to get physical pathSZEDER Gábor, May 9, 2012
  27. Johannes SixtMay 9, 2012
  28. 10/19 completion: use bash builtins to search for repositorySZEDER Gábor, May 9, 2012
  29. Junio C HamanoMay 9, 2012
  30. SZEDER GáborMay 9, 2012
  31. Junio C HamanoMay 9, 2012
  32. 11/19 bash prompt: use bash builtins to find out current branchSZEDER Gábor, May 9, 2012
  33. Junio C HamanoMay 9, 2012
  34. SZEDER GáborMay 9, 2012
  35. Junio C HamanoMay 9, 2012
  36. SZEDER GáborMay 9, 2012
  37. Junio C HamanoMay 9, 2012
  38. 12/19 bash prompt: use bash builtins to check whether inside git dirSZEDER Gábor, May 9, 2012
  39. Johannes SixtMay 9, 2012
  40. Junio C HamanoMay 9, 2012
  41. 13/19 bash prompt: check whether inside the worktree only when necessarySZEDER Gábor, May 9, 2012
  42. 14/19 bash prompt: use bash builtins to find out current branch during rebaseSZEDER Gábor, May 9, 2012
  43. 15/19 bash prompt: use bash builtins to get detached HEAD abbrev. object nameSZEDER Gábor, May 9, 2012
  44. 16/19 bash prompt: display stash and upstream state even inside the repositorySZEDER Gábor, May 9, 2012
  45. 17/19 bash prompt: use bash builtins to check stash stateSZEDER Gábor, May 9, 2012
  46. 18/19 bash prompt: avoid command substitution when checking for untracked filesSZEDER Gábor, May 9, 2012
  47. Junio C HamanoMay 9, 2012
  48. 19/19 bash prompt: alternative git prompt without command substitutionSZEDER Gábor, May 9, 2012
  49. Andrew SayersMay 9, 2012
  50. SZEDER GáborMay 9, 2012

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.