completion: zsh: stop leaking local cache variable

Commit Message

D. Ben Knoble April 30, 2024, 9:53 p.m. UTC

From: "D. Ben Knoble" <[e-mail]>

Completing commands like "git rebase" in one repository will leak the
local __git_repo_path into the shell's environment so that completing
commands after changing to a different repository will give the old
repository's references (or none at all).

The bug report on the mailing list [1] suggests one simple way to observe
this yourself:

Enter the following commands from some directory:
  mkdir a b b/c
  for d (a b); git -C $d init && git -C $d commit --allow-empty -m init
  cd a
  git branch foo
  pushd ../b/c
  git branch bar

Now type these:
  git rebase <TAB>… # completion for bar available; C-c to abort
  declare -p __git_repo_path # outputs /path/to/b/.git
  popd
  git branch # outputs foo, main
  git rebase <TAB>… # completion candidates are bar, main!

Ideally, the last typed <TAB> should be yielding foo, main.

Commit beb6ee7163 (completion: extract repository discovery from
__gitdir(), 2017-02-03) anticipated this problem by marking
__git_repo_path as local in __git_main and __gitk_main for Bash
completion but did not give the same mark to _git for Zsh completion.
Thus make __git_repo_path local for Zsh completion, too.

[1]: https://lore.kernel.org/git/[e-mail]/

Signed-off-by: D. Ben Knoble <[e-mail]>
---
    completion: zsh: stop leaking local cache variable
    
    Prevent leaking a local variable used to cache the repo path, which
    breaks future completions in different repositories using the shell,
    when using contributed Zsh completion.
    
    ------------------------------------------------------------------------
    
    I made a few attempts at starting a test script for this based on
    https://unix.stackexchange.com/a/668827/301073, but that code doesn't
    work and it was all becoming precariously complicated (sh starting zsh
    to start zsh in a pty which would receive keystrokes and check specific
    outputs: I couldn't make certain pieces work in a normal way locally,
    let alone as part of Git's test suite). Suffice to say I have tested
    this myself?

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1725%2Fbenknoble%2Ffix-zsh-completion-repo-cache-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1725/benknoble/fix-zsh-completion-repo-cache-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1725

 contrib/completion/git-completion.zsh | 1 +
 1 file changed, 1 insertion(+)


base-commit: 786a3e4b8d754d2b14b1208b98eeb0a554ef19a8

Comments

Junio C Hamano April 30, 2024, 11:18 p.m. UTC | #1

"D. Ben Knoble via GitGitGadget" <[e-mail]> writes:

> Commit beb6ee7163 (completion: extract repository discovery from
> __gitdir(), 2017-02-03) anticipated this problem by marking
> __git_repo_path as local in __git_main and __gitk_main for Bash
> completion but did not give the same mark to _git for Zsh completion.
> Thus make __git_repo_path local for Zsh completion, too.

Will queue.  The explanation makes tons of sense.

Thanks.

Patch

diff --git a/contrib/completion/git-completion.zsh b/contrib/completion/git-completion.zsh
index cac6f618817..f5877bd7a1e 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -272,6 +272,7 @@  _git ()
 {
 	local _ret=1
 	local cur cword prev
+	local __git_repo_path
 
 	cur=${words[CURRENT]}
 	prev=${words[CURRENT-1]}