git-gui: use rev-parse exclusively to find a repository

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 independent 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 is set as part of worktree discovery, but must be {} if not
running with a worktree. Initialze this as {} along with other global
variables, this is the correct value is no worktree is found.

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
Signed-off-by: Johannes Sixt <j6t@kdbg.org>

diff --git a/git-gui.sh b/git-gui.sh
index 84385c09a25de9248adebe98e79f85f18b7f6112..a057b3f675189eab5c023d30d34168966d8de9fc 100755 (executable)
--- 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@@}
 
@@ -1125,6 +1126,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
        set env(GIT_DIR) $_gitdir
@@ -1139,16 +1158,22 @@ proc unset_gitdir_vars {} {
        catch {unset env(GIT_WORK_TREE)}
 }
 
+# find repository
+set _gitdir {}
+if {$_gitdir eq {}} {
+       if {[catch {
+                       set _gitdir [git rev-parse --absolute-git-dir]
+               } err]} {
+               if {[is_gitvars_error $err]} {
+                       exit 1
+               }
+               set _gitdir {}
+       }
+}
+
 set picked 0
-if {[catch {
-               set _gitdir $env(GIT_DIR)
-               set _prefix {}
-               }]
-       && [catch {
-               # beware that from the .git dir this sets _prefix to the empty string
-               set _gitdir [git rev-parse --absolute-git-dir]
-               set _prefix [git rev-parse --show-prefix]
-       } err]} {
+if {$_gitdir eq {}} {
+       unset_gitdir_vars
        load_config 1
        apply_config
        choose_repository::pick
@@ -1159,7 +1184,6 @@ if {[catch {
                error_popup [strcat [mc "Unusable repo/worktree:"] " [pwd] \n\n$err"]
                exit 1
        }
-       set _prefix {}
        set picked 1
 }
 
@@ -1174,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