[PATCH v3 00/12] Improve git gui operation without a worktree

From

Mark Levedahl <mlevedahl@gmail.com>

Date

May 31, 2026, 23:02 UTC

Message-ID

<20260531230225.126817-1-mlevedahl@gmail.com>

In-Reply-To

<20260520202411.108764-1-mlevedahl@gmail.com>

git gui has a number of inter-related problems that result in problems
during startup from anything but a checked out worktree pointing at a
valid git repository. Some of the symptoms are:
- blame / browser subcommands, and launching gitk, are intended to be
  useful without a worktree, but fail to work.
- unlike git, git-gui is supposed to use the parent directory as a
  worktree if started from the .git subdirectory in the very common
  single worktree + embedded git repository format. This does not
  work.
- git-gui includes a repository picker allowing a user to select a
  worktree from a list and/or start a new repo+worktree: this dialog can
  appear at unexpected times, masking useful error feedback on
  configuration problems.

Johannes Sixt (1):
  git-gui: remove unnecessary 'cd $_gitworktree' from do_gitk

Mark Levedahl (11):
  git-gui: use HEAD as current branch when detached
  git-gui: guard set/unset of GIT_DIR and GIT_WORK_TREE
  git-gui: do not change global vars in choose_repository::pick
  git-gui: use --absolute-git-dir
  git-gui: use rev-parse exclusively to find a repository
  git-gui: use git rev-parse for worktree discovery
  git-gui: simplify [is_bare] to report if a worktree is known
  git-gui: try harder to find worktree from gitdir
  git-gui: allow specifying path '.' to the browser
  git-gui: check browser/blame arguments carefully
  git-gui: add gui and pick as explicit subcommands

 git-gui.sh                | 377 ++++++++++++++++++++++----------------
 lib/choose_repository.tcl |  21 +--
 2 files changed, 223 insertions(+), 175 deletions(-)

Interdiff against v2:

diff --git a/git-gui.sh b/git-gui.sh
index 299c1a0292..933e72c9b2 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1114,33 +1114,29 @@ unset argv0dir
 ## repository setup
 
 proc find_worktree_from_gitdir {} {
-	# Directory 'parent' of a repository named 'parent/.git' might be the worktree.
-	# Assure parent is a worktree and using the git repository already discovered.
-	# Also, handle case of being in a worktree's gitdir, where file "gitdir" points to
-	# gitlink file .git in the real worktree.
+	# this is invoked only if the current directory is inside the repository
 	set worktree {}
 	if {[file tail $::_gitdir] eq {.git}} {
+		# the dir containing .git is a worktree if repo allows it
+		# Check that git reports parent as a worktree (gitdir might not allow a worktree)
 		if {[catch {
-			set gitdir_parent [file dirname $::_gitdir]
-			set worktree [git -C $gitdir_parent rev-parse --show-toplevel]
-			set parent_gitdir [git -C $worktree rev-parse --absolute-git-dir]
-			if {$::_gitdir ne $parent_gitdir} {
-				set worktree {}
-			}
-		}]} {
+				set parent [file dirname $::_gitdir]
+				set worktree [git -C $parent rev-parse --show-toplevel]
+			}]} {
 			set worktree {}
 		}
 	} elseif [file exists {gitdir}] {
+		# a worktree gitdir has .gitdir naming worktree/.git
+		# assure git run there reports this dir as the gitdir (links might be broken)
 		if {[catch {
-			set fd_gitdir [open {gitdir} {r}]
-			set gitlink_parent [file dirname [read $fd_gitdir]]
-			catch {close $fd_gitdir}
-			set worktree [git -C $gitlink_parent rev-parse --show-toplevel]
-			set parent_gitdir [git -C $worktree rev-parse --absolute-git-dir]
-			if {$::_gitdir ne $parent_gitdir} {
-				set worktree {}
-			}
-		}]} {
+				set fd_gitdir [open {gitdir} {r}]
+				set worktree [file dirname [read $fd_gitdir]]
+				catch {close $fd_gitdir}
+				set worktree_gitdir [git -C $worktree rev-parse --absolute-git-dir]
+				if {$::_gitdir ne $worktree_gitdir} {
+					set worktree {}
+				}
+			}]} {
 			catch {close $fd_gitdir}
 			set worktree {}
 		}
@@ -1153,7 +1149,7 @@ proc is_gitvars_error {err} {
 	set GIT_DIR {}
 	set GIT_WORK_TREE {}
 	catch {set GIT_DIR $::env(GIT_DIR); set havevars 1}
-	catch {set GIT_WORK_TREE $::env(GIT_WORK_TREE) ; set havevars 1}
+	catch {set GIT_WORK_TREE $::env(GIT_WORK_TREE); set havevars 1}
 
 	if {$havevars} {
 		catch {wm withdraw .}
@@ -1168,9 +1164,7 @@ proc is_gitvars_error {err} {
 
 proc set_gitdir_vars {} {
 	global _gitdir _gitworktree env
-	if {$_gitdir ne {}} {
-		set env(GIT_DIR) $_gitdir
-	}
+	set env(GIT_DIR) $_gitdir
 	if {$_gitworktree ne {}} {
 		set env(GIT_WORK_TREE) $_gitworktree
 	}
@@ -1182,12 +1176,12 @@ proc unset_gitdir_vars {} {
 	catch {unset env(GIT_WORK_TREE)}
 }
 
-# find repository.
+# find repository
 set _gitdir {}
 if {[is_enabled gitdir_discovery]} {
 	if {[catch {
-		set _gitdir [git rev-parse --absolute-git-dir]
-	} err]} {
+			set _gitdir [git rev-parse --absolute-git-dir]
+		} err]} {
 		if {[is_gitvars_error $err]} {
 			exit 1
 		}
@@ -1200,14 +1194,13 @@ if {$_gitdir eq {} && [is_enabled picker]} {
 	unset_gitdir_vars
 	load_config 1
 	apply_config
-	if {![choose_repository::pick]} {
-		exit 1
-	}
+	choose_repository::pick
 	if {[catch {
-		set _gitdir [git rev-parse --absolute-git-dir]
-	} err]} {
+			set _gitdir [git rev-parse --absolute-git-dir]
+		} err]} {
 		catch {wm withdraw .}
-		error_popup [strcat [mc "Unusable repo/worktree:"] " [pwd] "\n\n$err"]
+		error_popup [strcat [mc "Unusable repo/worktree:"] " [pwd] \n\n$err"]
+		exit 1
 	}
 	set picked 1
 }
@@ -1220,9 +1213,9 @@ if {$_gitdir eq {}} {
 
 # find worktree, continue without if not required
 if {[catch {
-	set _gitworktree [git rev-parse --show-toplevel]
-	set _prefix [git rev-parse --show-prefix]
-} err]} {
+		set _gitworktree [git rev-parse --show-toplevel]
+		set _prefix [git rev-parse --show-prefix]
+	} err]} {
 	if {[is_gitvars_error $err]} {
 		exit 1
 	}
@@ -1238,17 +1231,14 @@ if {[is_bare]} {
 }
 
 if {![is_bare]} {
-	if {[catch {
-		cd $_gitworktree
-	} err]} {
+	if {[catch {cd $_gitworktree} err]} {
 		catch {wm withdraw .}
-		error_popup [strcat [mc "Cannot change to discovered worktree: "] \
-			"$_gitworktree" "\n\n$err"]
-		exit 1;
+		error_popup [strcat [mc "No working directory"] " $_gitworktree:\n\n$err"]
+		exit 1
 	}
 } elseif {![is_enabled bare]} {
 	catch {wm withdraw .}
-	error_popup [strcat [mc "Cannot use bare repository:"] "\n\n" $_gitdir]
+	error_popup [strcat [mc "Cannot use bare repository:"] "\n\n$_gitdir"]
 	exit 1
 }
 
@@ -2086,11 +2076,7 @@ proc do_gitk {revs {is_submodule false}} {
 	} else {
 		set pwd [pwd]
 
-		if {!$is_submodule} {
-			if {![is_bare]} {
-				cd $_gitworktree
-			}
-		} else {
+		if {$is_submodule} {
 			cd $current_diff_path
 			if {$revs eq {--}} {
 				set s $file_states($current_diff_path)
@@ -3026,26 +3012,18 @@ proc normalize_relpath {path} {
 	if {$elements ne {}} {
 		return [eval file join $elements]
 	} else {
-		return {./}
+		return {.}
 	}
 }
 
-proc find_path_type {head path} {
-	if {$path eq {./}} {
-		# the root-tree exists in every rev, ls-tree gives data on the contents,
-		# not the type of tree itself. So, if the rev exists, return {tree}
-		if {[catch {set objtype [git ls-tree $head]}]} {
-			set objtype {}
-		} else {
-			set objtype {tree}
-		}
+proc show_parse_err {err} {
+	if {[tk windowingsystem] eq "win32"} {
+		catch {wm withdraw .}
+		error_popup $err
 	} else {
-		# test that the path exists in head, ls-tree gives info on the path only
-		if {[catch {set objtype [git ls-tree {--format=%(objecttype)} $head $path]}]} {
-			set objtype {}
-		}
+		puts stderr $err
 	}
-	return $objtype
+	exit 1
 }
 
 # -- Not a normal commit type invocation?  Do that instead!
@@ -3054,91 +3032,80 @@ switch -- $subcommand {
 browser -
 blame {
 	if {$subcommand eq "blame"} {
-		set subcommand_args {[--line=<num>] <[rev] [--] filename | [--] filename rev>}
-		set required_objtype blob
+		set subcommand_args {[--line=<num>] [rev] [--] <filename>}
+		set required_pathtype blob
 	} else {
-		set subcommand_args {<[rev] [--] directory | [--] directory rev>}
-		set required_objtype tree
+		set subcommand_args {[rev] [--] <dirname>}
+		set required_pathtype tree
 	}
 	set maxargs [llength $subcommand_args]
 	set nargs [llength $argv]
 	if {$nargs < 1 || $nargs > $maxargs} usage
 	set head {}
-	set althead {}
 	set path {}
-	set altpath {}
-	set canswap 1
 	set jump_spec {}
 
-	# assume: [--line=num] [head] [--] path as the possible arguments, in order.
-	# head and path may need a swap later.
-	for {set iarg 0} {$iarg < $nargs} {incr iarg} {
-		set arg [lindex $argv $iarg]
-		if {$arg eq {--}} {
-			# next arg is the path, prevent or FORCE swap?
-			if {$iarg == $nargs - 2} {
-				set canswap 0
-			} elseif {$iarg == $nargs - 3} {
-				set canswap 2
-			} else {
+	set iarg 0
+	foreach a $argv {
+		incr iarg
+		if {$iarg == $nargs} {
+			# final argument is path
+			set path [normalize_relpath [file join $_prefix $a]]
+		} elseif {$a eq {--}} {
+			# allow before required final arg that must be path
+			if {$iarg != $nargs - 1} {
 				usage
 			}
-		} elseif {[regexp {^--line=(\d+)$} $arg arg lnum]} {
+		} elseif {[regexp {^--line=(\d+)$} $a a lnum]} {
 			# --line can only be the first arg
-			if {$iarg != 0 || $maxargs < 4} usage
+			if {$iarg != 1 || $subcommand ne {blame}} usage
 			set jump_spec [list $lnum]
-		} elseif {$iarg == $nargs - 1} {
-			# assume final argument is path
-			set path [normalize_relpath [file join $_prefix $arg]]
-			set althead $arg
 		} elseif {$head eq {}} {
-			# assume the other argument is head
-			set head $arg
-			set altpath [normalize_relpath [file join $_prefix $arg]]
+			set head $a
 		} else {
 			usage
 		}
 	}
 
-	# no swapping allowed if head not given, use current branch (HEAD)
+	# If head not given, use current branch (HEAD),
+	# and blame will use worktree if there is one.
+	set use_worktree 0
 	if {$head eq {}} {
 		load_current_branch
 		set head $current_branch
-		set canswap 0
-	}
-
-	# -- before "rev" arg means we got -- path head
-	if {$canswap == 2} {
-		set head $althead
-		set path $altpath
-		set canswap 0
-	}
-
-	set objtype [find_path_type $head $path]
-	if {$objtype eq {} && $canswap} {
-		set objtype [find_path_type $althead $altpath]
-		if {$objtype ne {}} {
-			set head $althead
-			set path $altpath
+		if {$subcommand eq {blame} && ![is_bare]} {
+			if {![file isfile $path]} {
+				show_parse_err [mc "fatal: no such file '%s' in worktree" $path]
+			}
+			set use_worktree 1
 		}
-	}
-	set current_branch $head
-
-	# check that path exists in head, and objtype matches need
-	if {$objtype ne $required_objtype} {
-		switch -- $required_objtype {
-			tree {set err [strcat \
-				[mc "'%s' is not a directory in rev '%s'" $path $head]]}
-			blob {set err [strcat \
-				[mc "'%s' is not a filename in rev '%s'" $path $head]]}
-		}
-		if {[tk windowingsystem] eq "win32"} {
-			catch {wm withdraw .}
-			error_popup $err
+	} else {
+		if {[catch {
+				set commitid \
+					[git rev-parse --verify --end-of-options \
+					[strcat $head "^{commit}"]]
+			}]} {
+			show_parse_err [mc "fatal: '%s' is not a valid rev'" $head]
 		} else {
-			puts stderr $err
+			set current_branch $head
+		}
+	}
+
+	# check path is known in head, and is file / directory as required
+	set pathtype {}
+	catch {set pathtype [git ls-tree {--format=%(objecttype)} $head $path]}
+	if {$pathtype ne {} && $path eq {.}} {
+		# ls-tree gives contents of root-dir, we need root-dir itself
+		set pathtype {tree}
+	}
+
+	if {$pathtype ne $required_pathtype} {
+		switch -- $required_pathtype {
+			tree {show_parse_err \
+				[mc "'%s' is not a directory in rev '%s'" $path $head]}
+			blob {show_parse_err \
+				[mc "'%s' is not a filename in rev '%s'" $path $head]}
 		}
-		exit 1
 	}
 
 	wm deiconify .
@@ -3146,8 +3113,8 @@ blame {
 	browser {
 		browser::new $head $path
 	}
-	blame {
-		blame::new $head $path $jump_spec
+	blame   {
+		blame::new [expr {$use_worktree ? {} : $head}] $path $jump_spec
 	}
 	}
 	return

-- 
2.54.0.99.14