Re: [PATCH v1 10/11] git-gui: improve worktree discovery
- From
Johannes Sixt <j6t@kdbg.org>
- Date
- May 19, 2026, 08:16 UTC
- Message-ID
- <5081fcc5-19b5-49aa-a33c-2c13aba7edb1@kdbg.org>
- In-Reply-To
- <f654eab3-2157-4591-9ae0-440efb052e8e@gmail.com>
Am 16.05.26 um 17:28 schrieb Mark Levedahl:
Show 21 quoted lines
> On 5/16/26 4:16 AM, Johannes Sixt wrote:
>> Am 14.05.26 um 16:33 schrieb Mark Levedahl:
>>> + 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].
>
> [is_enabled_bare] is correct. This code handles the case:
> - neither the startup directory nor GIT_WORK_TREE are useable worktrees, so [is_bare]
> is currently true.
> - the command given is browser or blame so a worktree is not needed. We can proceed.But in the case where the command is browser or blame, the argument parser must later check for the existence of files, provided that a worktree is present. But this conditional would change directory to somewhere that is not a worktree at all even though a worktree is available. So, I am still convinced that [is_bare] is correct.
> The bigger question is whether to change directory at all: git-gui should never touch > files that are neither in the worktree nor in the repository. Leaving the current > directory as neither of those could be troublesome. I have no strong feeling here though, > will delete this.
OK. We need the conditional, but not change the directory.
Show 21 quoted lines
>>> + } 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?
> We need to load the system and user global config before running the repository picker. We
> (re-) load the full config including the repository after we have a repository. I think
> this is correct: git-config explicitly lists worktree dependent includeif statements,
> meaning the config can be worktree dependent, and we must not load the final config until
> repository and worktree discovery are complete.Good point.
> > git rev-parse, etc., perform discovery and config file loading each time they are invoked, > those are unaffected by git-gui's internal config.
Also very true. Only calls to proc get_config would be affected by a different setup, but there are none in the code that has swapped places with working tree discovery. Only --show-object-format would be affected by the discovered environment, and for that case it is more correct to operate in the final setup.
-- Hannes