From: Mark Levedahl Date: Sat, 16 May 2026 14:25:26 GMT Subject: Re: [PATCH v1 03/11] git-gui: guard set/unset of GIT_DIR and GIT_WORK_TREE Message-ID: In-Reply-To: On 5/15/26 11:58 AM, Johannes Sixt wrote: > Am 14.05.26 um 16:33 schrieb Mark Levedahl: >> 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. > >> Signed-off-by: Mark Levedahl >> --- >> 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 > Will update to only ever set GIT_DIR, will still remove GIT_WORK_TREE on unset. Mark