[PATCH 00/19] Bash prompt speedup

[PATCH 00/19] Bash prompt speedup

[SZEDER Gábor]

Hi,

It was pointed out about a year ago that displaying the git-specific bash
prompt on Windows/MinGW takes quite long, long enough to be noticeable.
And indeed, while in a subdirectory of a repository and stash indicator
enabled and my laptop running from battery I get this:

  $ time prompt=$(__git_ps1)

  real    0m0.412s
  user    0m0.048s
  sys     0m0.210s

This is mainly caused by the numerous fork()s and exec()s to create
subshells and run git commands, which are rather expensive on Windows.

This patch series eliminates many command substitutions and git commands
from __git_ps1() by reorganizing code or replacing them with bash
builtins.  This speeds up the prompt immensely: now I get the same prompt
as above in about 10ms(!).  Timing results are shown in the log message of
patch 19.

Unfortunately, to achive this users have to change their configuration, in
particular change their $PS1 and $PROMPT_COMMAND (see patch 19) and should
enable the discovery of git repositories across filesystem boundaries (see
patch 10).

There are two RFC patches in there (9 and 18), please have a look.


Here's an outline of the series:

First, a couple of tests for the bash prompt, to lessen the chance that I
break something ;)  These are basically the same patches that I sent out a
while ago in [1], the only noteworthy change is that I renamed the helper
library to lib-bash.sh (from lib-completion.sh), because all it does is to
run tests under bash and there is nothing completion-specific in there.

[1]: http://thread.gmane.org/gmane.comp.version-control.git/195587/focus=195711

  [PATCH 01/19] tests: move code to run tests under bash into a helper library
  [PATCH 02/19] tests: add tests for the bash prompt functions in the completion script

The next four patches contain a couple of fixes and cleanups:

  [PATCH 03/19] completion: use __gitdir() in _git_log()
  [PATCH 04/19] completion: respect $GIT_DIR
  [PATCH 05/19] bash prompt: don't show the prompt when .git/HEAD is unreadable
  [PATCH 06/19] bash prompt: return early from __git_ps1() when not in a git repository

These four make __gitdir() faster, so they'll benefit not only the
bash prompt but completing refs, aliases, config variables, etc., too.

  [PATCH 07/19] completion: make __gitdir() store repository path in $__git_dir
  [PATCH 08/19] completion: use $__git_dir instead of $(__gitdir)
  [RFC PATCH 09/19] completion: platform-specific helper function to get physical path
  [PATCH 10/19] completion: use bash builtins to search for repository

These three make the main codepath in __git_ps1() faster by eliminating a
couple of command substitutions and git commands.

  [PATCH 11/19] bash prompt: use bash builtins to find out current branch
  [PATCH 12/19] bash prompt: use bash builtins to check whether inside git dir
  [PATCH 13/19] bash prompt: check whether inside the worktree only when necessary

Eliminate some more command substitutions and git commands that are not
necessarily run for every prompt:

  [PATCH 14/19] bash prompt: use bash builtins to find out current branch during rebase
  [PATCH 15/19] bash prompt: use bash builtins to get detached HEAD abbrev. object name
  [PATCH 16/19] bash prompt: display stash and upstream state even inside the repository
  [PATCH 17/19] bash prompt: use bash builtins to check stash state
  [RFC PATCH 18/19] bash prompt: avoid command substitution when checking for untracked files

And finally get rid of the command substitution used to include the git
prompt in $PS1:

  [PATCH 19/19] bash prompt: alternative git prompt without command substitution

Enjoy.


Best,
Gábor


SZEDER Gábor (19):
  tests: move code to run tests under bash into a helper library
  tests: add tests for the bash prompt functions in the completion
    script
  completion: use __gitdir() in _git_log()
  completion: respect $GIT_DIR
  bash prompt: don't show the prompt when .git/HEAD is unreadable
  bash prompt: return early from __git_ps1() when not in a git
    repository
  completion: make __gitdir() store repository path in $__git_dir
  completion: use $__git_dir instead of $(__gitdir)
  completion: platform-specific helper function to get physical path
  completion: use bash builtins to search for repository
  bash prompt: use bash builtins to find out current branch
  bash prompt: use bash builtins to check whether inside git dir
  bash prompt: check whether inside the worktree only when necessary
  bash prompt: use bash builtins to find out current branch during
    rebase
  bash prompt: use bash builtins to get detached HEAD abbrev. object
    name
  bash prompt: display stash and upstream state even inside the
    repository
  bash prompt: use bash builtins to check stash state
  bash prompt: avoid command substitution when checking for untracked
    files
  bash prompt: alternative git prompt without command substitution

 contrib/completion/git-completion.bash | 315 ++++++++++-------
 t/lib-bash.sh                          |  18 +
 t/t9902-completion.sh                  |  14 +-
 t/t9903-bash-prompt.sh                 | 593 +++++++++++++++++++++++++++++++++
 4 files changed, 815 insertions(+), 125 deletions(-)
 create mode 100644 t/lib-bash.sh
 create mode 100755 t/t9903-bash-prompt.sh

-- 
1.7.10.1.541.gb1be298

[PATCH 01/19] tests: move code to run tests under bash into a helper library

[SZEDER Gábor]

The following patch will add tests for the bash prompt functions as a
new test script, which also has to be run under bash.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 t/lib-bash.sh         | 18 ++++++++++++++++++
 t/t9902-completion.sh | 14 +-------------
 2 files changed, 19 insertions(+), 13 deletions(-)
 create mode 100755 t/lib-bash.sh

diff --git a/t/lib-bash.sh b/t/lib-bash.sh
new file mode 100644
index 00000000..11397f74
--- /dev/null
+++ b/t/lib-bash.sh
@@ -0,0 +1,18 @@
+#!/bin/sh
+#
+# Ensures that tests are run under Bash; primarily intended for running tests
+# of the completion script.
+
+if test -n "$BASH" && test -z "$POSIXLY_CORRECT"; then
+	# we are in full-on bash mode
+	true
+elif type bash >/dev/null 2>&1; then
+	# execute in full-on bash mode
+	unset POSIXLY_CORRECT
+	exec bash "$0" "$@"
+else
+	echo '1..0 #SKIP skipping bash completion tests; bash not available'
+	exit 0
+fi
+
+. ./test-lib.sh
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 5bda6b6e..a0ea9463 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -3,21 +3,9 @@
 # Copyright (c) 2012 Felipe Contreras
 #
 
-if test -n "$BASH" && test -z "$POSIXLY_CORRECT"; then
-	# we are in full-on bash mode
-	true
-elif type bash >/dev/null 2>&1; then
-	# execute in full-on bash mode
-	unset POSIXLY_CORRECT
-	exec bash "$0" "$@"
-else
-	echo '1..0 #SKIP skipping bash completion tests; bash not available'
-	exit 0
-fi
-
 test_description='test bash completion'
 
-. ./test-lib.sh
+. ./lib-bash.sh
 
 complete ()
 {
-- 
1.7.10.1.541.gb1be298

[PATCH 02/19] tests: add tests for the bash prompt functions in the completion script

[SZEDER Gábor]

The tests cover the discovery of the '.git' directory in the
__gitdir() function in different scenarios, and the prompt itself,
i.e. branch name, detached heads, operations (rebase, merge,
cherry-pick, bisect), and status indicators (dirty, stash, untracked
files; but not the upstream status).

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 t/t9903-bash-prompt.sh | 448 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 448 insertions(+)
 create mode 100755 t/t9903-bash-prompt.sh

diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
new file mode 100755
index 00000000..a6c9ce94
--- /dev/null
+++ b/t/t9903-bash-prompt.sh
@@ -0,0 +1,448 @@
+#!/bin/sh
+#
+# Copyright (c) 2012 SZEDER Gábor
+#
+
+test_description='test git-specific bash prompt functions'
+
+. ./lib-bash.sh
+
+. "$GIT_BUILD_DIR/contrib/completion/git-completion.bash"
+
+actual="$TRASH_DIRECTORY/actual"
+
+test_expect_success 'setup for prompt tests' '
+	mkdir -p subdir/subsubdir &&
+	git init otherrepo &&
+	echo 1 > file &&
+	git add file &&
+	test_tick &&
+	git commit -m initial &&
+	git tag -a -m msg1 t1 &&
+	git checkout -b b1 &&
+	echo 2 > file &&
+	git commit -m "second b1" file &&
+	echo 3 > file &&
+	git commit -m "third b1" file &&
+	git tag -a -m msg2 t2 &&
+	git checkout -b b2 master &&
+	echo 0 > file &&
+	git commit -m "second b2" file &&
+	git checkout master
+'
+
+test_expect_success 'gitdir - from command line (through $__git_dir)' '
+	echo "$TRASH_DIRECTORY/otherrepo/.git" > expected &&
+	(
+		__git_dir="$TRASH_DIRECTORY/otherrepo/.git" &&
+		__gitdir > "$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'gitdir - repo as argument' '
+	echo "otherrepo/.git" > expected &&
+	__gitdir "otherrepo" > "$actual" &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'gitdir - remote as argument' '
+	echo "remote" > expected &&
+	__gitdir "remote" > "$actual" &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'gitdir - .git directory in cwd' '
+	echo ".git" > expected &&
+	__gitdir > "$actual" &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'gitdir - .git directory in parent' '
+	echo "$TRASH_DIRECTORY/.git" > expected &&
+	(
+		cd subdir/subsubdir &&
+		__gitdir > "$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'gitdir - cwd is a .git directory' '
+	echo "." > expected &&
+	(
+		cd .git &&
+		__gitdir > "$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'gitdir - parent is a .git directory' '
+	echo "$TRASH_DIRECTORY/.git" > expected &&
+	(
+		cd .git/refs/heads &&
+		__gitdir > "$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_failure 'gitdir - $GIT_DIR set while .git directory in cwd' '
+	echo "$TRASH_DIRECTORY/otherrepo/.git" > expected &&
+	(
+		GIT_DIR="$TRASH_DIRECTORY/otherrepo/.git" &&
+		export GIT_DIR &&
+		__gitdir > "$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'gitdir - $GIT_DIR set while .git directory in parent' '
+	echo "$TRASH_DIRECTORY/otherrepo/.git" > expected &&
+	(
+		GIT_DIR="$TRASH_DIRECTORY/otherrepo/.git" &&
+		export GIT_DIR &&
+		cd subdir &&
+		__gitdir > "$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'gitdir - gitfile in cwd' '
+	echo "$TRASH_DIRECTORY/otherrepo/.git" > expected &&
+	echo "gitdir: $TRASH_DIRECTORY/otherrepo/.git" > subdir/.git &&
+	test_when_finished "rm -f subdir/.git" &&
+	(
+		cd subdir &&
+		__gitdir > "$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'gitdir - gitfile in parent' '
+	echo "$TRASH_DIRECTORY/otherrepo/.git" > expected &&
+	echo "gitdir: $TRASH_DIRECTORY/otherrepo/.git" > subdir/.git &&
+	test_when_finished "rm -f subdir/.git" &&
+	(
+		cd subdir/subsubdir &&
+		__gitdir > "$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success SYMLINKS 'gitdir - resulting path avoids symlinks' '
+	echo "$TRASH_DIRECTORY/otherrepo/.git" > expected &&
+	mkdir otherrepo/dir &&
+	test_when_finished "rm -rf otherrepo/dir" &&
+	ln -s otherrepo/dir link &&
+	test_when_finished "rm -f link" &&
+	(
+		cd link &&
+		__gitdir > "$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+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_expect_success 'prompt - branch name' '
+	printf " (master)" > expected &&
+	__git_ps1 > "$actual" &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - detached head' '
+	printf " ((%s...))" $(git log -1 --format="%h" b1^) > expected &&
+	git checkout b1^ &&
+	test_when_finished "git checkout master" &&
+	__git_ps1 > "$actual" &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - describe detached head - contains' '
+	printf " ((t2~1))" > expected &&
+	git checkout b1^ &&
+	test_when_finished "git checkout master" &&
+	(
+		GIT_PS1_DESCRIBE_STYLE=contains &&
+		__git_ps1 > "$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - describe detached head - branch' '
+	printf " ((b1~1))" > expected &&
+	git checkout b1^ &&
+	test_when_finished "git checkout master" &&
+	(
+		GIT_PS1_DESCRIBE_STYLE=branch &&
+		__git_ps1 > "$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - describe detached head - describe' '
+	printf " ((t1-1-g%s))" $(git log -1 --format="%h" b1^) > expected &&
+	git checkout b1^ &&
+	test_when_finished "git checkout master" &&
+	(
+		GIT_PS1_DESCRIBE_STYLE=describe &&
+		__git_ps1 > "$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - describe detached head - default' '
+	printf " ((t2))" > expected &&
+	git checkout --detach b1 &&
+	test_when_finished "git checkout master" &&
+	__git_ps1 > "$actual" &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - inside .git directory' '
+	printf " (GIT_DIR!)" > expected &&
+	(
+		cd .git &&
+		__git_ps1 > "$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - deep inside .git directory' '
+	printf " (GIT_DIR!)" > expected &&
+	(
+		cd .git/refs/heads &&
+		__git_ps1 > "$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - inside bare repository' '
+	printf " (BARE:master)" > expected &&
+	git init --bare bare.git &&
+	test_when_finished "rm -rf bare.git" &&
+	(
+		cd bare.git &&
+		__git_ps1 > "$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - interactive rebase' '
+	printf " (b1|REBASE-i)" > expected
+	echo "#!$SHELL_PATH" >fake_editor.sh &&
+	cat >>fake_editor.sh <<\EOF &&
+echo "edit $(git log -1 --format="%h")" > "$1"
+EOF
+	test_when_finished "rm -f fake_editor.sh" &&
+	chmod a+x fake_editor.sh &&
+	test_set_editor "$TRASH_DIRECTORY/fake_editor.sh" &&
+	git checkout b1 &&
+	test_when_finished "git checkout master" &&
+	git rebase -i HEAD^ &&
+	test_when_finished "git rebase --abort"
+	__git_ps1 > "$actual" &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - rebase merge' '
+	printf " (b2|REBASE-m)" > expected &&
+	git checkout b2 &&
+	test_when_finished "git checkout master" &&
+	test_must_fail git rebase --merge b1 b2 &&
+	test_when_finished "git rebase --abort" &&
+	__git_ps1 > "$actual" &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - rebase' '
+	printf " ((t2)|REBASE)" > expected &&
+	git checkout b2 &&
+	test_when_finished "git checkout master" &&
+	test_must_fail git rebase b1 b2 &&
+	test_when_finished "git rebase --abort" &&
+	__git_ps1 > "$actual" &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - merge' '
+	printf " (b1|MERGING)" > expected &&
+	git checkout b1 &&
+	test_when_finished "git checkout master" &&
+	test_must_fail git merge b2 &&
+	test_when_finished "git reset --hard" &&
+	__git_ps1 > "$actual" &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - cherry-pick' '
+	printf " (master|CHERRY-PICKING)" > expected &&
+	test_must_fail git cherry-pick b1 &&
+	test_when_finished "git reset --hard" &&
+	__git_ps1 > "$actual" &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - bisect' '
+	printf " (master|BISECTING)" > expected &&
+	git bisect start &&
+	test_when_finished "git bisect reset" &&
+	__git_ps1 > "$actual" &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - dirty status indicator - clean' '
+	printf " (master)" > expected &&
+	(
+		GIT_PS1_SHOWDIRTYSTATE=y &&
+		__git_ps1 > "$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - dirty status indicator - dirty worktree' '
+	printf " (master *)" > expected &&
+	echo "dirty" > file &&
+	test_when_finished "git reset --hard" &&
+	(
+		GIT_PS1_SHOWDIRTYSTATE=y &&
+		__git_ps1 > "$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - dirty status indicator - dirty index' '
+	printf " (master +)" > expected &&
+	echo "dirty" > file &&
+	test_when_finished "git reset --hard" &&
+	git add -u &&
+	(
+		GIT_PS1_SHOWDIRTYSTATE=y &&
+		__git_ps1 > "$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - dirty status indicator - dirty index and worktree' '
+	printf " (master *+)" > expected &&
+	echo "dirty index" > file &&
+	test_when_finished "git reset --hard" &&
+	git add -u &&
+	echo "dirty worktree" > file &&
+	(
+		GIT_PS1_SHOWDIRTYSTATE=y &&
+		__git_ps1 > "$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - dirty status indicator - before root commit' '
+	printf " (master #)" > expected &&
+	(
+		GIT_PS1_SHOWDIRTYSTATE=y &&
+		cd otherrepo &&
+		__git_ps1 > "$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - dirty status indicator - disabled by config' '
+	printf " (master)" > expected &&
+	echo "dirty" > file &&
+	test_when_finished "git reset --hard" &&
+	test_config bash.showDirtyState false &&
+	(
+		GIT_PS1_SHOWDIRTYSTATE=y &&
+		__git_ps1 > "$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - dirty status indicator - not shown inside .git directory' '
+	printf " (GIT_DIR!)" > expected &&
+	echo "dirty" > file &&
+	test_when_finished "git reset --hard" &&
+	(
+		GIT_PS1_SHOWDIRTYSTATE=y &&
+		cd .git &&
+		__git_ps1 > "$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - stash status indicator - no stash' '
+	printf " (master)" > expected &&
+	(
+		GIT_PS1_SHOWSTASHSTATE=y &&
+		__git_ps1 > "$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - stash status indicator - stash' '
+	printf " (master $)" > expected &&
+	echo 2 >file &&
+	git stash &&
+	test_when_finished "git stash drop" &&
+	(
+		GIT_PS1_SHOWSTASHSTATE=y &&
+		__git_ps1 > "$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - stash status indicator - not shown inside .git directory' '
+	printf " (GIT_DIR!)" > expected &&
+	echo 2 >file &&
+	git stash &&
+	test_when_finished "git stash drop" &&
+	(
+		GIT_PS1_SHOWSTASHSTATE=y &&
+		cd .git &&
+		__git_ps1 > "$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - untracked files status indicator - no untracked files' '
+	printf " (master)" > expected &&
+	(
+		GIT_PS1_SHOWUNTRACKEDFILES=y &&
+		cd otherrepo &&
+		__git_ps1 > "$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - untracked files status indicator - untracked files' '
+	printf " (master %%)" > expected &&
+	(
+		GIT_PS1_SHOWUNTRACKEDFILES=y &&
+		__git_ps1 > "$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - untracked files status indicator - not shown inside .git directory' '
+	printf " (GIT_DIR!)" > expected &&
+	(
+		GIT_PS1_SHOWUNTRACKEDFILES=y &&
+		cd .git &&
+		__git_ps1 > "$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - format string starting with dash' '
+	printf -- "-master" > expected &&
+	__git_ps1 "-%s" > "$actual" &&
+	test_cmp expected "$actual"
+'
+
+test_done
-- 
1.7.10.1.541.gb1be298

Re: [PATCH 02/19] tests: add tests for the bash prompt functions in the completion script

[Johannes Sixt]

Am 5/9/2012 2:44, schrieb SZEDER Gábor:
> The tests cover the discovery of the '.git' directory in the
> __gitdir() function in different scenarios, and the prompt itself,
> i.e. branch name, detached heads, operations (rebase, merge,
> cherry-pick, bisect), and status indicators (dirty, stash, untracked
> files; but not the upstream status).

The following patch contains fixups are needed to pass the tests at
this point plus a few more changes.

--- >8 ---
From: Johannes Sixt <j6t@kdbg.org>
Subject: [PATCH] fixup! tests: add tests for the bash prompt functions in the
 completion script

- $TRASH_DIRECTORY is /c/dir style path, but git produces c:/dir style
  paths. Use $(pwd), which is short for $(pwd -W) that produces the
  latter, to assemble expected test data and paths read by git.

- As long as GIT_DIR is only exported for use by git and not used by
  the completion functions, the move to $(pwd) is only cosmetic and
  for consistency.

- Insert a #!/bin/sh in the shell script to ensure the test will pass
  should git-rebase ever be ported to C.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 t/t9903-bash-prompt.sh |   29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index a6c9ce9..3880c56 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh
@@ -32,9 +32,9 @@ test_expect_success 'setup for prompt tests' '
 '
 
 test_expect_success 'gitdir - from command line (through $__git_dir)' '
-	echo "$TRASH_DIRECTORY/otherrepo/.git" > expected &&
+	echo "$(pwd)/otherrepo/.git" > expected &&
 	(
-		__git_dir="$TRASH_DIRECTORY/otherrepo/.git" &&
+		__git_dir="$(pwd)/otherrepo/.git" &&
 		__gitdir > "$actual"
 	) &&
 	test_cmp expected "$actual"
@@ -59,7 +59,7 @@ test_expect_success 'gitdir - .git directory in cwd' '
 '
 
 test_expect_success 'gitdir - .git directory in parent' '
-	echo "$TRASH_DIRECTORY/.git" > expected &&
+	echo "$(pwd)/.git" > expected &&
 	(
 		cd subdir/subsubdir &&
 		__gitdir > "$actual"
@@ -77,7 +77,7 @@ test_expect_success 'gitdir - cwd is a .git directory' '
 '
 
 test_expect_success 'gitdir - parent is a .git directory' '
-	echo "$TRASH_DIRECTORY/.git" > expected &&
+	echo "$(pwd)/.git" > expected &&
 	(
 		cd .git/refs/heads &&
 		__gitdir > "$actual"
@@ -86,9 +86,9 @@ test_expect_success 'gitdir - parent is a .git directory' '
 '
 
 test_expect_failure 'gitdir - $GIT_DIR set while .git directory in cwd' '
-	echo "$TRASH_DIRECTORY/otherrepo/.git" > expected &&
+	echo "$(pwd)/otherrepo/.git" > expected &&
 	(
-		GIT_DIR="$TRASH_DIRECTORY/otherrepo/.git" &&
+		GIT_DIR="$(pwd)/otherrepo/.git" &&
 		export GIT_DIR &&
 		__gitdir > "$actual"
 	) &&
@@ -96,9 +96,9 @@ test_expect_failure 'gitdir - $GIT_DIR set while .git directory in cwd' '
 '
 
 test_expect_success 'gitdir - $GIT_DIR set while .git directory in parent' '
-	echo "$TRASH_DIRECTORY/otherrepo/.git" > expected &&
+	echo "$(pwd)/otherrepo/.git" > expected &&
 	(
-		GIT_DIR="$TRASH_DIRECTORY/otherrepo/.git" &&
+		GIT_DIR="$(pwd)/otherrepo/.git" &&
 		export GIT_DIR &&
 		cd subdir &&
 		__gitdir > "$actual"
@@ -107,8 +107,8 @@ test_expect_success 'gitdir - $GIT_DIR set while .git directory in parent' '
 '
 
 test_expect_success 'gitdir - gitfile in cwd' '
-	echo "$TRASH_DIRECTORY/otherrepo/.git" > expected &&
-	echo "gitdir: $TRASH_DIRECTORY/otherrepo/.git" > subdir/.git &&
+	echo "$(pwd)/otherrepo/.git" > expected &&
+	echo "gitdir: $(pwd)/otherrepo/.git" > subdir/.git &&
 	test_when_finished "rm -f subdir/.git" &&
 	(
 		cd subdir &&
@@ -118,8 +118,8 @@ test_expect_success 'gitdir - gitfile in cwd' '
 '
 
 test_expect_success 'gitdir - gitfile in parent' '
-	echo "$TRASH_DIRECTORY/otherrepo/.git" > expected &&
-	echo "gitdir: $TRASH_DIRECTORY/otherrepo/.git" > subdir/.git &&
+	echo "$(pwd)/otherrepo/.git" > expected &&
+	echo "gitdir: $(pwd)/otherrepo/.git" > subdir/.git &&
 	test_when_finished "rm -f subdir/.git" &&
 	(
 		cd subdir/subsubdir &&
@@ -129,7 +129,7 @@ test_expect_success 'gitdir - gitfile in parent' '
 '
 
 test_expect_success SYMLINKS 'gitdir - resulting path avoids symlinks' '
-	echo "$TRASH_DIRECTORY/otherrepo/.git" > expected &&
+	echo "$(pwd)/otherrepo/.git" > expected &&
 	mkdir otherrepo/dir &&
 	test_when_finished "rm -rf otherrepo/dir" &&
 	ln -s otherrepo/dir link &&
@@ -238,11 +238,12 @@ test_expect_success 'prompt - interactive rebase' '
 	printf " (b1|REBASE-i)" > expected
 	echo "#!$SHELL_PATH" >fake_editor.sh &&
 	cat >>fake_editor.sh <<\EOF &&
+#!/bin/sh
 echo "edit $(git log -1 --format="%h")" > "$1"
 EOF
 	test_when_finished "rm -f fake_editor.sh" &&
 	chmod a+x fake_editor.sh &&
-	test_set_editor "$TRASH_DIRECTORY/fake_editor.sh" &&
+	test_set_editor "$(pwd)/fake_editor.sh" &&
 	git checkout b1 &&
 	test_when_finished "git checkout master" &&
 	git rebase -i HEAD^ &&
-- 
1.7.10.1.1689.gacdfbde

Re: [PATCH 02/19] tests: add tests for the bash prompt functions in the completion script

[Junio C Hamano]

Johannes Sixt <j.sixt@viscovery.net> writes:

> From: Johannes Sixt <j6t@kdbg.org>
> Subject: [PATCH] fixup! tests: add tests for the bash prompt functions in the
>  completion script
>
> - $TRASH_DIRECTORY is /c/dir style path, but git produces c:/dir style
>   paths. Use $(pwd), which is short for $(pwd -W) that produces the
>   latter, to assemble expected test data and paths read by git.

This comes up very often whenever somebody (including me) touches test
scripts.  We do have a write-up in t/README in "Dos and Don'ts" section,
but apparently that is not sufficient to avoid fix-ups.

Would it be possible to arrange so that $TRASH_DIRECTORY, $TEST_DIRECTORY
and $PWD are set to c:/dir style paths in Windows environment?  What would
we break if we did so?

The other direction of changing the Windows port of git to produce /c/dir
style paths would probably not work, as it would involve ripping out the
path mangling feature of bash in MSYS, which is done for some reason, I
presume.

Re: [PATCH 02/19] tests: add tests for the bash prompt functions in the completion script

[Johannes Sixt]

Am 5/9/2012 20:08, schrieb Junio C Hamano:
> Would it be possible to arrange so that $TRASH_DIRECTORY, $TEST_DIRECTORY
> and $PWD are set to c:/dir style paths in Windows environment?  What would
> we break if we did so?

I don't think that this is possible. The POSIX emulation in MSYS (or bash,
dunno) always converts to POSIX style:

> bash -c "cd c:/temp; PWD=$(pwd -W); echo $PWD; cd $PWD; pwd; echo $PWD"
c:/temp
/c/temp
/c/temp

That is, even if I use Windows style path to chdir around, the result is
POSIX style.

> The other direction of changing the Windows port of git to produce /c/dir
> style paths would probably not work, as it would involve ripping out the
> path mangling feature of bash in MSYS, which is done for some reason, I
> presume.

Doing that is certainly not something that I would be prepared to do just
to make the bash prompt work right ;-) And the added maintainance burden
incurred by the status quo is certainly much cheaper than yet another fork
of MSYS and/or bash.

-- Hannes

Re: [PATCH 02/19] tests: add tests for the bash prompt functions in the completion script

[Junio C Hamano]

SZEDER Gábor <szeder@ira.uka.de> writes:

> The tests cover the discovery of the '.git' directory in the __gitdir()
> function in different scenarios, and the prompt itself, i.e. branch
> name, detached heads, operations (rebase, merge, cherry-pick, bisect),
> and status indicators (dirty, stash, untracked files; but not the
> upstream status).
>
> Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>

Looks like a quite comprehensive tests around GIT_PS1_$MANY_DIFFERENT_STYLES
(except that GIT_PS1_SHOWUPSTREAM seems to be missing); very nice.

> +	echo 1 > file &&

When you are going to re-roll to add the missing SHOWUPSTREAM test, in
addition to J6t's $PWD vs $(pwd) vs $TRASH_DIRECTORY fix, please fix these
redirections to match the coding styles (i.e. "cmd >file" and "cmd <file",
with SP before and without SP after redirection operators).

Re: [PATCH 02/19] tests: add tests for the bash prompt functions in the completion script

[SZEDER Gábor]

On Wed, May 09, 2012 at 11:36:25AM -0700, Junio C Hamano wrote:
> SZEDER Gábor <szeder@ira.uka.de> writes:
> 
> > The tests cover the discovery of the '.git' directory in the __gitdir()
> > function in different scenarios, and the prompt itself, i.e. branch
> > name, detached heads, operations (rebase, merge, cherry-pick, bisect),
> > and status indicators (dirty, stash, untracked files; but not the
> > upstream status).
> >
> > Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
> 
> Looks like a quite comprehensive tests around GIT_PS1_$MANY_DIFFERENT_STYLES
> (except that GIT_PS1_SHOWUPSTREAM seems to be missing); very nice.

I've mentioned in the commit message that "... status indicators ...
but not the upstream status".  I didn't wrote tests for that because
this series doesn't changes anything in the function producing the
upstream status indicator.

However, thinking about it now, this series, in particular patch 16
(bash prompt: display stash and upstream state even inside the
repository), does change the context in which that function might be
invoked, i.e. not only from the work tree but even from within the
repository.  I don't think that would break anything (famous last
words ;), because that function runs git config, rev-list, and log,
and AFAICT these commands should work in a repository just as well,
and the rest of the function is just preparing their arguments and
processing their output.

Anyway, it would be definitely better to have a test to show that it
indeed works from within a repository, but I didn't want to fiddle
with svn upstreams.  Perhaps patch 16 should leave the upstream status
indicator as it is until someone ;) writes tests for it; since that
function doesn't affect the main codepath and it involves several
subshells and git processes anyway, there is not that much to be
gained anyway.

> > +	echo 1 > file &&
> 
> When you are going to re-roll to add the missing SHOWUPSTREAM test, in
> addition to J6t's $PWD vs $(pwd) vs $TRASH_DIRECTORY fix, please fix these
> redirections to match the coding styles (i.e. "cmd >file" and "cmd <file",
> with SP before and without SP after redirection operators).

OK.

I just followed suit of the recently added t9902-completion.sh, which
uses SP on both sides of redirection operators.

[PATCH 03/19] completion: use __gitdir() in _git_log()

[SZEDER Gábor]

The standard way to find out the path to the repository in the
completion script is the __gitdir() helper function, because that
handles the repository path given on the command line (i.e. git
--git-dir=/path/to/repo log --<TAB>).  However, there is one
exception: the completion function for 'git log' still uses 'git
rev-parse --git-dir' directly, and could offer (or not) the '--merge'
option erroneously when the repository is specified on the command
line.

Use __gitdir() there, too.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 9f56ec7a..f17abccb 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1551,7 +1551,7 @@ _git_log ()
 {
 	__git_has_doubledash && return
 
-	local g="$(git rev-parse --git-dir 2>/dev/null)"
+	local g="$(__gitdir)"
 	local merge=""
 	if [ -f "$g/MERGE_HEAD" ]; then
 		merge="--merge"
-- 
1.7.10.1.541.gb1be298

Re: [PATCH 03/19] completion: use __gitdir() in _git_log()

[Junio C Hamano]

SZEDER Gábor <szeder@ira.uka.de> writes:

> The standard way to find out the path to the repository in the
> completion script is the __gitdir() helper function, because that
> handles the repository path given on the command line (i.e. git
> --git-dir=/path/to/repo log --<TAB>).  However, there is one
> exception: the completion function for 'git log' still uses 'git
> rev-parse --git-dir' directly, and could offer (or not) the '--merge'
> option erroneously when the repository is specified on the command
> line.

Here `--merge` is the visible symptom, and the real issue you fixed is
that it used to be looking into a repository that is different from the
user is working with, right [*1*]?

Well spotted, and the fix sounds correct.

Thanks.

[Footnote]

*1* I am just making sure I am reading the above right; I am not
suggesting to omit description of visible symptom at all---quite
the opposite, I do want to see these visible symptom descriptions
in the log messages.

Re: [PATCH 03/19] completion: use __gitdir() in _git_log()

[SZEDER Gábor]

On Wed, May 09, 2012 at 11:41:13AM -0700, Junio C Hamano wrote:
> SZEDER Gábor <szeder@ira.uka.de> writes:
> 
> > The standard way to find out the path to the repository in the
> > completion script is the __gitdir() helper function, because that
> > handles the repository path given on the command line (i.e. git
> > --git-dir=/path/to/repo log --<TAB>).  However, there is one
> > exception: the completion function for 'git log' still uses 'git
> > rev-parse --git-dir' directly, and could offer (or not) the '--merge'
> > option erroneously when the repository is specified on the command
> > line.
> 
> Here `--merge` is the visible symptom, and the real issue you fixed is
> that it used to be looking into a repository that is different from the
> user is working with, right [*1*]?

Exactly; will add a sentence about it to be more explicit in the
reroll.

Note, however, that this doesn't influence refs completion, because
__git_refs() does use __gitdir(), so it will look into the right
repository.


Gábor

[PATCH 04/19] completion: respect $GIT_DIR

[SZEDER Gábor]

The __gitdir() helper function finds out the path of the git
repository by running 'git rev-parse --git-dir'.  However, it has a
shortcut first to avoid the overhead of running a git command in a
subshell when the current directory is at the top of the work tree,
i.e. when it contains a '.git' subdirectory.

If the 'GIT_DIR' environment variable is set then it specifies the
path to the git repository, and the autodetection of the '.git'
directory is not necessary.  However, $GIT_DIR is only taken into
acocunt by 'git rev-parse --git-dir', and the check for the '.git'
subdirectory is performed first, so it wins over the path given in
$GIT_DIR.

There are several completion (helper) functions that depend on
__gitdir(), and when the above case triggers the completion script
will do weird things, like offering refs, aliases, or stashes from a
different repository, or displaying wrong or broken prompt, etc.

So check first whether $GIT_DIR is set, and only proceed with checking
the '.git' directory in the current directory if it isn't.  'git
rev-parse' would also check whether the path in $GIT_DIR is a proper
'.git' directory, i.e. 'HEAD', 'refs/', and 'objects/' are present and
accessible, but we don't have to be that thorough for the bash prompt.
And we've lived with an equally permissive check for '.git' in the
current working directory for years anyway.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash |  3 +++
 t/t9903-bash-prompt.sh                 | 10 +++++++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index f17abccb..ab26bdc8 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -77,6 +77,9 @@ __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"
 		elif [ -d .git ]; then
 			echo .git
 		else
diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index a6c9ce94..96468ceb 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh
@@ -85,7 +85,7 @@ test_expect_success 'gitdir - parent is a .git directory' '
 	test_cmp expected "$actual"
 '
 
-test_expect_failure 'gitdir - $GIT_DIR set while .git directory in cwd' '
+test_expect_success 'gitdir - $GIT_DIR set while .git directory in cwd' '
 	echo "$TRASH_DIRECTORY/otherrepo/.git" > expected &&
 	(
 		GIT_DIR="$TRASH_DIRECTORY/otherrepo/.git" &&
@@ -106,6 +106,14 @@ test_expect_success 'gitdir - $GIT_DIR set while .git directory in parent' '
 	test_cmp expected "$actual"
 '
 
+test_expect_success 'gitdir - non-existing $GIT_DIR' '
+	(
+		GIT_DIR="$TRASH_DIRECTORY/non-existing" &&
+		export GIT_DIR &&
+		test_must_fail __gitdir
+	)
+'
+
 test_expect_success 'gitdir - gitfile in cwd' '
 	echo "$TRASH_DIRECTORY/otherrepo/.git" > expected &&
 	echo "gitdir: $TRASH_DIRECTORY/otherrepo/.git" > subdir/.git &&
-- 
1.7.10.1.541.gb1be298

Re: [PATCH 04/19] completion: respect $GIT_DIR

[Johannes Sixt]

Am 5/9/2012 2:44, schrieb SZEDER Gábor:
> +test_expect_success 'gitdir - non-existing $GIT_DIR' '
> +	(
> +		GIT_DIR="$TRASH_DIRECTORY/non-existing" &&
> +		export GIT_DIR &&
> +		test_must_fail __gitdir
> +	)
> +'

Another fixup, but it is only for consistency.

--- >8 ---
From: Johannes Sixt <j6t@kdbg.org>
Subject: [PATCH] fixup! completion: respect $GIT_DIR

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 t/t9903-bash-prompt.sh |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index cf8e0ca..0318288 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh
@@ -108,7 +108,7 @@ test_expect_success 'gitdir - $GIT_DIR set while .git directory in parent' '
 
 test_expect_success 'gitdir - non-existing $GIT_DIR' '
 	(
-		GIT_DIR="$TRASH_DIRECTORY/non-existing" &&
+		GIT_DIR="$(pwd)/non-existing" &&
 		export GIT_DIR &&
 		test_must_fail __gitdir
 	)
-- 
1.7.10.1.1689.gacdfbde

Re: [PATCH 04/19] completion: respect $GIT_DIR

[Junio C Hamano]

SZEDER Gábor <szeder@ira.uka.de> writes:

> The __gitdir() helper function finds out the path of the git
> repository by running 'git rev-parse --git-dir'.  However, it has a
> shortcut first to avoid the overhead of running a git command in a
> subshell when the current directory is at the top of the work tree,
> i.e. when it contains a '.git' subdirectory.
>
> If the 'GIT_DIR' environment variable is set then it specifies the
> path to the git repository, and the autodetection of the '.git'
> directory is not necessary.  However, $GIT_DIR is only taken into
> acocunt by 'git rev-parse --git-dir', and the check for the '.git'
> subdirectory is performed first, so it wins over the path given in
> $GIT_DIR.

Strictly speaking, you have to be a bit careful here, though.  If GIT_DIR
is set as a shell variable without being exported, it will not affect
where the "git" process you will spawn from your interactive shell session
will find the repository.  Only when it is exported it does.

It is a different matter if the distinction matters in the real life, in
other words, GIT_DIR set but not exported is a use case that is worth
worrying about.  But note that our own git-sh-setup script is one such use
case, so I wouldn't be surprised if somebody uses it for a strange
workflow (I suspect that might involve a working tree that has its .git
dir in a totally unrelated place, and the user runs "GIT_DIR=$GIT_DIR git
subcmd", using the set but not exported GIT_DIR as a typesaver).

> diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
> index a6c9ce94..96468ceb 100755
> --- a/t/t9903-bash-prompt.sh
> +++ b/t/t9903-bash-prompt.sh
> @@ -85,7 +85,7 @@ test_expect_success 'gitdir - parent is a .git directory' '
>  	test_cmp expected "$actual"
>  '
>  
> -test_expect_failure 'gitdir - $GIT_DIR set while .git directory in cwd' '
> +test_expect_success 'gitdir - $GIT_DIR set while .git directory in cwd' '
>  	echo "$TRASH_DIRECTORY/otherrepo/.git" > expected &&
>  	(
>  		GIT_DIR="$TRASH_DIRECTORY/otherrepo/.git" &&

And it is a good thing that the next line that comes after the above
context is "export GIT_DIR".  If we were to declare "set but not exported"
an uninteresting use case whose outcome is undefined (and that might be
fine), this still tests the defined behaviour of having the GIT_DIR in the
environment.

[PATCH 05/19] bash prompt: don't show the prompt when .git/HEAD is unreadable

[SZEDER Gábor]

__git_ps1() has a chain of commands to figure out what to display in
the prompt as current branch, i.e. the branch name, or a described
detached head, or the 7 hexdigits abbreviated object name, or
"(unknown)" when all of the above fails.

Now, when the 7 hexdigits case fails, then '/path/to/.git/HEAD' can't
be read.  This can happen when the file became unreadable after
__gitdir() found it: unlikely, but a parallel process can racily
delete it or change it's permissions in that short timeframe.
Alternatively, it is possible that either the subshell or the 'cut'
command fail.  Either way, when HEAD is not readable, then the path is
not considered to be a git repository, therefore the bash prompt
shouldn't be displayed at all.  And there is no point in continuing
the execution of __git_ps1(), because its subsequent git commands will
error out anyway.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index ab26bdc8..cd6a5f12 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -258,7 +258,7 @@ __git_ps1 ()
 				esac 2>/dev/null)" ||
 
 				b="$(cut -c1-7 "$g/HEAD" 2>/dev/null)..." ||
-				b="unknown"
+				return
 				b="($b)"
 			}
 		fi
-- 
1.7.10.1.541.gb1be298

Re: [PATCH 05/19] bash prompt: don't show the prompt when .git/HEAD is unreadable

[Junio C Hamano]

SZEDER Gábor <szeder@ira.uka.de> writes:

> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index ab26bdc8..cd6a5f12 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -258,7 +258,7 @@ __git_ps1 ()
>  				esac 2>/dev/null)" ||
>  
>  				b="$(cut -c1-7 "$g/HEAD" 2>/dev/null)..." ||
> -				b="unknown"
> +				return

While you are touching the vicinity of the code, could we lose that "cut"
and replace it with "rev-parse --short HEAD", without the hardcoded 1-7?

I wondered if we can use a single "git describe" output for all the
describe/default and failure cases but didn't come up with a good way to
do so only by using bash built-ins.

Re: [PATCH 05/19] bash prompt: don't show the prompt when .git/HEAD is unreadable

[SZEDER Gábor]

On Wed, May 09, 2012 at 12:32:36PM -0700, Junio C Hamano wrote:
> SZEDER Gábor <szeder@ira.uka.de> writes:
> 
> > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> > index ab26bdc8..cd6a5f12 100755
> > --- a/contrib/completion/git-completion.bash
> > +++ b/contrib/completion/git-completion.bash
> > @@ -258,7 +258,7 @@ __git_ps1 ()
> >  				esac 2>/dev/null)" ||
> >  
> >  				b="$(cut -c1-7 "$g/HEAD" 2>/dev/null)..." ||
> > -				b="unknown"
> > +				return
> 
> While you are touching the vicinity of the code, could we lose that "cut"
> and replace it with "rev-parse --short HEAD", without the hardcoded 1-7?

Patch 15 (bash prompt: use bash builtins to get detached HEAD abbrev.
object name) eliminates that 'cut' with some parameter expansions.
While it doesn't respect the 'core.abbrev' config variable, it's much
much faster, so IMHO it's worth it.


Gábor

[PATCH 06/19] bash prompt: return early from __git_ps1() when not in a git repository

[SZEDER Gábor]

... to gain one level of indentation for the bulk of the function.

(The patch looks quite unreadable, you'd better check it with 'git
diff -w'.)

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash | 154 +++++++++++++++++----------------
 1 file changed, 78 insertions(+), 76 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index cd6a5f12..eaa3df9d 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -217,94 +217,96 @@ __git_ps1_show_upstream ()
 __git_ps1 ()
 {
 	local g="$(__gitdir)"
-	if [ -n "$g" ]; then
-		local r=""
-		local b=""
-		if [ -f "$g/rebase-merge/interactive" ]; then
-			r="|REBASE-i"
-			b="$(cat "$g/rebase-merge/head-name")"
-		elif [ -d "$g/rebase-merge" ]; then
-			r="|REBASE-m"
-			b="$(cat "$g/rebase-merge/head-name")"
-		else
-			if [ -d "$g/rebase-apply" ]; then
-				if [ -f "$g/rebase-apply/rebasing" ]; then
-					r="|REBASE"
-				elif [ -f "$g/rebase-apply/applying" ]; then
-					r="|AM"
-				else
-					r="|AM/REBASE"
-				fi
-			elif [ -f "$g/MERGE_HEAD" ]; then
-				r="|MERGING"
-			elif [ -f "$g/CHERRY_PICK_HEAD" ]; then
-				r="|CHERRY-PICKING"
-			elif [ -f "$g/BISECT_LOG" ]; then
-				r="|BISECTING"
-			fi
+	if [ -z "$g" ]; then
+		return
+	fi
 
-			b="$(git symbolic-ref HEAD 2>/dev/null)" || {
-
-				b="$(
-				case "${GIT_PS1_DESCRIBE_STYLE-}" in
-				(contains)
-					git describe --contains HEAD ;;
-				(branch)
-					git describe --contains --all HEAD ;;
-				(describe)
-					git describe HEAD ;;
-				(* | default)
-					git describe --tags --exact-match HEAD ;;
-				esac 2>/dev/null)" ||
-
-				b="$(cut -c1-7 "$g/HEAD" 2>/dev/null)..." ||
-				return
-				b="($b)"
-			}
+	local r=""
+	local b=""
+	if [ -f "$g/rebase-merge/interactive" ]; then
+		r="|REBASE-i"
+		b="$(cat "$g/rebase-merge/head-name")"
+	elif [ -d "$g/rebase-merge" ]; then
+		r="|REBASE-m"
+		b="$(cat "$g/rebase-merge/head-name")"
+	else
+		if [ -d "$g/rebase-apply" ]; then
+			if [ -f "$g/rebase-apply/rebasing" ]; then
+				r="|REBASE"
+			elif [ -f "$g/rebase-apply/applying" ]; then
+				r="|AM"
+			else
+				r="|AM/REBASE"
+			fi
+		elif [ -f "$g/MERGE_HEAD" ]; then
+			r="|MERGING"
+		elif [ -f "$g/CHERRY_PICK_HEAD" ]; then
+			r="|CHERRY-PICKING"
+		elif [ -f "$g/BISECT_LOG" ]; then
+			r="|BISECTING"
 		fi
 
-		local w=""
-		local i=""
-		local s=""
-		local u=""
-		local c=""
-		local p=""
+		b="$(git symbolic-ref HEAD 2>/dev/null)" || {
+
+			b="$(
+			case "${GIT_PS1_DESCRIBE_STYLE-}" in
+			(contains)
+				git describe --contains HEAD ;;
+			(branch)
+				git describe --contains --all HEAD ;;
+			(describe)
+				git describe HEAD ;;
+			(* | default)
+				git describe --tags --exact-match HEAD ;;
+			esac 2>/dev/null)" ||
+
+			b="$(cut -c1-7 "$g/HEAD" 2>/dev/null)..." ||
+			return
+			b="($b)"
+		}
+	fi
 
-		if [ "true" = "$(git rev-parse --is-inside-git-dir 2>/dev/null)" ]; then
-			if [ "true" = "$(git rev-parse --is-bare-repository 2>/dev/null)" ]; then
-				c="BARE:"
-			else
-				b="GIT_DIR!"
-			fi
-		elif [ "true" = "$(git rev-parse --is-inside-work-tree 2>/dev/null)" ]; then
-			if [ -n "${GIT_PS1_SHOWDIRTYSTATE-}" ]; then
-				if [ "$(git config --bool bash.showDirtyState)" != "false" ]; then
-					git diff --no-ext-diff --quiet --exit-code || w="*"
-					if git rev-parse --quiet --verify HEAD >/dev/null; then
-						git diff-index --cached --quiet HEAD -- || i="+"
-					else
-						i="#"
-					fi
-				fi
-			fi
-			if [ -n "${GIT_PS1_SHOWSTASHSTATE-}" ]; then
-				git rev-parse --verify refs/stash >/dev/null 2>&1 && s="$"
-			fi
+	local w=""
+	local i=""
+	local s=""
+	local u=""
+	local c=""
+	local p=""
 
-			if [ -n "${GIT_PS1_SHOWUNTRACKEDFILES-}" ]; then
-				if [ -n "$(git ls-files --others --exclude-standard)" ]; then
-					u="%"
+	if [ "true" = "$(git rev-parse --is-inside-git-dir 2>/dev/null)" ]; then
+		if [ "true" = "$(git rev-parse --is-bare-repository 2>/dev/null)" ]; then
+			c="BARE:"
+		else
+			b="GIT_DIR!"
+		fi
+	elif [ "true" = "$(git rev-parse --is-inside-work-tree 2>/dev/null)" ]; then
+		if [ -n "${GIT_PS1_SHOWDIRTYSTATE-}" ]; then
+			if [ "$(git config --bool bash.showDirtyState)" != "false" ]; then
+				git diff --no-ext-diff --quiet --exit-code || w="*"
+				if git rev-parse --quiet --verify HEAD >/dev/null; then
+					git diff-index --cached --quiet HEAD -- || i="+"
+				else
+					i="#"
 				fi
 			fi
+		fi
+		if [ -n "${GIT_PS1_SHOWSTASHSTATE-}" ]; then
+			git rev-parse --verify refs/stash >/dev/null 2>&1 && s="$"
+		fi
 
-			if [ -n "${GIT_PS1_SHOWUPSTREAM-}" ]; then
-				__git_ps1_show_upstream
+		if [ -n "${GIT_PS1_SHOWUNTRACKEDFILES-}" ]; then
+			if [ -n "$(git ls-files --others --exclude-standard)" ]; then
+				u="%"
 			fi
 		fi
 
-		local f="$w$i$s$u"
-		printf -- "${1:- (%s)}" "$c${b##refs/heads/}${f:+ $f}$r$p"
+		if [ -n "${GIT_PS1_SHOWUPSTREAM-}" ]; then
+			__git_ps1_show_upstream
+		fi
 	fi
+
+	local f="$w$i$s$u"
+	printf -- "${1:- (%s)}" "$c${b##refs/heads/}${f:+ $f}$r$p"
 }
 
 __gitcomp_1 ()
-- 
1.7.10.1.541.gb1be298

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

[SZEDER Gábor]

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

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

[Junio C Hamano]

SZEDER Gábor <szeder@ira.uka.de> writes:

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

Good; this was actually the only major thing I worried about when I saw
the tail part of the series.  So $__git_dir is global across the call
chain of a single invocation to show __git_ps1, but it always is reset
once __git_ps1 is called again, so that it will always know where the
then-current git repository is, right?

Looks quite a sane and valid optimization to me.

[PATCH 08/19] completion: use $__git_dir instead of $(__gitdir)

[SZEDER Gábor]

The previous commit changed __gitdir() to store the repository path in
the $__git_dir variable.  Now we change all call sites to just call
__gitdir() directly and then use $__git_dir instead of doing
'dir="$(__gitdir)"' command substitution, thereby sparing the overhead
of fork()ing a subshell.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash | 106 ++++++++++++++++++---------------
 1 file changed, 58 insertions(+), 48 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 85b933f2..5c8d4aea 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -219,33 +219,33 @@ __git_ps1_show_upstream ()
 __git_ps1 ()
 {
 	local __git_dir=""
-	local g="$(__gitdir)"
-	if [ -z "$g" ]; then
+	__gitdir >/dev/null
+	if [ -z "$__git_dir" ]; then
 		return
 	fi
 
 	local r=""
 	local b=""
-	if [ -f "$g/rebase-merge/interactive" ]; then
+	if [ -f "$__git_dir/rebase-merge/interactive" ]; then
 		r="|REBASE-i"
-		b="$(cat "$g/rebase-merge/head-name")"
-	elif [ -d "$g/rebase-merge" ]; then
+		b="$(cat "$__git_dir/rebase-merge/head-name")"
+	elif [ -d "$__git_dir/rebase-merge" ]; then
 		r="|REBASE-m"
-		b="$(cat "$g/rebase-merge/head-name")"
+		b="$(cat "$__git_dir/rebase-merge/head-name")"
 	else
-		if [ -d "$g/rebase-apply" ]; then
-			if [ -f "$g/rebase-apply/rebasing" ]; then
+		if [ -d "$__git_dir/rebase-apply" ]; then
+			if [ -f "$__git_dir/rebase-apply/rebasing" ]; then
 				r="|REBASE"
-			elif [ -f "$g/rebase-apply/applying" ]; then
+			elif [ -f "$__git_dir/rebase-apply/applying" ]; then
 				r="|AM"
 			else
 				r="|AM/REBASE"
 			fi
-		elif [ -f "$g/MERGE_HEAD" ]; then
+		elif [ -f "$__git_dir/MERGE_HEAD" ]; then
 			r="|MERGING"
-		elif [ -f "$g/CHERRY_PICK_HEAD" ]; then
+		elif [ -f "$__git_dir/CHERRY_PICK_HEAD" ]; then
 			r="|CHERRY-PICKING"
-		elif [ -f "$g/BISECT_LOG" ]; then
+		elif [ -f "$__git_dir/BISECT_LOG" ]; then
 			r="|BISECTING"
 		fi
 
@@ -263,7 +263,7 @@ __git_ps1 ()
 				git describe --tags --exact-match HEAD ;;
 			esac 2>/dev/null)" ||
 
-			b="$(cut -c1-7 "$g/HEAD" 2>/dev/null)..." ||
+			b="$(cut -c1-7 "$__git_dir/HEAD" 2>/dev/null)..." ||
 			return
 			b="($b)"
 		}
@@ -522,9 +522,9 @@ __gitcomp_nl ()
 
 __git_heads ()
 {
-	local dir="$(__gitdir)"
-	if [ -d "$dir" ]; then
-		git --git-dir="$dir" for-each-ref --format='%(refname:short)' \
+	__gitdir "${1-}" >/dev/null
+	if [ -d "$__git_dir" ]; then
+		git --git-dir="$__git_dir" for-each-ref --format='%(refname:short)' \
 			refs/heads
 		return
 	fi
@@ -532,9 +532,9 @@ __git_heads ()
 
 __git_tags ()
 {
-	local dir="$(__gitdir)"
-	if [ -d "$dir" ]; then
-		git --git-dir="$dir" for-each-ref --format='%(refname:short)' \
+	__gitdir "${1-}" >/dev/null
+	if [ -d "$__git_dir" ]; then
+		git --git-dir="$__git_dir" for-each-ref --format='%(refname:short)' \
 			refs/tags
 		return
 	fi
@@ -545,9 +545,10 @@ __git_tags ()
 # by checkout for tracking branches
 __git_refs ()
 {
-	local i hash dir="$(__gitdir "${1-}")" track="${2-}"
+	local i hash track="${2-}"
 	local format refs
-	if [ -d "$dir" ]; then
+	__gitdir "${1-}" >/dev/null
+	if [ -d "$__git_dir" ]; then
 		case "$cur" in
 		refs|refs/*)
 			format="refname"
@@ -556,20 +557,20 @@ __git_refs ()
 			;;
 		*)
 			for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD; do
-				if [ -e "$dir/$i" ]; then echo $i; fi
+				if [ -e "$__git_dir/$i" ]; then echo $i; fi
 			done
 			format="refname:short"
 			refs="refs/tags refs/heads refs/remotes"
 			;;
 		esac
-		git --git-dir="$dir" for-each-ref --format="%($format)" \
+		git --git-dir="$__git_dir" 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 --git-dir="$__git_dir" for-each-ref --shell --format="ref=%(refname:short)" \
 				"refs/remotes/" | \
 			while read -r entry; do
 				eval "$entry"
@@ -583,7 +584,7 @@ __git_refs ()
 	fi
 	case "$cur" in
 	refs|refs/*)
-		git ls-remote "$dir" "$cur*" 2>/dev/null | \
+		git ls-remote "$__git_dir" "$cur*" 2>/dev/null | \
 		while read -r hash i; do
 			case "$i" in
 			*^{}) ;;
@@ -592,7 +593,7 @@ __git_refs ()
 		done
 		;;
 	*)
-		git ls-remote "$dir" HEAD ORIG_HEAD 'refs/tags/*' 'refs/heads/*' 'refs/remotes/*' 2>/dev/null | \
+		git ls-remote "$__git_dir" HEAD ORIG_HEAD 'refs/tags/*' 'refs/heads/*' 'refs/remotes/*' 2>/dev/null | \
 		while read -r hash i; do
 			case "$i" in
 			*^{}) ;;
@@ -625,9 +626,10 @@ __git_refs_remotes ()
 
 __git_remotes ()
 {
-	local i IFS=$'\n' d="$(__gitdir)"
-	test -d "$d/remotes" && ls -1 "$d/remotes"
-	for i in $(git --git-dir="$d" config --get-regexp 'remote\..*\.url' 2>/dev/null); do
+	local i IFS=$'\n'
+	__gitdir >/dev/null
+	test -d "$__git_dir/remotes" && ls -1 "$__git_dir/remotes"
+	for i in $(git --git-dir="$__git_dir" config --get-regexp 'remote\..*\.url' 2>/dev/null); do
 		i="${i#remote.}"
 		echo "${i/.url*/}"
 	done
@@ -685,8 +687,9 @@ __git_complete_revlist_file ()
 		esac
 
 		local IFS=$'\n'
+		__gitdir >/dev/null
 		COMPREPLY=($(compgen -P "$pfx" \
-			-W "$(git --git-dir="$(__gitdir)" ls-tree "$ls" \
+			-W "$(git --git-dir="$__git_dir" ls-tree "$ls" \
 				| sed '/^100... blob /{
 				           s,^.*	,,
 				           s,$, ,
@@ -936,7 +939,8 @@ __git_compute_porcelain_commands ()
 __git_pretty_aliases ()
 {
 	local i IFS=$'\n'
-	for i in $(git --git-dir="$(__gitdir)" config --get-regexp "pretty\..*" 2>/dev/null); do
+	__gitdir >/dev/null
+	for i in $(git --git-dir="$__git_dir" config --get-regexp "pretty\..*" 2>/dev/null); do
 		case "$i" in
 		pretty.*)
 			i="${i#pretty.}"
@@ -949,7 +953,8 @@ __git_pretty_aliases ()
 __git_aliases ()
 {
 	local i IFS=$'\n'
-	for i in $(git --git-dir="$(__gitdir)" config --get-regexp "alias\..*" 2>/dev/null); do
+	__gitdir >/dev/null
+	for i in $(git --git-dir="$__git_dir" config --get-regexp "alias\..*" 2>/dev/null); do
 		case "$i" in
 		alias.*)
 			i="${i#alias.}"
@@ -962,7 +967,8 @@ __git_aliases ()
 # __git_aliased_command requires 1 argument
 __git_aliased_command ()
 {
-	local word cmdline=$(git --git-dir="$(__gitdir)" \
+	__gitdir >/dev/null
+	local word cmdline=$(git --git-dir="$__git_dir" \
 		config --get "alias.$1")
 	for word in $cmdline; do
 		case "$word" in
@@ -1013,8 +1019,8 @@ __git_whitespacelist="nowarn warn error error-all fix"
 
 _git_am ()
 {
-	local dir="$(__gitdir)"
-	if [ -d "$dir"/rebase-apply ]; then
+	__gitdir >/dev/null
+	if [ -d "$__git_dir"/rebase-apply ]; then
 		__gitcomp "--skip --continue --resolved --abort"
 		return
 	fi
@@ -1099,7 +1105,8 @@ _git_bisect ()
 	local subcommands="start bad good skip reset visualize replay log run"
 	local subcommand="$(__git_find_on_cmdline "$subcommands")"
 	if [ -z "$subcommand" ]; then
-		if [ -f "$(__gitdir)"/BISECT_START ]; then
+		__gitdir >/dev/null
+		if [ -f "$__git_dir"/BISECT_START ]; then
 			__gitcomp "$subcommands"
 		else
 			__gitcomp "replay start"
@@ -1559,9 +1566,9 @@ _git_log ()
 {
 	__git_has_doubledash && return
 
-	local g="$(__gitdir)"
+	__gitdir >/dev/null
 	local merge=""
-	if [ -f "$g/MERGE_HEAD" ]; then
+	if [ -f "$__git_dir/MERGE_HEAD" ]; then
 		merge="--merge"
 	fi
 	case "$cur" in
@@ -1745,8 +1752,8 @@ _git_push ()
 
 _git_rebase ()
 {
-	local dir="$(__gitdir)"
-	if [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then
+	__gitdir >/dev/null
+	if [ -d "$__git_dir"/rebase-apply ] || [ -d "$__git_dir"/rebase-merge ]; then
 		__gitcomp "--continue --skip --abort"
 		return
 	fi
@@ -1845,7 +1852,8 @@ __git_config_get_set_variables ()
 		c=$((--c))
 	done
 
-	git --git-dir="$(__gitdir)" config $config_file --list 2>/dev/null |
+	__gitdir >/dev/null
+	git --git-dir="$__git_dir" config $config_file --list 2>/dev/null |
 	while read -r line
 	do
 		case "$line" in
@@ -1880,7 +1888,8 @@ _git_config ()
 	remote.*.push)
 		local remote="${prev#remote.}"
 		remote="${remote%.push}"
-		__gitcomp_nl "$(git --git-dir="$(__gitdir)" \
+		__gitdir >/dev/null
+		__gitcomp_nl "$(git --git-dir="$__git_dir" \
 			for-each-ref --format='%(refname):%(refname)' \
 			refs/heads)"
 		return
@@ -2304,7 +2313,8 @@ _git_remote ()
 		;;
 	update)
 		local i c='' IFS=$'\n'
-		for i in $(git --git-dir="$(__gitdir)" config --get-regexp "remotes\..*" 2>/dev/null); do
+		__gitdir >/dev/null
+		for i in $(git --git-dir="$__git_dir" config --get-regexp "remotes\..*" 2>/dev/null); do
 			i="${i#remotes.}"
 			c="$c ${i/ */}"
 		done
@@ -2441,7 +2451,8 @@ _git_stash ()
 			COMPREPLY=()
 			;;
 		show,*|apply,*|drop,*|pop,*|branch,*)
-			__gitcomp_nl "$(git --git-dir="$(__gitdir)" stash list \
+			__gitdir >/dev/null
+			__gitcomp_nl "$(git --git-dir="$__git_dir" stash list \
 					| sed -n -e 's/:.*//p')"
 			;;
 		*)
@@ -2693,10 +2704,9 @@ _gitk ()
 
 	__git_has_doubledash && return
 
-	local __git_dir=""
-	local g="$(__gitdir)"
-	local merge=""
-	if [ -f "$g/MERGE_HEAD" ]; then
+	local __git_dir="" merge=""
+	__gitdir >/dev/null
+	if [ -f "$__git_dir/MERGE_HEAD" ]; then
 		merge="--merge"
 	fi
 	case "$cur" in
-- 
1.7.10.1.541.gb1be298

Re: [PATCH 08/19] completion: use $__git_dir instead of $(__gitdir)

[Junio C Hamano]

SZEDER Gábor <szeder@ira.uka.de> writes:

> +	__gitdir >/dev/null

If this becomes the only call site of __gitdir helper function (and that
was the way I read the log message), it would be sane to rename it to
a more descriptive __setup_dash_dash_git_dir function and lose the need to
redirect its output, no?

> @@ -962,7 +967,8 @@ __git_aliases ()
>  # __git_aliased_command requires 1 argument
>  __git_aliased_command ()
>  {
> -	local word cmdline=$(git --git-dir="$(__gitdir)" \
> +	__gitdir >/dev/null
> +	local word cmdline=$(git --git-dir="$__git_dir" \
>  		config --get "alias.$1")
>  	for word in $cmdline; do
>  		case "$word" in

Now this worries me.  The way I read 07/19 was that the local __git_dir=""
declarations in __git_ps1 and __git were what protected this whole
machinery to protect us against surprises from user doing "cd" between
interactive commands, but you have the same __gitdir call to set up the
global $__git_dir variable there, without the initialization to "".

Having to have a call to __gitdir seems to indicate to me that you cannot
assume that the other initialization sites may not have been called before
we get to this point.  Then why is 'local __git_dir=""' unneeded here?

Re: [PATCH 08/19] completion: use $__git_dir instead of $(__gitdir)

[SZEDER Gábor]

On Wed, May 09, 2012 at 12:43:33PM -0700, Junio C Hamano wrote:
> SZEDER Gábor <szeder@ira.uka.de> writes:
> 
> > +	__gitdir >/dev/null
> 
> If this becomes the only call site of __gitdir helper function (and that
> was the way I read the log message), it would be sane to rename it to
> a more descriptive __setup_dash_dash_git_dir function and lose the need to
> redirect its output, no?

There might be user-defined completion functions using __gitdir(), and
I didn't want to break them by changing the function's name or
altering its existing behavior, i.e. printing the path.

> > @@ -962,7 +967,8 @@ __git_aliases ()
> >  # __git_aliased_command requires 1 argument
> >  __git_aliased_command ()
> >  {
> > -	local word cmdline=$(git --git-dir="$(__gitdir)" \
> > +	__gitdir >/dev/null
> > +	local word cmdline=$(git --git-dir="$__git_dir" \
> >  		config --get "alias.$1")
> >  	for word in $cmdline; do
> >  		case "$word" in
> 
> Now this worries me.  The way I read 07/19 was that the local __git_dir=""
> declarations in __git_ps1 and __git were what protected this whole
> machinery to protect us against surprises from user doing "cd" between
> interactive commands, but you have the same __gitdir call to set up the
> global $__git_dir variable there, without the initialization to "".
> 
> Having to have a call to __gitdir seems to indicate to me that you cannot
> assume that the other initialization sites may not have been called before
> we get to this point.  Then why is 'local __git_dir=""' unneeded here?

Your comments to the previous patch apply here.

All completion functions are called either from _git() or from
_gitk(), where $__git_dir is declared as local.  So no matter how deep
is $__git_dir set in the callchain, it can't leak into the
environment.  It won't even survive between two subsequent completions
on the same command line.

Now, it would definitely be simpler to just initialize $__git_dir in
the two toplevel functions.  But there are many codepatch that don't
need $__git_dir at all, and would only be slowed down by an additional
$(git rev-parse --git-dir).


Gábor

Re: [PATCH 08/19] completion: use $__git_dir instead of $(__gitdir)

[Junio C Hamano]

SZEDER Gábor <szeder@ira.uka.de> writes:

>> > @@ -962,7 +967,8 @@ __git_aliases ()
>> >  # __git_aliased_command requires 1 argument
>> >  __git_aliased_command ()
>> >  {
>> > -	local word cmdline=$(git --git-dir="$(__gitdir)" \
>> > +	__gitdir >/dev/null
>> > +	local word cmdline=$(git --git-dir="$__git_dir" \
>> >  		config --get "alias.$1")
>> >  	for word in $cmdline; do
>> >  		case "$word" in
>> 
>> Now this worries me.  The way I read 07/19 was that the local __git_dir=""
>> declarations in __git_ps1 and __git were what protected this whole
>> machinery to protect us against surprises from user doing "cd" between
>> interactive commands, but you have the same __gitdir call to set up the
>> global $__git_dir variable there, without the initialization to "".
>> 
>> Having to have a call to __gitdir seems to indicate to me that you cannot
>> assume that the other initialization sites may not have been called before
>> we get to this point.  Then why is 'local __git_dir=""' unneeded here?
>
> Your comments to the previous patch apply here.

Not really.  __git_ps1 and __gitk seems to do __gitdir very early to make
sure anybody that use $__git_dir can rely on it, but having to sprinkle
"set up $__git_dir variable" everywhere means anybody who wants to update
need to know if it is already called, which defeats the point of "we can
use $__git_dir instead of calling $(__gitdir)" from maintainability's
point of view.

Re: [PATCH 08/19] completion: use $__git_dir instead of $(__gitdir)

[SZEDER Gábor]

On Wed, May 09, 2012 at 01:56:02PM -0700, Junio C Hamano wrote:
> SZEDER Gábor <szeder@ira.uka.de> writes:
> 
> >> > @@ -962,7 +967,8 @@ __git_aliases ()
> >> >  # __git_aliased_command requires 1 argument
> >> >  __git_aliased_command ()
> >> >  {
> >> > -	local word cmdline=$(git --git-dir="$(__gitdir)" \
> >> > +	__gitdir >/dev/null
> >> > +	local word cmdline=$(git --git-dir="$__git_dir" \
> >> >  		config --get "alias.$1")
> >> >  	for word in $cmdline; do
> >> >  		case "$word" in
> >> 
> >> Now this worries me.  The way I read 07/19 was that the local __git_dir=""
> >> declarations in __git_ps1 and __git were what protected this whole
> >> machinery to protect us against surprises from user doing "cd" between
> >> interactive commands, but you have the same __gitdir call to set up the
> >> global $__git_dir variable there, without the initialization to "".
> >> 
> >> Having to have a call to __gitdir seems to indicate to me that you cannot
> >> assume that the other initialization sites may not have been called before
> >> we get to this point.  Then why is 'local __git_dir=""' unneeded here?
> >
> > Your comments to the previous patch apply here.
> 
> Not really.  __git_ps1 and __gitk seems to do __gitdir very early to make
> sure anybody that use $__git_dir can rely on it

No, __git_ps1() and __gitk() do __gitdir() early, because they need
the path to the repository very early.

> but having to sprinkle
> "set up $__git_dir variable" everywhere means anybody who wants to update
> need to know if it is already called, which defeats the point of "we can
> use $__git_dir instead of calling $(__gitdir)" from maintainability's
> point of view.

Well, the point is better explained in the body of the commit message:

  just call __gitdir() directly and then use $__git_dir instead of
  doing 'dir="$(__gitdir)"' command substitution

Unfortunately, I just couldn't manage to squeeze all this into a
one-line short description.  So it really is about performance, and
not maintainability.

[RFC PATCH 09/19] completion: platform-specific helper function to get physical path

[SZEDER Gábor]

Two upcoming optimizations will replace invocations of git commands in
command substitutions with bash builtins examining the path of the
current working directory.  Since git works with physical paths, we
can't use logical path provided in $PWD, but have to resort to the
'$(pwd -P)' command substitution to get the physical path of the
current directory.

However, on platforms not supporting symbolic links, such as MinGW,
the path in $PWD is bound to be the physical path.  So on those
platforms we could avoid the command substitution and use $PWD
directly.  Great for MinGW, because the overhead of forking a subshell
is relatively large there.

So add a platform-specific helper function to get the physical path of
the current directory: on MinGW it's defined such that it gets the
physical path from $PWD, while on other platforms from '$(pwd -P)'.
The path is stored in a variable whose name is passed as argument, so
no command substitution is needed when invoking this function.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---

I'm not sure how to check for MinGW; I just looked through the output from
'set', and saw OSTYPE=msys there.

 contrib/completion/git-completion.bash | 14 ++++++++++++++
 t/t9903-bash-prompt.sh                 | 21 +++++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 5c8d4aea..bd7d39e3 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -70,6 +70,20 @@ case "$COMP_WORDBREAKS" in
 *)   COMP_WORDBREAKS="$COMP_WORDBREAKS:"
 esac
 
+# __git_pwd_p() stores the physical path of the current working directory
+# in the variable whose name is given as argument
+if [ ${OSTYPE-} = "msys" ]; then
+__git_pwd_p ()
+{
+	eval $1="$PWD"
+}
+else
+__git_pwd_p ()
+{
+	eval $1=\"$(pwd -P)\"
+}
+fi
+
 # __gitdir accepts 0 or 1 arguments (i.e., location)
 # Prints the path to the .git directory, and stores it in $__git_dir as well.
 __gitdir ()
diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index 496e04ad..3d722b25 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh
@@ -32,6 +32,27 @@ test_expect_success 'setup for prompt tests' '
 	git checkout master
 '
 
+test_expect_success 'getting pwd -P' '
+	echo "$TRASH_DIRECTORY" > expected &&
+	(
+		__git_pwd_p p &&
+		echo "$p" > "$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success SYMLINKS 'getting pwd -P - avoids symlinks' '
+	echo "$TRASH_DIRECTORY/otherrepo" > expected &&
+	ln -s otherrepo link &&
+	test_when_finished "rm -f link" &&
+	(
+		cd link &&
+		__git_pwd_p p &&
+		echo "$p" > "$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
 test_expect_success 'gitdir - from command line (through $__git_dir)' '
 	echo "$TRASH_DIRECTORY/otherrepo/.git" > expected &&
 	(
-- 
1.7.10.1.541.gb1be298

Re: [RFC PATCH 09/19] completion: platform-specific helper function to get physical path

[Johannes Sixt]

Am 5/9/2012 2:44, schrieb SZEDER Gábor:
> I'm not sure how to check for MinGW; I just looked through the output from
> 'set', and saw OSTYPE=msys there.

Ususally, we check uname -s for *MINGW* for "portability", but
since a subshell is counter-productive and we are sure that we have a
bash here, I think your check is OK.

> +# __git_pwd_p() stores the physical path of the current working directory
> +# in the variable whose name is given as argument
> +if [ ${OSTYPE-} = "msys" ]; then
> +__git_pwd_p ()
> +{
> +	eval $1="$PWD"
> +}
> +else
> +__git_pwd_p ()
> +{
> +	eval $1=\"$(pwd -P)\"
> +}
> +fi
> +

The following fixup of the quoting is needed at any rate to make the
eval'd commands resistent against directory names with blanks and
double-quotes.

--- >8 ---
From: Johannes Sixt <j6t@kdbg.org>
Subject: [PATCH] fixup! completion: platform-specific helper function to get
 physical path

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 contrib/completion/git-completion.bash |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index bd7d39e..2cab4a0 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -75,12 +75,12 @@ esac
 if [ ${OSTYPE-} = "msys" ]; then
 __git_pwd_p ()
 {
-	eval $1="$PWD"
+	eval "$1=\$PWD"
 }
 else
 __git_pwd_p ()
 {
-	eval $1=\"$(pwd -P)\"
+	eval "$1=\$(pwd -P)"
 }
 fi
 
-- 
1.7.10.1.1689.gacdfbde

[PATCH 10/19] completion: use bash builtins to search for repository

[SZEDER Gábor]

When not at the top of the work tree or when no path is specified by
'git --git-dir=...' or $GIT_DIR, the __gitdir() helper function finds
out the physical path to the '.git' directory by running the '$(git
rev-parse --git-dir)' command substitution.  fork()ing a subshell and
fork()+exec()ing a git command take some time; during the same amount
of time we can stat a lot of files and directories using bash
builtins, likely finding the '.git' directory faster.

This patch extends __gitdir() to search for a '.git' directory using
only bash builtins.  Like the existing check for '.git' in the current
working directory or the previously added check for $GIT_DIR, this
search is not that thorough either, as it doesn't check whether the
found '.git' directory or the directory pointed to by a gitfile is a
valid '.git' repository.

Since git doesn't search for a .git directory beyond filesystem
boundaries and beyond paths specified in $GIT_CEILING_DIRECTORIES, the
search in __gitdir() should not do that either.  However, bash doesn't
provide builtins to check that two paths are on the same filesystem,
so we can't limit the search cheaply to one filesystem.  Therefore,
__gitdir() will only use bash builtins when neither of these limits
are active, i.e. $GIT_DISCOVERY_ACROSS_FILESYSTEM is set and
$GIT_CEILING_DIRECTORIES is empty, otherwise it will fall back on
executing 'git rev-parse'.  $GIT_CEILING_DIRECTORIES is empty by
default, but users have to set $GIT_DISCOVERY_ACROSS_FILESYSTEM
explicitly to enable this optimisation, when they find its
consequences acceptable.

Of course, stat()ing in C is much faster than in bash, so there is a
point when bash builtins will be slower than '$(git rev-parse
--git-dir)' despite all the fork()s+exec() overhead.  On MinGW this
overhead is considerable, and the builtins version is faster even at a
depth of 50 directories.  On Linux the builtins version is only faster
for paths less than 10 directories deeper than the toplevel; with my
usage patterns that covers 99.9% of the prompts displayed.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash | 30 ++++++++++++
 t/t9903-bash-prompt.sh                 | 83 ++++++++++++++++++++++++++++++++++
 2 files changed, 113 insertions(+)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index bd7d39e3..dd69e56e 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -89,13 +89,43 @@ fi
 __gitdir ()
 {
 	if [ -z "${1-}" ]; then
+		local gitfile
+
 		if [ -n "${__git_dir-}" ]; then
 			:
 		elif [ -n "${GIT_DIR-}" ]; then
 			test -d "${GIT_DIR-}" || return 1
 			__git_dir="$GIT_DIR"
+		elif [ -f .git ]; then
+			read gitfile <.git 2>/dev/null || return 1
+			__git_dir="${gitfile#gitdir: }"
 		elif [ -d .git ]; then
 			__git_dir=.git
+		elif [ -d refs ] && [ -r HEAD ] && \
+				[ -d "${GIT_OBJECT_DIRECTORY:-objects}" ]; then
+			__git_dir=.
+		elif [ -n "${GIT_DISCOVERY_ACROSS_FILESYSTEM-}" -a \
+				-z "${GIT_CEILING_DIRECTORIES-}" ]; then
+			local p
+			__git_pwd_p p
+			while true; do
+				p="${p%/*}"
+				if [ -f "$p/.git" ]; then
+					read gitfile <"$p/.git" 2>/dev/null || return 1
+					__git_dir="${gitfile#gitdir: }"
+					break
+				elif [ -d "$p/.git" ]; then
+					__git_dir="$p/.git"
+					break
+				elif [ -d "$p/refs" ] && [ -r "$p/HEAD" ] && \
+						[ -d "${GIT_OBJECT_DIRECTORY:-$p/objects}" ]; then
+					__git_dir="${p:-/}"
+					break
+				fi
+				if [ -z "$p" ]; then
+					return 1
+				fi
+			done
 		else
 			__git_dir="$(git rev-parse --git-dir 2>/dev/null)" || return 1
 		fi
diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index 3d722b25..ffa22d39 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh
@@ -105,6 +105,18 @@ test_expect_success 'gitdir - .git directory in parent' '
 	test_cmp expected "$actual_var"
 '
 
+test_expect_success 'gitdir - .git directory in parent - with builtins' '
+	echo "$TRASH_DIRECTORY/.git" > expected &&
+	(
+		GIT_DISCOVERY_ACROSS_FILESYSTEM=true &&
+		cd subdir/subsubdir &&
+		__gitdir > "$actual" &&
+		echo "$__git_dir" > "$actual_var"
+	) &&
+	test_cmp expected "$actual" &&
+	test_cmp expected "$actual_var"
+'
+
 test_expect_success 'gitdir - cwd is a .git directory' '
 	echo "." > expected &&
 	(
@@ -116,6 +128,20 @@ test_expect_success 'gitdir - cwd is a .git directory' '
 	test_cmp expected "$actual_var"
 '
 
+test_expect_success 'gitdir - cwd is a .git directory - GIT_OBJECT_DIRECTORY' '
+	echo "." > expected &&
+	mv .git/objects _objects &&
+	test_when_finished "mv _objects .git/objects" &&
+	(
+		GIT_OBJECT_DIRECTORY="$TRASH_DIRECTORY/_objects" &&
+		cd .git &&
+		__gitdir > "$actual" &&
+		echo "$__git_dir" > "$actual_var"
+	) &&
+	test_cmp expected "$actual" &&
+	test_cmp expected "$actual_var"
+'
+
 test_expect_success 'gitdir - parent is a .git directory' '
 	echo "$TRASH_DIRECTORY/.git" > expected &&
 	(
@@ -127,6 +153,33 @@ test_expect_success 'gitdir - parent is a .git directory' '
 	test_cmp expected "$actual_var"
 '
 
+test_expect_success 'gitdir - parent is a .git directory - with builtins' '
+	echo "$TRASH_DIRECTORY/.git" > expected &&
+	(
+		GIT_DISCOVERY_ACROSS_FILESYSTEM=true &&
+		cd .git/refs/heads &&
+		__gitdir > "$actual" &&
+		echo "$__git_dir" > "$actual_var"
+	) &&
+	test_cmp expected "$actual" &&
+	test_cmp expected "$actual_var"
+'
+
+test_expect_success 'gitdir - parent is a .git directory - with builtins and GIT_OBJECT_DIRECTORY' '
+	echo "$TRASH_DIRECTORY/.git" > expected &&
+	mv .git/objects _objects &&
+	test_when_finished "mv _objects .git/objects" &&
+	(
+		GIT_OBJECT_DIRECTORY="$TRASH_DIRECTORY/_objects" &&
+		GIT_DISCOVERY_ACROSS_FILESYSTEM=true &&
+		cd .git/refs/heads &&
+		__gitdir > "$actual" &&
+		echo "$__git_dir" > "$actual_var"
+	) &&
+	test_cmp expected "$actual" &&
+	test_cmp expected "$actual_var"
+'
+
 test_expect_success 'gitdir - $GIT_DIR set while .git directory in cwd' '
 	echo "$TRASH_DIRECTORY/otherrepo/.git" > expected &&
 	(
@@ -187,6 +240,20 @@ test_expect_success 'gitdir - gitfile in parent' '
 	test_cmp expected "$actual_var"
 '
 
+test_expect_success 'gitdir - gitfile in parent - with builtins' '
+	echo "$TRASH_DIRECTORY/otherrepo/.git" > expected &&
+	echo "gitdir: $TRASH_DIRECTORY/otherrepo/.git" > subdir/.git &&
+	test_when_finished "rm -f subdir/.git" &&
+	(
+		GIT_DISCOVERY_ACROSS_FILESYSTEM=true &&
+		cd subdir/subsubdir &&
+		__gitdir > "$actual" &&
+		echo "$__git_dir" > "$actual_var"
+	) &&
+	test_cmp expected "$actual" &&
+	test_cmp expected "$actual_var"
+'
+
 test_expect_success SYMLINKS 'gitdir - resulting path avoids symlinks' '
 	echo "$TRASH_DIRECTORY/otherrepo/.git" > expected &&
 	mkdir otherrepo/dir &&
@@ -202,6 +269,22 @@ test_expect_success SYMLINKS 'gitdir - resulting path avoids symlinks' '
 	test_cmp expected "$actual_var"
 '
 
+test_expect_success SYMLINKS 'gitdir - resulting path avoids symlinks - with builtins' '
+	echo "$TRASH_DIRECTORY/otherrepo/.git" >expected &&
+	mkdir otherrepo/dir &&
+	test_when_finished "rm -rf otherrepo/dir" &&
+	ln -s otherrepo/dir link &&
+	test_when_finished "rm -f link" &&
+	(
+		GIT_DISCOVERY_ACROSS_FILESYSTEM=true &&
+		cd link &&
+		__gitdir > "$actual" &&
+		echo "$__git_dir" > "$actual_var"
+	) &&
+	test_cmp expected "$actual" &&
+	test_cmp expected "$actual_var"
+'
+
 test_expect_success 'gitdir - not a git repository' '
 	(
 		cd subdir/subsubdir &&
-- 
1.7.10.1.541.gb1be298

Re: [PATCH 10/19] completion: use bash builtins to search for repository

[Junio C Hamano]

SZEDER Gábor <szeder@ira.uka.de> writes:

> ..., this
> search is not that thorough either, as it doesn't check whether the
> found '.git' directory or the directory pointed to by a gitfile is a
> valid '.git' repository.
> ...
> Of course, stat()ing in C is much faster than in bash, so there is a
> point when bash builtins will be slower than '$(git rev-parse
> --git-dir)' despite all the fork()s+exec() overhead.

I'd feel safer if this new logic were an opt-in feature, at least in the
beginning, with these pros-and-cons summarized near the beginning of the
file to let the users choose if they want to use "exactly matches the
command the prompt script is trying to help" version (i.e. rev-parse) vs
"matches most of the time and faster under these conditions" version
(i.e. the new logic).

Thanks.

Re: [PATCH 10/19] completion: use bash builtins to search for repository

[SZEDER Gábor]

On Wed, May 09, 2012 at 12:52:49PM -0700, Junio C Hamano wrote:
> SZEDER Gábor <szeder@ira.uka.de> writes:
> 
> > ..., this
> > search is not that thorough either, as it doesn't check whether the
> > found '.git' directory or the directory pointed to by a gitfile is a
> > valid '.git' repository.
> > ...
> > Of course, stat()ing in C is much faster than in bash, so there is a
> > point when bash builtins will be slower than '$(git rev-parse
> > --git-dir)' despite all the fork()s+exec() overhead.
> 
> I'd feel safer if this new logic were an opt-in feature, at least in the
> beginning, with these pros-and-cons summarized near the beginning of the
> file to let the users choose if they want to use "exactly matches the
> command the prompt script is trying to help" version (i.e. rev-parse) vs
> "matches most of the time and faster under these conditions" version
> (i.e. the new logic).

I'm not sure what you mean by opt-in.  It's already opt-in in the
sense that users have to set $GIT_DISCOVERY_ACROSS_FILESYSTEM to
enable this logic.  Or do you mean that
$GIT_DISCOVERY_ACROSS_FILESYSTEM should not implicitly enable this
logic, but it should be controlled by a new dedicated variable?

Re: [PATCH 10/19] completion: use bash builtins to search for repository

[Junio C Hamano]

SZEDER Gábor <szeder@ira.uka.de> writes:

> I'm not sure what you mean by opt-in.  It's already opt-in in the
> sense that users have to set $GIT_DISCOVERY_ACROSS_FILESYSTEM to
> enable this logic.  Or do you mean that
> $GIT_DISCOVERY_ACROSS_FILESYSTEM should not implicitly enable this
> logic, but it should be controlled by a new dedicated variable?

Exactly.  In the three words in DISCOVERY_ACROSS_FILESYSTEM, I do not see
anything that indicates that the user wishes to use a "may be faster in
some situations but may be less correct" behaviour for one thing.  Also,
some day you or somebody else may find out how to take advantage of the
new code in this patch series and still stop at the filesystem boundary
and at that point, people may want to still forbid git to go up across
filesystem boundary and want to use the new prompt code.  Tying these two
independent concepts (i.e. "do I want to use the new experimental prompt
code?" and "do I want git to stop going up across fs boundaries?")
together only because the initial implementation happens to rely on it is
not such a good idea.

[PATCH 11/19] bash prompt: use bash builtins to find out current branch

[SZEDER Gábor]

__git_ps1() runs the '$(git symbolic-ref HEAD)' command substitution
to find out whether we are on a branch and to find out the name of
that branch.  This imposes the overhead of fork()ing a subshell and
fork()+exec()ing a git process.

Since HEAD is a single-line file and the symbolic ref format is quite
simple to recognize and parse, read and parse it using only bash
builtins, thereby sparing all that fork()+exec() overhead.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index dd69e56e..ed372c41 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -293,8 +293,11 @@ __git_ps1 ()
 			r="|BISECTING"
 		fi
 
-		b="$(git symbolic-ref HEAD 2>/dev/null)" || {
-
+		local head=""
+		read head 2>/dev/null <"$__git_dir/HEAD" || return
+		# is it a symbolic ref?
+		b="${head#ref: }"
+		if [ "$head" = "$b" ]; then
 			b="$(
 			case "${GIT_PS1_DESCRIBE_STYLE-}" in
 			(contains)
@@ -310,7 +313,7 @@ __git_ps1 ()
 			b="$(cut -c1-7 "$__git_dir/HEAD" 2>/dev/null)..." ||
 			return
 			b="($b)"
-		}
+		fi
 	fi
 
 	local w=""
-- 
1.7.10.1.541.gb1be298

Re: [PATCH 11/19] bash prompt: use bash builtins to find out current branch

[Junio C Hamano]

SZEDER Gábor <szeder@ira.uka.de> writes:

> Since HEAD is a single-line file and the symbolic ref format is quite
> simple to recognize and parse,...

Strictly speaking, that is true only if you somehow know that HEAD is not
a symlinked symref.  You may end up reading [0-9a-f]{40} out of HEAD
without learning where the symbolic link pointed at.

I personally do not _know_ of anybody who is still using a symlinked
symref, but the reasoning behind 9f0bb90 (core.prefersymlinkrefs: use
symlinks for .git/HEAD, 2006-05-02) cannot go away by definition until
every project that benefited from the configuration the commit introduced
goes extinct, so I wouldn't be surprised if you get complaints from the
users if we adopt this patch.

Re: [PATCH 11/19] bash prompt: use bash builtins to find out current branch

[SZEDER Gábor]

On Wed, May 09, 2012 at 01:02:41PM -0700, Junio C Hamano wrote:
> SZEDER Gábor <szeder@ira.uka.de> writes:
> 
> > Since HEAD is a single-line file and the symbolic ref format is quite
> > simple to recognize and parse,...
> 
> Strictly speaking, that is true only if you somehow know that HEAD is not
> a symlinked symref.  You may end up reading [0-9a-f]{40} out of HEAD
> without learning where the symbolic link pointed at.
> 
> I personally do not _know_ of anybody who is still using a symlinked
> symref, but the reasoning behind 9f0bb90 (core.prefersymlinkrefs: use
> symlinks for .git/HEAD, 2006-05-02) cannot go away by definition until
> every project that benefited from the configuration the commit introduced
> goes extinct, so I wouldn't be surprised if you get complaints from the
> users if we adopt this patch.

Symlinked symref, wow.  That was long before my time ;)

So, let's see whether I understand it correctly:

- If HEAD is a symlink, then it's a symlinked symref, and points to
  a real ref file somewhere under refs/.
- If HEAD is a regular file, then it's either a symref containing
  'ref: refs/...', or it's a detached HEAD containing 40 hexdigits.

If the above is right, then we could check with bash builtins whether
HEAD is a symbolic link, which is cheap, and stick to '$(git
symbolic-ref HEAD)' if it is, or use bash builtins if it isn't, right?  
This way we could get most of the performance benefits for modern
HEADs, while still supporting symlinked symrefs.

Gábor

Re: [PATCH 11/19] bash prompt: use bash builtins to find out current branch

[Junio C Hamano]

SZEDER Gábor <szeder@ira.uka.de> writes:

> If the above is right, then we could check with bash builtins whether
> HEAD is a symbolic link, which is cheap, and stick to '$(git
> symbolic-ref HEAD)' if it is, or use bash builtins if it isn't, right?  

Sure.  Alternatively, you could run "readlink" on it if that is available
built-in, and manipulate the result in string builtins, but that is a b/c
slow path anyway, so I wouldn't bother.

Thanks.

Re: [PATCH 11/19] bash prompt: use bash builtins to find out current branch

[SZEDER Gábor]

On Wed, May 09, 2012 at 02:25:46PM -0700, Junio C Hamano wrote:
> SZEDER Gábor <szeder@ira.uka.de> writes:
> 
> > If the above is right, then we could check with bash builtins whether
> > HEAD is a symbolic link, which is cheap, and stick to '$(git
> > symbolic-ref HEAD)' if it is, or use bash builtins if it isn't, right?  
> 
> Sure.  Alternatively, you could run "readlink" on it if that is available
> built-in, and manipulate the result in string builtins, but that is a b/c
> slow path anyway, so I wouldn't bother.

OK, will do that then.

'readlink' is not a bash builtin, so it would need the same number of
fork()s and exec() as 'symbolic-ref'.  Of course, the 'readlink'
binary is much smaller than git and has less to do, so it might be a
tiny bit faster, but for this rare corner case it really doesn't
matter.

Re: [PATCH 11/19] bash prompt: use bash builtins to find out current branch

[Junio C Hamano]

SZEDER Gábor <szeder@ira.uka.de> writes:

> 'readlink' is not a bash builtin, so it would need the same number of
> fork()s and exec() as 'symbolic-ref'.  Of course, the 'readlink'
> binary is much smaller than git and has less to do, so it might be a
> tiny bit faster, but for this rare corner case it really doesn't
> matter.

Correct, and by not using it you do not have to worry about systems
that does not install the binary.

[PATCH 12/19] bash prompt: use bash builtins to check whether inside git dir

[SZEDER Gábor]

__git_ps1() runs the '$(git rev-parse --is-inside-git-dir)' command
substitution to check whether we are inside a .git directory and the
bash prompt needs to be adjusted accordingly (i.e. display 'BARE!' or
'GIT_DIR!').  This imposes the overhead of fork()ing a subshell and
fork()+exec()ing a git process.

Perform this check by comparing the path to the repository and the
current directory using only bash builtins, thereby sparing all that
fork()+exec() overhead.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index ed372c41..72f7d0ed 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -323,7 +323,11 @@ __git_ps1 ()
 	local c=""
 	local p=""
 
-	if [ "true" = "$(git rev-parse --is-inside-git-dir 2>/dev/null)" ]; then
+	local pwd_p
+	__git_pwd_p pwd_p
+	# inside .git dir?
+	if [ "$__git_dir" = "." -o \
+			"${pwd_p#$__git_dir}" != "$pwd_p" ]; then
 		if [ "true" = "$(git rev-parse --is-bare-repository 2>/dev/null)" ]; then
 			c="BARE:"
 		else
-- 
1.7.10.1.541.gb1be298

Re: [PATCH 12/19] bash prompt: use bash builtins to check whether inside git dir

[Johannes Sixt]

Am 5/9/2012 2:44, schrieb SZEDER Gábor:
> __git_ps1() runs the '$(git rev-parse --is-inside-git-dir)' command
> substitution to check whether we are inside a .git directory and the
> bash prompt needs to be adjusted accordingly (i.e. display 'BARE!' or
> 'GIT_DIR!').  This imposes the overhead of fork()ing a subshell and
> fork()+exec()ing a git process.
> 
> Perform this check by comparing the path to the repository and the
> current directory using only bash builtins, thereby sparing all that
> fork()+exec() overhead.

> -	if [ "true" = "$(git rev-parse --is-inside-git-dir 2>/dev/null)" ]; then
> +	local pwd_p
> +	__git_pwd_p pwd_p
> +	# inside .git dir?
> +	if [ "$__git_dir" = "." -o \
> +			"${pwd_p#$__git_dir}" != "$pwd_p" ]; then

At this point, $__git_dir is c:/dir style, whereas $pwd_p is /c/dir style,
and the intended prefix check does not trigger.

As long as $__git_dir is only used to access files, it does not matter
whether it is Windows style or POSIX style. But if $__git_dir is used in a
comparison, then you must make 100% sure that the involved paths are of
the same vintage.

What would be lost if this patch were dropped?

-- Hannes

Re: [PATCH 12/19] bash prompt: use bash builtins to check whether inside git dir

[Junio C Hamano]

Johannes Sixt <j.sixt@viscovery.net> writes:

>> __git_ps1() runs the '$(git rev-parse --is-inside-git-dir)' command
>> substitution to check whether we are inside a .git directory and the
>> bash prompt needs to be adjusted accordingly (i.e. display 'BARE!' or
>> 'GIT_DIR!').  This imposes the overhead of fork()ing a subshell and
>> fork()+exec()ing a git process.
>> 
>> Perform this check by comparing the path to the repository and the
>> current directory using only bash builtins, thereby sparing all that
>> fork()+exec() overhead.
>
>> -	if [ "true" = "$(git rev-parse --is-inside-git-dir 2>/dev/null)" ]; then
>> +	local pwd_p
>> +	__git_pwd_p pwd_p
>> +	# inside .git dir?
>> +	if [ "$__git_dir" = "." -o \
>> +			"${pwd_p#$__git_dir}" != "$pwd_p" ]; then
>
> At this point, $__git_dir is c:/dir style, whereas $pwd_p is /c/dir style,
> and the intended prefix check does not trigger.
>
> As long as $__git_dir is only used to access files, it does not matter
> whether it is Windows style or POSIX style. But if $__git_dir is used in a
> comparison, then you must make 100% sure that the involved paths are of
> the same vintage.
>
> What would be lost if this patch were dropped?

One loss of fork/exec is what would be lost, I would guess, and that seems
to be the primary point of this entire series, so...

At the conceptual level, I think the optimization in this patch makes
sense, but if the assumed primary beneficiary (i.e. Windows) cannot
benefit from this particular optimization due to two different path
representations, it wouldn't help being conceptually sound X-<.

[PATCH 13/19] bash prompt: check whether inside the worktree only when necessary

[SZEDER Gábor]

Besides the current branch name or detached head info __git_ps1() can
also display status indicators in the prompt for changes in the work
tree, stashes, untracked files, and divergence from upstream.  These
are only displayed when enabled by specific environment variables and
when we are in a work tree.  The latter condition is checked by the
'$(git rev-parse --is-inside-work-tree)' command substitution,
impossing the overhead of fork()ing a subshell and fork()+exec()ing a
git process.

However, the check for the work tree preceeds the check of the
environment variables enabling status indicators, so it's evaluated
even when all these indicators are disabled.

Check upfront whether any of these indicators are enabled, i.e. a
corresponding environment variable is set, to spare the unnecessary
fork()+exec() overhead when all of them are disabled.

(Ideally we could check whether we are in a work tree using only bash
builtins, like we did in the previous commit for .git directory, but
the path of the work tree can be specified by the 'core.worktree'
config variable, and running 'git config' to get its value would be
just as expensive.)

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash | 42 +++++++++++++++++++---------------
 1 file changed, 24 insertions(+), 18 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 72f7d0ed..64b96f13 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -333,29 +333,35 @@ __git_ps1 ()
 		else
 			b="GIT_DIR!"
 		fi
-	elif [ "true" = "$(git rev-parse --is-inside-work-tree 2>/dev/null)" ]; then
-		if [ -n "${GIT_PS1_SHOWDIRTYSTATE-}" ]; then
-			if [ "$(git config --bool bash.showDirtyState)" != "false" ]; then
-				git diff --no-ext-diff --quiet --exit-code || w="*"
-				if git rev-parse --quiet --verify HEAD >/dev/null; then
-					git diff-index --cached --quiet HEAD -- || i="+"
-				else
-					i="#"
+	elif [ -n "${GIT_PS1_SHOWDIRTYSTATE-}" \
+			-o -n "${GIT_PS1_SHOWSTASHSTATE-}" \
+			-o -n "${GIT_PS1_SHOWUNTRACKEDFILES-}" \
+			-n "${GIT_PS1_SHOWUPSTREAM-}" ]; then
+		if [ "true" = "$(git rev-parse --is-inside-work-tree 2>/dev/null)" ]; then
+			if [ -n "${GIT_PS1_SHOWDIRTYSTATE-}" ]; then
+				if [ "$(git config --bool bash.showDirtyState)" != "false" ]; then
+					git diff --no-ext-diff --quiet --exit-code || w="*"
+					if git rev-parse --quiet --verify HEAD >/dev/null; then
+						git diff-index --cached --quiet HEAD -- || i="+"
+					else
+						i="#"
+					fi
 				fi
 			fi
-		fi
-		if [ -n "${GIT_PS1_SHOWSTASHSTATE-}" ]; then
-			git rev-parse --verify refs/stash >/dev/null 2>&1 && s="$"
-		fi
 
-		if [ -n "${GIT_PS1_SHOWUNTRACKEDFILES-}" ]; then
-			if [ -n "$(git ls-files --others --exclude-standard)" ]; then
-				u="%"
+			if [ -n "${GIT_PS1_SHOWSTASHSTATE-}" ]; then
+			        git rev-parse --verify refs/stash >/dev/null 2>&1 && s="$"
 			fi
-		fi
 
-		if [ -n "${GIT_PS1_SHOWUPSTREAM-}" ]; then
-			__git_ps1_show_upstream
+			if [ -n "${GIT_PS1_SHOWUNTRACKEDFILES-}" ]; then
+				if [ -n "$(git ls-files --others --exclude-standard)" ]; then
+					u="%"
+				fi
+			fi
+
+			if [ -n "${GIT_PS1_SHOWUPSTREAM-}" ]; then
+				__git_ps1_show_upstream
+			fi
 		fi
 	fi
 
-- 
1.7.10.1.541.gb1be298

[PATCH 14/19] bash prompt: use bash builtins to find out current branch during rebase

[SZEDER Gábor]

During an ongoing interactive rebase __git_ps1() finds out the name of
the rebased branch by executing the '$(cat .git/rebase-merge/head-name)'
command substitution.  That is not quite the most efficient way to
read a single line single word file, because it imposes the overhead
of fork()ing a subshell and fork()+exec()ing 'cat'.

Use the 'read' bash builtin instead to avoid that overhead.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 64b96f13..671032bf 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -272,10 +272,10 @@ __git_ps1 ()
 	local b=""
 	if [ -f "$__git_dir/rebase-merge/interactive" ]; then
 		r="|REBASE-i"
-		b="$(cat "$__git_dir/rebase-merge/head-name")"
+		read b <"$__git_dir/rebase-merge/head-name"
 	elif [ -d "$__git_dir/rebase-merge" ]; then
 		r="|REBASE-m"
-		b="$(cat "$__git_dir/rebase-merge/head-name")"
+		read b <"$__git_dir/rebase-merge/head-name"
 	else
 		if [ -d "$__git_dir/rebase-apply" ]; then
 			if [ -f "$__git_dir/rebase-apply/rebasing" ]; then
-- 
1.7.10.1.541.gb1be298

[PATCH 15/19] bash prompt: use bash builtins to get detached HEAD abbrev. object name

[SZEDER Gábor]

When describing a detached HEAD according to the $GIT_PS1_DESCRIBE
environment variable fails, __git_ps1() runs the '$(cut -c1-7
.git/HEAD)' command substitution to put the 7 hexdigits abbreviated
commit object name in the prompt.  This imposes the overhead of
fork()ing a subshell and fork(+exec()ing 'cut'.

Thanks to an earlier commit in this series the contents of HEAD is
already read into a local variable, so we can get the 7 hexdigits
using only parameter expansions, sparing the fork()+exec() overhead.

Since zsh doesn't implement substring expansion we can't just use
${head:0:7}, hence the "remove everything except the first 7 chars"
parameter expansion combination.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 671032bf..2346962d 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -310,8 +310,8 @@ __git_ps1 ()
 				git describe --tags --exact-match HEAD ;;
 			esac 2>/dev/null)" ||
 
-			b="$(cut -c1-7 "$__git_dir/HEAD" 2>/dev/null)..." ||
-			return
+			# detached head abbreviated object name
+			b="${head%${head#???????}}..."
 			b="($b)"
 		fi
 	fi
-- 
1.7.10.1.541.gb1be298

[PATCH 16/19] bash prompt: display stash and upstream state even inside the repository

[SZEDER Gábor]

Besides the current branch name or detached head info __git_ps1() can
also display some status indicators in the prompt.  The first such
indicator was for changes in the work tree in 738a94a9 (bash: offer to
show (un)staged changes, 2009-02-03), and was only checked/displayed
when inside the work tree.  Later other indicators were added in
2414b45c (Show presence of stashed changes in bash prompt.,
2009-06-02), 397f7c63 (Show the presence of untracked files in the
bash prompt., 2009-07-22), and 6d158cba (bash completion: Support
"divergence from upstream" messages in __git_ps1, 2010-06-17).  All of
these just followed suit and were checked only when inside the work
tree, i.e. after checking the results of the '$(git rev-parse
--is-inside-work-tree)' command substitution, imposing the overhead of
fork()ing a subshell and fork()+exec()ing a git process.

However, the presence of stashes and the divergence from upstream is
not a property of the work tree but a property of the repository, and
the implementation of their indicators doesn't actually require a work
tree.  Therefore, we can display these two indicators even inside the
repository.  Not that it's very useful to see the stash status while
poking around deep inside the .git directory, but this way users
enabling only the stash indicator won't pay the additional performance
penalty of the check for the work tree.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash | 20 +++++++++-----------
 t/t9903-bash-prompt.sh                 |  4 ++--
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 2346962d..64207e3c 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -334,9 +334,7 @@ __git_ps1 ()
 			b="GIT_DIR!"
 		fi
 	elif [ -n "${GIT_PS1_SHOWDIRTYSTATE-}" \
-			-o -n "${GIT_PS1_SHOWSTASHSTATE-}" \
-			-o -n "${GIT_PS1_SHOWUNTRACKEDFILES-}" \
-			-n "${GIT_PS1_SHOWUPSTREAM-}" ]; then
+			-o -n "${GIT_PS1_SHOWUNTRACKEDFILES-}" ]; then
 		if [ "true" = "$(git rev-parse --is-inside-work-tree 2>/dev/null)" ]; then
 			if [ -n "${GIT_PS1_SHOWDIRTYSTATE-}" ]; then
 				if [ "$(git config --bool bash.showDirtyState)" != "false" ]; then
@@ -349,22 +347,22 @@ __git_ps1 ()
 				fi
 			fi
 
-			if [ -n "${GIT_PS1_SHOWSTASHSTATE-}" ]; then
-			        git rev-parse --verify refs/stash >/dev/null 2>&1 && s="$"
-			fi
-
 			if [ -n "${GIT_PS1_SHOWUNTRACKEDFILES-}" ]; then
 				if [ -n "$(git ls-files --others --exclude-standard)" ]; then
 					u="%"
 				fi
 			fi
-
-			if [ -n "${GIT_PS1_SHOWUPSTREAM-}" ]; then
-				__git_ps1_show_upstream
-			fi
 		fi
 	fi
 
+	if [ -n "${GIT_PS1_SHOWSTASHSTATE-}" ]; then
+	        git rev-parse --verify refs/stash >/dev/null 2>&1 && s="$"
+	fi
+
+	if [ -n "${GIT_PS1_SHOWUPSTREAM-}" ]; then
+		__git_ps1_show_upstream
+	fi
+
 	local f="$w$i$s$u"
 	printf -- "${1:- (%s)}" "$c${b##refs/heads/}${f:+ $f}$r$p"
 }
diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index ffa22d39..a43d402a 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh
@@ -542,8 +542,8 @@ test_expect_success 'prompt - stash status indicator - stash' '
 	test_cmp expected "$actual"
 '
 
-test_expect_success 'prompt - stash status indicator - not shown inside .git directory' '
-	printf " (GIT_DIR!)" > expected &&
+test_expect_success 'prompt - stash status indicator - stash while inside .git directory' '
+	printf " (GIT_DIR! $)" > expected &&
 	echo 2 >file &&
 	git stash &&
 	test_when_finished "git stash drop" &&
-- 
1.7.10.1.541.gb1be298

[PATCH 17/19] bash prompt: use bash builtins to check stash state

[SZEDER Gábor]

When the environment variable $GIT_PS1_SHOWSTASHSTATE is set
__git_ps1() checks the presence of stashes by running 'git rev-parse
--verify refs/stash'.  This command not only checks that the
'refs/stash' ref exists but also, well, verifies that it's a valid
ref.

However, we don't need to be that thorough for the bash prompt.  We
can omit that verification and only check whether 'refs/stash' exists
or not.  Since 'git pack-refs' never packs 'refs/stash', it's a matter
of checking the existence of a ref file.  Perform this check using
only bash builtins to spare the overhead of fork()+exec()ing a git
process.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 64207e3c..c4feab68 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -356,7 +356,9 @@ __git_ps1 ()
 	fi
 
 	if [ -n "${GIT_PS1_SHOWSTASHSTATE-}" ]; then
-	        git rev-parse --verify refs/stash >/dev/null 2>&1 && s="$"
+		if [ -r "$__git_dir/refs/stash" ]; then
+			s="$"
+		fi
 	fi
 
 	if [ -n "${GIT_PS1_SHOWUPSTREAM-}" ]; then
-- 
1.7.10.1.541.gb1be298

[RFC PATCH 18/19] bash prompt: avoid command substitution when checking for untracked files

[SZEDER Gábor]

When enabled, the bash prompt can indicate the presence of untracked
files with a '%' sign.  __git_ps1() checks for untracked files by running the
'$(git ls-files --others --exclude-standard)' command substitution,
and displays the indicator when there is no output.

Avoid this command substitution by additionally passing
'--error-unmatch *', and checking the command's return value.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---

This seems to do the right thing, but I'm not quite sure, so I would
appreciate a pair of expert eyeballs on it.

 contrib/completion/git-completion.bash | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index c4feab68..5ea19018 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -348,9 +348,8 @@ __git_ps1 ()
 			fi
 
 			if [ -n "${GIT_PS1_SHOWUNTRACKEDFILES-}" ]; then
-				if [ -n "$(git ls-files --others --exclude-standard)" ]; then
-					u="%"
-				fi
+				git ls-files --others --exclude-standard --error-unmatch -- '*' >/dev/null 2>/dev/null &&
+				u="%"
 			fi
 		fi
 	fi
-- 
1.7.10.1.541.gb1be298

Re: [RFC PATCH 18/19] bash prompt: avoid command substitution when checking for untracked files

[Junio C Hamano]

SZEDER Gábor <szeder@ira.uka.de> writes:

> When enabled, the bash prompt can indicate the presence of untracked
> files with a '%' sign.  __git_ps1() checks for untracked files by running the
> '$(git ls-files --others --exclude-standard)' command substitution,
> and displays the indicator when there is no output.
>
> Avoid this command substitution by additionally passing
> '--error-unmatch *', and checking the command's return value.

This is too subtle and needs to be explained in a in-code comment.  For
example, it is unclear to me how this '*' pathspec and an untracked file
that does not fnmatch(3) with the pattern (e.g. ".trash").


        $ rm -fr /var/tmp/x && git init /var/tmp/x && cd /var/tmp/x
	$ args='ls-files --others --exclude-standard'

	$ git $args | wc -l
        0
        $ git $args --error-unmatch -- '*' >/dev/null 2>&1 ; echo $?
        1

        $ >a
        $ git $args | wc -l
	1
        $ git $args --error-unmatch -- '*' >/dev/null 2>&1 ; echo $?
	0

	$ mv a .a
        $ git $args | wc -l
	1
        $ git $args --error-unmatch -- '*' >/dev/null 2>&1 ; echo $?
	0

The first two cases seem to be fine, but isn't the last one showing that
your update is incorrect?

>
> Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
> ---
>
> This seems to do the right thing, but I'm not quite sure, so I would
> appreciate a pair of expert eyeballs on it.
>
>  contrib/completion/git-completion.bash | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index c4feab68..5ea19018 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -348,9 +348,8 @@ __git_ps1 ()
>  			fi
>  
>  			if [ -n "${GIT_PS1_SHOWUNTRACKEDFILES-}" ]; then
> -				if [ -n "$(git ls-files --others --exclude-standard)" ]; then
> -					u="%"
> -				fi
> +				git ls-files --others --exclude-standard --error-unmatch -- '*' >/dev/null 2>/dev/null &&
> +				u="%"
>  			fi
>  		fi
>  	fi

[PATCH 19/19] bash prompt: alternative git prompt without command substitution

[SZEDER Gábor]

__git_ps1() prints the branch name, status indicators, etc. to stdout,
therefore it has to be included in $PS1 through a command substitution
to display that information in the prompt.  The configuration is
straightforward, but it imposes the overhead of fork()ing a subshell
for the command substitution.

However, bash has the $PROMPT_COMMAND shell variable, which "if set,
the value is executed as a command prior to issuing each primary
prompt" (quoted from bash man page).  Its value isn't executed in a
subshell but in the context of the "main" shell, hence (non-local)
variables set in invoked shell functions are available when expanding
$PS1.  We can use this facility to avoid that command substitution for
__git_ps1().

So split out the meat of __git_ps1() into the new
__git_prompt_command() function, which stores the branch name & co.
in the $__git_ps1_string variable.  This function, as its name
suggests, should be included in $PROMPT_COMMAND, and $__git_ps1_string
should in turn be included in $PS1 with a bit of a twist to put the
parentheses around it:

   PROMPT_COMMAND=__git_prompt_command
   PS1='[\u@\h \W${__git_ps1_string:+ ($__git_ps1_string)}]\$ '

Turn __git_ps1() into a wrapper around __git_prompt_command() such
that it's functionality remains unaltered, so already configured
prompts won't break.

The whole series speeds up the bash prompt on Windows/MinGW
immensely, in many cases brings it down to around 10ms on my
machine while in powersave mode.  Here are some timing results in
three common scenarios (repeated 10 times, because the after cases
were too fast to measure a single execution accurately with 'time'):

In my home directory, i.e. not in a git repository, before:

    /c/Users/szeder
    $ time for i in {0..9} ; do prompt=$(__git_ps1) ; done

    real    0m0.952s
    user    0m0.214s
    sys     0m0.444s

  After:

    /c/Users/szeder
    $ time for i in {0..9} ; do __git_prompt_command ;
           prompt=${__git_ps1_string:+ ($__git_ps1_string)} ; done

    real    0m0.718s
    user    0m0.136s
    sys     0m0.354s

  After, with discovery across filesystems enabled:

    /c/Users/szeder
    $ time for i in {0..9} ; do __git_prompt_command ;
           prompt=${__git_ps1_string:+ ($__git_ps1_string)} ; done

    real    0m0.078s
    user    0m0.016s
    sys     0m0.062s

At the top of a work tree, before:

    /c/Users/szeder/repo (master)
    $ time for i in {0..9} ; do prompt=$(__git_ps1) ; done

    real    0m2.901s
    user    0m0.391s
    sys     0m1.468s

  After:

    /c/Users/szeder/repo (master)
    $ time for i in {0..9} ; do __git_prompt_command ;
           prompt=${__git_ps1_string:+ ($__git_ps1_string)} ; done

    real    0m0.094s
    user    0m0.047s
    sys     0m0.047s

In a subdirectory, stash indicator enabled, before:

    /c/Users/szeder/repo/subdir (master $)
    $ time for i in {0..9} ; do prompt=$(__git_ps1) ; done

    real    0m4.118s
    user    0m0.468s
    sys     0m2.056s

  After:

    /c/Users/szeder/repo/subdir (master $)
    $ time for i in {0..9} ; do __git_prompt_command ;
           prompt=${__git_ps1_string:+ ($__git_ps1_string)} ; done

    real    0m0.858s
    user    0m0.152s
    sys     0m0.322s

  After, discovery across filesystems enabled:

    /c/Users/szeder/repo/subdir (master $)
    $ time for i in {0..9} ; do __git_prompt_command ;
           prompt=${__git_ps1_string:+ ($__git_ps1_string)} ; done

    real    0m0.109s
    user    0m0.047s
    sys     0m0.063s

Well, that's about 97% improvement.

The performance gain on Linux is smaller, the latter case goes down
from 0.264s to 0.047, but since it was fast enough to begin with I
won't lengthen this commit message with further timing results on
Linux.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---

We had some discussions recently about putting user-facing functions into
a separate "namespace".  This patch doesn't take that into account, but
once a consensus is reached __git_prompt_command() should be put in that
namespace.

 contrib/completion/git-completion.bash | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 5ea19018..1c29f3d0 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -29,6 +29,11 @@
 #       are currently in a git repository.  The %s token will be
 #       the name of the current branch.
 #
+#       Alternatively, to make the above Bash prompt a bit faster:
+#               PROMPT_COMMAND=__git_prompt_command
+#               PS1='[\u@\h \W${__git_ps1_string:+ ($__git_ps1_string)}]\$ '
+#               GIT_DISCOVERY_ACROSS_FILESYSTEM=true
+#
 #       In addition, if you set GIT_PS1_SHOWDIRTYSTATE to a nonempty
 #       value, unstaged (*) and staged (+) changes will be shown next
 #       to the branch name.  You can configure this per-repository
@@ -258,11 +263,12 @@ __git_ps1_show_upstream ()
 }
 
 
-# __git_ps1 accepts 0 or 1 arguments (i.e., format string)
-# returns text to add to bash PS1 prompt (includes branch name)
-__git_ps1 ()
+# Stores the text to be added to the bash prompt (branch name, status
+# indicators, etc.) in the $__git_ps1_string variable.
+__git_prompt_command ()
 {
 	local __git_dir=""
+	__git_ps1_string=""
 	__gitdir >/dev/null
 	if [ -z "$__git_dir" ]; then
 		return
@@ -365,7 +371,18 @@ __git_ps1 ()
 	fi
 
 	local f="$w$i$s$u"
-	printf -- "${1:- (%s)}" "$c${b##refs/heads/}${f:+ $f}$r$p"
+	__git_ps1_string="$c${b##refs/heads/}${f:+ $f}$r$p"
+}
+
+# __git_ps1 accepts 0 or 1 arguments (i.e., format string)
+# returns text to add to bash PS1 prompt (includes branch name)
+__git_ps1 ()
+{
+	local __git_ps1_string
+	__git_prompt_command
+	if [ -n "$__git_ps1_string" ]; then
+		printf -- "${1:- (%s)}" "$__git_ps1_string"
+	fi
 }
 
 __gitcomp_1 ()
-- 
1.7.10.1.541.gb1be298

Re: [PATCH 19/19] bash prompt: alternative git prompt without command substitution

[Andrew Sayers]

On 09/05/12 01:44, SZEDER Gábor wrote:
> __git_ps1() prints the branch name, status indicators, etc. to stdout,
> therefore it has to be included in $PS1 through a command substitution
> to display that information in the prompt.  The configuration is
> straightforward, but it imposes the overhead of fork()ing a subshell
> for the command substitution.
> 
> However, bash has the $PROMPT_COMMAND shell variable, which "if set,
> the value is executed as a command prior to issuing each primary
> prompt" (quoted from bash man page).  Its value isn't executed in a
> subshell but in the context of the "main" shell, hence (non-local)
> variables set in invoked shell functions are available when expanding
> $PS1.  We can use this facility to avoid that command substitution for
> __git_ps1().
> 
> So split out the meat of __git_ps1() into the new
> __git_prompt_command() function, which stores the branch name & co.
> in the $__git_ps1_string variable.  This function, as its name
> suggests, should be included in $PROMPT_COMMAND, and $__git_ps1_string
> should in turn be included in $PS1 with a bit of a twist to put the
> parentheses around it:
> 
>    PROMPT_COMMAND=__git_prompt_command

Rather than overwrite any existing PROMPT_COMMAND, it would be better to
do something like:

PROMPT_COMMAND="__git_prompt_command; $PROMPT_COMMAND"

>    PS1='[\u@\h \W${__git_ps1_string:+ ($__git_ps1_string)}]\$ '
> 
> Turn __git_ps1() into a wrapper around __git_prompt_command() such
> that it's functionality remains unaltered, so already configured
> prompts won't break.
> 
> The whole series speeds up the bash prompt on Windows/MinGW
> immensely, in many cases brings it down to around 10ms on my
> machine while in powersave mode.  Here are some timing results in
> three common scenarios (repeated 10 times, because the after cases
> were too fast to measure a single execution accurately with 'time'):
> 
> In my home directory, i.e. not in a git repository, before:
> 
>     /c/Users/szeder
>     $ time for i in {0..9} ; do prompt=$(__git_ps1) ; done
> 
>     real    0m0.952s
>     user    0m0.214s
>     sys     0m0.444s
> 
>   After:
> 
>     /c/Users/szeder
>     $ time for i in {0..9} ; do __git_prompt_command ;
>            prompt=${__git_ps1_string:+ ($__git_ps1_string)} ; done
> 
>     real    0m0.718s
>     user    0m0.136s
>     sys     0m0.354s
> 
>   After, with discovery across filesystems enabled:
> 
>     /c/Users/szeder
>     $ time for i in {0..9} ; do __git_prompt_command ;
>            prompt=${__git_ps1_string:+ ($__git_ps1_string)} ; done
> 
>     real    0m0.078s
>     user    0m0.016s
>     sys     0m0.062s
> 
> At the top of a work tree, before:
> 
>     /c/Users/szeder/repo (master)
>     $ time for i in {0..9} ; do prompt=$(__git_ps1) ; done
> 
>     real    0m2.901s
>     user    0m0.391s
>     sys     0m1.468s
> 
>   After:
> 
>     /c/Users/szeder/repo (master)
>     $ time for i in {0..9} ; do __git_prompt_command ;
>            prompt=${__git_ps1_string:+ ($__git_ps1_string)} ; done
> 
>     real    0m0.094s
>     user    0m0.047s
>     sys     0m0.047s
> 
> In a subdirectory, stash indicator enabled, before:
> 
>     /c/Users/szeder/repo/subdir (master $)
>     $ time for i in {0..9} ; do prompt=$(__git_ps1) ; done
> 
>     real    0m4.118s
>     user    0m0.468s
>     sys     0m2.056s
> 
>   After:
> 
>     /c/Users/szeder/repo/subdir (master $)
>     $ time for i in {0..9} ; do __git_prompt_command ;
>            prompt=${__git_ps1_string:+ ($__git_ps1_string)} ; done
> 
>     real    0m0.858s
>     user    0m0.152s
>     sys     0m0.322s
> 
>   After, discovery across filesystems enabled:
> 
>     /c/Users/szeder/repo/subdir (master $)
>     $ time for i in {0..9} ; do __git_prompt_command ;
>            prompt=${__git_ps1_string:+ ($__git_ps1_string)} ; done
> 
>     real    0m0.109s
>     user    0m0.047s
>     sys     0m0.063s
> 
> Well, that's about 97% improvement.
> 
> The performance gain on Linux is smaller, the latter case goes down
> from 0.264s to 0.047, but since it was fast enough to begin with I
> won't lengthen this commit message with further timing results on
> Linux.
> 
> Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
> ---
> 
> We had some discussions recently about putting user-facing functions into
> a separate "namespace".  This patch doesn't take that into account, but
> once a consensus is reached __git_prompt_command() should be put in that
> namespace.
> 
>  contrib/completion/git-completion.bash | 25 +++++++++++++++++++++----
>  1 file changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 5ea19018..1c29f3d0 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -29,6 +29,11 @@
>  #       are currently in a git repository.  The %s token will be
>  #       the name of the current branch.
>  #
> +#       Alternatively, to make the above Bash prompt a bit faster:
> +#               PROMPT_COMMAND=__git_prompt_command

As above, I'd recommend a simple documentation change:
PROMPT_COMMAND="__git_prompt_command; $PROMPT_COMMAND"
(to show people how to chain any other prompt commands they have)

> +#               PS1='[\u@\h \W${__git_ps1_string:+ ($__git_ps1_string)}]\$ '
> +#               GIT_DISCOVERY_ACROSS_FILESYSTEM=true
> +#
>  #       In addition, if you set GIT_PS1_SHOWDIRTYSTATE to a nonempty
>  #       value, unstaged (*) and staged (+) changes will be shown next
>  #       to the branch name.  You can configure this per-repository
> @@ -258,11 +263,12 @@ __git_ps1_show_upstream ()
>  }
>  
>  
> -# __git_ps1 accepts 0 or 1 arguments (i.e., format string)
> -# returns text to add to bash PS1 prompt (includes branch name)
> -__git_ps1 ()
> +# Stores the text to be added to the bash prompt (branch name, status
> +# indicators, etc.) in the $__git_ps1_string variable.
> +__git_prompt_command ()
>  {
>  	local __git_dir=""
> +	__git_ps1_string=""
>  	__gitdir >/dev/null
>  	if [ -z "$__git_dir" ]; then
>  		return
> @@ -365,7 +371,18 @@ __git_ps1 ()
>  	fi
>  
>  	local f="$w$i$s$u"
> -	printf -- "${1:- (%s)}" "$c${b##refs/heads/}${f:+ $f}$r$p"
> +	__git_ps1_string="$c${b##refs/heads/}${f:+ $f}$r$p"
> +}
> +
> +# __git_ps1 accepts 0 or 1 arguments (i.e., format string)
> +# returns text to add to bash PS1 prompt (includes branch name)
> +__git_ps1 ()
> +{
> +	local __git_ps1_string
> +	__git_prompt_command
> +	if [ -n "$__git_ps1_string" ]; then
> +		printf -- "${1:- (%s)}" "$__git_ps1_string"
> +	fi

How hard/appropriate would it be to export individual parts of the
prompt here?  Something like:

__git_ps1_string_dirtystate="$i"
__git_ps1_string_untrackedfiles="$u"

There have been requests in the past to let people individually
colourise different bits of the prompt, which this would make practical.

>  }
>  
>  __gitcomp_1 ()

Re: [PATCH 19/19] bash prompt: alternative git prompt without command substitution

[SZEDER Gábor]

Hi,

On Wed, May 09, 2012 at 08:38:22PM +0100, Andrew Sayers wrote:
> On 09/05/12 01:44, SZEDER Gábor wrote:
> >    PROMPT_COMMAND=__git_prompt_command
> 
> Rather than overwrite any existing PROMPT_COMMAND, it would be better to
> do something like:
> 
> PROMPT_COMMAND="__git_prompt_command; $PROMPT_COMMAND"

> > +#       Alternatively, to make the above Bash prompt a bit faster:
> > +#               PROMPT_COMMAND=__git_prompt_command
> 
> As above, I'd recommend a simple documentation change:
> PROMPT_COMMAND="__git_prompt_command; $PROMPT_COMMAND"
> (to show people how to chain any other prompt commands they have)

That's a good idea.  In fact I did manage to overwrite my
$PROMPT_COMMAND and was wondering why did the title of my terminal
windows disappear so suddenly...


> > @@ -365,7 +371,18 @@ __git_ps1 ()
> >  	fi
> >  
> >  	local f="$w$i$s$u"
> > -	printf -- "${1:- (%s)}" "$c${b##refs/heads/}${f:+ $f}$r$p"
> > +	__git_ps1_string="$c${b##refs/heads/}${f:+ $f}$r$p"
> > +}
> > +
> > +# __git_ps1 accepts 0 or 1 arguments (i.e., format string)
> > +# returns text to add to bash PS1 prompt (includes branch name)
> > +__git_ps1 ()
> > +{
> > +	local __git_ps1_string
> > +	__git_prompt_command
> > +	if [ -n "$__git_ps1_string" ]; then
> > +		printf -- "${1:- (%s)}" "$__git_ps1_string"
> > +	fi
> 
> How hard/appropriate would it be to export individual parts of the
> prompt here?  Something like:
> 
> __git_ps1_string_dirtystate="$i"
> __git_ps1_string_untrackedfiles="$u"
> 
> There have been requests in the past to let people individually
> colourise different bits of the prompt, which this would make practical.

We can't do that from __git_ps1(), because, as I mentioned in the commit
message, it must be invoked in a command substitution from $PS1, and
what's exported in a subshell that stays in that subshell.

Doing so from __git_prompt_command() would be quite simple: just
rename the appropriate variables and don't declare them as local.
It would even be more pleasing to the eyes than the current one-letter
variable names.


Gábor