Re: [PATCH v1 03/11] git-gui: guard set/unset of GIT_DIR and GIT_WORK_TREE
- From
Johannes Sixt <j6t@kdbg.org>
- Date
- May 15, 2026, 15:58 UTC
- Message-ID
- <d9af8b60-e354-4cfc-87b3-a3e708b362da@kdbg.org>
- In-Reply-To
- <20260514143322.865587-4-mlevedahl@gmail.com>
Am 14.05.26 um 16:33 schrieb Mark Levedahl:
Show 11 quoted lines
> git-gui unconditionally exports GIT_DIR and GIT_WORK_TREE to the
> environment, and furthmore unconditionally unsets these in many places.
> But, GIT_WORK_TREE should be set only if it is not {} as the empty
> value, really meaning no work-tree is found, causes git to throw fatal
> errors (git-gui gets the error from branch --show-current). Fixing this
> is required to allow blame and browser to operate from a repository
> without a worktree.
>
> Establish a pair of functions to remove GIT_DIR and GIT_WORK_TREE from
> the environment, avoiding any error if they do not exist. Also, add a
> function to export these, but export GIT_WORK_TREE only if not empty.Good. But as I said in a parallel thread, I actually concur with your assessment in the coverletter of this patch series that GIT_WORK_TREE should be not set at all. At least in the modes that require a working tree.
Show 79 quoted lines
>
> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
> ---
> git-gui.sh | 32 ++++++++++++++++++++++----------
> 1 file changed, 22 insertions(+), 10 deletions(-)
>
> diff --git a/git-gui.sh b/git-gui.sh
> index a951fcd..387cad6 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -1122,6 +1122,22 @@ unset argv0dir
> ##
> ## repository setup
>
> +proc set_gitdir_vars {} {
> + global _gitdir _gitworktree env
> + if {$_gitdir ne {}} {
> + set env(GIT_DIR) $_gitdir
> + }
> + if {$_gitworktree ne {}} {
> + set env(GIT_WORK_TREE) $_gitworktree
> + }
> +}
> +
> +proc unset_gitdir_vars {} {
> + global env
> + catch {unset env(GIT_DIR)}
> + catch {unset env(GIT_WORK_TREE)}
> +}
> +
> set picked 0
> if {[catch {
> set _gitdir $env(GIT_DIR)
> @@ -1207,8 +1223,8 @@ if {[lindex $_reponame end] eq {.git}} {
> set _reponame [lindex $_reponame end]
> }
>
> -set env(GIT_DIR) $_gitdir
> -set env(GIT_WORK_TREE) $_gitworktree
> +# Export the final paths
> +set_gitdir_vars
>
> ######################################################################
> ##
> @@ -2050,13 +2066,11 @@ proc do_gitk {revs {is_submodule false}} {
> # TODO we could make life easier (start up faster?) for gitk
> # by setting these to the appropriate values to allow gitk
> # to skip the heuristics to find their proper value
> - unset env(GIT_DIR)
> - unset env(GIT_WORK_TREE)
> + unset_gitdir_vars
> }
> safe_exec_bg [concat $cmd $revs "--" "--"]
>
> - set env(GIT_DIR) $_gitdir
> - set env(GIT_WORK_TREE) $_gitworktree
> + set_gitdir_vars
> cd $pwd
>
> if {[info exists main_status]} {
> @@ -2084,16 +2098,14 @@ proc do_git_gui {} {
>
> # see note in do_gitk about unsetting these vars when
> # running tools in a submodule
> - unset env(GIT_DIR)
> - unset env(GIT_WORK_TREE)
> + unset_gitdir_vars
>
> set pwd [pwd]
> cd $current_diff_path
>
> safe_exec_bg [concat $exe gui]
>
> - set env(GIT_DIR) $_gitdir
> - set env(GIT_WORK_TREE) $_gitworktree
> + set_gitdir_vars
> cd $pwd
>
> set status_operation [$::main_status \After these changes, a 'global env' probably becomes stale and could be removed.
-- Hannes