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@xxxxxxxxxx> --- 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html