From: Johannes Sixt <j6t@kdbg.org>
Date: Sun, 24 May 2026 07:16:33 GMT
Subject: Re: [PATCH v2 00/11] Improve git gui operation without a worktree
Message-ID: <43f070e4-e624-4a33-8c24-294520fb503a@kdbg.org>
In-Reply-To: <20260520202411.108764-1-mlevedahl@gmail.com>

Am 20.05.26 um 22:23 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 to select a
>   worktree from a list and/or start a new repo+worktree: this dialog can
>   appear at unexpected times, masking useful error feedback on
>   configuration problems.
> 
> This patch series addresses the above issues, substantially rewriting
> the initial repository/worktree process to rely upon git rev-parse so
> that git's knowledge of access rules, repository configuration, and use
> of GIT_DIR / GIT_WORK_TREE (or git --gitdir / --work-tree) is used
> throughout, replacing code largely based upon what git did in 2008. This
> also means that git gui will naturally gain any new rules implmented in
> git-core.
> 
> With this, git-gui only exports GIT_WORK_TREE when non-empty.
> GIT_WORK_TREE is needed, and must be exported, if the user is overriding
> core.worktree in the git repository. But, GIT_WORK_TREE cannot be used
> to specify the lack of a worktree, so exporting an empty GIT_WORK_TREE
> is one of the problems fixed by this series.
> 
> v2 of this series is a very substantial rewrite driven by j6t's review,
> with patches reoranized and squashed, interfaces to the repository
> chooser changed, a different code structure to allow user control of the
> repository picker, a different approach to fixing the command line
> parser for blame / browser, and other more minor changes. Patches
> for fixing blame / browser are now after all discovery refactoring as
> they cannot be tested without some of those fixes.
> 
> Many subtle things are fixed beyond the list at the top, including
> better compatibility with git blame and repeatable browser / blame
> operation for specific revs not in the worktree, regardless of the
> worktree state. j6t indicated that in the git-gui project, the following
> fails in the current release:
> 
> cd lib
> GIT_DIR=$PWD/../.git GIT_WORK_TREE=$PWD/.. ../git-gui.sh browser origin/master .
> 
> This is due to a _prefix issue, and is fixed as of the patch
>      git-gui: use git rev-parse for worktree discovery
> 

I've completed my review of this iteration.

Repository and working tree discovery is already converging fast.
However, I have issues with the proposed argument parsing of the browser
and blame modes, in particular, I don't think that we need to
accommodate the uncanny file-before-rev argument order and that it
disregards the worktree completely. Maybe we should postpone any changes
in this area, if possible?

Throughout, we use a strange indentation style of 'if {[catch ...' that
is violated in new code, but I left uncommented. It should indent the
catch body one additional level like so:

	if {catch {
			commands that can fail
		} err]} {
		error handling here
	}

Thank you very much for working on this topic.

-- Hannes