threads / patch / 61400

patchcompletion: zsh: stop leaking local cache variable

Subject: [PATCH] completion: zsh: stop leaking local cache variable

## tl;dr

2 messages between Apr 30, 2024 and Apr 30, 2024. Diffs are folded; open one to read it.

replies: 1people: 2as markdown or json

D. Ben Knoble via GitGitGadget· Apr 30, 2024, 21:53 UTC · lore
From: "D. Ben Knoble" <ben.knoble+github@gmail.com>

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/CALnO6CBv3+e2WL6n6Mh7ZZHCX2Ni8GpvM4a-bQYxNqjmgZdwdg@mail.gmail.com/
Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail.com>
---
    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(+)
Show changes to contrib/completion/git-completion.zsh +1 −0
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]}

base-commit: 786a3e4b8d754d2b14b1208b98eeb0a554ef19a8
-- 
gitgitgadget
Junio C Hamano· Apr 30, 2024, 23:18 UTC · re: D. Ben Knoble via GitGitGadget · lore

Re: [PATCH] completion: zsh: stop leaking local cache variable

"D. Ben Knoble via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 5 quoted lines
> 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.

← back to recent threads