[PATCH] git-gui: handle bare repo or missing worktree

[PATCH] git-gui: handle bare repo or missing worktree

[Shroom Moo]

When starting git-gui from a directory that Git recognizes as a valid
repository but the repository is either bare or its working tree is
missing, git-gui previously attempted to execute 'rev-parse
--show-toplevel' without error handling. This caused a fatal Tcl error
("this operation must be run in a work tree") and prevented the user
from opening the repository selection dialog.

Improve the repository setup logic:
- After obtaining the git directory via 'rev-parse --git-dir', check
  whether the repository is bare or if the working tree can be
  successfully located.
- If the repository is unusable as a working tree, display a warning
  and present the "Create/Clone/Open" repository selection dialog.

This makes git-gui robust when launched from a bare repository, from
inside a .git directory without a worktree, or when GIT_DIR points to
an invalid location. No regressions observed in normal working trees.

Signed-off-by: Shroom Moo <egg_mushroomcow@foxmail.com>
---
 git-gui/git-gui.sh | 75 +++++++++++++++++++++++++++++++++++-----------
 1 file changed, 57 insertions(+), 18 deletions(-)

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index 23fe76e498..2e4bc2f226 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -1123,24 +1123,63 @@ unset argv0dir
 ## repository setup
 
 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
-		set _gitdir [git rev-parse --git-dir]
-		set _prefix [git rev-parse --show-prefix]
-	} err]} {
-	load_config 1
-	apply_config
-	choose_repository::pick
-	if {![file isdirectory $_gitdir]} {
-		exit 1
-	}
-	set picked 1
-}
+# Save temporarily for restoration later
+set _startup_cwd [pwd]
+
+set need_pick 0
+if {[catch {set _gitdir $env(GIT_DIR); set _prefix {}}]} {
+    # GIT_DIR not set in environment, attempt auto-detection
+	# beware that from the .git dir this sets _gitdir to .
+	# and _prefix to the empty string
+    if {[catch {set _gitdir [git rev-parse --git-dir]; set _prefix [git rev-parse --show-prefix]} err]} {
+        # Not inside any Git repository, proceed to repository selection UI
+        set need_pick 1
+    } else {
+        # Inside a Git repository, but need to verify its usability
+        cd [file dirname $_gitdir]
+        set bare 0
+        set worktree_missing 0
+		# Check if this is a bare repository (no working tree)
+        if {![catch {set bare [git rev-parse --is-bare-repository]}]} {
+            if {$bare eq "true"} {
+                set bare 1
+            }
+        }
+		# Check if working tree is present and accessible
+        if {[catch {git rev-parse --show-toplevel}]} {
+            set worktree_missing 1
+        }
+
+        cd $_startup_cwd
+
+        # For bare repos or missing worktrees, warn and guide to selection
+        if {$bare || $worktree_missing} {
+            set msg [mc "The repository at '%s' cannot be opened:" [file normalize $_gitdir]]
+            if {$bare} {
+                append msg "\n\n" [mc "It is a bare repository (no working tree)."]
+            } else {
+                append msg "\n\n" [mc "The working tree appears to be missing or inaccessible."]
+            }
+            append msg "\n\n" [mc "Please select a valid working repository, or create/clone one."]
+            tk_messageBox -icon warning -type ok -title [mc "Repository Unusable"] -message $msg
+
+            set need_pick 1
+        }
+    }
+}
+
+if {$need_pick} {
+    load_config 1
+    apply_config
+    choose_repository::pick
+    if {![file isdirectory $_gitdir]} {
+        exit 1
+    }
+    set picked 1
+}
+
+# Clean up working tree checking temporary variables
+unset -nocomplain _startup_cwd need_pick bare worktree_missing
 
 # Use object format as hash algorithm (either "sha1" or "sha256")
 set hashalgorithm [git rev-parse --show-object-format]
-- 
2.52.0.windows.1

Re: [PATCH] git-gui: handle bare repo or missing worktree

[Johannes Sixt]

Am 21.04.26 um 18:28 schrieb Shroom Moo:
> When starting git-gui from a directory that Git recognizes as a valid
> repository but the repository is either bare or its working tree is
> missing, git-gui previously attempted to execute 'rev-parse
> --show-toplevel' without error handling. This caused a fatal Tcl error
> ("this operation must be run in a work tree") and prevented the user
> from opening the repository selection dialog.
> 
> Improve the repository setup logic:
> - After obtaining the git directory via 'rev-parse --git-dir', check
>   whether the repository is bare or if the working tree can be
>   successfully located.
> - If the repository is unusable as a working tree, display a warning
>   and present the "Create/Clone/Open" repository selection dialog.

I consider it harmful to allow the user to create (or clone) a
repository after Git GUI has already determined the existence of a
repository. If the worktree was not found, we should report an error and
exit after the message is dismissed, and not give the option to create a
repository.

> 
> This makes git-gui robust when launched from a bare repository, from
> inside a .git directory without a worktree, or when GIT_DIR points to
> an invalid location. No regressions observed in normal working trees.
> 
> Signed-off-by: Shroom Moo <egg_mushroomcow@foxmail.com>
> ---
>  git-gui/git-gui.sh | 75 +++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 57 insertions(+), 18 deletions(-)
> 
> diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
> index 23fe76e498..2e4bc2f226 100755
> --- a/git-gui/git-gui.sh
> +++ b/git-gui/git-gui.sh
> @@ -1123,24 +1123,63 @@ unset argv0dir
>  ## repository setup
>  
>  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
> -		set _gitdir [git rev-parse --git-dir]
> -		set _prefix [git rev-parse --show-prefix]
> -	} err]} {
> -	load_config 1
> -	apply_config
> -	choose_repository::pick
> -	if {![file isdirectory $_gitdir]} {
> -		exit 1
> -	}
> -	set picked 1
> -}
> +# Save temporarily for restoration later
> +set _startup_cwd [pwd]
> +
> +set need_pick 0
> +if {[catch {set _gitdir $env(GIT_DIR); set _prefix {}}]} {
> +    # GIT_DIR not set in environment, attempt auto-detection
> +	# beware that from the .git dir this sets _gitdir to .
> +	# and _prefix to the empty string
> +    if {[catch {set _gitdir [git rev-parse --git-dir]; set _prefix [git rev-parse --show-prefix]} err]} {
> +        # Not inside any Git repository, proceed to repository selection UI
> +        set need_pick 1
> +    } else {
> +        # Inside a Git repository, but need to verify its usability
> +        cd [file dirname $_gitdir]
> +        set bare 0
> +        set worktree_missing 0
> +		# Check if this is a bare repository (no working tree)
> +        if {![catch {set bare [git rev-parse --is-bare-repository]}]} {
> +            if {$bare eq "true"} {
> +                set bare 1
> +            }
> +        }
> +		# Check if working tree is present and accessible
> +        if {[catch {git rev-parse --show-toplevel}]} {
> +            set worktree_missing 1
> +        }
> +
> +        cd $_startup_cwd
> +
> +        # For bare repos or missing worktrees, warn and guide to selection
> +        if {$bare || $worktree_missing} {
> +            set msg [mc "The repository at '%s' cannot be opened:" [file normalize $_gitdir]]
> +            if {$bare} {
> +                append msg "\n\n" [mc "It is a bare repository (no working tree)."]
> +            } else {
> +                append msg "\n\n" [mc "The working tree appears to be missing or inaccessible."]
> +            }
> +            append msg "\n\n" [mc "Please select a valid working repository, or create/clone one."]
> +            tk_messageBox -icon warning -type ok -title [mc "Repository Unusable"] -message $msg
> +
> +            set need_pick 1
> +        }
> +    }
> +}
> +
> +if {$need_pick} {
> +    load_config 1
> +    apply_config
> +    choose_repository::pick
> +    if {![file isdirectory $_gitdir]} {
> +        exit 1
> +    }
> +    set picked 1
> +}
> +
> +# Clean up working tree checking temporary variables
> +unset -nocomplain _startup_cwd need_pick bare worktree_missing

This adds quite a lot of code with failure cases, but after this point
we already have some quite extensive error diagnosis, too. Except that
the call to git rev-parse --show-toplevel is not protected.

If we do not invoke the Create/Clone/Open dialog after a repository has
already been discovered, would it not be sufficient to just add the
protection around the --show-toplevel call? Or is there something else
missing in the existing error paths?

-- Hannes

[PATCH v2 1/1] git-gui: protect rev-parse --show-toplevel call

[Shroom Moo]

When starting git-gui from a directory that is a bare repository or
where the working tree is missing, git-gui previously executed
'rev-parse --show-toplevel' without error handling. This caused a
fatal Tcl error ("this operation must be run in a work tree").

Wrap the call in a catch to prevent the fatal error. The existing
error paths after this call already handle bare repos and missing
worktrees appropriately.

Signed-off-by: Shroom Moo <egg_mushroomcow@foxmail.com>
---
 git-gui/git-gui.sh | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index 23fe76e498..aee37685e1 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -1169,7 +1169,9 @@ if {![file isdirectory $_gitdir]} {
 load_config 0
 apply_config
 
-set _gitworktree [git rev-parse --show-toplevel]
+if {[catch {set _gitworktree [git rev-parse --show-toplevel]}]} {
+    set _gitworktree {}
+}
 
 if {$_prefix ne {}} {
 	if {$_gitworktree eq {}} {
-- 
2.52.0.windows.1

Re: [PATCH v2 1/1] git-gui: protect rev-parse --show-toplevel call

[Mark Levedahl]

On 4/29/26 1:32 PM, Shroom Moo wrote:
> When starting git-gui from a directory that is a bare repository or
> where the working tree is missing, git-gui previously executed
> 'rev-parse --show-toplevel' without error handling. This caused a
> fatal Tcl error ("this operation must be run in a work tree").
> 
> Wrap the call in a catch to prevent the fatal error. The existing
> error paths after this call already handle bare repos and missing
> worktrees appropriately.
> 
> Signed-off-by: Shroom Moo <egg_mushroomcow@foxmail.com>
> ---
>  git-gui/git-gui.sh | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
> index 23fe76e498..aee37685e1 100755
> --- a/git-gui/git-gui.sh
> +++ b/git-gui/git-gui.sh
> @@ -1169,7 +1169,9 @@ if {![file isdirectory $_gitdir]} {
>  load_config 0
>  apply_config
>  
> -set _gitworktree [git rev-parse --show-toplevel]
> +if {[catch {set _gitworktree [git rev-parse --show-toplevel]}]} {
> +    set _gitworktree {}
> +}
>  
>  if {$_prefix ne {}} {
>  	if {$_gitworktree eq {}} {

Unfortunately, this allows starting git-gui inside the separate gitdir created by

	git clone --separate-git-dir=/some/where/else ...

There is no hint where the workdir is, but git recognizes the repository is not bare:

	git rev-parse --is-bare-repository  ==> false
	git rev-parse --is-inside-git-dir   ==> true
	git rev-parse --is-inside-work-tree ==> false
	git rev-parse --show-toplevel       ==> fatal: must be run in a work tree
	git worktree list                   ==> absolute path to gitdir (not a worktree)

As git-gui has no protection against modifying what is effectively a bare repository,
allowing git-gui to run in this directory is dangerous, or possibly just very confusing.

git refuses to work in this gitdir: "git status" run in the above gitdir gives: "fatal:
this operation must be run in a work tree."

The simplest safe thing is to catch the error and abort with a more useful message than
currently provided. Or perhaps, check git rev-parse --is-inside-git-dir and abort, and do
so before trying --show-toplevel.


Mark

[PATCH v3 1/1] git-gui: handle missing worktree and separated gitdir

[Shroom Moo]

When git-gui is started from a directory that Git recognizes as a
valid repository but the working tree is not accessible (e.g., a
separated gitdir created by `git clone --separate-git-dir`, a bare
repository, or a case where the worktree directory was removed),
it previously called `rev-parse --show-toplevel` without error
handling, causing a fatal Tcl error ("this operation must be run
in a work tree").

Wrap the call in a `catch` and handle the failure as follows:

- For bare repositories, keep `_gitworktree` empty so that the
  existing `is_bare` check shows "Cannot use bare repository" and
  exits.  No behavioral change.

- For non‑bare repositories, try to locate the worktree from the
  parent directory using `git -C $parent rev-parse --show-toplevel`.
  If the parent is a valid worktree, change to it; this covers the
  legitimate case of starting git-gui from within the .git
  subdirectory of a normal working tree.

- If the parent directory is not a worktree, refuse to start with
  a clear error message.  This prevents dangerous operations in a
  separated gitdir, where ordinary Git commands like `git status`
  would themselves refuse to run.

The approach intentionally avoids two pitfalls:

  - Testing `--is-inside-git-dir` before calling `--show-toplevel`
    would break the normal use case of starting git-gui from within
    a .git subdirectory (where --show-toplevel would succeed).

  - A simple “non‑bare” check after a failed --show-toplevel would
    reject a normal repository whose worktree was only temporarily
    removed.

The chosen method keeps the original behavior for bare repositories
and for regular working trees, fixes the crash, and properly blocks
separated gitdirs without a reachable worktree.

Signed-off-by: Shroom Moo <egg_mushroomcow@foxmail.com>
---
 git-gui/git-gui.sh | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index 23fe76e498..2392282df3 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -1169,7 +1169,28 @@ if {![file isdirectory $_gitdir]} {
 load_config 0
 apply_config
 
-set _gitworktree [git rev-parse --show-toplevel]
+if {[catch {set _gitworktree [git rev-parse --show-toplevel]}]} {
+	# For bare repositories, use the existing error handling
+	if {![catch {set bare [git rev-parse --is-bare-repository]}] && $bare eq {true}} {
+		set _gitworktree {}
+	} else {
+		# Non-bare: try to find the worktree from the parent directory
+		set parent [file dirname [pwd]]
+		# Cannot go higher than the root directory; leave _gitworktree empty
+		if {[file normalize $parent] eq [file normalize [pwd]]} {
+			# Already at the filesystem root; let existing paths cope
+			set _gitworktree {}
+		} elseif {![catch {
+			set _gitworktree [git -C $parent rev-parse --show-toplevel]
+		}]} {
+			cd $parent
+		} else {
+			catch {wm withdraw .}
+			error_popup [mc "Cannot start git-gui from inside the Git directory."]
+			exit 1
+		}
+	}
+}
 
 if {$_prefix ne {}} {
 	if {$_gitworktree eq {}} {
-- 
2.52.0.windows.1

Re: [PATCH v3 1/1] git-gui: handle missing worktree and separated gitdir

[Mark Levedahl]

On 4/30/26 6:02 AM, Shroom Moo wrote:
> When git-gui is started from a directory that Git recognizes as a
> valid repository but the working tree is not accessible (e.g., a
> separated gitdir created by `git clone --separate-git-dir`, a bare
> repository, or a case where the worktree directory was removed),
> it previously called `rev-parse --show-toplevel` without error
> handling, causing a fatal Tcl error ("this operation must be run
> in a work tree").
>
> Wrap the call in a `catch` and handle the failure as follows:
>
> - For bare repositories, keep `_gitworktree` empty so that the
>   existing `is_bare` check shows "Cannot use bare repository" and
>   exits.  No behavioral change.
>
> - For non‑bare repositories, try to locate the worktree from the
>   parent directory using `git -C $parent rev-parse --show-toplevel`.
>   If the parent is a valid worktree, change to it; this covers the
>   legitimate case of starting git-gui from within the .git
>   subdirectory of a normal working tree.
>
> - If the parent directory is not a worktree, refuse to start with
>   a clear error message.  This prevents dangerous operations in a
>   separated gitdir, where ordinary Git commands like `git status`
>   would themselves refuse to run.
>
> The approach intentionally avoids two pitfalls:
>
>   - Testing `--is-inside-git-dir` before calling `--show-toplevel`
>     would break the normal use case of starting git-gui from within
>     a .git subdirectory (where --show-toplevel would succeed).
>
>   - A simple “non‑bare” check after a failed --show-toplevel would
>     reject a normal repository whose worktree was only temporarily
>     removed.
>
> The chosen method keeps the original behavior for bare repositories
> and for regular working trees, fixes the crash, and properly blocks
> separated gitdirs without a reachable worktree.
>
> Signed-off-by: Shroom Moo <egg_mushroomcow@foxmail.com>
> ---
>  git-gui/git-gui.sh | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
> index 23fe76e498..2392282df3 100755
> --- a/git-gui/git-gui.sh
> +++ b/git-gui/git-gui.sh
> @@ -1169,7 +1169,28 @@ if {![file isdirectory $_gitdir]} {
>  load_config 0
>  apply_config
>  
> -set _gitworktree [git rev-parse --show-toplevel]
> +if {[catch {set _gitworktree [git rev-parse --show-toplevel]}]} {
> +	# For bare repositories, use the existing error handling
> +	if {![catch {set bare [git rev-parse --is-bare-repository]}] && $bare eq {true}} {
> +		set _gitworktree {}
> +	} else {
> +		# Non-bare: try to find the worktree from the parent directory
> +		set parent [file dirname [pwd]]
> +		# Cannot go higher than the root directory; leave _gitworktree empty
> +		if {[file normalize $parent] eq [file normalize [pwd]]} {
> +			# Already at the filesystem root; let existing paths cope
> +			set _gitworktree {}
> +		} elseif {![catch {
> +			set _gitworktree [git -C $parent rev-parse --show-toplevel]
> +		}]} {
> +			cd $parent
> +		} else {
> +			catch {wm withdraw .}
> +			error_popup [mc "Cannot start git-gui from inside the Git directory."]
> +			exit 1
> +		}
> +	}
> +}
>  
>  if {$_prefix ne {}} {
>  	if {$_gitworktree eq {}} {

A bare repository can be contained in a workdir / worktree pointing at a different gitdir:
the logic above can thus a workdir that doesn't use the gitdir where git-gui was started.
The bigger issue is that a gitdir can support multiple checked-out directories with no
one-to-one mapping and no clear idea of which of those a user may have intended.

So, I believe the correct fix is to test "rev-parse --is-inside-git-dir, and if so throw a
clear error message and exit. This will give the user something to start with to solve the
problem of why they started git-gui in a gitdir, and not in a worktree.

Mark

Re: [PATCH v3 1/1] git-gui: handle missing worktree and separated

[Shroom Moo]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1255 bytes --]

Hi Mark,

Thanks for catching the multi‑worktree ambiguity. The parent‑directory
fallback can surely accidentally pick the wrong worktree. As a 
reminder, the current code deliberately supports starting git-gui from 
within a regular repository's .git directory.

The comment says:
# beware that from the .git dir this sets _gitdir to .
# and _prefix to the empty string

In that case, _gitdir is ".", _prefix is empty, and the later logic
falls back to using [file dirname $_gitdir] as the worktree. A blanket
"if --is-inside-git-dir then exit" would make that case useless.

I'll send a v4 that first checks --is-bare-repository (preserving the
original bare‑repo error), then checks --is-inside-git-dir and refuses
if inside a gitdir. This accepts the .git‑startup limitation in
exchange for safety, and keeps the bare‑repo message unchanged.

Two alternatives still exist if a different trade‑off is preferred: 
- Only check --is-inside-git-dir (simpler, but makes the
bare‑repo error_popup useless).
- After --is-inside-git-dir, consult git worktree list and
switch to the single worktree if unambiguous (keeps .git‑startup
but adds complexity and a runtime dependency).

Shroom

Re: [PATCH v3 1/1] git-gui: handle missing worktree and separated gitdir

[Johannes Sixt]

Am 30.04.26 um 18:18 schrieb Mark Levedahl:
> 
> 
> On 4/30/26 6:02 AM, Shroom Moo wrote:
>> When git-gui is started from a directory that Git recognizes as a
>> valid repository but the working tree is not accessible (e.g., a
>> separated gitdir created by `git clone --separate-git-dir`, a bare
>> repository, or a case where the worktree directory was removed),
>> it previously called `rev-parse --show-toplevel` without error
>> handling, causing a fatal Tcl error ("this operation must be run
>> in a work tree").
>>
>> Wrap the call in a `catch` and handle the failure as follows:
>>
>> - For bare repositories, keep `_gitworktree` empty so that the
>>   existing `is_bare` check shows "Cannot use bare repository" and
>>   exits.  No behavioral change.
>>
>> - For non‑bare repositories, try to locate the worktree from the
>>   parent directory using `git -C $parent rev-parse --show-toplevel`.
>>   If the parent is a valid worktree, change to it; this covers the
>>   legitimate case of starting git-gui from within the .git
>>   subdirectory of a normal working tree.
>>
>> - If the parent directory is not a worktree, refuse to start with
>>   a clear error message.  This prevents dangerous operations in a
>>   separated gitdir, where ordinary Git commands like `git status`
>>   would themselves refuse to run.
>>
>> The approach intentionally avoids two pitfalls:
>>
>>   - Testing `--is-inside-git-dir` before calling `--show-toplevel`
>>     would break the normal use case of starting git-gui from within
>>     a .git subdirectory (where --show-toplevel would succeed).
>>
>>   - A simple “non‑bare” check after a failed --show-toplevel would
>>     reject a normal repository whose worktree was only temporarily
>>     removed.
>>
>> The chosen method keeps the original behavior for bare repositories
>> and for regular working trees, fixes the crash, and properly blocks
>> separated gitdirs without a reachable worktree.
>>
>> Signed-off-by: Shroom Moo <egg_mushroomcow@foxmail.com>
>> ---
>>  git-gui/git-gui.sh | 23 ++++++++++++++++++++++-
>>  1 file changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
>> index 23fe76e498..2392282df3 100755
>> --- a/git-gui/git-gui.sh
>> +++ b/git-gui/git-gui.sh
>> @@ -1169,7 +1169,28 @@ if {![file isdirectory $_gitdir]} {
>>  load_config 0
>>  apply_config
>>  
>> -set _gitworktree [git rev-parse --show-toplevel]
>> +if {[catch {set _gitworktree [git rev-parse --show-toplevel]}]} {
>> +	# For bare repositories, use the existing error handling
>> +	if {![catch {set bare [git rev-parse --is-bare-repository]}] && $bare eq {true}} {
>> +		set _gitworktree {}
>> +	} else {
>> +		# Non-bare: try to find the worktree from the parent directory
>> +		set parent [file dirname [pwd]]
>> +		# Cannot go higher than the root directory; leave _gitworktree empty
>> +		if {[file normalize $parent] eq [file normalize [pwd]]} {
>> +			# Already at the filesystem root; let existing paths cope
>> +			set _gitworktree {}
>> +		} elseif {![catch {
>> +			set _gitworktree [git -C $parent rev-parse --show-toplevel]
>> +		}]} {
>> +			cd $parent
>> +		} else {
>> +			catch {wm withdraw .}
>> +			error_popup [mc "Cannot start git-gui from inside the Git directory."]
>> +			exit 1
>> +		}
>> +	}
>> +}
>>  
>>  if {$_prefix ne {}} {
>>  	if {$_gitworktree eq {}} {
> 
> A bare repository can be contained in a workdir / worktree pointing at a different gitdir:
> the logic above can thus a workdir that doesn't use the gitdir where git-gui was started.
> The bigger issue is that a gitdir can support multiple checked-out directories with no
> one-to-one mapping and no clear idea of which of those a user may have intended.
> 
> So, I believe the correct fix is to test "rev-parse --is-inside-git-dir, and if so throw a
> clear error message and exit. This will give the user something to start with to solve the
> problem of why they started git-gui in a gitdir, and not in a worktree.
We have quite a bit of code that attempts to make Git GUI work from the
.git directory and also in bare repositories.

87cd09f43e56 ("git-gui: work from the .git dir", 2010-01-23) made the
first step. The original code just used the $_gitdir as the working
directory. However, at that time we did not have alternate worktrees,
and the old code, when used today, does not work in a `git
worktree`-created worktree. Later, the `git rev-parse --show-toplevel`
call came with 38ec8d3e2652 ("git-gui: correct assignment of work-tree",
2010-10-20). However, it also changes the fall-back code slightly, so
that running Git GUI from the .git directory would not work the same way
as before and takes the .git directory as the work tree (because in the
.git directory --show-cdup is not "..", but empty).

I think we need to restructure the existing flow a bit and not just fix
a single spot in the code. I suggest this order of operation:

1. Handle the bare repository case. If not enabled, fail. Otherwise, we
can work with an empty $_gitworktree.

2. Collect --show-toplevel into $_gitworktree.

2a. If this failed: If --is-inside-git-dir is true, and the last
$_gitdir directory component is exactly ".git", take the parent
repository as $_gitworktree. Otherwise, fail.

3. Handle all the other edge cases, if any, with the so determined
$_gitworktree. (I didn't think through, yet, what needs to be done.)

-- Hannes

Re: [PATCH v3 1/1] git-gui: handle missing worktree and separated gitdir

[Mark Levedahl]

On 5/1/26 9:13 AM, Johannes Sixt wrote:
> Am 30.04.26 um 18:18 schrieb Mark Levedahl:
>>
>> On 4/30/26 6:02 AM, Shroom Moo wrote:
>> We have quite a bit of code that attempts to make Git GUI work from the
> .git directory and also in bare repositories.
>
> 87cd09f43e56 ("git-gui: work from the .git dir", 2010-01-23) made the
> first step. The original code just used the $_gitdir as the working
> directory. However, at that time we did not have alternate worktrees,
> and the old code, when used today, does not work in a `git
> worktree`-created worktree. Later, the `git rev-parse --show-toplevel`
> call came with 38ec8d3e2652 ("git-gui: correct assignment of work-tree",
> 2010-10-20). However, it also changes the fall-back code slightly, so
> that running Git GUI from the .git directory would not work the same way
> as before and takes the .git directory as the work tree (because in the
> .git directory --show-cdup is not "..", but empty).
>
> I think we need to restructure the existing flow a bit and not just fix
> a single spot in the code. I suggest this order of operation:
>
> 1. Handle the bare repository case. If not enabled, fail. Otherwise, we
> can work with an empty $_gitworktree.
>
> 2. Collect --show-toplevel into $_gitworktree.
>
> 2a. If this failed: If --is-inside-git-dir is true, and the last
> $_gitdir directory component is exactly ".git", take the parent
> repository as $_gitworktree. Otherwise, fail.
>
> 3. Handle all the other edge cases, if any, with the so determined
> $_gitworktree. (I didn't think through, yet, what needs to be done.)
>
> -- Hannes
>

I found one horrid edge case: 

Start git-gui in a gitdir not embedded in a worktree, with core.bare=false as there are
one or more gitfile and/or symlinked worktrees elsewhere.
- current git-gui aborts with an uncaught error. Good.
- git-gui with the wrapped --show-toplevel call finds no worktree to switch to, so runs in
the gitdir allowing commits of the gitdir items.
-  I just added and  committed the *file* refs/heads/master to branch master in such a gitdir.

git-gui's normal gui must be started ONLY if rev-parse --is-inside-work-tree is true. (The
blame view invoked by gitk in theory could be allowed to run in a bare repository
read-only mode.).

For read/write mode:
if --is-inside-git-dir == true at startup, we must abort, or find a valid worktree and
switch to that.

My personal preference is for git-gui to abort:
   I started git-gui where it cannot run. 
   My error. Let me learn and fix that.

Alternatively, ask me what to do:
    e.g., prepare a dialog after looking at git-worktree list, and the parent dir IFF this
dir is named .git, telling me of my mistake and offering me one or more worktrees to
switch to.

But please, don't just switch to another directory without asking. This is just
encouraging me to make careless errors.

Mark

Re: [PATCH v3 1/1] git-gui: handle missing worktree and separated gitdir

[Mark Levedahl]

On 5/1/26 12:42 PM, Mark Levedahl wrote:
> On 5/1/26 9:13 AM, Johannes Sixt wrote:
>> Am 30.04.26 um 18:18 schrieb Mark Levedahl:
>>> On 4/30/26 6:02 AM, Shroom Moo wrote:
>>> We have quite a bit of code that attempts to make Git GUI work from the
>> .git directory and also in bare repositories.
>>
>> 87cd09f43e56 ("git-gui: work from the .git dir", 2010-01-23) made the
>> first step. The original code just used the $_gitdir as the working
>> directory. However, at that time we did not have alternate worktrees,
>> and the old code, when used today, does not work in a `git
>> worktree`-created worktree. Later, the `git rev-parse --show-toplevel`
>> call came with 38ec8d3e2652 ("git-gui: correct assignment of work-tree",
>> 2010-10-20). However, it also changes the fall-back code slightly, so
>> that running Git GUI from the .git directory would not work the same way
>> as before and takes the .git directory as the work tree (because in the
>> .git directory --show-cdup is not "..", but empty).
>>
>> I think we need to restructure the existing flow a bit and not just fix
>> a single spot in the code. I suggest this order of operation:
>>
>> 1. Handle the bare repository case. If not enabled, fail. Otherwise, we
>> can work with an empty $_gitworktree.
>>
>> 2. Collect --show-toplevel into $_gitworktree.
>>
>> 2a. If this failed: If --is-inside-git-dir is true, and the last
>> $_gitdir directory component is exactly ".git", take the parent
>> repository as $_gitworktree. Otherwise, fail.
>>
>> 3. Handle all the other edge cases, if any, with the so determined
>> $_gitworktree. (I didn't think through, yet, what needs to be done.)
>>
>> -- Hannes
>>
> I found one horrid edge case: 
>
> Start git-gui in a gitdir not embedded in a worktree, with core.bare=false as there are
> one or more gitfile and/or symlinked worktrees elsewhere.
> - current git-gui aborts with an uncaught error. Good.
> - git-gui with the wrapped --show-toplevel call finds no worktree to switch to, so runs in
> the gitdir allowing commits of the gitdir items.
> -  I just added and  committed the *file* refs/heads/master to branch master in such a gitdir.
>
> git-gui's normal gui must be started ONLY if rev-parse --is-inside-work-tree is true. (The
> blame view invoked by gitk in theory could be allowed to run in a bare repository
> read-only mode.).
>
> For read/write mode:
> if --is-inside-git-dir == true at startup, we must abort, or find a valid worktree and
> switch to that.
>
> My personal preference is for git-gui to abort:
>    I started git-gui where it cannot run. 
>    My error. Let me learn and fix that.
>
> Alternatively, ask me what to do:
>     e.g., prepare a dialog after looking at git-worktree list, and the parent dir IFF this
> dir is named .git, telling me of my mistake and offering me one or more worktrees to
> switch to.
>
> But please, don't just switch to another directory without asking. This is just
> encouraging me to make careless errors.
>
> Mark
>
I dug a bit more into the startup logic, and I think I better understand rework that is
needed.

Two basic problems I see here are beyond the question of if (and when) git-gui should try
to locate a worktree:

- git gui blame in a gitdir was apparently broken by the git repo commit 2d92ab32fd
("rev-parse: make --show-toplevel without a worktree an error", 2019-11-19). Prior to that
commit, git gui would stay in the startup directory enabling only features that cannot
modify the repository, and gitk could bring this view up in a gitdir. This doesn't work
right now.

- git-gui's logic includes a conceptual error embodied in proc is_bare: is_bare uses $(git
rev-parse --is-bare-repository) but what we need is $(git rev-parse --is-inside-git-dir),
and these are not synonyms.
It does not matter whether a worktree exists that points at the gitdir, and as discussed
before, main worktrees can easily exist that we cannot locate from the gitdir. At best,
is_bare is a guess.
So, is_bare should be replaced by is_inside_gitdir, and we should also have
is_inside_worktree. These are mutually exclusive, though both can be false.

My current idea of an improved startup flow enables features only at the end:

1) If not in a gitdir or worktree, 
    1a)   if git gui's subcommand is not 'gui', abort with an error (citool, browser, or
blame invocations carry information specific to a gitdir/worktree). 
    1b)   otherwise, invoke repository_chooser, which either aborts, or changes directory
to a worktree.

-- we are now in a gitdir or a worktree, and this may or may not be the startup directory.

2) Look at the combination of git gui subcommand and directory type (worktree / gitdir) to
decide to continue.
    2a) blame / browser are ok in either directory type.
    2b) citool requires a specific worktree, which should have been the initial startup
directory. Abort if not.
    2b) gui requires a worktree. Abort if not (my recommendation), or offer to find (or
automatically find) a worktree.

3) Change directory to the top level of of the directory_type (git rev-parse knows
toplevel of a worktree, different code is needed for a gitdir).

4) Enable features based upon subcommand and directory type.

There are 12 combinations of initial directory type (gitdir, worktree, neither) and
subcommand (gui, blame, browser, citool) to consider, with a lot of duplicated code
amongst the 12 cases. So, obviously, steps 1 and 2 can be convolved in many ways that are
different than what I wrote above.

Mark

Re: [PATCH v3 1/1] git-gui: handle missing worktree and separated gitdir

[Johannes Sixt]

Am 02.05.26 um 23:51 schrieb Mark Levedahl:
> Two basic problems I see here are beyond the question of if (and when) git-gui should try
> to locate a worktree:
> 
> - git gui blame in a gitdir was apparently broken by the git repo commit 2d92ab32fd
> ("rev-parse: make --show-toplevel without a worktree an error", 2019-11-19). Prior to that
> commit, git gui would stay in the startup directory enabling only features that cannot
> modify the repository, and gitk could bring this view up in a gitdir. This doesn't work
> right now.

True, `blame` used to work in bare repositories, but is broken now.

> - git-gui's logic includes a conceptual error embodied in proc is_bare: is_bare uses $(git
> rev-parse --is-bare-repository) but what we need is $(git rev-parse --is-inside-git-dir),
> and these are not synonyms.
> It does not matter whether a worktree exists that points at the gitdir, and as discussed
> before, main worktrees can easily exist that we cannot locate from the gitdir. At best,
> is_bare is a guess.
> So, is_bare should be replaced by is_inside_gitdir, and we should also have
> is_inside_worktree. These are mutually exclusive, though both can be false.

I would not call the use of --is-bar-repository instead of
--is-inside-git-dir an error, just a choice that has been made. In
particular, when the startup directory is named '.git' and is not marked
as bare, then its parent directory can very reasonably be taken as its
worktree. (That's how things worked before --show-toplevel was used.) If
the check is for --is-inside-git-dir, this treatment would be ruled out
early.


> My current idea of an improved startup flow enables features only at the end:
> 
> 1) If not in a gitdir or worktree, 
>     1a)   if git gui's subcommand is not 'gui', abort with an error (citool, browser, or
> blame invocations carry information specific to a gitdir/worktree). 
>     1b)   otherwise, invoke repository_chooser, which either aborts, or changes directory
> to a worktree.
> 
> -- we are now in a gitdir or a worktree, and this may or may not be the startup directory.
> 
> 2) Look at the combination of git gui subcommand and directory type (worktree / gitdir) to
> decide to continue.
>     2a) blame / browser are ok in either directory type.
>     2b) citool requires a specific worktree, which should have been the initial startup
> directory. Abort if not.
>     2b) gui requires a worktree. Abort if not (my recommendation), or offer to find (or
> automatically find) a worktree.
> 
> 3) Change directory to the top level of of the directory_type (git rev-parse knows
> toplevel of a worktree, different code is needed for a gitdir).
> 
> 4) Enable features based upon subcommand and directory type.
> 
> There are 12 combinations of initial directory type (gitdir, worktree, neither) and
> subcommand (gui, blame, browser, citool) to consider, with a lot of duplicated code
> amongst the 12 cases. So, obviously, steps 1 and 2 can be convolved in many ways that are
> different than what I wrote above.
So true.

But perhaps there is a simpler solution: Let's present an error if
--show-toplevel fails except in the case where the startup directory is
named '.git' (and is a valid Git repository) and is not bare (then the
worktree is the parent). I insist in this exception, because this
use-case was considered important in the past (87cd09f43e56 "git-gui:
work from the .git dir", 2010-01-23).

-- Hannes

Re: [PATCH v3 1/1] git-gui: handle missing worktree and separated gitdir

[Mark Levedahl]

On 5/3/26 4:53 AM, Johannes Sixt wrote:
> I would not call the use of --is-bar-repository instead of
> --is-inside-git-dir an error, just a choice that has been made. In
> particular, when the startup directory is named '.git' and is not marked
> as bare, then its parent directory can very reasonably be taken as its
> worktree. (That's how things worked before --show-toplevel was used.) If
> the check is for --is-inside-git-dir, this treatment would be ruled out
> early.
>
Whether being in a gitdir is ok, or a worktree required, is of fundamental importance and
is not explicitly checked now. This is my issue. (Whether the repo is bare, or embedded in
a worktree, is relevant only when automatically fixing a user error.)
> But perhaps there is a simpler solution: Let's present an error if
> --show-toplevel fails except in the case where the startup directory is
> named '.git' (and is a valid Git repository) and is not bare (then the
> worktree is the parent). I insist in this exception, because this
> use-case was considered important in the past (87cd09f43e56 "git-gui:
> work from the .git dir", 2010-01-23).
>
> -- Hannes
>

This would not fix gitk's blame / browse from a gitdir, and I don't really see a one or
two line fix as being adequate.

git-gui sets GIT_WORK_TREE and GIT_DIR at startup. GIT_DIR passes my simple tests, but
mishandles GIT_WORK_TREE.

I expect these two invocations to be equivalent, both starting git-gui in the worktree
'/some/path':

    GIT_WORK_TREE=/some/path git gui
    git -C /some/path gui

But, the GIT_WORK_TREE approach:
    works as I expect ONLY when the current directory is a valid worktree
    when started from a gitdir, uses that gitdir in conjunction with the requested worktree
    when started from an uncontrolled directory, shows the repository picker.

The git -C approach is indifferent to the current directory, of course.

GIT_WORK_TREE enters much too late in the process, and rather should handled first:
    if GIT_WORK_TREE is in the environment, cd to that first. Throw an error if that
directory is not a valid worktree.

I don't actually understand the use case of defining GIT_DIR or GIT_WORK_TREE to git gui,
and I wonder what other bugs are lurking... maybe the better approach is to just abort if
GIT_DIR or GIT_WORK_TREE are defined?

Mark

Re: [PATCH v3 1/1] git-gui: handle missing worktree and separated gitdir

[Mark Levedahl]

>
> On 5/3/26 4:53 AM, Johannes Sixt wrote:
>> But perhaps there is a simpler solution: Let's present an error if
>> --show-toplevel fails except in the case where the startup directory is
>> named '.git' (and is a valid Git repository) and is not bare (then the
>> worktree is the parent). I insist in this exception, because this
>> use-case was considered important in the past (87cd09f43e56 "git-gui:
>> work from the .git dir", 2010-01-23).
>>
>> -- Hannes
>>

I've restructured startup code in line with what I suggested before, allowing operation in
a worktree or a gitdir, and with various combinations of GIT_DIR and GIT_WORK_TREE
environment variables set. Unfortunately, git-gui's blame and browser commands simply do
not now work without a valid worktree. The error(s) are not obvious to me, and bisecting
requires git version 2.24 or earlier to remove 2d92ab32fd ("rev-parse: make
--show-toplevel without a worktree an error", 2019-11-19) to even start: many gcc and
git/git-gui version compatibility issues are certain to arise. I won't be doing this.

So, I believe your suggestion above is the best path, leaving behind dead code that
purports to support operation from a gitdir but does not.

Mark

Re: [PATCH v3 1/1] git-gui: handle missing worktree and separated gitdir

[Johannes Sixt]

Am 04.05.26 um 17:13 schrieb Mark Levedahl:
> On 5/3/26 4:53 AM, Johannes Sixt wrote:
>> I would not call the use of --is-bar-repository instead of
>> --is-inside-git-dir an error, just a choice that has been made. In
>> particular, when the startup directory is named '.git' and is not marked
>> as bare, then its parent directory can very reasonably be taken as its
>> worktree. (That's how things worked before --show-toplevel was used.) If
>> the check is for --is-inside-git-dir, this treatment would be ruled out
>> early.
>>
> Whether being in a gitdir is ok, or a worktree required, is of fundamental importance and
> is not explicitly checked now. This is my issue. (Whether the repo is bare, or embedded in
> a worktree, is relevant only when automatically fixing a user error.)

I don't quite follow what you a trying to say here.

>> But perhaps there is a simpler solution: Let's present an error if
>> --show-toplevel fails except in the case where the startup directory is
>> named '.git' (and is a valid Git repository) and is not bare (then the
>> worktree is the parent). I insist in this exception, because this
>> use-case was considered important in the past (87cd09f43e56 "git-gui:
>> work from the .git dir", 2010-01-23).
>>
>> -- Hannes
>>
> 
> This would not fix gitk's blame / browse from a gitdir, and I don't really see a one or
> two line fix as being adequate.
> 
> git-gui sets GIT_WORK_TREE and GIT_DIR at startup. GIT_DIR passes my simple tests, but
> mishandles GIT_WORK_TREE.
> 
> I expect these two invocations to be equivalent, both starting git-gui in the worktree
> '/some/path':
> 
>     GIT_WORK_TREE=/some/path git gui
>     git -C /some/path gui
> 
> But, the GIT_WORK_TREE approach:
>     works as I expect ONLY when the current directory is a valid worktree
>     when started from a gitdir, uses that gitdir in conjunction with the requested worktree
>     when started from an uncontrolled directory, shows the repository picker.

The important aspect here isn't about the worktree, but whether a gitdir
can be determined for the current directory. All three observations make
total sense.

That said, setting GIT_WORK_TREE without also setting GIT_DIR is
undefined and need not be considered further.

> The git -C approach is indifferent to the current directory, of course.
> 
> GIT_WORK_TREE enters much too late in the process, and rather should handled first:
>     if GIT_WORK_TREE is in the environment, cd to that first. Throw an error if that
> directory is not a valid worktree.

As I said, GIT_WORK_TREE without GIT_DIR is an invalid use-case. For
this reason, the first thing to do is find the database, and from there
work out the worktree. In the most common use-case it is the current
directory.

> I don't actually understand the use case of defining GIT_DIR or GIT_WORK_TREE to git gui,
> and I wonder what other bugs are lurking... maybe the better approach is to just abort if
> GIT_DIR or GIT_WORK_TREE are defined?
I lean towards setting GIT_DIR always. This is necessary, because Git
GUI can be run from a subdirectory of the worktree, and then changes
directory to the top-level. It must be ensured that the same GIT_DIR is
used that was detected from the subdirectory.

Now that the current directory is at the top-level of the worktree, we
could just not set GIT_WORK_TREE at all, provided that setting GIT_DIR
without GIT_WORK_TREE is a valid use-case for Git. I am not yet sure
about that.

-- Hannes

Re: [PATCH v3 1/1] git-gui: handle missing worktree and separated gitdir

[Mark Levedahl]

On 5/6/26 3:32 AM, Johannes Sixt wrote:
>> Whether being in a gitdir is ok, or a worktree required, is of fundamental importance and
>> is not explicitly checked now. This is my issue. (Whether the repo is bare, or embedded in
>> a worktree, is relevant only when automatically fixing a user error.)
> I don't quite follow what you a trying to say here.
I played a bit more:

A git repository (gitdir) can have config.bare true | false | not set
git rev-parse --is-bare-repository tells you that whatever gitdir is discovered from the
current directory has core.bare==true. This happens whether the call is from inside the
gitdir, or in the parent dir of a gitdir named '.git', or in a directory containing a
symlink or a gitfile link to the gitdir. This call never tells you what directory you are
actually in.

git rev-parse --is-inside-work-tree gives:
    true - the call is made from a directory that is suported/supportable as a worktree of
a gitdir.
    false - the call is made from inside a gitdir, or from a directory linked to a to a
gitdir with core.bare == true.
    and error is thrown if no gitdir is discovered.

I find --is-inside-work-tree a much better call to make early in setup. 
    true - full git-gui is ok, 
    false - blame/browser is ok (gitdir might have core.bare true)
    error - no gitdir found, the repository picker should be called.

So, the only need to test if the repo is marked bare is when looking for a possible
worktree when git-gui was started inside the gitdir, or started in a directory linked to
said gitdir, or GIT_DIR in the environment points to said gitdir: I consider all of this a
user (or configuration) error, and there are many possible causes to explore to give
useful feedback to the user.

But, there are many ways to code this. I started down a path of using
--is-inside-worktree, but in the end there are still a lot of corner cases to find.

>>> But perhaps there is a simpler solution: Let's present an error if
>>> --show-toplevel fails except in the case where the startup directory is
>>> named '.git' (and is a valid Git repository) and is not bare (then the
>>> worktree is the parent). I insist in this exception, because this
>>> use-case was considered important in the past (87cd09f43e56 "git-gui:
>>> work from the .git dir", 2010-01-23).
>>>
>>> -- Hannes
>>>
>> This would not fix gitk's blame / browse from a gitdir, and I don't really see a one or
>> two line fix as being adequate.

As you mentioned elsewhere, the problem on browser/blame is that _gitworktree is empty
when no worktree is found, so GIT_WORK_TREE is exported to the environment as an empty
variable. This cause is in a commit from 12 years ago:

    3decb8e0ac ("git-gui: tolerate major version changes when comparing the git version",
2014-05-17)

Prior to that commit and if not using git v1.7.x, an alternate branch of code not using
git rev-parse was used for worktree discovery, and that code set _gitworktree = _gitdir
when in a gitdir. The alternate code was removed more recently as it was unreachable from
non-ancient git versions.

The fix is to set _gitworktree to _gitdir before exporting GIT_WORK_TREE, or to just not
export an empty GIT_WORK_TREE. Obviously, having GIT_WORK_TREE = GIT_DIR is asking for
trouble, but perhaps is ok as git-gui is running in a read-only mode for browse/blame. My
limited testing shows this works.


Mark

Re: [PATCH v3 1/1] git-gui: handle missing worktree and separated gitdir

[Johannes Sixt]

Am 06.05.26 um 13:27 schrieb Mark Levedahl:
> A git repository (gitdir) can have config.bare true | false | not set
> git rev-parse --is-bare-repository tells you that whatever gitdir is discovered from the
> current directory has core.bare==true. This happens whether the call is from inside the
> gitdir, or in the parent dir of a gitdir named '.git', or in a directory containing a
> symlink or a gitfile link to the gitdir. This call never tells you what directory you are
> actually in.

OK. But how does "find out which directory we are in" come into play
here? If we find a bare repository, we do not need a worktree. If we are
in a non-bare repository, we can find the worktree with `rev-parse
--show-toplevel`.

> 
> git rev-parse --is-inside-work-tree gives:
>     true - the call is made from a directory that is suported/supportable as a worktree of
> a gitdir.
>     false - the call is made from inside a gitdir, or from a directory linked to a to a
> gitdir with core.bare == true.
>     and error is thrown if no gitdir is discovered.
> 
> I find --is-inside-work-tree a much better call to make early in setup. 
>     true - full git-gui is ok, 
>     false - blame/browser is ok (gitdir might have core.bare true)
>     error - no gitdir found, the repository picker should be called.

But we would still make an exception for the case that $PWD is a
non-bare repository named ".git", because then, by Git GUI's definition,
its parent is the corresponding worktree.

> So, the only need to test if the repo is marked bare is when looking for a possible
> worktree when git-gui was started inside the gitdir, or started in a directory linked to
> said gitdir, or GIT_DIR in the environment points to said gitdir: I consider all of this a
> user (or configuration) error, and there are many possible causes to explore to give
> useful feedback to the user.

How does this scheme work when the user starts `git gui blame` in a bare
repository that does not have a worktree? Would this not produce an
error because no worktree was found?

> As you mentioned elsewhere, the problem on browser/blame is that _gitworktree is empty
> when no worktree is found, so GIT_WORK_TREE is exported to the environment as an empty
> variable. This cause is in a commit from 12 years ago:
> 
>     3decb8e0ac ("git-gui: tolerate major version changes when comparing the git version",
> 2014-05-17)

I don't think that this commit very relevant. The problem is in `git
branch --show-current` (and probably other git command variants) that
want to turn an empty $GIT_WORK_TREE into an absolute path even in cases
where no worktree is needed. I haven't tried to figure out which commit
(in the Git repository) started to do this.

> The fix is to set _gitworktree to _gitdir before exporting GIT_WORK_TREE, or to just not
> export an empty GIT_WORK_TREE. Obviously, having GIT_WORK_TREE = GIT_DIR is asking for
> trouble, but perhaps is ok as git-gui is running in a read-only mode for browse/blame. My
> limited testing shows this works.
Good to know. My preference is to not set GIT_WORK_TREE at all provided
that setting GIT_DIR without GIT_WORK_TREE is a use-case supported by Git.

-- Hannes

Re: [PATCH v3 1/1] git-gui: handle missing worktree and separated gitdir

[Mark Levedahl]

On 5/6/26 8:57 AM, Johannes Sixt wrote:
> Am 06.05.26 um 13:27 schrieb Mark Levedahl:
>> A git repository (gitdir) can have config.bare true | false | not set
>> git rev-parse --is-bare-repository tells you that whatever gitdir is discovered from the
>> current directory has core.bare==true. This happens whether the call is from inside the
>> gitdir, or in the parent dir of a gitdir named '.git', or in a directory containing a
>> symlink or a gitfile link to the gitdir. This call never tells you what directory you are
>> actually in.
> OK. But how does "find out which directory we are in" come into play
> here? If we find a bare repository, we do not need a worktree. If we are
> in a non-bare repository, we can find the worktree with `rev-parse
> --show-toplevel`.
>
>> git rev-parse --is-inside-work-tree gives:
>>     true - the call is made from a directory that is suported/supportable as a worktree of
>> a gitdir.
>>     false - the call is made from inside a gitdir, or from a directory linked to a to a
>> gitdir with core.bare == true.
>>     and error is thrown if no gitdir is discovered.
>>
>> I find --is-inside-work-tree a much better call to make early in setup. 
>>     true - full git-gui is ok, 
>>     false - blame/browser is ok (gitdir might have core.bare true)
>>     error - no gitdir found, the repository picker should be called.
> But we would still make an exception for the case that $PWD is a
> non-bare repository named ".git", because then, by Git GUI's definition,
> its parent is the corresponding worktree.

I find the organization using rev-parse --is-inside-work-tree easier to reason about, and
if I were writing this from scratch, I would do it that way. But, you have one or more
patches in progress, if this idea is useful there great, otherwise, drop it.

>> As you mentioned elsewhere, the problem on browser/blame is that _gitworktree is empty
>> when no worktree is found, so GIT_WORK_TREE is exported to the environment as an empty
>> variable. This cause is in a commit from 12 years ago:
>>
>>     3decb8e0ac ("git-gui: tolerate major version changes when comparing the git version",
>> 2014-05-17)
> I don't think that this commit very relevant. The problem is in `git
> branch --show-current` (and probably other git command variants) that
> want to turn an empty $GIT_WORK_TREE into an absolute path even in cases
> where no worktree is needed. I haven't tried to figure out which commit
> (in the Git repository) started to do this.

Reverting that commit in any way has nothing to do with fixing this problem now. But,
detecting breakage at that commit is what lead me to discover the problem was _gitworktree
== {} and GIT_WORK_TREE="". As you say, browser/blame may well not have broken until a
more recent commit to git itself. I cannot say, even the 2019 git prior to rev parse
--top-level being taught to error out will not build on my computer due to incompatibilities.

>> The fix is to set _gitworktree to _gitdir before exporting GIT_WORK_TREE, or to just not
>> export an empty GIT_WORK_TREE. Obviously, having GIT_WORK_TREE = GIT_DIR is asking for
>> trouble, but perhaps is ok as git-gui is running in a read-only mode for browse/blame. My
>> limited testing shows this works.
> Good to know. My preference is to not set GIT_WORK_TREE at all provided
> that setting GIT_DIR without GIT_WORK_TREE is a use-case supported by Git.
>
>

I just confirmed that git-gui modified to export GIT_DIR only, not GIT_WORK_TREE, and
actually to make sure GIT_WORK_TREE is not in env, has blame/browser working correctly in
a gitdir with no worktree.

Mark

Re: [PATCH v3 1/1] git-gui: handle missing worktree and separated gitdir

[Mark Levedahl]

On 5/6/26 10:05 AM, Mark Levedahl wrote:
>
> On 5/6/26 8:57 AM, Johannes Sixt wrote:
>> Am 06.05.26 um 13:27 schrieb Mark Levedahl:
>>> A git repository (gitdir) can have config.bare true | false | not set
>>> git rev-parse --is-bare-repository tells you that whatever gitdir is discovered from the
>>> current directory has core.bare==true. This happens whether the call is from inside the
>>> gitdir, or in the parent dir of a gitdir named '.git', or in a directory containing a
>>> symlink or a gitfile link to the gitdir. This call never tells you what directory you are
>>> actually in.
>> OK. But how does "find out which directory we are in" come into play
>> here? If we find a bare repository, we do not need a worktree. If we are
>> in a non-bare repository, we can find the worktree with `rev-parse
>> --show-toplevel`.
The influence of config variable core.bare in a git repository breaks the design I
contemplated using rev-parse --is-inside-work-tree. core.bare is a strange tri-state item,
usually only set if the repository is explicitly bare. With core.bare undefined, rev-parse
can give different answers inside and outside a gitdir. For instance
      rev-parse --is-bare-repository gives true inside a repository, and false outside,
--is-inside-work-tree is also influenced by core.bare, so results cannot be interpreted as
I initially thought.

Anyway, after some experiments, the minimal sufficient tests I find for blame|browser and
gui|citool are different:

- blame|browser need a readable gitdir, and do not need a worktree. The current directory
can be inside the gitdir, core.bare is irrelevant.

    a sufficient startup test:

        git rev-parse --git-dir returns the name of a gitdir, rather than throwing an error.
               core.bare and being inside/outside of the gitdir do affect error / no error.
                (the return value may be a relative path, so can depend upon current
directory)

    only GIT_DIR should be exported. GIT_WORK_TREE should not be in env.

- gui|citool, require a gitdir, a worktree outside the gitdir, and core.bare is not true

    a sufficient and minimal startup test:

        git rev-parse --show-toplevel returns the worktree root, rather than throwing an
error.
                core.bare = true in the gitdir will cause error in the above.
                being inside the gitdir causes error in the above.

        But, starting in a .git directory supporting a worktree is allowed by the fallback
logic

        git rev-parse --git-dir returns '.', (current dir is the root of the gitdir),
rather than throwing an error.
        the current directory is named ".git", meaning the parent dir is this gitdir's
worktree if core.bare is not true
        git -C .. rev-parse --show-toplevel  returns the worktree root: (will throw an
error if core.bare is true)

    the current directory should be changed to the worktree root.
    GIT_DIR and GIT_WORK_TREE should be exported to env.

Mark

[PATCH v4 1/1] git-gui: handle missing worktree and separated gitdir

[Shroom Moo]

When git-gui is started from a directory that Git recognizes as a
valid repository but the working tree is not accessible (e.g., a
separated gitdir created by `git clone --separate-git-dir`, a bare
repository, or a case where the worktree directory was removed),
it previously called `rev-parse --show-toplevel` without error
handling, causing a fatal Tcl error ("this operation must be run
in a work tree").

Wrap the call in a `catch` and handle the failure as follows:

- For bare repositories, keep `_gitworktree` empty so that the
  existing `is_bare` check shows "Cannot use bare repository" and
  exits.  No behavioral change.

- If we are inside the gitdir (i.e., `--is-inside-git-dir` is
  `true`), refuse to start with a clear message: "Cannot start
  git-gui from inside the Git directory. Please run git-gui from a
  working tree."  This prevents dangerous operations in a separated
  gitdir or when starting from a .git subdirectory, where ordinary
  Git commands like `git status` also refuse to run.

- Otherwise, leave `_gitworktree` empty so that the existing error
  paths (e.g., missing worktree) can display their own diagnostics.

This fix intentionally drops the previous ability to start git-gui
from within a regular repository's .git subdirectory.  That behavior
was never reliable (Git itself forbids operations inside .git) and
broke completely in the multi‑worktree case where the parent
directory is not guaranteed to be the intended worktree.  The error
message guides the user to start git-gui from a worktree instead.

Signed-off-by: Shroom Moo <egg_mushroomcow@foxmail.com>
---
 git-gui/git-gui.sh | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index 23fe76e498..9e848aa26b 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -1169,7 +1169,17 @@ if {![file isdirectory $_gitdir]} {
 load_config 0
 apply_config
 
-set _gitworktree [git rev-parse --show-toplevel]
+if {[catch {set _gitworktree [git rev-parse --show-toplevel]}]} {
+    if {![catch {set bare [git rev-parse --is-bare-repository]}] && $bare eq {true}} {
+        set _gitworktree {}
+    } elseif {![catch {set inside [git rev-parse --is-inside-git-dir]}] && $inside eq {true}} {
+        catch {wm withdraw .}
+        error_popup [mc "Cannot start git-gui from inside the Git directory\nPlease run git-gui from a working tree"]
+        exit 1
+    } else {
+        set _gitworktree {}
+    }
+}
 
 if {$_prefix ne {}} {
 	if {$_gitworktree eq {}} {
-- 
2.52.0.windows.1

[PATCH v5 1/1] git-gui: restructure repository startup

[Shroom Moo]

When git-gui is started inside a .git directory of a non-bare
repository, it should treat the parent directory as the worktree,
as it did before commit 2d92ab32fd (rev-parse: make --show-toplevel
without a worktree an error, 2019-11-19).  However, a bare repository
or a separated gitdir without a worktree must be rejected early.

Protect the previously unguarded calls to `git rev-parse
--show-object-format` and `--show-toplevel`.  Restructure the startup
sequence to:

- Check for a bare repository right after loading the config.  If the
  repository is bare and the current subcommand does not allow bare
  repos (e.g. normal commit mode), show "Cannot use bare repository"
  and exit.

- When `rev-parse --show-toplevel` fails and the repository is
  non-bare, the gitdir path ends with ".git", and we are inside that
  gitdir, use the parent directory as the worktree.  This preserves
  the ability to start git-gui from within a regular repository’s
  .git directory, which was intentionally supported since 87cd09f43e56
  (git-gui: work from the .git dir, 2010-01-23).

- Otherwise, show a descriptive error and exit.

- Wrap `rev-parse --show-object-format` in a catch to avoid a crash
  when the repository configuration is broken (e.g. core.worktree
  pointing to an invalid path).

Also removes the old `_prefix`‑based fallback that computed a relative
path to the worktree top from a subdirectory, and the unconditional
`[file dirname $_gitdir]` guess.  Both are unnecessary now that
`rev‑parse --show‑toplevel` directly provides the absolute top‑level
path and we can `cd` to it.  The guess is further unsafe in
multi‑worktree setups, where a gitdir may have more than one worktree.
The only remaining fallback is the explicit “.git directory” rule for
non‑bare repositories, which mirrors the historical behaviour.

This fixes the fatal Tcl error when the working tree is missing, while
keeping the .git startup feature and avoiding any automatic directory
switching that could be dangerous in multi‑worktree setups.

Signed-off-by: Shroom Moo <egg_mushroomcow@foxmail.com>
---
 git-gui/git-gui.sh | 72 +++++++++++++++++++++++++++++-----------------
 1 file changed, 45 insertions(+), 27 deletions(-)

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index 23fe76e498..c06d85b8d9 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -1129,7 +1129,8 @@ if {[catch {
 		}]
 	&& [catch {
 		# beware that from the .git dir this sets _gitdir to .
-		# and _prefix to the empty string
+		# and _prefix to the empty string; this is handled by
+		# the startup safety checks below
 		set _gitdir [git rev-parse --git-dir]
 		set _prefix [git rev-parse --show-prefix]
 	} err]} {
@@ -1142,8 +1143,20 @@ if {[catch {
 	set picked 1
 }
 
+if {![file isdirectory $_gitdir]} {
+	catch {wm withdraw .}
+	error_popup [strcat 
+		[mc "Git directory not found:"] "\n\n$_gitdir\n\n" \
+		[mc "Please ensure GIT_DIR points to a valid Git repository"]]
+	exit 1
+}
+
 # Use object format as hash algorithm (either "sha1" or "sha256")
-set hashalgorithm [git rev-parse --show-object-format]
+if {[catch {set hashalgorithm [git rev-parse --show-object-format]} err]} {
+	catch {wm withdraw .}
+	error_popup [strcat [mc "Failed to determine hash algorithm:"] "\n\n$err"]
+	exit 1
+}
 if {$hashalgorithm eq "sha1"} {
 	set hashlength 40
 } elseif {$hashalgorithm eq "sha256"} {
@@ -1160,46 +1173,50 @@ if {$_gitdir eq "."} {
 	set _gitdir [pwd]
 }
 
-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
 
-set _gitworktree [git rev-parse --show-toplevel]
+# Handle bare repository early: if not allowed, abort
+if {[is_bare] && ![is_enabled bare]} {
+	catch {wm withdraw .}
+	error_popup [strcat [mc "Cannot use bare repository:"] "\n\n" [file normalize $_gitdir]]
+	exit 1
+}
 
-if {$_prefix ne {}} {
-	if {$_gitworktree eq {}} {
-		regsub -all {[^/]+/} $_prefix ../ cdup
-	} else {
-		set cdup $_gitworktree
-	}
-	if {[catch {cd $cdup} err]} {
-		catch {wm withdraw .}
-		error_popup [strcat [mc "Cannot move to top of working directory:"] "\n\n$err"]
-		exit 1
+# Determine the working tree
+if {[is_bare] && [is_enabled bare]} {
+	set _gitworktree {}
+} else {
+	if {[catch {set _gitworktree [git rev-parse --show-toplevel]} err]} {
+		# If we are inside a .git directory of a non-bare repo,
+		# the worktree is the parent directory
+		set inside_gitdir 0
+		catch {set inside_gitdir [git rev-parse --is-inside-git-dir]}
+		if {![is_bare] && $inside_gitdir eq {true} && [file tail [file normalize $_gitdir]] eq {.git}} {
+			set _gitworktree [file normalize [file dirname $_gitdir]]
+		} else {
+			catch {wm withdraw .}
+			error_popup [strcat [mc "Cannot determine working tree:"] "\n\n$err"]
+			exit 1
+		}
 	}
-	set _gitworktree [pwd]
-	unset cdup
-} elseif {![is_enabled bare]} {
-	if {[is_bare]} {
+
+	if {$_gitworktree eq {}} {
 		catch {wm withdraw .}
-		error_popup [strcat [mc "Cannot use bare repository:"] "\n\n$_gitdir"]
+		error_popup [mc "Cannot determine working tree (unexpected empty result)"]
 		exit 1
 	}
-	if {$_gitworktree eq {}} {
-		set _gitworktree [file dirname $_gitdir]
-	}
+
 	if {[catch {cd $_gitworktree} err]} {
 		catch {wm withdraw .}
-		error_popup [strcat [mc "No working directory"] " $_gitworktree:\n\n$err"]
+		error_popup [strcat [mc "Cannot move to working directory:"] "\n\n$err"]
 		exit 1
 	}
 	set _gitworktree [pwd]
 }
+
+# Derive a human-readable repository name
 set _reponame [file split [file normalize $_gitdir]]
 if {[lindex $_reponame end] eq {.git}} {
 	set _reponame [lindex $_reponame end-1]
@@ -1207,6 +1224,7 @@ if {[lindex $_reponame end] eq {.git}} {
 	set _reponame [lindex $_reponame end]
 }
 
+# Export the final paths
 set env(GIT_DIR) $_gitdir
 set env(GIT_WORK_TREE) $_gitworktree
 
-- 
2.52.0.windows.1

Re: [PATCH v5 1/1] git-gui: restructure repository startup

[Johannes Sixt]

Am 04.05.26 um 16:59 schrieb Shroom Moo:
> When git-gui is started inside a .git directory of a non-bare
> repository, it should treat the parent directory as the worktree,
> as it did before commit 2d92ab32fd (rev-parse: make --show-toplevel
> without a worktree an error, 2019-11-19).  However, a bare repository
> or a separated gitdir without a worktree must be rejected early.
> 
> Protect the previously unguarded calls to `git rev-parse
> --show-object-format` and `--show-toplevel`.  Restructure the startup
> sequence to:
> 
> - Check for a bare repository right after loading the config.  If the
>   repository is bare and the current subcommand does not allow bare
>   repos (e.g. normal commit mode), show "Cannot use bare repository"
>   and exit.
> 
> - When `rev-parse --show-toplevel` fails and the repository is
>   non-bare, the gitdir path ends with ".git", and we are inside that
>   gitdir, use the parent directory as the worktree.  This preserves
>   the ability to start git-gui from within a regular repository’s
>   .git directory, which was intentionally supported since 87cd09f43e56
>   (git-gui: work from the .git dir, 2010-01-23).
> 
> - Otherwise, show a descriptive error and exit.

Very good. This does things in the right order, IMO.

> 
> - Wrap `rev-parse --show-object-format` in a catch to avoid a crash
>   when the repository configuration is broken (e.g. core.worktree
>   pointing to an invalid path).

Nice catch. This could be moved into its own patch. But it is acceptable
in this patch as it loosely fits the topic.

> 
> Also removes the old `_prefix`‑based fallback that computed a relative
> path to the worktree top from a subdirectory, and the unconditional
> `[file dirname $_gitdir]` guess.  Both are unnecessary now that
> `rev‑parse --show‑toplevel` directly provides the absolute top‑level
> path and we can `cd` to it.  The guess is further unsafe in
> multi‑worktree setups, where a gitdir may have more than one worktree.
> The only remaining fallback is the explicit “.git directory” rule for
> non‑bare repositories, which mirrors the historical behaviour.

Nice cleanup.

> 
> This fixes the fatal Tcl error when the working tree is missing, while
> keeping the .git startup feature and avoiding any automatic directory
> switching that could be dangerous in multi‑worktree setups.

Good!

However, this doesn't fix `git gui blame HEAD file` in a bare
repository, because `git branch --show-current` fails with an empty
GIT_WORK_TREE value. Fixing this needs to consider whether to set
GIT_DIR and GIT_WORK_TREE at all, as alluded to by Mark in a near-by
message. This is a separate topic.

> 
> Signed-off-by: Shroom Moo <egg_mushroomcow@foxmail.com>
> ---
>  git-gui/git-gui.sh | 72 +++++++++++++++++++++++++++++-----------------
>  1 file changed, 45 insertions(+), 27 deletions(-)
> 
> diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
> index 23fe76e498..c06d85b8d9 100755
> --- a/git-gui/git-gui.sh
> +++ b/git-gui/git-gui.sh
> @@ -1129,7 +1129,8 @@ if {[catch {
>  		}]
>  	&& [catch {
>  		# beware that from the .git dir this sets _gitdir to .
> -		# and _prefix to the empty string
> +		# and _prefix to the empty string; this is handled by
> +		# the startup safety checks below
>  		set _gitdir [git rev-parse --git-dir]
>  		set _prefix [git rev-parse --show-prefix]
>  	} err]} {
> @@ -1142,8 +1143,20 @@ if {[catch {
>  	set picked 1
>  }
>  
> +if {![file isdirectory $_gitdir]} {
> +	catch {wm withdraw .}
> +	error_popup [strcat 
> +		[mc "Git directory not found:"] "\n\n$_gitdir\n\n" \
> +		[mc "Please ensure GIT_DIR points to a valid Git repository"]]
> +	exit 1
> +}
> +
>  # Use object format as hash algorithm (either "sha1" or "sha256")
> -set hashalgorithm [git rev-parse --show-object-format]
> +if {[catch {set hashalgorithm [git rev-parse --show-object-format]} err]} {
> +	catch {wm withdraw .}
> +	error_popup [strcat [mc "Failed to determine hash algorithm:"] "\n\n$err"]
> +	exit 1
> +}
>  if {$hashalgorithm eq "sha1"} {
>  	set hashlength 40
>  } elseif {$hashalgorithm eq "sha256"} {
> @@ -1160,46 +1173,50 @@ if {$_gitdir eq "."} {
>  	set _gitdir [pwd]
>  }
>  
> -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
>  
> -set _gitworktree [git rev-parse --show-toplevel]
> +# Handle bare repository early: if not allowed, abort
> +if {[is_bare] && ![is_enabled bare]} {
> +	catch {wm withdraw .}
> +	error_popup [strcat [mc "Cannot use bare repository:"] "\n\n" [file normalize $_gitdir]]
> +	exit 1
> +}
>  
> -if {$_prefix ne {}} {
> -	if {$_gitworktree eq {}} {
> -		regsub -all {[^/]+/} $_prefix ../ cdup
> -	} else {
> -		set cdup $_gitworktree
> -	}
> -	if {[catch {cd $cdup} err]} {
> -		catch {wm withdraw .}
> -		error_popup [strcat [mc "Cannot move to top of working directory:"] "\n\n$err"]
> -		exit 1
> +# Determine the working tree
> +if {[is_bare] && [is_enabled bare]} {
> +	set _gitworktree {}

I strongly suggest to collapse this branch and the previous 'if
{[is_bare] && ![is_enabled bare]}' into a single 'if {[is_bare]}',
because then in the else-branch below we can be sure not to have a bare
repository.

> +} else {
> +	if {[catch {set _gitworktree [git rev-parse --show-toplevel]} err]} {
> +		# If we are inside a .git directory of a non-bare repo,
> +		# the worktree is the parent directory
> +		set inside_gitdir 0
> +		catch {set inside_gitdir [git rev-parse --is-inside-git-dir]}
> +		if {![is_bare] && $inside_gitdir eq {true} && [file tail [file normalize $_gitdir]] eq {.git}} {

Do we need to 'file normalize' before taking the 'file tail'? If not,
then the line would be shorter.

> +			set _gitworktree [file normalize [file dirname $_gitdir]]
> +		} else {
> +			catch {wm withdraw .}
> +			error_popup [strcat [mc "Cannot determine working tree:"] "\n\n$err"]
> +			exit 1
> +		}
>  	}
> -	set _gitworktree [pwd]
> -	unset cdup
> -} elseif {![is_enabled bare]} {
> -	if {[is_bare]} {
> +
> +	if {$_gitworktree eq {}} {
>  		catch {wm withdraw .}
> -		error_popup [strcat [mc "Cannot use bare repository:"] "\n\n$_gitdir"]
> +		error_popup [mc "Cannot determine working tree (unexpected empty result)"]
>  		exit 1
>  	}
> -	if {$_gitworktree eq {}} {
> -		set _gitworktree [file dirname $_gitdir]
> -	}
> +
>  	if {[catch {cd $_gitworktree} err]} {
>  		catch {wm withdraw .}
> -		error_popup [strcat [mc "No working directory"] " $_gitworktree:\n\n$err"]
> +		error_popup [strcat [mc "Cannot move to working directory:"] "\n\n$err"]
>  		exit 1
>  	}
>  	set _gitworktree [pwd]
>  }
> +
> +# Derive a human-readable repository name
>  set _reponame [file split [file normalize $_gitdir]]
>  if {[lindex $_reponame end] eq {.git}} {
>  	set _reponame [lindex $_reponame end-1]
> @@ -1207,6 +1224,7 @@ if {[lindex $_reponame end] eq {.git}} {
>  	set _reponame [lindex $_reponame end]
>  }
>  
> +# Export the final paths
>  set env(GIT_DIR) $_gitdir
>  set env(GIT_WORK_TREE) $_gitworktree
>  

-- Hannes

[PATCH v6 0/3] git-gui: robustify startup and fix environment handling

[Shroom Moo]

This series addresses the startup crash introduced by Git commit
"2d92ab32fd ("rev-parse: make --show-toplevel without a worktree an
error", 2019-11-19)", which causes `git gui` to die with a Tcl error
when a worktree is missing (e.g. inside a .git directory without a
working tree, or in a bare repository).

Additionally, it resolves two historically inconsistent behaviours:
- The "Visualize ... History" menu items were enabled in bare
  repositories but triggered Tcl errors due to the missing worktree.
- `GIT_WORK_TREE` and `GIT_DIR` environment variables were not
  respected early enough, so `GIT_WORK_TREE=/some/path git gui`
  often ignored the explicit worktree and brought up the repository
  picker, and an exported empty `GIT_WORK_TREE` confused commands
  like `git branch --show-current` in bare repositories.

Shroom Moo (3):
  git-gui: restructure repository startup
  git-gui: disable gitk visualization when no worktree available
  git-gui: handle GIT_DIR and GIT_WORK_TREE early

 git-gui/git-gui.sh | 170 ++++++++++++++++++++++++++++++++++-----------
 1 file changed, 128 insertions(+), 42 deletions(-)

-- 
2.52.0.windows.1

[PATCH v7 0/3] git-gui: robustify startup and fix environment handling

[Shroom Moo]

This series addresses the startup crash introduced by Git commit
"2d92ab32fd ("rev-parse: make --show-toplevel without a worktree an
error", 2019-11-19)", which causes `git gui` to die with a Tcl error
when a worktree is missing (e.g. inside a .git directory without a
working tree, or in a bare repository).

Additionally, it resolves two historically inconsistent behaviours:
- The "Visualize ... History" menu items were enabled in bare
  repositories but triggered Tcl errors due to the missing worktree.
- `GIT_WORK_TREE` and `GIT_DIR` environment variables were not
  respected early enough, so `GIT_WORK_TREE=/some/path git gui`
  often ignored the explicit worktree and brought up the repository
  picker, and an exported empty `GIT_WORK_TREE` confused commands
  like `git branch --show-current` in bare repositories.

Shroom Moo (3):
  git-gui: restructure repository startup
  git-gui: disable gitk visualization when no worktree available
  git-gui: handle GIT_DIR and GIT_WORK_TREE early

 git-gui/git-gui.sh | 187 ++++++++++++++++++++++++++++++++++-----------
 1 file changed, 141 insertions(+), 46 deletions(-)

-- 
2.52.0.windows.1

Re: [PATCH v7 0/3] git-gui: robustify startup and fix environment handling

[Mark Levedahl]

On 5/9/26 9:37 AM, Shroom Moo wrote:
> Shroom Moo (3):
>   git-gui: restructure repository startup
>   git-gui: disable gitk visualization when no worktree available
>   git-gui: handle GIT_DIR and GIT_WORK_TREE early
>
>
After careful consideration, I find starting off by fixing what is broken in git-gui about
using a bare-repository, and letting git core handle GIT_DIR and GIT_WORK_TREE, leads to a
much more complete and different solution. A patch series (attempting to) do so will follow.

Mark

[PATCH v1 00/11] Improve git gui operation without a worktree

[Mark Levedahl]

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 select a worktree
  from a list and/or start a new repo+worktree: this dialog appears at
  unexpected times, masking useful error feedback on configuration
  problems.

This patch series addresses the above issues, substantially rewriting
the blame / browser command line process, the initial repository and
worktree discovery processes, and using git rev-parse when possible to
handle repository / worktree discovery including any specification of
GIT_DIR or GIT_WORK_TREE to reduce the future likelihood of conflict
with command line git. This also allows explicit user control to avoid
the repository picker masking a configuration error.

Note: I question why git-gui ever exports GIT_WORK_TREE. If it is not
empty, that is the current directory when startup is complete and any
git command will use the current directory as the worktree. If empty,
there is no worktree and the current directory should be (and after this
series, is) at the toplevel of the gitdir: again, there is nothing to
communicate to another process. If a process being launched needs a
different worktree, that should be the startup directory given to the
process without changing git-gui's current directory.

Mark Levedahl (11):
  git-gui: allow specifying path '.' to the browser
  git-gui: refactor browser / blame argument parsing
  git-gui: guard set/unset of GIT_DIR and GIT_WORK_TREE
  git-gui: put choose_repository::pick in a proc
  git-gui: use --absolute-git-dir
  git gui: GIT_DIR / GIT_WORK_TREE make any discovery error fatal
  git-gui: use rev-parse exclusively to find a repository
  git-gui: simplify [is_bare] to report if a worktree is known
  git-gui: support using repository parent dir as a worktree
  git-gui: improve worktree discovery
  git-gui: add gui and pick as explicit subcommands

 git-gui.sh | 276 ++++++++++++++++++++++++++---------------------------
 1 file changed, 135 insertions(+), 141 deletions(-)

-- 
2.54.0.99.14

[PATCH v1 01/11] git-gui: allow specifying path '.' to the browser

[Mark Levedahl]

Invoking "git-gui browser rev ." should show the file browser for the
commitish rev, starting at the root directory. This errors out in
normalize_relpath because the '.' is removed, yielding an empty list as
argument to [file join ...]. Fix this.

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
 git-gui.sh | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/git-gui.sh b/git-gui.sh
index 23fe76e..6048f92 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -2965,7 +2965,11 @@ proc normalize_relpath {path} {
 		}
 		lappend elements $item
 	}
-	return [eval file join $elements]
+	if {$elements ne {}} {
+		return [eval file join $elements]
+	} else {
+		return {}
+	}
 }
 
 # -- Not a normal commit type invocation?  Do that instead!
-- 
2.54.0.99.14

Re: [PATCH v1 01/11] git-gui: allow specifying path '.' to the browser

[Johannes Sixt]

Am 14.05.26 um 16:33 schrieb Mark Levedahl:
> Invoking "git-gui browser rev ." should show the file browser for the
> commitish rev, starting at the root directory. This errors out in
> normalize_relpath because the '.' is removed, yielding an empty list as
> argument to [file join ...]. Fix this.

Good catch!

The description isn't precise, though. '.' means to list the current
directory. The mentioned problem happens only if this is also the root
of the working tree.

> 
> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
> ---
>  git-gui.sh | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/git-gui.sh b/git-gui.sh
> index 23fe76e..6048f92 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -2965,7 +2965,11 @@ proc normalize_relpath {path} {
>  		}
>  		lappend elements $item
>  	}
> -	return [eval file join $elements]
> +	if {$elements ne {}} {
> +		return [eval file join $elements]
> +	} else {
> +		return {}
> +	}
>  }
>  
>  # -- Not a normal commit type invocation?  Do that instead!


-- Hannes

[PATCH v1 02/11] git-gui: refactor browser / blame argument parsing

[Mark Levedahl]

git-gui has subcommands blame and browser, both of which accept a
pathname, possibly preceded by a commit-ish item to specify a revision.
Also, blame can take a first argument that gives a line number to focus.

The command line parser for the above is more complex than needed, and
cannot work without a worktree as the pathname objects are checked
against the current worktree for existence. This also precludes naming a
directory or file that does not exist on the currently checked out
branch.

So, replace this with a simpler parser that looks at argument number and
number of arguments to know what value to expect. The blame and browser
backends already have error checking with diagnostic information, so
defer most error checking to those. Also, allow a line-number selection
to be given and silently ignored for the browser, further simplifying
this code.

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
 git-gui.sh | 66 +++++++++++++-----------------------------------------
 1 file changed, 16 insertions(+), 50 deletions(-)

diff --git a/git-gui.sh b/git-gui.sh
index 6048f92..a951fcd 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -2986,51 +2986,34 @@ blame {
 	set head {}
 	set path {}
 	set jump_spec {}
-	set is_path 0
+	set nargs [llength $argv]
+	if {$nargs < 1} {
+		usage
+	}
+	set argn 0
 	foreach a $argv {
-		set p [file join $_prefix $a]
+		set argn [expr {$argn + 1}]
 
-		if {$is_path || [file exists $p]} {
-			if {$path ne {}} usage
-			set path [normalize_relpath $p]
-			break
-		} elseif {$a eq {--}} {
-			if {$path ne {}} {
-				if {$head ne {}} usage
-				set head $path
-				set path {}
+		if {$argn < $nargs} {
+			# revision or line number
+			if {[regexp {^--line=(\d+)$} $a a lnum]} {
+				set jump_spec [list $lnum]
+			} else {
+				set head $a
 			}
-			set is_path 1
-		} elseif {[regexp {^--line=(\d+)$} $a a lnum]} {
-			if {$jump_spec ne {} || $head ne {}} usage
-			set jump_spec [list $lnum]
-		} elseif {$head eq {}} {
-			if {$head ne {}} usage
-			set head $a
-			set is_path 1
-		} else {
-			usage
-		}
-	}
-	unset is_path
-
-	if {$head ne {} && $path eq {}} {
-		if {[string index $head 0] eq {/}} {
-			set path [normalize_relpath $head]
-			set head {}
 		} else {
-			set path [normalize_relpath $_prefix$head]
-			set head {}
+			set path [normalize_relpath $a]
 		}
 	}
 
 	if {$head eq {}} {
 		load_current_branch
+		set head $current_branch
 	} else {
 		if {[regexp [string map "@@ [expr $hashlength - 1]" {^[0-9a-f]{1,@@}$}] $head]} {
 			if {[catch {
-					set head [git rev-parse --verify $head]
-				} err]} {
+				set head [git rev-parse --verify $head]
+			} err]} {
 				if {[tk windowingsystem] eq "win32"} {
 					tk_messageBox -icon error -title [mc Error] -message $err
 				} else {
@@ -3046,26 +3029,9 @@ blame {
 	switch -- $subcommand {
 	browser {
 		if {$jump_spec ne {}} usage
-		if {$head eq {}} {
-			if {$path ne {} && [file isdirectory $path]} {
-				set head $current_branch
-			} else {
-				set head $path
-				set path {}
-			}
-		}
 		browser::new $head $path
 	}
 	blame   {
-		if {$head eq {} && ![file exists $path]} {
-			catch {wm withdraw .}
-			tk_messageBox \
-				-icon error \
-				-type ok \
-				-title [mc "git-gui: fatal error"] \
-				-message [mc "fatal: cannot stat path %s: No such file or directory" $path]
-			exit 1
-		}
 		blame::new $head $path $jump_spec
 	}
 	}
-- 
2.54.0.99.14

Re: [PATCH v1 02/11] git-gui: refactor browser / blame argument parsing

[Johannes Sixt]

Am 14.05.26 um 16:33 schrieb Mark Levedahl:
> git-gui has subcommands blame and browser, both of which accept a
> pathname, possibly preceded by a commit-ish item to specify a revision.
> Also, blame can take a first argument that gives a line number to focus.
> 
> The command line parser for the above is more complex than needed, and
> cannot work without a worktree as the pathname objects are checked
> against the current worktree for existence. This also precludes naming a
> directory or file that does not exist on the currently checked out
> branch.

While the old browser isn't simple, it implements the strategy "revs
before paths, no revs after the first path or '--'" that is applied by
every git command. The rewritten parser is only slightly simpler. It
should not ignore "--". Furthermore, the old parser ignored excessive
trailing arguments, while the new parser ignores excessive leading
arguments. Neither is desirable, and we should report an incorrect
argument list (if we don't, then I prefer the old behavior).

> 
> So, replace this with a simpler parser that looks at argument number and
> number of arguments to know what value to expect. The blame and browser
> backends already have error checking with diagnostic information, so
> defer most error checking to those. Also, allow a line-number selection
> to be given and silently ignored for the browser, further simplifying
> this code.

The line number selection isn't ignored by the browser, but reported as
an incorrect usage before and after this patch.

> 
> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
> ---
>  git-gui.sh | 66 +++++++++++++-----------------------------------------
>  1 file changed, 16 insertions(+), 50 deletions(-)
> 
> diff --git a/git-gui.sh b/git-gui.sh
> index 6048f92..a951fcd 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -2986,51 +2986,34 @@ blame {
>  	set head {}
>  	set path {}
>  	set jump_spec {}
> -	set is_path 0
> +	set nargs [llength $argv]
> +	if {$nargs < 1} {
> +		usage
> +	}
> +	set argn 0
>  	foreach a $argv {
> -		set p [file join $_prefix $a]
> +		set argn [expr {$argn + 1}]
>  
> -		if {$is_path || [file exists $p]} {
> -			if {$path ne {}} usage
> -			set path [normalize_relpath $p]
> -			break
> -		} elseif {$a eq {--}} {
> -			if {$path ne {}} {
> -				if {$head ne {}} usage
> -				set head $path
> -				set path {}
> +		if {$argn < $nargs} {
> +			# revision or line number
> +			if {[regexp {^--line=(\d+)$} $a a lnum]} {
> +				set jump_spec [list $lnum]
> +			} else {
> +				set head $a
>  			}
> -			set is_path 1
> -		} elseif {[regexp {^--line=(\d+)$} $a a lnum]} {
> -			if {$jump_spec ne {} || $head ne {}} usage
> -			set jump_spec [list $lnum]
> -		} elseif {$head eq {}} {
> -			if {$head ne {}} usage
> -			set head $a
> -			set is_path 1
> -		} else {
> -			usage
> -		}
> -	}
> -	unset is_path
> -
> -	if {$head ne {} && $path eq {}} {
> -		if {[string index $head 0] eq {/}} {
> -			set path [normalize_relpath $head]
> -			set head {}
>  		} else {
> -			set path [normalize_relpath $_prefix$head]
> -			set head {}
> +			set path [normalize_relpath $a]

This loses the capability to request a browser relative to a
subdirectory of the working tree. For example, "git gui browser main ."
now shows the top-level directory instead of the subdirectory when
invoked from a subdirectory.

>  		}
>  	}
>  
>  	if {$head eq {}} {
>  		load_current_branch
> +		set head $current_branch
>  	} else {
>  		if {[regexp [string map "@@ [expr $hashlength - 1]" {^[0-9a-f]{1,@@}$}] $head]} {
>  			if {[catch {
> -					set head [git rev-parse --verify $head]
> -				} err]} {
> +				set head [git rev-parse --verify $head]
> +			} err]} {

Please leave the indentation unchanged.

>  				if {[tk windowingsystem] eq "win32"} {
>  					tk_messageBox -icon error -title [mc Error] -message $err
>  				} else {
> @@ -3046,26 +3029,9 @@ blame {
>  	switch -- $subcommand {
>  	browser {
>  		if {$jump_spec ne {}} usage
> -		if {$head eq {}} {
> -			if {$path ne {} && [file isdirectory $path]} {
> -				set head $current_branch
> -			} else {
> -				set head $path
> -				set path {}
> -			}
> -		}
>  		browser::new $head $path
>  	}
>  	blame   {
> -		if {$head eq {} && ![file exists $path]} {
> -			catch {wm withdraw .}
> -			tk_messageBox \
> -				-icon error \
> -				-type ok \
> -				-title [mc "git-gui: fatal error"] \
> -				-message [mc "fatal: cannot stat path %s: No such file or directory" $path]
> -			exit 1
> -		}
>  		blame::new $head $path $jump_spec
>  	}
>  	}

The check for the existence of files is actually necessary to
disambiguate the meaning of the argument. If a file "maint" exists, then
the argument is to be interpreted as path, not as the ref "maint", even
if that exists, too.

I suggest to protect the "file exists" calls with ($_gitworktree ne {}
&& ...) or (![is_bare] && ...) to handle being invoked from a bare
repository. That is, in a bare repository we treat arguments the same as
files that do not exist in the currently checked-out branch.

-- Hannes

[PATCH v1 03/11] git-gui: guard set/unset of GIT_DIR and GIT_WORK_TREE

[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.

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 \
-- 
2.54.0.99.14

Re: [PATCH v1 03/11] git-gui: guard set/unset of GIT_DIR and GIT_WORK_TREE

[Johannes Sixt]

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 <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

[PATCH v1 04/11] git-gui: put choose_repository::pick in a proc

[Mark Levedahl]

git-gui includes a 'repository picker', which allows creating a new
repository + worktree, or selecting a worktree from a recent list.
git-gui runs the picker when a valid git repository is not found. All of
the code for this is embedded in the discovery process block, making the
latter more difficult to read, and also making things more difficult if
we want to have an explicit 'pick' subcommand to force this to run.

Let's move this invocation and supporting code to a separate proc,
aiding in subsequent refactoring. Assure GIT_DIR and GIT_WORK_TREE are
unset, configuration is loaded, ant that _gitdir is correctly set
afterwards. As this is invoked before worktree discovery, later code
will set that anyway so need not be included here.

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
 git-gui.sh | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/git-gui.sh b/git-gui.sh
index 387cad6..0b73c35 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1139,6 +1139,16 @@ proc unset_gitdir_vars {} {
 }
 
 set picked 0
+proc pick_repo {} {
+	unset_gitdir_vars
+	load_config 1
+	apply_config
+	choose_repository::pick
+	set _gitdir [git rev-parse --absolute-git-dir]
+	set _prefix {}
+	set picked 1
+}
+
 if {[catch {
 		set _gitdir $env(GIT_DIR)
 		set _prefix {}
@@ -1149,13 +1159,7 @@ if {[catch {
 		set _gitdir [git rev-parse --git-dir]
 		set _prefix [git rev-parse --show-prefix]
 	} err]} {
-	load_config 1
-	apply_config
-	choose_repository::pick
-	if {![file isdirectory $_gitdir]} {
-		exit 1
-	}
-	set picked 1
+		pick_repo
 }
 
 # Use object format as hash algorithm (either "sha1" or "sha256")
-- 
2.54.0.99.14

Re: [PATCH v1 04/11] git-gui: put choose_repository::pick in a proc

[Aina Boot]

On 5/14/26 2:33 PM, Mark Levedahl wrote:
>  set picked 0
> +proc pick_repo {} {
> +	unset_gitdir_vars
> +	load_config 1
> +	apply_config
> +	choose_repository::pick
> +	set _gitdir [git rev-parse --absolute-git-dir]
> +	set _prefix {}
> +	set picked 1
> +}
> +
>

Here inside the proc it create vars locally, "global..." is missing.

Aina

Re: [PATCH v1 04/11] git-gui: put choose_repository::pick in a proc

[Mark Levedahl]

On 5/15/26 7:00 AM, Aina Boot wrote:
> On 5/14/26 2:33 PM, Mark Levedahl wrote:
>>  set picked 0
>> +proc pick_repo {} {
>> +	unset_gitdir_vars
>> +	load_config 1
>> +	apply_config
>> +	choose_repository::pick
>> +	set _gitdir [git rev-parse --absolute-git-dir]
>> +	set _prefix {}
>> +	set picked 1
>> +}
>> +
>>
> Here inside the proc it create vars locally, "global..." is missing.
>
> Aina
Yes, also, I missed copying the check on the return variable ::_gitdir. That proc should be:

proc pick_repo {} {
    global _gitdir picked
    unset_gitdir_vars
    load_config 1
    apply_config
    choose_repository::pick
    if {![file isdirectory $_gitdir]} {
        exit 1
    }
    set _gitdir [git rev-parse --absolute-git-dir]
    set picked 1
}

Mark

Re: [PATCH v1 04/11] git-gui: put choose_repository::pick in a proc

[Johannes Sixt]

Am 14.05.26 um 16:33 schrieb Mark Levedahl:
> git-gui includes a 'repository picker', which allows creating a new
> repository + worktree, or selecting a worktree from a recent list.
> git-gui runs the picker when a valid git repository is not found. All of
> the code for this is embedded in the discovery process block, making the
> latter more difficult to read, and also making things more difficult if
> we want to have an explicit 'pick' subcommand to force this to run.

OK, let's see how useful this becomes.

> 
> Let's move this invocation and supporting code to a separate proc,
> aiding in subsequent refactoring. Assure GIT_DIR and GIT_WORK_TREE are
> unset, configuration is loaded, ant that _gitdir is correctly set

s/ant/and/

> afterwards. As this is invoked before worktree discovery, later code
> will set that anyway so need not be included here.
> 
> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
> ---
>  git-gui.sh | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/git-gui.sh b/git-gui.sh
> index 387cad6..0b73c35 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -1139,6 +1139,16 @@ proc unset_gitdir_vars {} {
>  }
>  
>  set picked 0
> +proc pick_repo {} {
> +	unset_gitdir_vars
> +	load_config 1
> +	apply_config
> +	choose_repository::pick
> +	set _gitdir [git rev-parse --absolute-git-dir]
> +	set _prefix {}
> +	set picked 1
> +}
> +

So, this isn't intended as a plain move of code? Since we set _gitdir
here, we could remove the corresonding lines from lib/choose_repository.tcl.

Is the variable "picked" only needed for this particular picker
invocation? Then it should not be set in the function, but at the call site.

>  if {[catch {
>  		set _gitdir $env(GIT_DIR)
>  		set _prefix {}
> @@ -1149,13 +1159,7 @@ if {[catch {
>  		set _gitdir [git rev-parse --git-dir]
>  		set _prefix [git rev-parse --show-prefix]
>  	} err]} {
> -	load_config 1
> -	apply_config
> -	choose_repository::pick
> -	if {![file isdirectory $_gitdir]} {
> -		exit 1
> -	}
> -	set picked 1
> +		pick_repo

The indentation is off here.

>  }
>  
>  # Use object format as hash algorithm (either "sha1" or "sha256")

-- Hannes

[PATCH v1 05/11] git-gui: use --absolute-git-dir

[Mark Levedahl]

git-gui uses git rev-parse --git-dir to get the pathname of the
discovered git repository. The returned value can be relative, and is
'.' if the current directory is the top of the repository directory
itself.  git-gui has code to change '.' to [pwd] in this case so that
subsequent logic runs.

But, git rev-parse supports --absolute-git-dir from fac60b8925
("rev-parse: add option for absolute or relative path formatting",
2020-12-13), and included in git 2.31. git-gui requires git >= 2.36, so
this more useful form is always available. Use --absolute-git-dir to
always get an absolute path, avoiding the need for other checks.
---
 git-gui.sh | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/git-gui.sh b/git-gui.sh
index 0b73c35..c2cf5f1 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1156,7 +1156,7 @@ if {[catch {
 	&& [catch {
 		# beware that from the .git dir this sets _gitdir to .
 		# and _prefix to the empty string
-		set _gitdir [git rev-parse --git-dir]
+		set _gitdir [git rev-parse --absolute-git-dir]
 		set _prefix [git rev-parse --show-prefix]
 	} err]} {
 		pick_repo
@@ -1173,18 +1173,12 @@ if {$hashalgorithm eq "sha1"} {
 	exit 1
 }
 
-# we expand the _gitdir when it's just a single dot (i.e. when we're being
-# run from the .git dir itself) lest the routines to find the worktree
-# get confused
-if {$_gitdir eq "."} {
-	set _gitdir [pwd]
-}
-
 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

Re: [PATCH v1 05/11] git-gui: use --absolute-git-dir

[Johannes Sixt]

Am 14.05.26 um 16:33 schrieb Mark Levedahl:
> git-gui uses git rev-parse --git-dir to get the pathname of the
> discovered git repository. The returned value can be relative, and is
> '.' if the current directory is the top of the repository directory
> itself.  git-gui has code to change '.' to [pwd] in this case so that
> subsequent logic runs.
> 
> But, git rev-parse supports --absolute-git-dir from fac60b8925
> ("rev-parse: add option for absolute or relative path formatting",
> 2020-12-13), and included in git 2.31. git-gui requires git >= 2.36, so
> this more useful form is always available. Use --absolute-git-dir to
> always get an absolute path, avoiding the need for other checks.

Nice!

However, the patch is incomplete. We set _gitdir also from
lib/choose_repository.tcl. I think it would be best to swap this patch
with patch 4/11, remove the _gitdir setters from the picker
implementation, and call `rev-parse --absolute-git-dir` like you did in
4/11. This depends on that the picker sets the current directory to the
top-level of the working tree with the embeded .git directory.

BTW, missing sign-off.

> ---
>  git-gui.sh | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/git-gui.sh b/git-gui.sh
> index 0b73c35..c2cf5f1 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -1156,7 +1156,7 @@ if {[catch {
>  	&& [catch {
>  		# beware that from the .git dir this sets _gitdir to .
>  		# and _prefix to the empty string
> -		set _gitdir [git rev-parse --git-dir]
> +		set _gitdir [git rev-parse --absolute-git-dir]
>  		set _prefix [git rev-parse --show-prefix]
>  	} err]} {
>  		pick_repo
> @@ -1173,18 +1173,12 @@ if {$hashalgorithm eq "sha1"} {
>  	exit 1
>  }
>  
> -# we expand the _gitdir when it's just a single dot (i.e. when we're being
> -# run from the .git dir itself) lest the routines to find the worktree
> -# get confused
> -if {$_gitdir eq "."} {
> -	set _gitdir [pwd]
> -}
> -
>  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

-- Hannes

[PATCH v1 06/11] git gui: GIT_DIR / GIT_WORK_TREE make any discovery error fatal

[Mark Levedahl]

git accepts any combination of GIT_DIR and GIT_WORK_TREE to override the
normal repository and worktree discovery process.  git-gui should accept
any such valid configuration, but overriding the discovery process means
the user has assured that the combination of current directory, GIT_DIR,
and GIT_WORK_TREE will lead to the correct repository and worktree. As
such, an error found during discovery where either or both of GIT_DIR
and GIT_WORK_TREE are set is a fatal error, no further exploration
should be tried.

Provide a common proc to support displaying an error message and exiting
if GIT_DIR or GIT_WORK_TREE are in the environment.

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
 git-gui.sh | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/git-gui.sh b/git-gui.sh
index c2cf5f1..2e2ddc0 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1122,6 +1122,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 {}} {
-- 
2.54.0.99.14

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

[Mark Levedahl]

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. Stop here if
the user set GIT_DIR or GIT_WORK_TREE. Otherwise, continue the existing
behavior and show the repository picker.

Also, remove a later check on whether _gitdir is a directory: that code
cannot be reached without rev-parse having validating the repository.

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
 git-gui.sh | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/git-gui.sh b/git-gui.sh
index 2e2ddc0..81789dd 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@@}
 
@@ -1167,19 +1168,18 @@ proc pick_repo {} {
 	set picked 1
 }
 
+# find repository.
 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
-		set _gitdir [git rev-parse --absolute-git-dir]
-		set _prefix [git rev-parse --show-prefix]
-	} err]} {
+	set _gitdir [git rev-parse --absolute-git-dir]
+} err]} {
+	if {[is_gitvars_error $err]} {
+		exit 1
+	} else {
 		pick_repo
+	}
 }
 
+
 # Use object format as hash algorithm (either "sha1" or "sha256")
 set hashalgorithm [git rev-parse --show-object-format]
 if {$hashalgorithm eq "sha1"} {
@@ -1191,12 +1191,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

Re: [PATCH v1 07/11] git-gui: use rev-parse exclusively to find a repository

[Johannes Sixt]

Am 14.05.26 um 16:33 schrieb Mark Levedahl:
> 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.

OK. If the user gave us GIT_DIR with our without GIT_WORK_TREE, then
that combination better be workable.

> 
> Let's just invoke rev-parse so all error checking is done. Stop here if
> the user set GIT_DIR or GIT_WORK_TREE. Otherwise, continue the existing
> behavior and show the repository picker.

OK. But the paragraph is confusing, because a big "If an error occurs"
is missing after the first sentence.

> 
> Also, remove a later check on whether _gitdir is a directory: that code
> cannot be reached without rev-parse having validating the repository.

Good.

> 
> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
> ---
>  git-gui.sh | 24 +++++++++---------------
>  1 file changed, 9 insertions(+), 15 deletions(-)
> 
> diff --git a/git-gui.sh b/git-gui.sh
> index 2e2ddc0..81789dd 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@@}
>  
> @@ -1167,19 +1168,18 @@ proc pick_repo {} {
>  	set picked 1
>  }
>  
> +# find repository.
>  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
> -		set _gitdir [git rev-parse --absolute-git-dir]
> -		set _prefix [git rev-parse --show-prefix]
> -	} err]} {
> +	set _gitdir [git rev-parse --absolute-git-dir]

Please do also set _prefix. It should fix the bug that the file chooser
uses an empty prefix after

cd lib
GIT_DIR=$PWD/../.git GIT_WORK_TREE=$PWD/.. ../git-gui.sh browser master .

(this is an old bug.)

Please keep the additional indentation of the catch body.

> +} err]} {
> +	if {[is_gitvars_error $err]} {
> +		exit 1
> +	} else {
>  		pick_repo
> +	}

Treat the 'if' as an early exist without an else, and we don't need the
previously strange indentation of 'pick_repo'.

>  }
>  
> +
>  # Use object format as hash algorithm (either "sha1" or "sha256")
>  set hashalgorithm [git rev-parse --show-object-format]
>  if {$hashalgorithm eq "sha1"} {
> @@ -1191,12 +1191,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

(Stopping the review here for today.)

-- Hannes

[PATCH v1 08/11] git-gui: simplify [is_bare] to report if a worktree is known

[Mark Levedahl]

git-gui includes proc is_bare, used in several places to make decisions
on whether a worktree exists, but also in discovery to tell if a
worktree can be supported.

But, is_bare is out of date with regard to multiple worktrees, safe
repository guards, and possibly other relevant features known to git
rev-parse. Also, is_bare caches its result on the first call, so is not
useful if a later step in the discovery process finds a worktree.

So, simplify is_bare to report whether git-gui has a worktree or is
working only from a repository.

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
 git-gui.sh | 25 +------------------------
 1 file changed, 1 insertion(+), 24 deletions(-)

diff --git a/git-gui.sh b/git-gui.sh
index 81789dd..a03eaa7 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -372,7 +372,6 @@ if {[tk windowingsystem] eq "aqua"} {
 set _appname {Git Gui}
 set _gitdir {}
 set _gitworktree {}
-set _isbare {}
 set _githtmldir {}
 set _prefix {}
 set _reponame {}
@@ -524,29 +523,7 @@ proc get_config {name} {
 }
 
 proc is_bare {} {
-	global _isbare
-	global _gitdir
-	global _gitworktree
-
-	if {$_isbare eq {}} {
-		if {[catch {
-			set _bare [git rev-parse --is-bare-repository]
-			switch  -- $_bare {
-			true { set _isbare 1 }
-			false { set _isbare 0}
-			default { throw }
-			}
-		}]} {
-			if {[is_config_true core.bare]
-				|| ($_gitworktree eq {}
-					&& [lindex [file split $_gitdir] end] ne {.git})} {
-				set _isbare 1
-			} else {
-				set _isbare 0
-			}
-		}
-	}
-	return $_isbare
+	return [expr {$::_gitworktree eq {}}]
 }
 
 ######################################################################
-- 
2.54.0.99.14

Re: [PATCH v1 08/11] git-gui: simplify [is_bare] to report if a worktree is known

[Johannes Sixt]

Am 14.05.26 um 16:33 schrieb Mark Levedahl:
> git-gui includes proc is_bare, used in several places to make decisions
> on whether a worktree exists, but also in discovery to tell if a
> worktree can be supported.
> 
> But, is_bare is out of date with regard to multiple worktrees, safe
> repository guards, and possibly other relevant features known to git
> rev-parse. Also, is_bare caches its result on the first call, so is not
> useful if a later step in the discovery process finds a worktree.
> 
> So, simplify is_bare to report whether git-gui has a worktree or is
> working only from a repository.
> 
> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
> ---
>  git-gui.sh | 25 +------------------------
>  1 file changed, 1 insertion(+), 24 deletions(-)
> 
> diff --git a/git-gui.sh b/git-gui.sh
> index 81789dd..a03eaa7 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -372,7 +372,6 @@ if {[tk windowingsystem] eq "aqua"} {
>  set _appname {Git Gui}
>  set _gitdir {}
>  set _gitworktree {}
> -set _isbare {}
>  set _githtmldir {}
>  set _prefix {}
>  set _reponame {}
> @@ -524,29 +523,7 @@ proc get_config {name} {
>  }
>  
>  proc is_bare {} {
> -	global _isbare
> -	global _gitdir
> -	global _gitworktree
> -
> -	if {$_isbare eq {}} {
> -		if {[catch {
> -			set _bare [git rev-parse --is-bare-repository]
> -			switch  -- $_bare {
> -			true { set _isbare 1 }
> -			false { set _isbare 0}
> -			default { throw }
> -			}
> -		}]} {
> -			if {[is_config_true core.bare]
> -				|| ($_gitworktree eq {}
> -					&& [lindex [file split $_gitdir] end] ne {.git})} {
> -				set _isbare 1
> -			} else {
> -				set _isbare 0
> -			}
> -		}
> -	}
> -	return $_isbare
> +	return [expr {$::_gitworktree eq {}}]
>  }
>  
>  ######################################################################

Very nice!

IMO, regardless of which way we end up rewriting repository discovery,
the end result should be that we can use $_gitworktree like this to tell
whether we are in a bare repository or not.

-- Hannes

[PATCH v1 09/11] git-gui: support using repository parent dir as a worktree

[Mark Levedahl]

git-gui, since 87cd09f43e ("git-gui: work from the .git dir",
2010-01-23), has had the intent to allow starting from inside a
repository, then switching to the parent directory if that is a valid
worktree.

This certainly hasn't worked since 2d92ab32fd ("rev-parse: make
--show-toplevel without a worktree an error", 2019-11-19) in git, but
breaking this git-gui feature was unintentional.

Add a proc to test if the parent of the git repository is a valid
worktree, and set that directory as the worktree if so. Use invocations
of git rev-parse to assure all validity and safety checks included in
git-core are executed.
---
 git-gui.sh | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/git-gui.sh b/git-gui.sh
index a03eaa7..e326401 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1100,6 +1100,23 @@ unset argv0dir
 ##
 ## repository setup
 
+proc is_parent_worktree {} {
+	# Directory 'parent' of a repository named 'parent/.git' might be the worktree
+	set ok 0
+	if {[file tail $::_gitdir] eq {.git}} {
+		set gitdir_parent [file join $::_gitdir {..}]
+		set expected_worktree [file normalize $gitdir_parent]
+		catch {set git_worktree [git -C $gitdir_parent rev-parse --show-toplevel]}
+		if {[string compare $expected_worktree $git_worktree] == 0} {
+			set ::_prefix {}
+			set ::_gitworktree $git_worktree
+			cd $git_worktree
+			set ok 1
+		}
+	}
+	return $ok
+}
+
 proc is_gitvars_error {err} {
 	set havevars 0
 	set GIT_DIR {}
-- 
2.54.0.99.14

Re: [PATCH v1 09/11] git-gui: support using repository parent dir as a worktree

[Johannes Sixt]

Am 14.05.26 um 16:33 schrieb Mark Levedahl:
> git-gui, since 87cd09f43e ("git-gui: work from the .git dir",
> 2010-01-23), has had the intent to allow starting from inside a
> repository, then switching to the parent directory if that is a valid
> worktree.
> 
> This certainly hasn't worked since 2d92ab32fd ("rev-parse: make
> --show-toplevel without a worktree an error", 2019-11-19) in git, but
> breaking this git-gui feature was unintentional.
> 
> Add a proc to test if the parent of the git repository is a valid
> worktree, and set that directory as the worktree if so. Use invocations
> of git rev-parse to assure all validity and safety checks included in
> git-core are executed.

BTW, missing sign-off.

> ---
>  git-gui.sh | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/git-gui.sh b/git-gui.sh
> index a03eaa7..e326401 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -1100,6 +1100,23 @@ unset argv0dir
>  ##
>  ## repository setup
>  
> +proc is_parent_worktree {} {
> +	# Directory 'parent' of a repository named 'parent/.git' might be the worktree
> +	set ok 0
> +	if {[file tail $::_gitdir] eq {.git}} {
> +		set gitdir_parent [file join $::_gitdir {..}]
> +		set expected_worktree [file normalize $gitdir_parent]

We have [file dirname ...]. Is there a reason to avoid it?

> +		catch {set git_worktree [git -C $gitdir_parent rev-parse --show-toplevel]}
> +		if {[string compare $expected_worktree $git_worktree] == 0} {

The purpose of this check should be explained in a comment. I think it is:

For a repository with the database in a directory named .git we assume
that the working tree is the directory containing .git. But
configuration may point to a different worktree. Then we do not want to
hold on to our assumption.

However, whether [git -C elsewhere ...] uses the same gitdir that we
have discovered so far cannot be told from this piece of code alone.
Therefore, I think it is wrong to extract this check into a function.

Also, I don't think we can use string comparison here. On Windows, the
command returns the Windows style path, but Tcl my operate with a POSIX
style path.

> +			set ::_prefix {}
> +			set ::_gitworktree $git_worktree
> +			cd $git_worktree

So many side-effects in a function whose name suggests that it only does
some checks. Please, don't do that.

> +			set ok 1
> +		}
> +	}
> +	return $ok
> +}
> +
>  proc is_gitvars_error {err} {
>  	set havevars 0
>  	set GIT_DIR {}

In general, I am not a fan of commits that add new functions, but no
call sites. Please squash this into 10/11. Ditto for is_gitvars_error in
06/11.

-- Hannes

[PATCH v1 10/11] git-gui: improve worktree discovery

[Mark Levedahl]

git gui's worktree discovery needs update based upon prior work in this
series. In the normal case, all information we need comes directly from
git rev-parse (--show-toplevel, and --show-prefix). Should this work, we
have a valid worktree and all git gui commands can run.

If not, we need to consider:
- if GIT_DIR or GIT_WORK_TREE are in the environment, just stop as we
  the input configuration was wrong, the user must fix that.
- if we have a browser or blame subcommand, no worktree is needed so
  git-gui can run without.
- using the git repository's parent is a valid worktree (if possible),
  restoring prior behavior.

The current directory should be either the root of the worktree, if one
is found, or the top-level of the git repository.

Make it so. Also, make worktree discover directly follow repository
discovery, reducing the locations that might need error trapping to
catch configuration issues.

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
 git-gui.sh | 56 ++++++++++++++++++++++--------------------------------
 1 file changed, 23 insertions(+), 33 deletions(-)

diff --git a/git-gui.sh b/git-gui.sh
index e326401..3a83dd5 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1173,6 +1173,28 @@ if {[catch {
 	}
 }
 
+# find worktree, continue without if not required
+if {[catch {
+	set _gitworktree [git rev-parse --show-toplevel]
+	set _prefix [git rev-parse --show-prefix]
+	cd $_gitworktree
+} err]} {
+	if {[is_gitvars_error $err]} {
+		exit 1
+	}
+	set _gitworktree {}
+	set _prefix {}
+	if {[is_enabled bare]} {
+		cd $_gitdir
+	} elseif {![is_parent_worktree]} {
+		catch {wm withdraw .}
+		error_popup [strcat [mc "Cannot use bare repository:"] "\n\n" $_gitdir]
+		exit 1
+	}
+}
+
+# repository and worktree config are complete, export them
+set_gitdir_vars
 
 # Use object format as hash algorithm (either "sha1" or "sha256")
 set hashalgorithm [git rev-parse --show-object-format]
@@ -1189,37 +1211,8 @@ if {$hashalgorithm eq "sha1"} {
 load_config 0
 apply_config
 
-set _gitworktree [git rev-parse --show-toplevel]
 
-if {$_prefix ne {}} {
-	if {$_gitworktree eq {}} {
-		regsub -all {[^/]+/} $_prefix ../ cdup
-	} else {
-		set cdup $_gitworktree
-	}
-	if {[catch {cd $cdup} err]} {
-		catch {wm withdraw .}
-		error_popup [strcat [mc "Cannot move to top of working directory:"] "\n\n$err"]
-		exit 1
-	}
-	set _gitworktree [pwd]
-	unset cdup
-} elseif {![is_enabled bare]} {
-	if {[is_bare]} {
-		catch {wm withdraw .}
-		error_popup [strcat [mc "Cannot use bare repository:"] "\n\n$_gitdir"]
-		exit 1
-	}
-	if {$_gitworktree eq {}} {
-		set _gitworktree [file dirname $_gitdir]
-	}
-	if {[catch {cd $_gitworktree} err]} {
-		catch {wm withdraw .}
-		error_popup [strcat [mc "No working directory"] " $_gitworktree:\n\n$err"]
-		exit 1
-	}
-	set _gitworktree [pwd]
-}
+# Derive a human-readable repository name
 set _reponame [file split [file normalize $_gitdir]]
 if {[lindex $_reponame end] eq {.git}} {
 	set _reponame [lindex $_reponame end-1]
@@ -1227,9 +1220,6 @@ if {[lindex $_reponame end] eq {.git}} {
 	set _reponame [lindex $_reponame end]
 }
 
-# Export the final paths
-set_gitdir_vars
-
 ######################################################################
 ##
 ## global init
-- 
2.54.0.99.14

Re: [PATCH v1 10/11] git-gui: improve worktree discovery

[Johannes Sixt]

Am 14.05.26 um 16:33 schrieb Mark Levedahl:
> git gui's worktree discovery needs update based upon prior work in this
> series. In the normal case, all information we need comes directly from
> git rev-parse (--show-toplevel, and --show-prefix). Should this work, we
> have a valid worktree and all git gui commands can run.
> 
> If not, we need to consider:
> - if GIT_DIR or GIT_WORK_TREE are in the environment, just stop as we
>   the input configuration was wrong, the user must fix that.
> - if we have a browser or blame subcommand, no worktree is needed so
>   git-gui can run without.
> - using the git repository's parent is a valid worktree (if possible),
>   restoring prior behavior.
> 
> The current directory should be either the root of the worktree, if one
> is found, or the top-level of the git repository.

I disagree in the case where no working tree is found. Then there is no
point in changing the current directory.

> 
> Make it so. Also, make worktree discover directly follow repository
> discovery, reducing the locations that might need error trapping to
> catch configuration issues.

Good!

> 
> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>

BTW, please make the subject line more descriptive. The word "improve"
does not convey anything of importance.

> ---
>  git-gui.sh | 56 ++++++++++++++++++++++--------------------------------
>  1 file changed, 23 insertions(+), 33 deletions(-)
> 
> diff --git a/git-gui.sh b/git-gui.sh
> index e326401..3a83dd5 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -1173,6 +1173,28 @@ if {[catch {
>  	}
>  }
>  
> +# find worktree, continue without if not required
> +if {[catch {
> +	set _gitworktree [git rev-parse --show-toplevel]
> +	set _prefix [git rev-parse --show-prefix]
> +	cd $_gitworktree
> +} err]} {

We have three commands, each with their own possible failure sources.
One of the outcomes of an error is that we proceed anyway. I think that
this is incorrect if the 'cd' fails: we must not proceed if it fails.
Therefore, we must handle its failure separately.

> +	if {[is_gitvars_error $err]} {
> +		exit 1
> +	}
> +	set _gitworktree {}
> +	set _prefix {}
> +	if {[is_enabled bare]} {
> +		cd $_gitdir

Why change the directory here? If we run `git gui browser master dir` we
do not want to change the directory in an uncontrolled manner. The
argument parser will want to check for the existence of files, and then
we do not want to operate from a random directory.

Also, I think that the check must be for [is_bare] and not [is_enabled
bare].

> +	} elseif {![is_parent_worktree]} {
> +		catch {wm withdraw .}
> +		error_popup [strcat [mc "Cannot use bare repository:"] "\n\n" $_gitdir]
> +		exit 1
> +	}
> +}
> +
> +# repository and worktree config are complete, export them
> +set_gitdir_vars
>  
>  # Use object format as hash algorithm (either "sha1" or "sha256")
>  set hashalgorithm [git rev-parse --show-object-format]

This moves code around. In particular, we see load_config and
apply_config in the context below, which now happens only after these
calls. How certain are we that these have no effect on the code that
runs now earlier?

> @@ -1189,37 +1211,8 @@ if {$hashalgorithm eq "sha1"} {
>  load_config 0
>  apply_config
>  
> -set _gitworktree [git rev-parse --show-toplevel]
>  
> -if {$_prefix ne {}} {
> -	if {$_gitworktree eq {}} {
> -		regsub -all {[^/]+/} $_prefix ../ cdup
> -	} else {
> -		set cdup $_gitworktree
> -	}
> -	if {[catch {cd $cdup} err]} {
> -		catch {wm withdraw .}
> -		error_popup [strcat [mc "Cannot move to top of working directory:"] "\n\n$err"]
> -		exit 1
> -	}
> -	set _gitworktree [pwd]
> -	unset cdup
> -} elseif {![is_enabled bare]} {
> -	if {[is_bare]} {
> -		catch {wm withdraw .}
> -		error_popup [strcat [mc "Cannot use bare repository:"] "\n\n$_gitdir"]
> -		exit 1
> -	}
> -	if {$_gitworktree eq {}} {
> -		set _gitworktree [file dirname $_gitdir]
> -	}
> -	if {[catch {cd $_gitworktree} err]} {
> -		catch {wm withdraw .}
> -		error_popup [strcat [mc "No working directory"] " $_gitworktree:\n\n$err"]
> -		exit 1
> -	}
> -	set _gitworktree [pwd]
> -}
> +# Derive a human-readable repository name
>  set _reponame [file split [file normalize $_gitdir]]
>  if {[lindex $_reponame end] eq {.git}} {
>  	set _reponame [lindex $_reponame end-1]
> @@ -1227,9 +1220,6 @@ if {[lindex $_reponame end] eq {.git}} {
>  	set _reponame [lindex $_reponame end]
>  }
>  
> -# Export the final paths
> -set_gitdir_vars
> -
>  ######################################################################
>  ##
>  ## global init

-- Hannes

[PATCH v1 11/11] git-gui: add gui and pick as explicit subcommands

[Mark Levedahl]

git-gui accepts subcommands blame | browser | citool, and assumes the
subcommand is 'gui' if none is actually given, But, git gui also has a
repository picker (choose_repository::pick) that can create a new
repository + worktree, or choose an existing one, switch to that, and
the run the gui. The user has no direct control over invoking the
picker, instead the picker is triggered by failure in the repository /
worktree discover process: this includes being started in a directory
not controlled by git, which is probably the intended use case.

The picker can appear when the user has no intention of creating a new
worktree, and the user cannot use the picker to create a new worktree
inside another.

So, add two new explicit subcommands:
    gui  - Run the gui if repository/worktree discovery succeeds, or die
           with an error message, but never run the picker.
    pick - First run the picker, regardless, then start the gui in
           the chosen worktree.

Nothing in this changes the prior behavior, the alternates above must be
explicitly selected to see any change.

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
 git-gui.sh | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/git-gui.sh b/git-gui.sh
index 3a83dd5..c56aeef 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1021,6 +1021,7 @@ proc load_config {include_global} {
 ##
 ## feature option selection
 
+set run_picker_on_error 1
 if {[regexp {^git-(.+)$} [file tail $argv0] _junk subcommand]} {
 	unset _junk
 } else {
@@ -1030,6 +1031,7 @@ if {$subcommand eq {gui.sh}} {
 	set subcommand gui
 }
 if {$subcommand eq {gui} && [llength $argv] > 0} {
+	set run_picker_on_error 0
 	set subcommand [lindex $argv 0]
 	set argv [lrange $argv 1 end]
 }
@@ -1047,6 +1049,7 @@ blame {
 	disable_option multicommit
 	disable_option branch
 	disable_option transport
+	set run_picker_on_error 0
 }
 citool {
 	enable_option singlecommit
@@ -1055,6 +1058,7 @@ citool {
 	disable_option multicommit
 	disable_option branch
 	disable_option transport
+	set run_picker_on_error 0
 
 	while {[llength $argv] > 0} {
 		set a [lindex $argv 0]
@@ -1162,14 +1166,28 @@ proc pick_repo {} {
 	set picked 1
 }
 
+# run repository picker if explicitly requested
+switch -- $subcommand {
+	pick {
+		pick_repo
+		set subcommand gui
+		set run_picker_on_error 0
+	}
+}
+
 # find repository.
 if {[catch {
 	set _gitdir [git rev-parse --absolute-git-dir]
 } err]} {
 	if {[is_gitvars_error $err]} {
 		exit 1
-	} else {
+	}
+	if {$run_picker_on_error} {
 		pick_repo
+	} else {
+		catch {wm withdraw .}
+		error_popup [strcat [mc "Git directory not found:"] "\n\n$err"]
+		exit 1
 	}
 }
 
@@ -3051,7 +3069,7 @@ gui {
 	# fall through to setup UI for commits
 }
 default {
-	set err "[mc usage:] $argv0 \[{blame|browser|citool}\]"
+	set err "[mc usage:] $argv0 \[{blame|browser|citool|gui|pick}\]"
 	if {[tk windowingsystem] eq "win32"} {
 		wm withdraw .
 		tk_messageBox -icon error -message $err \
-- 
2.54.0.99.14

Re: [PATCH v1 11/11] git-gui: add gui and pick as explicit subcommands

[Johannes Sixt]

Am 14.05.26 um 16:33 schrieb Mark Levedahl:
> git-gui accepts subcommands blame | browser | citool, and assumes the
> subcommand is 'gui' if none is actually given, But, git gui also has a
> repository picker (choose_repository::pick) that can create a new
> repository + worktree, or choose an existing one, switch to that, and
> the run the gui. The user has no direct control over invoking the
> picker, instead the picker is triggered by failure in the repository /
> worktree discover process: this includes being started in a directory
> not controlled by git, which is probably the intended use case.
> 
> The picker can appear when the user has no intention of creating a new
> worktree, and the user cannot use the picker to create a new worktree
> inside another.
> 
> So, add two new explicit subcommands:
>     gui  - Run the gui if repository/worktree discovery succeeds, or die
>            with an error message, but never run the picker.
>     pick - First run the picker, regardless, then start the gui in
>            the chosen worktree.
> 
> Nothing in this changes the prior behavior, the alternates above must be
> explicitly selected to see any change.

OK.

> 
> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
> ---
>  git-gui.sh | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/git-gui.sh b/git-gui.sh
> index 3a83dd5..c56aeef 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -1021,6 +1021,7 @@ proc load_config {include_global} {
>  ##
>  ## feature option selection
>  
> +set run_picker_on_error 1
>  if {[regexp {^git-(.+)$} [file tail $argv0] _junk subcommand]} {
>  	unset _junk
>  } else {
> @@ -1030,6 +1031,7 @@ if {$subcommand eq {gui.sh}} {
>  	set subcommand gui
>  }
>  if {$subcommand eq {gui} && [llength $argv] > 0} {
> +	set run_picker_on_error 0
>  	set subcommand [lindex $argv 0]
>  	set argv [lrange $argv 1 end]
>  }
> @@ -1047,6 +1049,7 @@ blame {
>  	disable_option multicommit
>  	disable_option branch
>  	disable_option transport
> +	set run_picker_on_error 0
>  }
>  citool {
>  	enable_option singlecommit
> @@ -1055,6 +1058,7 @@ citool {
>  	disable_option multicommit
>  	disable_option branch
>  	disable_option transport
> +	set run_picker_on_error 0
>  
>  	while {[llength $argv] > 0} {
>  		set a [lindex $argv 0]

Can we please use the available disable_option and enable_option feature
instead of a new variable. Just for consistency around repository discovery.

> @@ -1162,14 +1166,28 @@ proc pick_repo {} {
>  	set picked 1
>  }
>  
> +# run repository picker if explicitly requested
> +switch -- $subcommand {
> +	pick {
> +		pick_repo
> +		set subcommand gui
> +		set run_picker_on_error 0
> +	}
> +}
> +

It just feels wrong to have a new pick_repo call before repository
discovery. Can we not treat this case below as if regular repository
discovery failed and then end up in the existing call of pick_repo?

>  # find repository.
>  if {[catch {
>  	set _gitdir [git rev-parse --absolute-git-dir]
>  } err]} {
>  	if {[is_gitvars_error $err]} {
>  		exit 1
> -	} else {
> +	}
> +	if {$run_picker_on_error} {
>  		pick_repo
> +	} else {
> +		catch {wm withdraw .}
> +		error_popup [strcat [mc "Git directory not found:"] "\n\n$err"]
> +		exit 1
>  	}
>  }
>  
> @@ -3051,7 +3069,7 @@ gui {
>  	# fall through to setup UI for commits
>  }
>  default {
> -	set err "[mc usage:] $argv0 \[{blame|browser|citool}\]"
> +	set err "[mc usage:] $argv0 \[{blame|browser|citool|gui|pick}\]"
>  	if {[tk windowingsystem] eq "win32"} {
>  		wm withdraw .
>  		tk_messageBox -icon error -message $err \

We don't need to switch on the new subcommands?

As a follow-up to my comment on 04/11: How relevant is it that variable
$picked is set in a 'git gui pick' invocation?

-- Hannes

Re: [PATCH v1 00/11] Improve git gui operation without a worktree

[Johannes Sixt]

Am 14.05.26 um 16:33 schrieb Mark Levedahl:
> 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 select a worktree
>   from a list and/or start a new repo+worktree: this dialog appears at
>   unexpected times, masking useful error feedback on configuration
>   problems.
> 
> This patch series addresses the above issues, substantially rewriting
> the blame / browser command line process, the initial repository and
> worktree discovery processes, and using git rev-parse when possible to
> handle repository / worktree discovery including any specification of
> GIT_DIR or GIT_WORK_TREE to reduce the future likelihood of conflict
> with command line git. This also allows explicit user control to avoid
> the repository picker masking a configuration error.

OK. Overall, this goes in the right direction. There are still open
questions and potential problems with this implementation. We also
disagree in a few details; see my comments on the patches.

> 
> Note: I question why git-gui ever exports GIT_WORK_TREE. If it is not
> empty, that is the current directory when startup is complete and any
> git command will use the current directory as the worktree.

I fully agree with this.

> If empty,
> there is no worktree and the current directory should be (and after this
> series, is) at the toplevel of the gitdir: again, there is nothing to
> communicate to another process.

Here I disagree. We should not need to change directory if no working
tree was found.

> If a process being launched needs a
> different worktree, that should be the startup directory given to the
> process without changing git-gui's current directory.

I haven't thought this through, but this sounds very reasonable.

> 
> Mark Levedahl (11):
>   git-gui: allow specifying path '.' to the browser
>   git-gui: refactor browser / blame argument parsing
>   git-gui: guard set/unset of GIT_DIR and GIT_WORK_TREE
>   git-gui: put choose_repository::pick in a proc
>   git-gui: use --absolute-git-dir
>   git gui: GIT_DIR / GIT_WORK_TREE make any discovery error fatal
>   git-gui: use rev-parse exclusively to find a repository
>   git-gui: simplify [is_bare] to report if a worktree is known
>   git-gui: support using repository parent dir as a worktree
>   git-gui: improve worktree discovery
>   git-gui: add gui and pick as explicit subcommands
> 
>  git-gui.sh | 276 ++++++++++++++++++++++++++---------------------------
>  1 file changed, 135 insertions(+), 141 deletions(-)
> 

-- Hannes

[PATCH v7 1/3] git-gui: restructure repository startup

[Shroom Moo]

When git-gui is started inside a .git directory of a non-bare
repository, it should treat the parent directory as the worktree,
as it did before commit 2d92ab32fd (rev-parse: make --show-toplevel
without a worktree an error, 2019-11-19).  However, a bare repository
or a separated gitdir without a worktree must be rejected early.

Protect the previously unguarded calls to `git rev-parse
--show-object-format` and `--show-toplevel`.  Restructure the startup
sequence to:

- Check for a bare repository right after loading the config.  If the
  repository is bare and the current subcommand does not allow bare
  repos (e.g. normal commit mode), show "Cannot use bare repository"
  and exit.

- When `rev-parse --show-toplevel` fails and the repository is
  non-bare, the gitdir path ends with ".git", and we are inside that
  gitdir, use the parent directory as the worktree.  This preserves
  the ability to start git-gui from within a regular repository’s
  .git directory, which was intentionally supported since 87cd09f43e56
  (git-gui: work from the .git dir, 2010-01-23).

- Otherwise, show a descriptive error and exit.

- Wrap `rev-parse --show-object-format` in a catch to avoid a crash
  when the repository configuration is broken (e.g. core.worktree
  pointing to an invalid path).

Also removes the old `_prefix`‑based fallback that computed a relative
path to the worktree top from a subdirectory, and the unconditional
`[file dirname $_gitdir]` guess.  Both are unnecessary now that
`rev‑parse --show‑toplevel` directly provides the absolute top‑level
path and we can `cd` to it.  The guess is further unsafe in
multi‑worktree setups, where a gitdir may have more than one worktree.
The only remaining fallback is the explicit “.git directory” rule for
non‑bare repositories, which mirrors the historical behaviour.
Additionally, only export GIT_WORK_TREE when it is not empty, to avoid
confusing commands in bare-repository subcommands.

This fixes the fatal Tcl error when the working tree is missing, while
keeping the .git startup feature and avoiding any automatic directory
switching that could be dangerous in multi‑worktree setups.

Helped-by: Johannes Sixt <j6t@kdbg.org>
Helped-by: Mark Levedahl <mlevedahl@gmail.com>
Signed-off-by: Shroom Moo <egg_mushroomcow@foxmail.com>
---
 git-gui/git-gui.sh | 76 ++++++++++++++++++++++++++++++----------------
 1 file changed, 49 insertions(+), 27 deletions(-)

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index 23fe76e498..9eb93a76b5 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -1129,7 +1129,8 @@ if {[catch {
 		}]
 	&& [catch {
 		# beware that from the .git dir this sets _gitdir to .
-		# and _prefix to the empty string
+		# and _prefix to the empty string; this is handled by
+		# the startup safety checks below
 		set _gitdir [git rev-parse --git-dir]
 		set _prefix [git rev-parse --show-prefix]
 	} err]} {
@@ -1142,8 +1143,20 @@ if {[catch {
 	set picked 1
 }
 
+if {![file isdirectory $_gitdir]} {
+	catch {wm withdraw .}
+	error_popup [strcat 
+		[mc "Git directory not found:"] "\n\n$_gitdir\n\n" \
+		[mc "Please ensure GIT_DIR points to a valid Git repository"]]
+	exit 1
+}
+
 # Use object format as hash algorithm (either "sha1" or "sha256")
-set hashalgorithm [git rev-parse --show-object-format]
+if {[catch {set hashalgorithm [git rev-parse --show-object-format]} err]} {
+	catch {wm withdraw .}
+	error_popup [strcat [mc "Failed to determine hash algorithm:"] "\n\n$err"]
+	exit 1
+}
 if {$hashalgorithm eq "sha1"} {
 	set hashlength 40
 } elseif {$hashalgorithm eq "sha256"} {
@@ -1160,46 +1173,52 @@ if {$_gitdir eq "."} {
 	set _gitdir [pwd]
 }
 
-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
 
-set _gitworktree [git rev-parse --show-toplevel]
-
-if {$_prefix ne {}} {
-	if {$_gitworktree eq {}} {
-		regsub -all {[^/]+/} $_prefix ../ cdup
-	} else {
-		set cdup $_gitworktree
-	}
-	if {[catch {cd $cdup} err]} {
+# Handle bare repository and determine working tree
+if {[is_bare]} {
+	# Bare repository: only allowed for certain subcommands
+	if {![is_enabled bare]} {
 		catch {wm withdraw .}
-		error_popup [strcat [mc "Cannot move to top of working directory:"] "\n\n$err"]
+		error_popup [strcat [mc "Cannot use bare repository:"] "\n\n" [file normalize $_gitdir]]
 		exit 1
 	}
-	set _gitworktree [pwd]
-	unset cdup
-} elseif {![is_enabled bare]} {
-	if {[is_bare]} {
-		catch {wm withdraw .}
-		error_popup [strcat [mc "Cannot use bare repository:"] "\n\n$_gitdir"]
-		exit 1
+	# Allowed bare repo does not have a worktree
+	set _gitworktree {}
+} else {
+	# Non-bare repository: we must find a worktree
+	if {[catch {set _gitworktree [git rev-parse --show-toplevel]} err]} {
+		# The only acceptable failure is when we are inside
+		# the .git directory of a regular repository.
+		set inside_gitdir 0
+		catch {set inside_gitdir [git rev-parse --is-inside-git-dir]}
+		if {$inside_gitdir eq {true} && [file tail $_gitdir] eq {.git}} {
+			# Use the parent directory as worktree (historic behavior)
+			set _gitworktree [file normalize [file dirname $_gitdir]]
+		} else {
+			catch {wm withdraw .}
+			error_popup [strcat [mc "Cannot determine working tree:"] "\n\n$err"]
+			exit 1
+		}
 	}
+
 	if {$_gitworktree eq {}} {
-		set _gitworktree [file dirname $_gitdir]
+		catch {wm withdraw .}
+		error_popup [mc "Cannot determine working tree (unexpected empty result)"]
+		exit 1
 	}
+
 	if {[catch {cd $_gitworktree} err]} {
 		catch {wm withdraw .}
-		error_popup [strcat [mc "No working directory"] " $_gitworktree:\n\n$err"]
+		error_popup [strcat [mc "Cannot move to working directory:"] "\n\n$err"]
 		exit 1
 	}
 	set _gitworktree [pwd]
 }
+
+# Derive a human-readable repository name
 set _reponame [file split [file normalize $_gitdir]]
 if {[lindex $_reponame end] eq {.git}} {
 	set _reponame [lindex $_reponame end-1]
@@ -1207,8 +1226,11 @@ if {[lindex $_reponame end] eq {.git}} {
 	set _reponame [lindex $_reponame end]
 }
 
+# Export the final paths
 set env(GIT_DIR) $_gitdir
-set env(GIT_WORK_TREE) $_gitworktree
+if {$_gitworktree ne {}} {
+	set env(GIT_WORK_TREE) $_gitworktree
+}
 
 ######################################################################
 ##
-- 
2.52.0.windows.1

Re: [PATCH v7 1/3] git-gui: restructure repository startup

[Johannes Sixt]

Am 09.05.26 um 15:37 schrieb Shroom Moo:
> When git-gui is started inside a .git directory of a non-bare
> repository, it should treat the parent directory as the worktree,
> as it did before commit 2d92ab32fd (rev-parse: make --show-toplevel
> without a worktree an error, 2019-11-19).  However, a bare repository
> or a separated gitdir without a worktree must be rejected early.
> > Protect the previously unguarded calls to `git rev-parse
> --show-object-format` and `--show-toplevel`.  Restructure the startup
> sequence to:
> 
> - Check for a bare repository right after loading the config.  If the
>   repository is bare and the current subcommand does not allow bare
>   repos (e.g. normal commit mode), show "Cannot use bare repository"
>   and exit.
> 
> - When `rev-parse --show-toplevel` fails and the repository is
>   non-bare, the gitdir path ends with ".git", and we are inside that
>   gitdir, use the parent directory as the worktree.  This preserves
>   the ability to start git-gui from within a regular repository’s
>   .git directory, which was intentionally supported since 87cd09f43e56
>   (git-gui: work from the .git dir, 2010-01-23).
> 
> - Otherwise, show a descriptive error and exit.
> 
> - Wrap `rev-parse --show-object-format` in a catch to avoid a crash
>   when the repository configuration is broken (e.g. core.worktree
>   pointing to an invalid path).
> 
> Also removes the old `_prefix`‑based fallback that computed a relative
> path to the worktree top from a subdirectory, and the unconditional
> `[file dirname $_gitdir]` guess.  Both are unnecessary now that
> `rev‑parse --show‑toplevel` directly provides the absolute top‑level
> path and we can `cd` to it.  The guess is further unsafe in
> multi‑worktree setups, where a gitdir may have more than one worktree.
> The only remaining fallback is the explicit “.git directory” rule for
> non‑bare repositories, which mirrors the historical behaviour.
> Additionally, only export GIT_WORK_TREE when it is not empty, to avoid
> confusing commands in bare-repository subcommands.
> 
> This fixes the fatal Tcl error when the working tree is missing, while
> keeping the .git startup feature and avoiding any automatic directory
> switching that could be dangerous in multi‑worktree setups.

I think that the end result is useful. However, frankly, the patch
attempts to do too many things at once and should still be split further:

- The removal of the cdup fallback could be a preliminary patch.

- The protection of --show-object-format could be a follow-up patch.

> Helped-by: Johannes Sixt <j6t@kdbg.org>
> Helped-by: Mark Levedahl <mlevedahl@gmail.com>
> Signed-off-by: Shroom Moo <egg_mushroomcow@foxmail.com>
> ---
>  git-gui/git-gui.sh | 76 ++++++++++++++++++++++++++++++----------------
>  1 file changed, 49 insertions(+), 27 deletions(-)
> 
> diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
> index 23fe76e498..9eb93a76b5 100755
> --- a/git-gui/git-gui.sh
> +++ b/git-gui/git-gui.sh
> @@ -1129,7 +1129,8 @@ if {[catch {
>  		}]
>  	&& [catch {
>  		# beware that from the .git dir this sets _gitdir to .
> -		# and _prefix to the empty string
> +		# and _prefix to the empty string; this is handled by
> +		# the startup safety checks below
>  		set _gitdir [git rev-parse --git-dir]
>  		set _prefix [git rev-parse --show-prefix]
>  	} err]} {
> @@ -1142,8 +1143,20 @@ if {[catch {
>  	set picked 1
>  }
>  
> +if {![file isdirectory $_gitdir]} {
> +	catch {wm withdraw .}
> +	error_popup [strcat 
> +		[mc "Git directory not found:"] "\n\n$_gitdir\n\n" \
> +		[mc "Please ensure GIT_DIR points to a valid Git repository"]]
> +	exit 1
> +}
> +

This was moved from below. I would appreciated if there were no changes
in the moved code so that `git diff --color-moved` can show that no
changes were intended. I am not sure that the additional sentence that
mentions GIT_DIR is warranted. If you feel it is needed, please add it
in a separate patch with a justification.

>  # Use object format as hash algorithm (either "sha1" or "sha256")
> -set hashalgorithm [git rev-parse --show-object-format]
> +if {[catch {set hashalgorithm [git rev-parse --show-object-format]} err]} {
> +	catch {wm withdraw .}
> +	error_popup [strcat [mc "Failed to determine hash algorithm:"] "\n\n$err"]
> +	exit 1
> +}
>  if {$hashalgorithm eq "sha1"} {
>  	set hashlength 40
>  } elseif {$hashalgorithm eq "sha256"} {
> @@ -1160,46 +1173,52 @@ if {$_gitdir eq "."} {
>  	set _gitdir [pwd]
>  }
>  
> -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
>  
> -set _gitworktree [git rev-parse --show-toplevel]
> -
> -if {$_prefix ne {}} {
> -	if {$_gitworktree eq {}} {
> -		regsub -all {[^/]+/} $_prefix ../ cdup
> -	} else {
> -		set cdup $_gitworktree
> -	}
> -	if {[catch {cd $cdup} err]} {
> +# Handle bare repository and determine working tree
> +if {[is_bare]} {
> +	# Bare repository: only allowed for certain subcommands
> +	if {![is_enabled bare]} {
>  		catch {wm withdraw .}
> -		error_popup [strcat [mc "Cannot move to top of working directory:"] "\n\n$err"]
> +		error_popup [strcat [mc "Cannot use bare repository:"] "\n\n" [file normalize $_gitdir]]
>  		exit 1
>  	}
> -	set _gitworktree [pwd]
> -	unset cdup
> -} elseif {![is_enabled bare]} {
> -	if {[is_bare]} {
> -		catch {wm withdraw .}
> -		error_popup [strcat [mc "Cannot use bare repository:"] "\n\n$_gitdir"]
> -		exit 1
> +	# Allowed bare repo does not have a worktree
> +	set _gitworktree {}
> +} else {
> +	# Non-bare repository: we must find a worktree
> +	if {[catch {set _gitworktree [git rev-parse --show-toplevel]} err]} {
> +		# The only acceptable failure is when we are inside
> +		# the .git directory of a regular repository.
> +		set inside_gitdir 0
> +		catch {set inside_gitdir [git rev-parse --is-inside-git-dir]}
> +		if {$inside_gitdir eq {true} && [file tail $_gitdir] eq {.git}} {
> +			# Use the parent directory as worktree (historic behavior)
> +			set _gitworktree [file normalize [file dirname $_gitdir]]
> +		} else {
> +			catch {wm withdraw .}
> +			error_popup [strcat [mc "Cannot determine working tree:"] "\n\n$err"]
> +			exit 1
> +		}
>  	}
> +
>  	if {$_gitworktree eq {}} {
> -		set _gitworktree [file dirname $_gitdir]
> +		catch {wm withdraw .}
> +		error_popup [mc "Cannot determine working tree (unexpected empty result)"]
> +		exit 1
>  	}

An empty $_gitworktree should be practically impossible at this point.
Personally, I would let the following "cd" handle the case (it fails if
the argument is empty).

> +
>  	if {[catch {cd $_gitworktree} err]} {
>  		catch {wm withdraw .}
> -		error_popup [strcat [mc "No working directory"] " $_gitworktree:\n\n$err"]
> +		error_popup [strcat [mc "Cannot move to working directory:"] "\n\n$err"]
>  		exit 1
>  	}
>  	set _gitworktree [pwd]
>  }
> +
> +# Derive a human-readable repository name
>  set _reponame [file split [file normalize $_gitdir]]
>  if {[lindex $_reponame end] eq {.git}} {
>  	set _reponame [lindex $_reponame end-1]
> @@ -1207,8 +1226,11 @@ if {[lindex $_reponame end] eq {.git}} {
>  	set _reponame [lindex $_reponame end]
>  }
>  
> +# Export the final paths
>  set env(GIT_DIR) $_gitdir
> -set env(GIT_WORK_TREE) $_gitworktree
> +if {$_gitworktree ne {}} {
> +	set env(GIT_WORK_TREE) $_gitworktree
> +}
>  
>  ######################################################################
>  ##

-- Hannes

[PATCH v7 2/3] git-gui: disable gitk visualization when no worktree available

[Shroom Moo]

When git-gui is started in a bare repository with the 'bare' option
enabled (e.g., for blame/browser), there is no working tree.  The
"Visualize Current Branch's History" and "Visualize All Branch
History" menu items remain enabled, but clicking them triggers a Tcl
error because do_gitk tries to change directory to an empty
_gitworktree.

Fix this by disabling the two visualization menu items when the
repository is bare and the 'bare' option is active.  Also update
current_branch_write to keep the state consistent when the branch
changes, and add a defensive check in do_gitk to avoid the error
should the menu state somehow become out of sync.

This complements the startup sequence improvements in the previous
commit, which already correctly identifies bare repositories and
leaves _gitworktree empty in such cases.

Helped-by: Mark Levedahl <mlevedahl@gmail.com>
Helped-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Shroom Moo <egg_mushroomcow@foxmail.com>
---
 git-gui/git-gui.sh | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index 9eb93a76b5..ff022c5bbb 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -2036,6 +2036,10 @@ proc do_gitk {revs {is_submodule false}} {
 	#
 	set exe [_which gitk -script]
 	set cmd [list [info nameofexecutable] $exe]
+	if {$_gitworktree eq {}} {
+		error_popup [mc "Cannot visualize history: no working tree"]
+		return
+	}
 	if {$exe eq {}} {
 		error_popup [mc "Couldn't find gitk in PATH"]
 	} else {
@@ -2659,6 +2663,13 @@ set ui_visualize_current [.mbar.repository index last]
 .mbar.repository add command \
 	-label [mc "Visualize All Branch History"] \
 	-command {do_gitk --all}
+set ui_visualize_all [.mbar.repository index last]
+
+# Cannot work without a working tree
+if {[is_bare] && [is_enabled bare]} {
+	.mbar.repository entryconf $ui_visualize_current -state disabled
+	.mbar.repository entryconf $ui_visualize_all   -state disabled
+}
 .mbar.repository add separator
 
 proc current_branch_write {args} {
@@ -2667,6 +2678,13 @@ proc current_branch_write {args} {
 		-label [mc "Browse %s's Files" $current_branch]
 	.mbar.repository entryconf $::ui_visualize_current \
 		-label [mc "Visualize %s's History" $current_branch]
+	if {[is_bare] && [is_enabled bare]} {
+		.mbar.repository entryconf $::ui_visualize_current -state disabled
+		.mbar.repository entryconf $::ui_visualize_all   -state disabled
+	} else {
+		.mbar.repository entryconf $::ui_visualize_current -state normal
+		.mbar.repository entryconf $::ui_visualize_all   -state normal
+	}
 }
 trace add variable current_branch write current_branch_write
 
-- 
2.52.0.windows.1

Re: [PATCH v7 2/3] git-gui: disable gitk visualization when no worktree available

[Johannes Sixt]

Am 09.05.26 um 15:37 schrieb Shroom Moo:
> When git-gui is started in a bare repository with the 'bare' option
> enabled (e.g., for blame/browser), there is no working tree.  The
> "Visualize Current Branch's History" and "Visualize All Branch
> History" menu items remain enabled, but clicking them triggers a Tcl
> error because do_gitk tries to change directory to an empty
> _gitworktree.

I cannot reproduce this claim. The failure is not a Tcl error, but an
error in some `git` invocation that cannot handle an empty
GIT_WORK_TREE. And that happens only beginning with the *second*
invocation of one of the "Visualize" calls, because then an empty
GIT_WORK_TREE is exported into the environment.

> 
> Fix this by disabling the two visualization menu items when the
> repository is bare and the 'bare' option is active.  Also update
> current_branch_write to keep the state consistent when the branch
> changes, and add a defensive check in do_gitk to avoid the error
> should the menu state somehow become out of sync.

This change is not correct. Gitk can operate without a working tree. The
menu entries should not be disabled, ever. The bug is somewhere else.

See also my suggested replacement patch in my reply to 3/3.

-- Hannes

[PATCH v7 3/3] git-gui: handle GIT_DIR and GIT_WORK_TREE early

[Shroom Moo]

Users expect these two invocations to be equivalent:

    GIT_WORK_TREE=/some/path GIT_DIR=/some/path/.git git gui
    git -C /some/path gui

Currently, the environment variable variant often brings up the
repository picker or ignores the requested worktree because
GIT_WORK_TREE is processed too late.  Moreover, after determining
the working tree, git-gui unconditionally exports GIT_WORK_TREE.
When no worktree is found (e.g., in a bare repository with a
read-only subcommand like blame), an empty value is exported, which
confuses commands like `git branch --show-current`.

Fix both issues:

- Introduce resolve_initial_environment, which is called before any
  repository detection.  When both GIT_DIR and GIT_WORK_TREE are
  set, it changes to the specified worktree, verifies that it is a
  valid worktree, and adopts the given GIT_DIR.  In this case, the
  subsequent automatic detection is skipped.  After successful
  validation, GIT_WORK_TREE is unset so that later steps do not
  inherit it.  Export the absolute GIT_DIR before changing directory
  to ensure Git commands see the intended repository.

- Introduce set_worktree_env and unset_worktree_env helper procedures
  to safely manage the GIT_WORK_TREE environment variable:
  set_worktree_env sets it only when $_gitworktree is non-empty,
  and unset_worktree_env removes the variable without error.
  Replace all direct manipulations of GIT_WORK_TREE with these
  helpers to avoid accidentally exporting an empty value or
  causing 'unset' errors.

- At the end of startup, use set_worktree_env instead of exporting
  GIT_WORK_TREE unconditionally.  This leaves the variable unset
  when no worktree exists (bare repository), fixing commands that
  would fail with an empty GIT_WORK_TREE.

When only GIT_DIR is set without GIT_WORK_TREE, the existing logic
remains unchanged for backward compatibility.  Setting only
GIT_WORK_TREE is not a valid Git use-case and is silently ignored.

Helped-by: Mark Levedahl <mlevedahl@gmail.com>
Helped-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Shroom Moo <egg_mushroomcow@foxmail.com>
---
 git-gui/git-gui.sh | 103 ++++++++++++++++++++++++++++++++++-----------
 1 file changed, 79 insertions(+), 24 deletions(-)

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index ff022c5bbb..1123f9855b 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -1122,25 +1122,82 @@ unset argv0dir
 ##
 ## repository setup
 
+# Safely set/unset GIT_WORK_TREE
+proc set_worktree_env {} {
+    global _gitworktree
+    if {$_gitworktree ne {}} {
+        set ::env(GIT_WORK_TREE) $_gitworktree
+    }
+}
+
+proc unset_worktree_env {} {
+    catch {unset ::env(GIT_WORK_TREE)}
+}
+
+proc resolve_initial_environment {} {
+	global _gitdir env
+
+	# Only act if both GIT_DIR and GIT_WORK_TREE are set
+	if {[info exists env(GIT_DIR)] && [info exists env(GIT_WORK_TREE)]} {
+		# Validate GIT_DIR by resolving its absolute path
+		if {[catch {set _abs [git rev-parse --absolute-git-dir]} err]} {
+			catch {wm withdraw .}
+			error_popup [strcat [mc "Invalid GIT_DIR:"] "\n\n$err"]
+			exit 1
+		}
+		set env(GIT_DIR) $_abs
+
+		# Change current directory to GIT_WORK_TREE
+		if {[catch {cd $env(GIT_WORK_TREE)} err]} {
+			catch {wm withdraw .}
+			error_popup [strcat [mc "Cannot change to GIT_WORK_TREE:"] "\n\n$err"]
+			exit 1
+		}
+
+		# Verify that GIT_WORK_TREE is a valid Git worktree
+		if {[catch {git rev-parse --show-toplevel} err]} {
+			catch {wm withdraw .}
+			error_popup [strcat [mc "GIT_WORK_TREE is not a valid worktree:"] "\n\n$err"]
+			exit 1
+		}
+
+		# Use repository path specified by environment variables
+		set _gitdir $_abs
+		set ::_prefix {}
+
+		# Unset GIT_WORK_TREE to prevent it from being inherited by child processes
+		unset_worktree_env
+
+		return 1
+	}
+
+	# Other cases (only GIT_DIR, only GIT_WORK_TREE, or neither):
+	# Do nothing here and let the existing logic handle it later
+	return 0
+}
+set force_gitdir [resolve_initial_environment]
+
 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; this is handled by
-		# the startup safety checks below
-		set _gitdir [git rev-parse --git-dir]
-		set _prefix [git rev-parse --show-prefix]
-	} err]} {
-	load_config 1
-	apply_config
-	choose_repository::pick
-	if {![file isdirectory $_gitdir]} {
-		exit 1
+if {!$force_gitdir} {
+	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; this is handled by
+			# the startup safety checks below
+			set _gitdir [git rev-parse --git-dir]
+			set _prefix [git rev-parse --show-prefix]
+		} err]} {
+		load_config 1
+		apply_config
+		choose_repository::pick
+		if {![file isdirectory $_gitdir]} {
+			exit 1
+		}
+		set picked 1
 	}
-	set picked 1
 }
 
 if {![file isdirectory $_gitdir]} {
@@ -1228,9 +1285,7 @@ if {[lindex $_reponame end] eq {.git}} {
 
 # Export the final paths
 set env(GIT_DIR) $_gitdir
-if {$_gitworktree ne {}} {
-	set env(GIT_WORK_TREE) $_gitworktree
-}
+set_worktree_env
 
 ######################################################################
 ##
@@ -2077,12 +2132,12 @@ proc do_gitk {revs {is_submodule false}} {
 			# 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_worktree_env
 		}
 		safe_exec_bg [concat $cmd $revs "--" "--"]
 
 		set env(GIT_DIR) $_gitdir
-		set env(GIT_WORK_TREE) $_gitworktree
+		set_worktree_env
 		cd $pwd
 
 		if {[info exists main_status]} {
@@ -2111,7 +2166,7 @@ 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_worktree_env
 
 		set pwd [pwd]
 		cd $current_diff_path
@@ -2119,7 +2174,7 @@ proc do_git_gui {} {
 		safe_exec_bg [concat $exe gui]
 
 		set env(GIT_DIR) $_gitdir
-		set env(GIT_WORK_TREE) $_gitworktree
+		set_worktree_env
 		cd $pwd
 
 		set status_operation [$::main_status \
-- 
2.52.0.windows.1

Re: [PATCH v7 3/3] git-gui: handle GIT_DIR and GIT_WORK_TREE early

[Johannes Sixt]

Am 09.05.26 um 15:37 schrieb Shroom Moo:
> Users expect these two invocations to be equivalent:
> 
>     GIT_WORK_TREE=/some/path GIT_DIR=/some/path/.git git gui
>     git -C /some/path gui
> 
> Currently, the environment variable variant often brings up the
> repository picker or ignores the requested worktree because
> GIT_WORK_TREE is processed too late.

I cannot reproduce the case that brings the repository picker. All other
failure cases that I can produce are reasonable and do not indicate that
GIT_WORK_TREE is processed too late.

>  Moreover, after determining
> the working tree, git-gui unconditionally exports GIT_WORK_TREE.
> When no worktree is found (e.g., in a bare repository with a
> read-only subcommand like blame), an empty value is exported, which
> confuses commands like `git branch --show-current`.

True. I think the culprit is that we export GIT_WORK_TREE in the first
place.

I suggest the following patch to replace this and the previous patch.

---- 8< ----
From: Johannes Sixt <j6t@kdbg.org>
Subject: [PATCH] git-gui: operate git commands without GIT_WORK_TREE

The manual page of the git command states about the --git-dir option:

   Specifying the location of the ".git" directory using this option
   (or GIT_DIR environment variable) turns off the repository
   discovery [...], and tells Git that you are at the top level of
   the working tree.

Use this to our advantage:

- Set GIT_DIR in the environment to the value that was discovered, so
  that the invoked git commands operate on the same repository
  database that Git GUI uses even after it changes the working
  directory.

- After changing the working directory to the top level of the working
  tree, ensure that GIT_WORK_TREE is not set, because, as per
  documentation, all git invocations from then on will assume that the
  current working directory is also the top level working tree.

- Remove the now obsolete GIT_WORK_TREE dance when subordinate Gitk or
  Git GUI are invoked for a submodule.

Do keep the state of GIT_WORK_TREE if we are in a bare repository,
because Git GUI is not interested in the worktree at all, as no commit
mode is possible in a bare repository.

This avoids cases where an empty GIT_WORK_TREE was exported into the
environment, most notably by a call of `git gui blame HEAD file` in a
bare repository.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 git-gui.sh | 24 +++++++-----------------
 1 file changed, 7 insertions(+), 17 deletions(-)

diff --git a/git-gui.sh b/git-gui.sh
index 76560ec825cf..146a29a809a8 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1216,6 +1216,7 @@ if {[is_bare]} {
 		exit 1
 	}
 	set _gitworktree [pwd]
+	catch { unset env(GIT_WORK_TREE) }
 }
 
 # Derive a human-readable repository name
@@ -1228,9 +1229,6 @@ if {[lindex $_reponame end] eq {.git}} {
 
 # Export the final paths
 set env(GIT_DIR) $_gitdir
-if {$_gitworktree ne {}} {
-	set env(GIT_WORK_TREE) $_gitworktree
-}
 
 ######################################################################
 ##
@@ -2029,7 +2027,7 @@ proc incr_font_size {font {amt 1}} {
 
 proc do_gitk {revs {is_submodule false}} {
 	global current_diff_path file_states current_diff_side ui_index
-	global _gitdir _gitworktree
+	global _gitdir
 
 	# -- Always start gitk through whatever we were loaded with.  This
 	#    lets us bypass using shell process on Windows systems.
@@ -2043,11 +2041,7 @@ proc do_gitk {revs {is_submodule false}} {
 
 		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)
@@ -2067,18 +2061,16 @@ proc do_gitk {revs {is_submodule false}} {
 				}
 				set revs $old_sha1...$new_sha1
 			}
-			# GIT_DIR and GIT_WORK_TREE for the submodule are not the ones
-			# we've been using for the main repository, so unset them.
+			# GIT_DIR for the submodule is not the one we've been using for
+			# the main repository, so unset it. (GIT_WORK_TREE is already unset.)
 			# 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)
 		}
 		safe_exec_bg [concat $cmd $revs "--" "--"]
 
 		set env(GIT_DIR) $_gitdir
-		set env(GIT_WORK_TREE) $_gitworktree
 		cd $pwd
 
 		if {[info exists main_status]} {
@@ -2102,12 +2094,11 @@ proc do_git_gui {} {
 		error_popup [mc "Couldn't find git gui in PATH"]
 	} else {
 		global env
-		global _gitdir _gitworktree
+		global _gitdir
 
-		# see note in do_gitk about unsetting these vars when
+		# see note in do_gitk about unsetting this variable when
 		# running tools in a submodule
 		unset env(GIT_DIR)
-		unset env(GIT_WORK_TREE)
 
 		set pwd [pwd]
 		cd $current_diff_path
@@ -2115,7 +2106,6 @@ proc do_git_gui {} {
 		safe_exec_bg [concat $exe gui]
 
 		set env(GIT_DIR) $_gitdir
-		set env(GIT_WORK_TREE) $_gitworktree
 		cd $pwd
 
 		set status_operation [$::main_status \
-- 
2.54.0.215.g4fe990ec16

[PATCH v6 1/3] git-gui: restructure repository startup

[Shroom Moo]

When git-gui is started inside a .git directory of a non-bare
repository, it should treat the parent directory as the worktree,
as it did before commit 2d92ab32fd (rev-parse: make --show-toplevel
without a worktree an error, 2019-11-19).  However, a bare repository
or a separated gitdir without a worktree must be rejected early.

Protect the previously unguarded calls to `git rev-parse
--show-object-format` and `--show-toplevel`.  Restructure the startup
sequence to:

- Check for a bare repository right after loading the config.  If the
  repository is bare and the current subcommand does not allow bare
  repos (e.g. normal commit mode), show "Cannot use bare repository"
  and exit.

- When `rev-parse --show-toplevel` fails and the repository is
  non-bare, the gitdir path ends with ".git", and we are inside that
  gitdir, use the parent directory as the worktree.  This preserves
  the ability to start git-gui from within a regular repository’s
  .git directory, which was intentionally supported since 87cd09f43e56
  (git-gui: work from the .git dir, 2010-01-23).

- Otherwise, show a descriptive error and exit.

- Wrap `rev-parse --show-object-format` in a catch to avoid a crash
  when the repository configuration is broken (e.g. core.worktree
  pointing to an invalid path).

Also removes the old `_prefix`‑based fallback that computed a relative
path to the worktree top from a subdirectory, and the unconditional
`[file dirname $_gitdir]` guess.  Both are unnecessary now that
`rev‑parse --show‑toplevel` directly provides the absolute top‑level
path and we can `cd` to it.  The guess is further unsafe in
multi‑worktree setups, where a gitdir may have more than one worktree.
The only remaining fallback is the explicit “.git directory” rule for
non‑bare repositories, which mirrors the historical behaviour.

This fixes the fatal Tcl error when the working tree is missing, while
keeping the .git startup feature and avoiding any automatic directory
switching that could be dangerous in multi‑worktree setups.

Helped-by: Johannes Sixt <j6t@kdbg.org>
Helped-by: Mark Levedahl <mlevedahl@gmail.com>
Signed-off-by: Shroom Moo <egg_mushroomcow@foxmail.com>
---
 git-gui/git-gui.sh | 72 +++++++++++++++++++++++++++++-----------------
 1 file changed, 46 insertions(+), 26 deletions(-)

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index 23fe76e498..fbdc0b2a41 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -1129,7 +1129,8 @@ if {[catch {
 		}]
 	&& [catch {
 		# beware that from the .git dir this sets _gitdir to .
-		# and _prefix to the empty string
+		# and _prefix to the empty string; this is handled by
+		# the startup safety checks below
 		set _gitdir [git rev-parse --git-dir]
 		set _prefix [git rev-parse --show-prefix]
 	} err]} {
@@ -1142,8 +1143,20 @@ if {[catch {
 	set picked 1
 }
 
+if {![file isdirectory $_gitdir]} {
+	catch {wm withdraw .}
+	error_popup [strcat 
+		[mc "Git directory not found:"] "\n\n$_gitdir\n\n" \
+		[mc "Please ensure GIT_DIR points to a valid Git repository"]]
+	exit 1
+}
+
 # Use object format as hash algorithm (either "sha1" or "sha256")
-set hashalgorithm [git rev-parse --show-object-format]
+if {[catch {set hashalgorithm [git rev-parse --show-object-format]} err]} {
+	catch {wm withdraw .}
+	error_popup [strcat [mc "Failed to determine hash algorithm:"] "\n\n$err"]
+	exit 1
+}
 if {$hashalgorithm eq "sha1"} {
 	set hashlength 40
 } elseif {$hashalgorithm eq "sha256"} {
@@ -1160,46 +1173,52 @@ if {$_gitdir eq "."} {
 	set _gitdir [pwd]
 }
 
-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
 
-set _gitworktree [git rev-parse --show-toplevel]
-
-if {$_prefix ne {}} {
-	if {$_gitworktree eq {}} {
-		regsub -all {[^/]+/} $_prefix ../ cdup
-	} else {
-		set cdup $_gitworktree
-	}
-	if {[catch {cd $cdup} err]} {
+# Handle bare repository and determine working tree
+if {[is_bare]} {
+	# Bare repository: only allowed for certain subcommands
+	if {![is_enabled bare]} {
 		catch {wm withdraw .}
-		error_popup [strcat [mc "Cannot move to top of working directory:"] "\n\n$err"]
+		error_popup [strcat [mc "Cannot use bare repository:"] "\n\n" [file normalize $_gitdir]]
 		exit 1
 	}
-	set _gitworktree [pwd]
-	unset cdup
-} elseif {![is_enabled bare]} {
-	if {[is_bare]} {
-		catch {wm withdraw .}
-		error_popup [strcat [mc "Cannot use bare repository:"] "\n\n$_gitdir"]
-		exit 1
+	# Allowed bare repo does not have a worktree
+	set _gitworktree {}
+} else {
+	# Non-bare repository: we must find a worktree
+	if {[catch {set _gitworktree [git rev-parse --show-toplevel]} err]} {
+		# The only acceptable failure is when we are inside
+		# the .git directory of a regular repository.
+		set inside_gitdir 0
+		catch {set inside_gitdir [git rev-parse --is-inside-git-dir]}
+		if {$inside_gitdir eq {true} && [file tail $_gitdir] eq {.git}} {
+			# Use the parent directory as worktree (historic behavior)
+			set _gitworktree [file normalize [file dirname $_gitdir]]
+		} else {
+			catch {wm withdraw .}
+			error_popup [strcat [mc "Cannot determine working tree:"] "\n\n$err"]
+			exit 1
+		}
 	}
+
 	if {$_gitworktree eq {}} {
-		set _gitworktree [file dirname $_gitdir]
+		catch {wm withdraw .}
+		error_popup [mc "Cannot determine working tree (unexpected empty result)"]
+		exit 1
 	}
+
 	if {[catch {cd $_gitworktree} err]} {
 		catch {wm withdraw .}
-		error_popup [strcat [mc "No working directory"] " $_gitworktree:\n\n$err"]
+		error_popup [strcat [mc "Cannot move to working directory:"] "\n\n$err"]
 		exit 1
 	}
 	set _gitworktree [pwd]
 }
+
+# Derive a human-readable repository name
 set _reponame [file split [file normalize $_gitdir]]
 if {[lindex $_reponame end] eq {.git}} {
 	set _reponame [lindex $_reponame end-1]
@@ -1207,6 +1226,7 @@ if {[lindex $_reponame end] eq {.git}} {
 	set _reponame [lindex $_reponame end]
 }
 
+# Export the final paths
 set env(GIT_DIR) $_gitdir
 set env(GIT_WORK_TREE) $_gitworktree
 
-- 
2.52.0.windows.1

[PATCH v6 2/3] git-gui: disable gitk visualization when no worktree available

[Shroom Moo]

When git-gui is started in a bare repository with the 'bare' option
enabled (e.g., for blame/browser), there is no working tree.  The
"Visualize Current Branch's History" and "Visualize All Branch
History" menu items remain enabled, but clicking them triggers a Tcl
error because do_gitk tries to change directory to an empty
_gitworktree.

Fix this by disabling the two visualization menu items when the
repository is bare and the 'bare' option is active.  Also update
current_branch_write to keep the state consistent when the branch
changes, and add a defensive check in do_gitk to avoid the error
should the menu state somehow become out of sync.

This complements the startup sequence improvements in the previous
commit, which already correctly identifies bare repositories and
leaves _gitworktree empty in such cases.

Helped-by: Mark Levedahl <mlevedahl@gmail.com>
Helped-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Shroom Moo <egg_mushroomcow@foxmail.com>
---
 git-gui/git-gui.sh | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index fbdc0b2a41..1191e6654c 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -2034,6 +2034,10 @@ proc do_gitk {revs {is_submodule false}} {
 	#
 	set exe [_which gitk -script]
 	set cmd [list [info nameofexecutable] $exe]
+	if {$_gitworktree eq {}} {
+		error_popup [mc "Cannot visualize history: no working tree"]
+		return
+	}
 	if {$exe eq {}} {
 		error_popup [mc "Couldn't find gitk in PATH"]
 	} else {
@@ -2657,6 +2661,13 @@ set ui_visualize_current [.mbar.repository index last]
 .mbar.repository add command \
 	-label [mc "Visualize All Branch History"] \
 	-command {do_gitk --all}
+set ui_visualize_all [.mbar.repository index last]
+
+# Cannot work without a working tree
+if {[is_bare] && [is_enabled bare]} {
+	.mbar.repository entryconf $ui_visualize_current -state disabled
+	.mbar.repository entryconf $ui_visualize_all   -state disabled
+}
 .mbar.repository add separator
 
 proc current_branch_write {args} {
@@ -2665,6 +2676,13 @@ proc current_branch_write {args} {
 		-label [mc "Browse %s's Files" $current_branch]
 	.mbar.repository entryconf $::ui_visualize_current \
 		-label [mc "Visualize %s's History" $current_branch]
+	if {[is_bare] && [is_enabled bare]} {
+		.mbar.repository entryconf $::ui_visualize_current -state disabled
+		.mbar.repository entryconf $::ui_visualize_all   -state disabled
+	} else {
+		.mbar.repository entryconf $::ui_visualize_current -state normal
+		.mbar.repository entryconf $::ui_visualize_all   -state normal
+	}
 }
 trace add variable current_branch write current_branch_write
 
-- 
2.52.0.windows.1

[PATCH v6 3/3] git-gui: handle GIT_DIR and GIT_WORK_TREE early

[Shroom Moo]

Users expect these two invocations to be equivalent:

    GIT_WORK_TREE=/some/path GIT_DIR=/some/path/.git git gui
    git -C /some/path gui

Currently, the environment variable variant often brings up the
repository picker or ignores the requested worktree because
GIT_WORK_TREE is processed too late.  Moreover, after determining
the working tree, git-gui unconditionally exports GIT_WORK_TREE.
When no worktree is found (e.g., in a bare repository with a
read-only subcommand like blame), an empty value is exported, which
confuses commands like `git branch --show-current`.

Fix both issues:

- Introduce resolve_initial_environment, which is called before any
  repository detection.  When both GIT_DIR and GIT_WORK_TREE are
  set, it changes to the specified worktree, verifies that it is a
  valid worktree, and adopts the given GIT_DIR.  In this case, the
  subsequent automatic detection is skipped.  After successful
  validation, GIT_WORK_TREE is unset so that later steps do not
  inherit it.

- At the end of startup, only export GIT_DIR.  The current directory
  is already at the top of the worktree (when a worktree exists), so
  Git can discover the worktree automatically.  Not exporting an
  empty GIT_WORK_TREE fixes `git gui blame` in bare repositories.

When only GIT_DIR is set without GIT_WORK_TREE, the existing logic
remains unchanged for backward compatibility.  Setting only
GIT_WORK_TREE is not a valid Git use-case and is silently ignored.

Helped-by: Mark Levedahl <mlevedahl@gmail.com>
Helped-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Shroom Moo <egg_mushroomcow@foxmail.com>
---
 git-gui/git-gui.sh | 84 ++++++++++++++++++++++++++++++++++++----------
 1 file changed, 66 insertions(+), 18 deletions(-)

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index 1191e6654c..ca651dc576 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -1122,25 +1122,69 @@ unset argv0dir
 ##
 ## repository setup
 
+proc resolve_initial_environment {} {
+	global _gitdir env
+
+	# Only act if both GIT_DIR and GIT_WORK_TREE are set
+	if {[info exists env(GIT_DIR)] && [info exists env(GIT_WORK_TREE)]} {
+		# Validate GIT_DIR by resolving its absolute path
+		if {[catch {set _abs [git rev-parse --absolute-git-dir]} err]} {
+			catch {wm withdraw .}
+			error_popup [strcat [mc "Invalid GIT_DIR:"] "\n\n$err"]
+			exit 1
+		}
+
+		# Change current directory to GIT_WORK_TREE
+		if {[catch {cd $env(GIT_WORK_TREE)} err]} {
+			catch {wm withdraw .}
+			error_popup [strcat [mc "Cannot change to GIT_WORK_TREE:"] "\n\n$err"]
+			exit 1
+		}
+
+		# Verify that GIT_WORK_TREE is a valid Git worktree
+		if {[catch {git rev-parse --show-toplevel} err]} {
+			catch {wm withdraw .}
+			error_popup [strcat [mc "GIT_WORK_TREE is not a valid worktree:"] "\n\n$err"]
+			exit 1
+		}
+
+		# Use repository path specified by environment variables
+		set _gitdir $_abs
+		set ::_prefix {}
+
+		# Unset GIT_WORK_TREE to prevent it from being inherited by child processes
+		unset env(GIT_WORK_TREE)
+
+		return 1
+	}
+
+	# Other cases (only GIT_DIR, only GIT_WORK_TREE, or neither):
+	# Do nothing here and let the existing logic handle it later
+	return 0
+}
+set force_gitdir [resolve_initial_environment]
+
 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; this is handled by
-		# the startup safety checks below
-		set _gitdir [git rev-parse --git-dir]
-		set _prefix [git rev-parse --show-prefix]
-	} err]} {
-	load_config 1
-	apply_config
-	choose_repository::pick
-	if {![file isdirectory $_gitdir]} {
-		exit 1
+if {!$force_gitdir} {
+	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; this is handled by
+			# the startup safety checks below
+			set _gitdir [git rev-parse --git-dir]
+			set _prefix [git rev-parse --show-prefix]
+		} err]} {
+		load_config 1
+		apply_config
+		choose_repository::pick
+		if {![file isdirectory $_gitdir]} {
+			exit 1
+		}
+		set picked 1
 	}
-	set picked 1
 }
 
 if {![file isdirectory $_gitdir]} {
@@ -1228,7 +1272,11 @@ if {[lindex $_reponame end] eq {.git}} {
 
 # Export the final paths
 set env(GIT_DIR) $_gitdir
-set env(GIT_WORK_TREE) $_gitworktree
+# Do not export `GIT_WORK_TREE`. The current directory is already
+# the top-level of the working tree (if it exists), and Git will
+# automatically identify the working tree. For bare repositories,
+# `_gitworktree` is empty, and exporting an empty value would cause
+# commands like `git branch --show-current` to fail
 
 ######################################################################
 ##
-- 
2.52.0.windows.1

Re: [PATCH v6 3/3] git-gui: handle GIT_DIR and GIT_WORK_TREE early

[Mark Levedahl]

On 5/6/26 4:27 PM, Shroom Moo wrote:
> +proc resolve_initial_environment {} {
> +	global _gitdir env
> +
> +	# Only act if both GIT_DIR and GIT_WORK_TREE are set
> +	if {[info exists env(GIT_DIR)] && [info exists env(GIT_WORK_TREE)]} {
> +		# Validate GIT_DIR by resolving its absolute path
> +		if {[catch {set _abs [git rev-parse --absolute-git-dir]} err]} {
> +			catch {wm withdraw .}
> +			error_popup [strcat [mc "Invalid GIT_DIR:"] "\n\n$err"]
> +			exit 1
> +		}
> +
> +		# Change current directory to GIT_WORK_TREE
> +		if {[catch {cd $env(GIT_WORK_TREE)} err]} {
> +			catch {wm withdraw .}
> +			error_popup [strcat [mc "Cannot change to GIT_WORK_TREE:"] "\n\n$err"]
> +			exit 1
> +		}
GIT_DIR could be relative to the startup directory. The absolute variant should be
exported before the next step.



> +
> +		# Verify that GIT_WORK_TREE is a valid Git worktree
> +		if {[catch {git rev-parse --show-toplevel} err]} {
> +			catch {wm withdraw .}
> +			error_popup [strcat [mc "GIT_WORK_TREE is not a valid worktree:"] "\n\n$err"]
> +			exit 1
> +		}
> +
> +		# Use repository path specified by environment variables
> +		set _gitdir $_abs
> +		set ::_prefix {}
> +
> +		# Unset GIT_WORK_TREE to prevent it from being inherited by child processes
> +		unset env(GIT_WORK_TREE)
> +

There are many places in the code that have pairs of set/unset env(GIT_DIR) &
env(GIT_WORK_TREE).
Now that we have cases where $_gitworktree == {} and we don't want that exported, and
these are all unguarded.

Perhaps we need to create set/unset procs that do the right thing (setx assures
GIT_WORK_TREE is not in the env when $_gitworktree == {}), unsetx does not thow and error
when unset env(GIT_WORK_TREE) fails as the value doesn't exist, and invoke those common
procs to instead.

Mark

Re: [PATCH v6 3/3] git-gui: handle GIT_DIR and GIT_WORK_TREE early

[Aina Boot]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 921 bytes --]

After observing, I’d like to propose a clearer way to manage
GIT_WORK_TREE and GIT_DIR environment variables.

Five procedures in total:
1. set_worktree_env – sets GIT_WORK_TREE in the environment only if
$_gitworktree is non-empty.
2. unset_worktree_env – safely unsets GIT_WORK_TREE (ignores if it’s
not set).
3. clear_git_env – unsets both GIT_DIR and GIT_WORK_TREE before
entering a submodule context.
4. capture_git_env – captures the current GIT_DIR and GIT_WORK_TREE
values for later restoration.
5. restore_git_env – restores the saved values, setting GIT_WORK_TREE
only if it was previously non-empty.

These would replace scattered, unguarded set/unset env(GIT_WORK_TREE)
calls and ensure we don’t leave GIT_WORK_TREE defined when there is no
actual worktree. It also makes the intention more explicit and reduces
repetitive error-catching logic.

Aina

Re: [PATCH v6 3/3] git-gui: handle GIT_DIR and GIT_WORK_TREE early

[Shroom Moo]

These are functional, yet somewhat complex. Only 1 and 2 are necessary 
in this adjustment, I suppose. 

Saving and restoring old values in do_gitk and do_git_gui actually 
does not require additional encapsulation because: 
- First, GIT_DIR always exists, and directly using set and catch 
{unset} is inherently safe. 
- Second, the set/unset operations for GIT_WORK_TREE are already 
properly encapsulated. 

Since the save-restore pattern only appears in these two places, 
introducing capture_git_env / restore_git_env adds more conceptual 
overhead than what it saves. 

Shroom

Re: [PATCH] git-gui: handle bare repo or missing worktree

[Shroom Moo]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1651 bytes --]

Hi Hannes, 

Thanks for taking a close look at the patch.  You're right that the 
minimal crashfix is just protecting the `rev-parse --show-toplevel` 
call.  The existing error paths after that line already handle bare 
repos and missing worktrees, showing an error dialog before exiting. 
That alone would prevent the fatal Tcl error –the original crash is 
fixed. 

The extra logic is not about incomplete error diagnosis (the other 
checks cover that).  It's about improving the user experience in the 
specific case that prompted this report: a colleague double-clicked 
git-gui.exe inside the Git installation directory, where a third-party 
program had accidentally left a HEAD file.  That caused git to treat 
the directory as a bare repository, even though it has no . git folder. 
In that context, showing a "Cannot use bare repository" and quitting 
was confusing – the user simply started the program without choosing 
that path. 

So the additional detection and the `choose_repository::pick` call are 
a UX improvement, not a correctness requirement.  When the program 
detects that the current directory is an unusable repository, falling 
back to the repository-selection dialog (as we do when no repository 
is found) is better than failing with an error.  The user can pick a 
valid repository and continue without restarting the GUI. 

Since a minimal patch is preferred, I have prepared v2 which only 
protects `--show-toplevel`.  The dialog fallback could be revisited as 
a separate enhancement if desired. I've already sent the v2 patch to 
the list.

-- Shroom