From: Johannes Sixt Date: Wed, 06 May 2026 07:32:02 GMT Subject: Re: [PATCH v3 1/1] git-gui: handle missing worktree and separated gitdir Message-ID: In-Reply-To: <7d5cf952-badb-4071-a0eb-af9443fa8b5b@gmail.com> 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