Thread (122 messages) 122 messages, 4 authors, 6d ago
COOLING6d
Revisions (3)
  1. v1 [diff vs current]
  2. v2 current
  3. v3 [diff vs current]

[PATCH v2 04/11] git-gui: use rev-parse exclusively to find a repository

From: Mark Levedahl <hidden>
Date: 2026-05-20 20:24:36
Subsystem: the rest · Maintainer: Linus Torvalds

git-gui attempts to use env(GIT_DIR) directly as the git repository,
accepting GIT_DIR if it is a directory. Only if that fails is git
rev-parse used to discover the repository.  But, this avoids all of
git-core's validity checking on a repository, thus possibly deferring an
error to a later step, possibly unexpected. Repository validation should
be part of initial setup so that later processing does not need error
trapping for configuration errors.

Let's just invoke rev-parse so all error checking is done.

While here, let's cleanup the error handling.

Stop if an error occurs and the user set GIT_DIR or GIT_WORK_TREE.
Use of either or both of those variables is supported by git, but their
use also means the user has taken responsibility that they are correct,
so a failure is something the user must address.

Otherwise on error, continue the existing behavior and show the
repository picker. But, let's move the possible invocation of
repository_chooser::pick to a separate code block. This permits adding
separate conditions on using pick indepent of repository discovery, and
will be exploited later in the series.  Note that the picker always
returns with the current directory in the root of a worktree with the
git repository is in the .git subdirectory.  The variable "picked" is
used by git-gui to automatically execute the "Explore Working Copy" menu
item after the repository picker is run.  This is controlled by config
variable gui.autoexplore, and happens after all discovery is complete.

Remove a later check on whether _gitdir is a directory: that code
cannot be reached without rev-parse already validating the repository.

_prefix should not be set before worktree discovery: the prefix is only
known after the worktree is found, and at this point we have only
discovered the repository. This is true even when running the repository
picker: that option provides a list of prior selections, and does no
validation on the list beyond checking that the directories exist.  For
now, just initialize _prefix along with other global variables.

Signed-off-by: Mark Levedahl <redacted>
---
 git-gui.sh | 48 +++++++++++++++++++++++++++++++++---------------
 1 file changed, 33 insertions(+), 15 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index 233c975786..c61a6cbd8f 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -374,6 +374,7 @@ set _gitdir {}
 set _gitworktree {}
 set _isbare {}
 set _githtmldir {}
+set _prefix {}
 set _reponame {}
 set _shellpath {@@SHELL_PATH@@}
 
@@ -1122,6 +1123,24 @@ unset argv0dir
 ##
 ## repository setup
 
+proc is_gitvars_error {err} {
+	set havevars 0
+	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}
+
+	if {$havevars} {
+		catch {wm withdraw .}
+		error_popup [strcat [mc "Invalid configuration:"] \
+		   "\n" "GIT_DIR: " $GIT_DIR \
+		   "\n" "GIT_WORK_TREE: " $GIT_WORK_TREE \
+			"\n\n$err"]
+		return 1
+	}
+	return 0
+}
+
 proc set_gitdir_vars {} {
 	global _gitdir _gitworktree env
 	if {$_gitdir ne {}} {
@@ -1138,17 +1157,22 @@ proc unset_gitdir_vars {} {
 	catch {unset env(GIT_WORK_TREE)}
 }
 
-set picked 0
-if {[catch {
-		set _gitdir $env(GIT_DIR)
-		set _prefix {}
-		}]
-	&& [catch {
-		# beware that from the .git dir this sets _gitdir to .
-		# and _prefix to the empty string
+# find repository.
+set _gitdir {}
+if {$_gitdir eq {}} {
+	if {[catch {
 		set _gitdir [git rev-parse --absolute-git-dir]
-		set _prefix [git rev-parse --show-prefix]
 	} err]} {
+		if {[is_gitvars_error $err]} {
+			exit 1
+		}
+		set _gitdir {}
+	}
+}
+
+set picked 0
+if {$_gitdir eq {}} {
+	unset_gitdir_vars
 	load_config 1
 	apply_config
 	if {![choose_repository::pick]} {
@@ -1160,7 +1184,6 @@ if {[catch {
 		catch {wm withdraw .}
 		error_popup [strcat [mc "Unusable repo/worktree:"] " [pwd] "\n\n$err"]
 	}
-	set _prefix {}
 	set picked 1
 }
 
@@ -1175,11 +1198,6 @@ if {$hashalgorithm eq "sha1"} {
 	exit 1
 }
 
-if {![file isdirectory $_gitdir]} {
-	catch {wm withdraw .}
-	error_popup [strcat [mc "Git directory not found:"] "\n\n$_gitdir"]
-	exit 1
-}
 # _gitdir exists, so try loading the config
 load_config 0
 apply_config
-- 
2.54.0.99.14
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help