* [PATCH 0/2] [GSoC Patch] t2000: modernize path checks to use helper functions
From: Zakariyah Ali via GitGitGadget @ 2026-05-23 11:07 UTC (permalink / raw)
To: git
Cc: Christian Couder, Karthik Nayak, Justin Tobler, Siddharth Asthana,
Ayush Chandekar, Zakariyah Ali
This is my GSoC microproject submission modernizing test path checks in
t/t2000-conflict-when-checking-files-out.sh.
Replace old-style path checks using test -f, test -d, and test ! -h with
dedicated test helper functions for improved test clarity and consistency.
This modernization improves test script readability by using Git's dedicated
test helpers:
test -f → test_path_is_file test -d → test_path_is_dir test ! -h && test -f
→ test_path_is_file_not_symlink test ! -h && test -d →
test_path_is_dir_not_symlink Found instances using: git grep 'test -[efd]'
t/ | grep 'test -[efd].*&&'
Converted 5 instances in t/t2000-conflict-when-checking-files-out.sh
This improves test clarity and consistency across the test suite.
I'm excited to contribute to Git and look forward to your feedback!
Zakariyah Ali (2):
t2000: consolidate second scenario into a single test block
t2000: cleanup unused debug code and variables
t/t2000-conflict-when-checking-files-out.sh | 65 +++------------------
1 file changed, 8 insertions(+), 57 deletions(-)
base-commit: 60f07c4f5c5f81c8a994d9e06b31a4a3a1679864
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2256%2Falibaba0010%2Fmodernize-test-path-checking-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2256/alibaba0010/modernize-test-path-checking-v1
Pull-Request: https://github.com/git/git/pull/2256
--
gitgitgadget
^ permalink raw reply
* [PATCH 1/2] t2000: consolidate second scenario into a single test block
From: Zakariyah Ali via GitGitGadget @ 2026-05-23 11:07 UTC (permalink / raw)
To: git
Cc: Christian Couder, Karthik Nayak, Justin Tobler, Siddharth Asthana,
Ayush Chandekar, Zakariyah Ali, Zakariyah Ali
In-Reply-To: <pull.2256.git.git.1779534462.gitgitgadget@gmail.com>
From: Zakariyah Ali <zakariyahali100@gmail.com>
Now that the test script has been modernised, consolidate the eight
separate test_expect_success blocks that together form the second
test scenario (setup, tree writes, checkout, symlink creation, and
final state check) into one self-contained block.
This makes it easier to read: data set-up, the operations being
tested, and the expected outcome are now all in one place.
Helped-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: Zakariyah Ali <zakariyahali100@gmail.com>
---
t/t2000-conflict-when-checking-files-out.sh | 55 ++++-----------------
1 file changed, 9 insertions(+), 46 deletions(-)
diff --git a/t/t2000-conflict-when-checking-files-out.sh b/t/t2000-conflict-when-checking-files-out.sh
index af199d8191..43ec901f9e 100755
--- a/t/t2000-conflict-when-checking-files-out.sh
+++ b/t/t2000-conflict-when-checking-files-out.sh
@@ -83,59 +83,22 @@ test_expect_success SYMLINKS 'checkout-index -f twice with --prefix' '
# path path3 is occupied by a non-directory. With "-f" it should remove
# the symlink path3 and create directory path3 and file path3/file1.
-test_expect_success 'prepare path2/file0 and index' '
+test_expect_success 'checkout-index -f resolves symlink conflict on leading path' '
mkdir path2 &&
date >path2/file0 &&
- git update-index --add path2/file0
-'
-
-test_expect_success 'write tree with path2/file0' '
- tree1=$(git write-tree)
-'
-
-test_debug 'show_files $tree1'
-
-test_expect_success 'prepare path3/file1 and index' '
+ git update-index --add path2/file0 &&
+ tree1=$(git write-tree) &&
mkdir path3 &&
date >path3/file1 &&
- git update-index --add path3/file1
-'
-
-test_expect_success 'write tree with path3/file1' '
- tree2=$(git write-tree)
-'
-
-test_debug 'show_files $tree2'
-
-test_expect_success 'read previously written tree and checkout.' '
+ git update-index --add path3/file1 &&
+ tree2=$(git write-tree) &&
rm -fr path3 &&
git read-tree -m $tree1 &&
- git checkout-index -f -a
-'
-
-test_debug 'show_files $tree1'
-
-test_expect_success 'add a symlink' '
- test_ln_s_add path2 path3
-'
-
-test_expect_success 'write tree with symlink path3' '
- tree3=$(git write-tree)
-'
-
-test_debug 'show_files $tree3'
-
-# Morten says "Got that?" here.
-# Test begins.
-
-test_expect_success 'read previously written tree and checkout.' '
+ git checkout-index -f -a &&
+ test_ln_s_add path2 path3 &&
+ tree3=$(git write-tree) &&
git read-tree $tree2 &&
- git checkout-index -f -a
-'
-
-test_debug 'show_files $tree2'
-
-test_expect_success 'checking out conflicting path with -f' '
+ git checkout-index -f -a &&
test_path_is_dir_not_symlink path2 &&
test_path_is_dir_not_symlink path3 &&
test_path_is_file_not_symlink path2/file0 &&
--
gitgitgadget
^ permalink raw reply related
* [PATCH 2/2] t2000: cleanup unused debug code and variables
From: Zakariyah Ali via GitGitGadget @ 2026-05-23 11:07 UTC (permalink / raw)
To: git
Cc: Christian Couder, Karthik Nayak, Justin Tobler, Siddharth Asthana,
Ayush Chandekar, Zakariyah Ali, Zakariyah Ali
In-Reply-To: <pull.2256.git.git.1779534462.gitgitgadget@gmail.com>
From: Zakariyah Ali <zakariyahali100@gmail.com>
Remove the show_files function which is no longer used after removing
test_debug calls, and remove an unused tree3 variable assignment in
the second test scenario.
These cleanups address feedback from Junio C Hamano regarding the
modernization of this test script.
Signed-off-by: Zakariyah Ali <zakariyahali100@gmail.com>
---
t/t2000-conflict-when-checking-files-out.sh | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/t/t2000-conflict-when-checking-files-out.sh b/t/t2000-conflict-when-checking-files-out.sh
index 43ec901f9e..7b61370549 100755
--- a/t/t2000-conflict-when-checking-files-out.sh
+++ b/t/t2000-conflict-when-checking-files-out.sh
@@ -23,17 +23,6 @@ test_description='git conflicts when checking files out test.'
. ./test-lib.sh
-show_files() {
- # show filesystem files, just [-dl] for type and name
- find path? -ls |
- sed -e 's/^[0-9]* * [0-9]* * \([-bcdl]\)[^ ]* *[0-9]* *[^ ]* *[^ ]* *[0-9]* [A-Z][a-z][a-z] [0-9][0-9] [^ ]* /fs: \1 /'
- # what's in the cache, just mode and name
- git ls-files --stage |
- sed -e 's/^\([0-9]*\) [0-9a-f]* [0-3] /ca: \1 /'
- # what's in the tree, just mode and name.
- git ls-tree -r "$1" |
- sed -e 's/^\([0-9]*\) [^ ]* [0-9a-f]* /tr: \1 /'
-}
test_expect_success 'prepare files path0 and path1/file1' '
date >path0 &&
@@ -96,7 +85,6 @@ test_expect_success 'checkout-index -f resolves symlink conflict on leading path
git read-tree -m $tree1 &&
git checkout-index -f -a &&
test_ln_s_add path2 path3 &&
- tree3=$(git write-tree) &&
git read-tree $tree2 &&
git checkout-index -f -a &&
test_path_is_dir_not_symlink path2 &&
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH 0/4] doc: hook: small improvements
From: Kristoffer Haugsbakk @ 2026-05-23 11:43 UTC (permalink / raw)
To: Jean-Noël AVILA, git; +Cc: Adrian Ratiu
In-Reply-To: <2832179.mvXUDI8C0e@piment-oiseau>
On Sat, May 23, 2026, at 12:24, Jean-Noël AVILA wrote:
> On Thursday, 21 May 2026 18:25:54 CEST kristofferhaugsbakk@fastmail.com wrote:
>> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
>>
>> Topic name: kh/doc-hook
>>
>> Topic summary: Small improvements to git-hook(1) and the associated config.
>>
>> [1/4] doc: hook: remove stray backtick
>> [2/4] doc: hook: consistently capitalize Git
>> [3/4] doc: config: include existing git-hook(1) section
>> [4/4] doc: hook: don’t self-link via config include
>>
>> Documentation/config.adoc | 2 ++
>> Documentation/config/hook.adoc | 19 +++++++++++++------
>> Documentation/git-hook.adoc | 11 ++++++-----
>> 3 files changed, 21 insertions(+), 11 deletions(-)
>>
>>
>> base-commit: aec3f587505a472db67e9462d0702e7d463a449d
>
> This series looks good to me.
Thanks. Can I add your ack to the patches?
^ permalink raw reply
* Re: [PATCH v2 01/11] git-gui: guard set/unset of GIT_DIR and GIT_WORK_TREE
From: Aina Boot @ 2026-05-23 11:46 UTC (permalink / raw)
To: Johannes Sixt, Mark Levedahl; +Cc: Shroom Moo, git
In-Reply-To: <b332c7d9-c86b-4d4b-a873-1600d910a237@kdbg.org>
On 5/23/26 8:19 AM, Johannes Sixt wrote:
> The other patch that removes cd $_gitworktree from do_gitk should still
> be good, I think.
Agree. It either succeededly set the directory or work without a
worktree, cd is practically unnecessary.
Aina
^ permalink raw reply
* Re: [PATCH 0/4] doc: hook: small improvements
From: Jean-Noël AVILA @ 2026-05-23 10:24 UTC (permalink / raw)
To: git, kristofferhaugsbakk; +Cc: Kristoffer Haugsbakk, adrian.ratiu
In-Reply-To: <CV_doc_hook.6f0@msgid.xyz>
On Thursday, 21 May 2026 18:25:54 CEST kristofferhaugsbakk@fastmail.com wrote:
> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
>
> Topic name: kh/doc-hook
>
> Topic summary: Small improvements to git-hook(1) and the associated config.
>
> [1/4] doc: hook: remove stray backtick
> [2/4] doc: hook: consistently capitalize Git
> [3/4] doc: config: include existing git-hook(1) section
> [4/4] doc: hook: don’t self-link via config include
>
> Documentation/config.adoc | 2 ++
> Documentation/config/hook.adoc | 19 +++++++++++++------
> Documentation/git-hook.adoc | 11 ++++++-----
> 3 files changed, 21 insertions(+), 11 deletions(-)
>
>
> base-commit: aec3f587505a472db67e9462d0702e7d463a449d
This series looks good to me.
Thanks
^ permalink raw reply
* Re: [PATCH v2 07/11] git-gui: try harder to find worktree from gitdir
From: Shroom Moo @ 2026-05-23 11:47 UTC (permalink / raw)
To: Mark Levedahl, Johannes Sixt; +Cc: git, Aina Boot
In-Reply-To: <c8d1ab1e-e0cb-44e2-afcd-728b7b43774c@kdbg.org>
On 5/23/26 4:01 PM, Johannes Sixt wrote:
> Am 21.05.26 um 06:55 schrieb Shroom Moo:
>> On 5/21/26 4:24 AM, Mark Levedahl wrote:
>>> + } elseif [file exists {gitdir}] {
>>> + if {[catch {
>>> + set fd_gitdir [open {gitdir} {r}]
>>> + set gitlink_parent [file dirname [read $fd_gitdir]]
>>> + catch {close $fd_gitdir}
>>> + set worktree [git -C $gitlink_parent rev-parse --show-toplevel]
>>> + set parent_gitdir [git -C $worktree rev-parse --absolute-git-dir]
>>> + if {$::_gitdir ne $parent_gitdir} {
>>> + set worktree {}
>>> + }
>>> + }]} {
>>> + catch {close $fd_gitdir}
>>> + set worktree {}
>>> + }
>>> + }
>> Additionally, [file exists {gitdir}] checks for the gitdir file in
>> the current working directory. Since the function has not yet
>> switched to $_gitdir when this check runs, it is almost impossible
>> to find the file. Consequently, this logic never triggers, preventing
>> linked worktrees from being recognized.
> I think you are misunderstanding which use-case this code is addressing.
> The case can be triggered very easily.
> First, the code before the part we see above is intended for the special
> case where we start in a .git, where `--show-toplevel` bails out and we
> define the worktree to be the directory containing .git.
> However, if we start in .git/worktrees/feature, then the code cited
> above kicks in, because `--show-toplevel` still bails out,
> `--absolute-git-dir` does not end in '.git', but now we have a file
> named 'gitdir' in the current directory. In this case, we define (and
> this is new with this patch) that the worktree is the one where the
> 'gitdir' points.
>
> -- Hannes
I see. The condition is unrelated to this patch. Users should handle
this case as assigning manually by rule. Indeed we don't need to
modify it.
Shroom
^ permalink raw reply
* Re: [PATCH v2 06/11] git-gui: use git rev-parse for worktree discovery
From: Johannes Sixt @ 2026-05-23 13:26 UTC (permalink / raw)
To: Mark Levedahl; +Cc: egg_mushroomcow, bootaina702, git
In-Reply-To: <20260520202411.108764-7-mlevedahl@gmail.com>
Am 20.05.26 um 22:24 schrieb Mark Levedahl:
> git gui uses a combination of tcl code and git invocations to determine
> the worktree and the location with respect to the worktree root
> (_prefix). But, git rev-parse provides all of this information directly,
> and assures full error and configuration checking are done by git
> itself. The entirety of discovery in normal configurations involves
>
> git rev-parse --show-toplevel (gets worktree root)
> git rev-parse --show-prefix (shows location wrt the root)
>
> An error thrown on either of these lines means the worktree discovered
> by git is unusable, or git did not discover a worktree because the
> current directory is inside the repository. If the user has defined
> GIT_DIR or GIT_WORK_TREE, this is a user configuration error and git-gui
> should stop.
>
> Otherwise, the blame or browser subcommands can be used without a
> worktree.
>
> A separate error might occur when changing to the root of the discovered
> worktree. The cause would be file system related and completely outside
> of git's control. So, the final "cd $worktree_root" is separately
> trapped.
>
> Discovery of the repository and the worktree must be guarded to trap
> errors: the intent is that any configuration problems are caught during
> discovery, and later processing need not include error trapping and
> recovery. So, move all worktree discovery code to be immediately after
> repository discovery.
>
> This does move configuration loading to occur after worktree discovery
> rather than before. None of the code executed in worktree discovery has
> any option controlled by a git-gui configuration variable, so no impact
> is expected. git itself will always read the repository configuration,
> including worktree specific configuration data if that exists, so this
> is unaffected by when git-gui loads its own config data, and we cannot
> be sure the full worktree dependent configuration can be loaded before
> full discovery is complete.
Very good!
When you move code around, please do not apply style changes so that
git show --color-moved --color-moved-ws=allow-indentation-change
can prove that no change was intended.
>
> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
> ---
> git-gui.sh | 64 +++++++++++++++++++++++++-----------------------------
> 1 file changed, 30 insertions(+), 34 deletions(-)
>
> diff --git a/git-gui.sh b/git-gui.sh
> index 936c309e59..8fe25fe188 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -1164,6 +1164,36 @@ if {$_gitdir eq {}} {
> set picked 1
> }
>
> +# find worktree, continue without if not required
> +if {[catch {
> + set _gitworktree [git rev-parse --show-toplevel]
> + set _prefix [git rev-parse --show-prefix]
> +} err]} {
> + if {[is_gitvars_error $err]} {
> + exit 1
> + }
> + set _gitworktree {}
> + set _prefix {}
> +}
> +
> +if {![is_bare]} {
> + if {[catch {
> + cd $_gitworktree
> + } err]} {
> + catch {wm withdraw .}
> + error_popup [strcat [mc "Cannot change to discovered worktree: "] \
> + "$_gitworktree" "\n\n$err"]
> + exit 1;
> + }
> +} elseif {![is_enabled bare]} {
> + 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]
> if {$hashalgorithm eq "sha1"} {
> @@ -1179,37 +1209,6 @@ 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]
> -}
> set _reponame [file split [file normalize $_gitdir]]
> if {[lindex $_reponame end] eq {.git}} {
> set _reponame [lindex $_reponame end-1]
> @@ -1217,9 +1216,6 @@ if {[lindex $_reponame end] eq {.git}} {
> set _reponame [lindex $_reponame end]
> }
>
> -# Export the final paths
> -set_gitdir_vars
> -
> ######################################################################
> ##
> ## global init
-- Hannes
^ permalink raw reply
* Re: [PATCH v2 07/11] git-gui: try harder to find worktree from gitdir
From: Johannes Sixt @ 2026-05-23 14:06 UTC (permalink / raw)
To: Mark Levedahl; +Cc: egg_mushroomcow, bootaina702, git
In-Reply-To: <20260520202411.108764-8-mlevedahl@gmail.com>
Am 20.05.26 um 22:24 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.
I can imagine that this kind of use occurs in "Git GUI here" menu item
of the file explorer on Windows. So, we should resurrect the feature.
>
> 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.
>
> There are (at least) 3 cases where the gitdir can tell us where the
> worktree is, and we would like all to work:
>
> - core.worktree is set, and points to a valid worktree. This is already
> handled by git rev-parse --show-toplevel, even when not in the worktree.
> There is nothing more to do in this case.
>
> - the gitdir is embedded in a worktree as subdirectory .git. The parent
> is (or at least should be) a valid worktree. This worked long ago.
>
> - the gitdir is a worktree specific directory (under
> <mainrepo>/worktrees/worktree_name), within which there is a file
> "gitdir" pointing to .git in the worktree. git gui never learned to
> handle this case.
>
> Let's handle the latter two cases. Always check that the discovered
> worktree is valid and points to the already discovered gitdir according
> to git rev-parse. This avoids issues that may arise because we are
> discovering from the gitdir up, rather than the worktree down, and file
> system non-posix behavior or misconfiguration of git might cause
> confusion. For instance, a manually moved worktree might not be where
> the gitdir points.
>
> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
> ---
> git-gui.sh | 42 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> diff --git a/git-gui.sh b/git-gui.sh
> index 8fe25fe188..aeb7ed3548 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -1100,6 +1100,41 @@ unset argv0dir
> ##
> ## repository setup
>
> +proc find_worktree_from_gitdir {} {
> + # Directory 'parent' of a repository named 'parent/.git' might be the worktree.
> + # Assure parent is a worktree and using the git repository already discovered.
> + # Also, handle case of being in a worktree's gitdir, where file "gitdir" points to
> + # gitlink file .git in the real worktree.
> + set worktree {}
> + if {[file tail $::_gitdir] eq {.git}} {
> + if {[catch {
> + set gitdir_parent [file dirname $::_gitdir]
> + set worktree [git -C $gitdir_parent rev-parse --show-toplevel]
> + set parent_gitdir [git -C $worktree rev-parse --absolute-git-dir]
> + if {$::_gitdir ne $parent_gitdir} {
> + set worktree {}
I tried to come up with a situation where we end up here, but couldn't.
When would this happen? If it actually can't happen, I would prefer to
spawn fewer git processes and just take the result of 'file dirname'.
If the code must remain, can we please rename one of gitdir_parent or
parent_gitdir?
> + }
> + }]} {
> + set worktree {}
> + }
> + } elseif [file exists {gitdir}] {
> + if {[catch {
> + set fd_gitdir [open {gitdir} {r}]
> + set gitlink_parent [file dirname [read $fd_gitdir]]
> + catch {close $fd_gitdir}
> + set worktree [git -C $gitlink_parent rev-parse --show-toplevel]
> + set parent_gitdir [git -C $worktree rev-parse --absolute-git-dir]
Since worktrees can be messed up quite easily, it looks reasonable to
check whether the worktree points back to the gitdir. (But I haven't
tried to construct a case that passes the check in the next line.)
> + if {$::_gitdir ne $parent_gitdir} {
> + set worktree {}
> + }
> + }]} {
> + catch {close $fd_gitdir}
> + set worktree {}
> + }
> + }
> + return $worktree
> +}
> +
> proc is_gitvars_error {err} {
> set havevars 0
> set GIT_DIR {}
> @@ -1176,6 +1211,13 @@ if {[catch {
> set _prefix {}
> }
>
> +if {[is_bare]} {
> + # Maybe we are in an embedded or worktree specific gitdir
> + if {[set _gitworktree [find_worktree_from_gitdir]] ne {}} {
> + set _prefix {}
> + }
> +}
> +
> if {![is_bare]} {
> if {[catch {
> cd $_gitworktree
-- Hannes
^ permalink raw reply
* Re: [PATCH v2 08/11] git-gui: use HEAD as current branch when detached (bug fix)
From: Johannes Sixt @ 2026-05-23 14:08 UTC (permalink / raw)
To: Mark Levedahl, git; +Cc: egg_mushroomcow, bootaina702
In-Reply-To: <20260520202411.108764-9-mlevedahl@gmail.com>
Am 20.05.26 um 22:24 schrieb Mark Levedahl:
> commit f87a36b697 ("git-gui: use git-branch --show-current", 2024-02-12)
> changed git-gui to use git-branch to access refs, rather than directly
> reading files as doing the latter is not compatible with the reftable
> backend. git branch --show-current reports an empty branch name when the
> head is detached, and in this case load_current_branch needs to report
> HEAD using special case logic as it did prior to the above commit. Make
> it do so.
>
> This addresses an issue with git-gui browser failing with a detached
> head.
Nice catch. I'll reorder this as the first commit.
>
> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
> ---
> git-gui.sh | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/git-gui.sh b/git-gui.sh
> index aeb7ed3548..a72d8a59ec 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -648,6 +648,9 @@ proc load_current_branch {} {
>
> set current_branch [git branch --show-current]
> set is_detached [expr [string length $current_branch] == 0]
> + if {$is_detached} {
> + set current_branch {HEAD}
> + }
> }
>
> auto_load tk_optionMenu
-- Hannes
^ permalink raw reply
* Re: [PATCH v2 09/11] git-gui: allow specifying path '.' to the browser
From: Johannes Sixt @ 2026-05-23 14:23 UTC (permalink / raw)
To: Mark Levedahl; +Cc: egg_mushroomcow, bootaina702, git
In-Reply-To: <20260520202411.108764-10-mlevedahl@gmail.com>
Am 20.05.26 um 22:24 schrieb Mark Levedahl:
> Invoking "git-gui browser rev ." should show the file browser for the
> commitish rev, starting at the current directory. When the current
> directory is the working tree root, this errors out in normalize_relpath
> because the '.' is removed, yielding an empty list as argument to [file
> join ...]. The browser function demands "./" in this case, so make it
> so. (./ works on Windows as well because g4w accepts posix file
> naming).
I wonder why we need "./" instead of plain ".". The latter works just
fine in my tests (on Linux).
>
> 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 a72d8a59ec..d373457901 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -3007,7 +3007,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
^ permalink raw reply
* Re: [PATCH v3 6/8] promisor-remote: trust known remotes matching acceptFromServerUrl
From: Kristoffer Haugsbakk @ 2026-05-23 15:17 UTC (permalink / raw)
To: Christian Couder, git
Cc: Junio C Hamano, Patrick Steinhardt, Taylor Blau, Karthik Nayak,
Elijah Newren, Toon Claes, Christian Couder
In-Reply-To: <20260519153808.494105-7-christian.couder@gmail.com>
On Tue, May 19, 2026, at 17:38, Christian Couder wrote:
>[snip]
>
> Let's then use this helper in should_accept_remote() so that, a known
> remote whose URL matches the allowlist is accepted.
I don’t understand this comma break?
>
> To prepare for this new logic, let's also:
>
>[snip]
>
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
The rest of the commit message looks good to me.
> ---
> Documentation/config/promisor.adoc | 74 +++++++++++++++++++
> Documentation/gitprotocol-v2.adoc | 9 ++-
> promisor-remote.c | 102 +++++++++++++++++++++++---
> t/t5710-promisor-remote-capability.sh | 71 ++++++++++++++++++
> 4 files changed, 242 insertions(+), 14 deletions(-)
>
> diff --git a/Documentation/config/promisor.adoc
>[snip]
> ++
> +Be _VERY_ careful with these patterns: `*` matches any sequence of
> +characters within the 'host' and 'path' parts of a URL (but cannot
> +cross part boundaries). An overly broad pattern is a major security
> +risk, as a matching URL allows a server to update fields (such as
> +authentication tokens) on known remotes without further confirmation.
> +To minimize security risks, follow these guidelines:
> ++
So this introduces a list of precautions to take.
> +1. Start with a secure protocol scheme, like `https://` or `ssh://`.
> ++
> +2. Only allow domain names or paths where you control and trust _ALL_
> + the content. Be especially careful with shared hosting platforms
> + like `github.com` or `gitlab.com`. A broad pattern like
> + `https://gitlab.com/*` is dangerous because it trusts every
> + repository on the entire platform. Always restrict such patterns to
> + your specific organization or namespace (e.g.,
> + `https://gitlab.com/your-org/*`).
> ++
> +3. Never use globs at the end of domain names. For example,
> + `https://cdn.your-org.com/*` might be safe, but
> + `https://cdn.your-org.com*/*` is a major security risk because
> + the latter matches `https://cdn.your-org.com.hacker.net/repo`.
> ++
> +4. Be careful using globs at the beginning of domain names. While the
> + code ensures a `*` in the host cannot cross into the path, a
> + pattern like `https://*.example.com/*` will still match any
> + subdomain. This is extremely dangerous on shared hosting platforms
> + (e.g., `https://*.github.io/*` trusts every user's site on the
> + entire platform).
The list seems to end here, because...
> ++
> +Before matching, both the advertised URL and the pattern are
> +normalized: the scheme and host are lowercased, percent-encoded
This next paragraph seems to go back to describing how things work. But
this paragraph as well as all of the following ones belong to this list
item:
4. Be careful using globs [...]
Before matching, [...]
The glob pattern can [...]
If a remote with the [...]
For the security implications [...]
promisor.checkFields
[...]
I don’t know what the intent is. But using an open block will delimit
the ordered list.
diff --git Documentation/config/promisor.adoc Documentation/config/promisor.adoc
index cc728bb0b5e..f07a2e883bd 100644
--- Documentation/config/promisor.adoc
+++ Documentation/config/promisor.adoc
@@ -109,6 +109,7 @@ and to update fields (such as authentication tokens) on known remotes
without further confirmation. To minimize security risks, follow these
guidelines:
+
+--
1. Start with a secure protocol scheme, like `https://` or `ssh://`.
+
2. Only allow domain names or paths where you control and trust _ALL_
@@ -130,6 +131,7 @@ guidelines:
subdomain. This is extremely dangerous on shared hosting platforms
(e.g., `https://*.github.io/*` trusts every user's site on the
entire platform).
+--
+
Before matching, both the advertised URL and the pattern are
normalized: the scheme and host are lowercased, percent-encoded
>[snip]
^ permalink raw reply
* Re: [PATCH v2 07/11] git-gui: try harder to find worktree from gitdir
From: Mark Levedahl @ 2026-05-23 15:33 UTC (permalink / raw)
To: Johannes Sixt; +Cc: egg_mushroomcow, bootaina702, git
In-Reply-To: <a1e9da65-f8dd-4544-bbc9-d3b01328cebe@kdbg.org>
On 5/23/26 10:06 AM, Johannes Sixt wrote:
> I tried to come up with a situation where we end up here, but couldn't.
> When would this happen? If it actually can't happen, I would prefer to
> spawn fewer git processes and just take the result of 'file dirname'.
>
> If the code must remain, can we please rename one of gitdir_parent or
> parent_gitdir?
There are some odd cases I've seen, mostly driven by network file systems (using Samba or
NFS or ...) that don't behave as POSIX. Perhaps that is reasonably out of scope. The more
I think about this, I'll delete that part.
Symlinks can create odd cases:
Consider /tmp/main and /tmp/worktree. The latter has a .git entry that is a symlink to
/tmp/main.git. git rev-parse --absolute-git-dir shows
in dir /tmp/worktree/.git /tmp/main.git
So, git-gui would start in /tmp/main, and not in /tmp/worktree.
If using git new-workdir (which is still in the wild), this creates a .git dir with
symlinks to all the subdirs. Now git rev-parse reports
in dir /tmp/worktree.git /tmp/worktree/.git
in dir /tmp/worktree/.git/logs /tmp/main/.git
So, as long as this is started in the top level of .git, it's ok.
While the shell understands we descended into a symlink and reports pwd does not
de-referencing the symlink, tcl always dereferences the symlinks. So, any ability to
contain this behavior is very likely system and shell dependent.
So, in summary, I'll probably simplify the check to just --show-toplevel run in the dir
above .git.
Mark
^ permalink raw reply
* Re: [PATCH v2 09/11] git-gui: allow specifying path '.' to the browser
From: Mark Levedahl @ 2026-05-23 15:43 UTC (permalink / raw)
To: Johannes Sixt; +Cc: egg_mushroomcow, bootaina702, git
In-Reply-To: <ae3cdc22-2f88-4222-bab7-403408373a53@kdbg.org>
On 5/23/26 10:23 AM, Johannes Sixt wrote:
> Am 20.05.26 um 22:24 schrieb Mark Levedahl:
>> Invoking "git-gui browser rev ." should show the file browser for the
>> commitish rev, starting at the current directory. When the current
>> directory is the working tree root, this errors out in normalize_relpath
>> because the '.' is removed, yielding an empty list as argument to [file
>> join ...]. The browser function demands "./" in this case, so make it
>> so. (./ works on Windows as well because g4w accepts posix file
>> naming).
> I wonder why we need "./" instead of plain ".". The latter works just
> fine in my tests (on Linux).
'.' caused errors in browser::new in for me before while './' worked, but now I find both
work. I'm confused, this must have been an interaction with something else in flight at
the time, will revert to '.' if that passes my tests on Windows as well as it is more
consistent of not adding '/' to a dirname.
Mark
^ permalink raw reply
* Re: [PATCH v2 01/11] git-gui: guard set/unset of GIT_DIR and GIT_WORK_TREE
From: Mark Levedahl @ 2026-05-23 16:08 UTC (permalink / raw)
To: Johannes Sixt; +Cc: egg_mushroomcow, bootaina702, git
In-Reply-To: <b332c7d9-c86b-4d4b-a873-1600d910a237@kdbg.org>
On 5/23/26 4:18 AM, Johannes Sixt wrote:
> Am 22.05.26 um 13:54 schrieb Mark Levedahl:
>> The manual page is incomplete: if the repository has set core.worktree=/somehere, that is
>> the root of the worktree and the current directory is always ignored. git rev-parse will
>> report /somewhere as the answer to --show-toplevel regardless of current directory, even
>> if inside the gitdir, and even if GIT_DIR is used.
>>
>> The user can override with GIT_WORK_TREE, and if so we must keep GIT_WORK_TREE in the
>> environment if it was set. [...]
> Oh, well, these intricacies! Let's scrap my patch and keep yours.
>
> The other patch that removes cd $_gitworktree from do_gitk should still
> be good, I think.
>
> -- Hannes
>
Removing cd $_gitworktree should be ok, we are already in that directory, or don't have a
worktree, and I did that once myself before dropping it as I don't really understand
do_gitk. It should not change any behavior. So, go ahead and add it wherever you wish.
But, I don't understand unsetting GIT_DIR and GIT_WORK_TREE for gitk. If we needed them
for git in the super module, we need them for submodules as well, but have no idea how to
adjust them. Simply unsetting them cannot be right. Out of scope for me. But, there are
some dragons lurking around this proc.
Mark
^ permalink raw reply
* Re: [PATCH] doc: fix typos via codespell
From: Weijie Yuan @ 2026-05-23 17:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Andrew Kreimer, git
In-Reply-To: <xmqqa4u6aotg.fsf@gitster.g>
Hi Mr. Hamano, I've checked the rest of patch, I believe Mr. Kreimer is doing
great. Meanwhile, I'm wondering if you are happy to accept patches
about typos or not, since I have other remaining corrections for typos
here. If you don't mind, may I send a version of it add sort this patch
series out.
Thanks,
Weijie Yuan
^ permalink raw reply
* [PATCH] fixup git-gui: allow blame to show uncommitted changes
From: Mark Levedahl @ 2026-05-23 18:19 UTC (permalink / raw)
To: git; +Cc: j6t, egg_mushroomcow, bootaina702, Mark Levedahl
In-Reply-To: <20260520202411.108764-11-mlevedahl@gmail.com>
Commit a0db0d61fb ("git-gui: Generate blame on uncommitted working tree
file", 2007-05-08) added ability for git-gui's blame to use uncommited
content in the worktree in the blame display. The specific mechanism
that allows this is passing head as {} to the blame constructor, and this
mode is enabled by specifying no head, which means the current branch is
used, and no swapping of head / path is possible. Path checking looks for the
path existing in the currently checked out branch, so files unknown to
git cannot be blamed. This mirrors git blame behavior. Both now will
use the worktree contents of the file $path as the basis for blame,
including showing an empty blame if the file exists and is empty.
error if $path is remove from the worktree, even if it exists on the
current branch.
The latter behavior is a change: before this patch, git-gui will show
the unknown file in its entirety with no annotations. The new behavior,
following git blame, reports that git knows nothing about the file.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
This patch will be squashed into 0011 in a v3, or kept separate. The
comment in patch 11 saying blame's use of the worktree is unaffected
is wrong, this fixes blame to do exactly what git-blame does with
worktree content. Slightly different than what git-gui did before
regarding unknown files, but I think git-blame's approach is correct.
git-gui.sh | 29 ++++++++++++++++++++++++-----
1 file changed, 24 insertions(+), 5 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index ae609f86f1..114511974a 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -3084,11 +3084,16 @@ blame {
}
}
- # no swapping allowed if head not given, use current branch (HEAD)
+ # If head not given, use current branch (HEAD), no swapping allowed,
+ # and blame may use the worktree file content.
+ set use_worktree 0
if {$head eq {}} {
load_current_branch
set head $current_branch
set canswap 0
+ if {$subcommand eq {blame} && ![is_bare]} {
+ set use_worktree 1
+ }
}
# -- before "rev" arg means we got -- path head
@@ -3098,7 +3103,16 @@ blame {
set canswap 0
}
- set objtype [find_path_type $head $path]
+ if {$use_worktree} {
+ if {[file isfile $path]} {
+ set objtype {blob}
+ } else {
+ set objtype {}
+ }
+ } else {
+ set objtype [find_path_type $head $path]
+ }
+
if {$objtype eq {} && $canswap} {
set objtype [find_path_type $althead $altpath]
if {$objtype ne {}} {
@@ -3108,7 +3122,7 @@ blame {
}
set current_branch $head
- # check that path exists in head, and objtype matches need
+ # check objtype matches need
if {$objtype ne $required_objtype} {
switch -- $required_objtype {
tree {set err [strcat \
@@ -3130,8 +3144,13 @@ blame {
browser {
browser::new $head $path
}
- blame {
- blame::new $head $path $jump_spec
+
+ blame {
+ if {$use_worktree} {
+ blame::new {} $path $jump_spec
+ } else {
+ blame::new $head $path $jump_spec
+ }
}
}
return
--
2.54.0.99.14
^ permalink raw reply related
* [PATCH v13 0/2] checkout: --track=fetch
From: Harald Nordgren via GitGitGadget @ 2026-05-23 19:48 UTC (permalink / raw)
To: git
Cc: Ramsay Jones, D. Ben Knoble, Kristoffer Haugsbakk, Marc Branchaud,
Phillip Wood, Harald Nordgren
In-Reply-To: <pull.2281.v12.git.git.1779358803652.gitgitgadget@gmail.com>
* Create a preparatory commit that exposes find_tracking_remote_for_ref()
and advise_ambiguous_fetch_refspec() from branch.c, so checkout can reuse
the same lookup git branch --track uses.
* Use advise_ambiguous_fetch_refspec() for the "multiple remotes match"
case, so the wording matches git branch --track.
Harald Nordgren (2):
branch: expose helpers for finding the remote owning a tracking ref
checkout: extend --track with a "fetch" mode to refresh start-point
Documentation/git-checkout.adoc | 17 +-
Documentation/git-switch.adoc | 5 +-
branch.c | 96 ++++++-----
branch.h | 16 ++
builtin/checkout.c | 139 +++++++++++++++-
t/t7201-co.sh | 276 ++++++++++++++++++++++++++++++++
6 files changed, 498 insertions(+), 51 deletions(-)
base-commit: aec3f587505a472db67e9462d0702e7d463a449d
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2281%2FHaraldNordgren%2Fcheckout-fetch-start-point-v13
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2281/HaraldNordgren/checkout-fetch-start-point-v13
Pull-Request: https://github.com/git/git/pull/2281
Range-diff vs v12:
-: ---------- > 1: 2369afad24 branch: expose helpers for finding the remote owning a tracking ref
1: bcd034dbed ! 2: 60adf0e67d checkout: extend --track with a "fetch" mode to refresh start-point
@@ Commit message
git checkout -b new_branch --track origin/some-branch
Identify the remote whose configured fetch refspec maps to
- <start-point>, then run "git fetch <remote> <src-ref>" for just that
- ref so other remote-tracking branches are left untouched. When
- <start-point> is a bare <remote> (e.g. "origin"), follow
+ <start-point> using find_tracking_remote_for_ref() (the same lookup
+ "--track" uses to pick which remote to record in
+ branch.<name>.remote), then run "git fetch <remote> <src-ref>" for
+ just that ref so other remote-tracking branches are left untouched.
+ When <start-point> is a bare <remote> (e.g. "origin"), follow
refs/remotes/<remote>/HEAD to learn which branch to refresh. If
"git fetch" fails but the remote-tracking ref already exists locally,
warn and proceed from the existing tip; otherwise abort.
@@ builtin/checkout.c: struct branch_info {
char *checkout;
};
-+struct fetch_target_cb {
-+ char *dst;
-+ struct string_list matches;
-+};
-+
-+static int match_fetch_target(struct remote *remote, void *priv)
-+{
-+ struct fetch_target_cb *cb = priv;
-+ struct refspec_item q = { .dst = cb->dst };
-+
-+ if (!remote_find_tracking(remote, &q) && q.src)
-+ string_list_append(&cb->matches, remote->name)->util = q.src;
-+ return 0;
-+}
-+
+static void fetch_remote_for_start_point(const char *arg, int quiet)
+{
+ struct strbuf dst = STRBUF_INIT;
-+ struct fetch_target_cb cb = { .matches = STRING_LIST_INIT_NODUP };
++ struct tracking tracking;
++ struct string_list tracking_srcs = STRING_LIST_INIT_DUP;
++ struct string_list ambiguous_remotes = STRING_LIST_INIT_DUP;
+ struct child_process cmd = CHILD_PROCESS_INIT;
+ struct object_id oid;
+ struct remote *named_remote;
+ int bare_ns;
-+ size_t i;
+
+ strbuf_addf(&dst, "refs/remotes/%s", arg);
+ if (check_refname_format(dst.buf, 0))
@@ builtin/checkout.c: struct branch_info {
+ free(head_path);
+ }
+
-+ cb.dst = dst.buf;
-+ for_each_remote(match_fetch_target, &cb);
-+
-+ if (cb.matches.nr > 1) {
-+ struct strbuf msg = STRBUF_INIT;
-+
-+ strbuf_addf(&msg,
-+ _("cannot fetch start-point '%s': fetch refspecs "
-+ "of multiple remotes map to the same destination:"),
-+ arg);
-+ for (i = 0; i < cb.matches.nr; i++)
-+ strbuf_addf(&msg, "\n %s", cb.matches.items[i].string);
-+ strbuf_addstr(&msg,
-+ _("\nadjust 'remote.<name>.fetch' so only one "
-+ "remote maps there, or omit '=fetch'"));
-+ die("%s", msg.buf);
++ memset(&tracking, 0, sizeof(tracking));
++ tracking.spec.dst = dst.buf;
++ tracking.srcs = &tracking_srcs;
++ find_tracking_remote_for_ref(&tracking, &ambiguous_remotes);
++
++ if (tracking.matches > 1) {
++ int status = die_message(_("cannot fetch start-point '%s': "
++ "fetch refspecs of multiple remotes "
++ "map to '%s'"), arg, dst.buf);
++ advise_ambiguous_fetch_refspec(dst.buf, &ambiguous_remotes);
++ exit(status);
+ }
+
-+ if (!cb.matches.nr) {
++ if (!tracking.matches) {
+ if (bare_ns && named_remote &&
+ remote_is_configured(named_remote, 1))
+ die(_("cannot fetch start-point '%s': "
@@ builtin/checkout.c: struct branch_info {
+ strvec_push(&cmd.args, "fetch");
+ if (quiet)
+ strvec_push(&cmd.args, "--quiet");
-+ strvec_pushl(&cmd.args, cb.matches.items[0].string,
-+ (char *)cb.matches.items[0].util, NULL);
++ strvec_pushl(&cmd.args, tracking.remote,
++ tracking_srcs.items[0].string, NULL);
+ cmd.git_cmd = 1;
+ if (run_command(&cmd)) {
+ if (!refs_read_ref(get_main_ref_store(the_repository),
@@ builtin/checkout.c: struct branch_info {
+ die(_("failed to fetch start-point '%s'"), arg);
+ }
+
-+ for (i = 0; i < cb.matches.nr; i++)
-+ free(cb.matches.items[i].util);
-+ string_list_clear(&cb.matches, 0);
++ string_list_clear(&tracking_srcs, 0);
++ string_list_clear(&ambiguous_remotes, 0);
+ strbuf_release(&dst);
+}
+
@@ t/t7201-co.sh: test_expect_success 'tracking info copied with autoSetupMerge=inh
+ test_must_fail git checkout --track=fetch -b local_ambig ambig_ns/fetch_ambig 2>err &&
+ test_grep "fetch_ambig_a" err &&
+ test_grep "fetch_ambig_b" err &&
-+ test_grep "remote.<name>.fetch" err &&
++ test_grep "tracking namespaces" err &&
+ test_must_fail git rev-parse --verify refs/heads/local_ambig
+'
+
--
gitgitgadget
^ permalink raw reply
* [PATCH v13 1/2] branch: expose helpers for finding the remote owning a tracking ref
From: Harald Nordgren via GitGitGadget @ 2026-05-23 19:48 UTC (permalink / raw)
To: git
Cc: Ramsay Jones, D. Ben Knoble, Kristoffer Haugsbakk, Marc Branchaud,
Phillip Wood, Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2281.v13.git.git.1779565714.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
The remote-lookup that setup_tracking() does is useful outside
branch.c too; for example, deciding which remote to "git fetch"
from given a remote-tracking ref.
Move 'struct tracking' to branch.h and add two helpers backed by the
existing for_each_remote walk: find_tracking_remote_for_ref() and
advise_ambiguous_fetch_refspec(). setup_tracking() uses both. No
behavior change.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
branch.c | 96 ++++++++++++++++++++++++++++++--------------------------
branch.h | 16 ++++++++++
2 files changed, 68 insertions(+), 44 deletions(-)
diff --git a/branch.c b/branch.c
index 243db7d0fc..46ae7f0035 100644
--- a/branch.c
+++ b/branch.c
@@ -20,16 +20,9 @@
#include "run-command.h"
#include "strmap.h"
-struct tracking {
- struct refspec_item spec;
- struct string_list *srcs;
- const char *remote;
- int matches;
-};
-
struct find_tracked_branch_cb {
struct tracking *tracking;
- struct string_list ambiguous_remotes;
+ struct string_list *ambiguous_remotes;
};
static int find_tracked_branch(struct remote *remote, void *priv)
@@ -45,10 +38,10 @@ static int find_tracked_branch(struct remote *remote, void *priv)
break;
case 2:
/* there are at least two remotes; backfill the first one */
- string_list_append(&ftb->ambiguous_remotes, tracking->remote);
+ string_list_append(ftb->ambiguous_remotes, tracking->remote);
/* fall through */
default:
- string_list_append(&ftb->ambiguous_remotes, remote->name);
+ string_list_append(ftb->ambiguous_remotes, remote->name);
free(tracking->spec.src);
string_list_clear(tracking->srcs, 0);
break;
@@ -59,6 +52,51 @@ static int find_tracked_branch(struct remote *remote, void *priv)
return 0;
}
+void find_tracking_remote_for_ref(struct tracking *tracking,
+ struct string_list *ambiguous_remotes)
+{
+ struct find_tracked_branch_cb ftb_cb = {
+ .tracking = tracking,
+ .ambiguous_remotes = ambiguous_remotes,
+ };
+
+ for_each_remote(find_tracked_branch, &ftb_cb);
+}
+
+void advise_ambiguous_fetch_refspec(const char *dst,
+ const struct string_list *ambiguous_remotes)
+{
+ struct strbuf remotes_advice = STRBUF_INIT;
+ struct string_list_item *item;
+
+ if (!advice_enabled(ADVICE_AMBIGUOUS_FETCH_REFSPEC))
+ return;
+
+ for_each_string_list_item(item, ambiguous_remotes)
+ /*
+ * TRANSLATORS: This is a line listing a remote with duplicate
+ * refspecs in the advice message below. For RTL languages you'll
+ * probably want to swap the "%s" and leading " " space around.
+ */
+ strbuf_addf(&remotes_advice, _(" %s\n"), item->string);
+
+ /*
+ * TRANSLATORS: The second argument is a \n-delimited list of
+ * duplicate refspecs, composed above.
+ */
+ advise(_("There are multiple remotes whose fetch refspecs map to the remote\n"
+ "tracking ref '%s':\n"
+ "%s"
+ "\n"
+ "This is typically a configuration error.\n"
+ "\n"
+ "To support setting up tracking branches, ensure that\n"
+ "different remotes' fetch refspecs map into different\n"
+ "tracking namespaces."), dst,
+ remotes_advice.buf);
+ strbuf_release(&remotes_advice);
+}
+
static int should_setup_rebase(const char *origin)
{
switch (autorebase) {
@@ -254,11 +292,8 @@ static void setup_tracking(const char *new_ref, const char *orig_ref,
{
struct tracking tracking;
struct string_list tracking_srcs = STRING_LIST_INIT_DUP;
+ struct string_list ambiguous_remotes = STRING_LIST_INIT_DUP;
int config_flags = quiet ? 0 : BRANCH_CONFIG_VERBOSE;
- struct find_tracked_branch_cb ftb_cb = {
- .tracking = &tracking,
- .ambiguous_remotes = STRING_LIST_INIT_DUP,
- };
if (!track)
BUG("asked to set up tracking, but tracking is disallowed");
@@ -267,7 +302,7 @@ static void setup_tracking(const char *new_ref, const char *orig_ref,
tracking.spec.dst = (char *)orig_ref;
tracking.srcs = &tracking_srcs;
if (track != BRANCH_TRACK_INHERIT)
- for_each_remote(find_tracked_branch, &ftb_cb);
+ find_tracking_remote_for_ref(&tracking, &ambiguous_remotes);
else if (inherit_tracking(&tracking, orig_ref))
goto cleanup;
@@ -293,34 +328,7 @@ static void setup_tracking(const char *new_ref, const char *orig_ref,
if (tracking.matches > 1) {
int status = die_message(_("not tracking: ambiguous information for ref '%s'"),
orig_ref);
- if (advice_enabled(ADVICE_AMBIGUOUS_FETCH_REFSPEC)) {
- struct strbuf remotes_advice = STRBUF_INIT;
- struct string_list_item *item;
-
- for_each_string_list_item(item, &ftb_cb.ambiguous_remotes)
- /*
- * TRANSLATORS: This is a line listing a remote with duplicate
- * refspecs in the advice message below. For RTL languages you'll
- * probably want to swap the "%s" and leading " " space around.
- */
- strbuf_addf(&remotes_advice, _(" %s\n"), item->string);
-
- /*
- * TRANSLATORS: The second argument is a \n-delimited list of
- * duplicate refspecs, composed above.
- */
- advise(_("There are multiple remotes whose fetch refspecs map to the remote\n"
- "tracking ref '%s':\n"
- "%s"
- "\n"
- "This is typically a configuration error.\n"
- "\n"
- "To support setting up tracking branches, ensure that\n"
- "different remotes' fetch refspecs map into different\n"
- "tracking namespaces."), orig_ref,
- remotes_advice.buf);
- strbuf_release(&remotes_advice);
- }
+ advise_ambiguous_fetch_refspec(orig_ref, &ambiguous_remotes);
exit(status);
}
@@ -347,7 +355,7 @@ static void setup_tracking(const char *new_ref, const char *orig_ref,
cleanup:
string_list_clear(&tracking_srcs, 0);
- string_list_clear(&ftb_cb.ambiguous_remotes, 0);
+ string_list_clear(&ambiguous_remotes, 0);
}
int read_branch_desc(struct strbuf *buf, const char *branch_name)
diff --git a/branch.h b/branch.h
index 3dc6e2a0ff..0aafa1673f 100644
--- a/branch.h
+++ b/branch.h
@@ -1,9 +1,25 @@
#ifndef BRANCH_H
#define BRANCH_H
+#include "refspec.h"
+#include "string-list.h"
+
struct repository;
struct strbuf;
+struct tracking {
+ struct refspec_item spec;
+ struct string_list *srcs;
+ const char *remote;
+ int matches;
+};
+
+void find_tracking_remote_for_ref(struct tracking *tracking,
+ struct string_list *ambiguous_remotes);
+
+void advise_ambiguous_fetch_refspec(const char *dst,
+ const struct string_list *ambiguous_remotes);
+
enum branch_track {
BRANCH_TRACK_UNSPECIFIED = -1,
BRANCH_TRACK_NEVER = 0,
--
gitgitgadget
^ permalink raw reply related
* [PATCH v13 2/2] checkout: extend --track with a "fetch" mode to refresh start-point
From: Harald Nordgren via GitGitGadget @ 2026-05-23 19:48 UTC (permalink / raw)
To: git
Cc: Ramsay Jones, D. Ben Knoble, Kristoffer Haugsbakk, Marc Branchaud,
Phillip Wood, Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2281.v13.git.git.1779565714.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
Add a "fetch" mode to the "--track" option of "git checkout" / "git
switch" that refreshes <start-point> before checking it out:
git checkout -b new_branch --track=fetch origin/some-branch
is shorthand for
git fetch origin some-branch
git checkout -b new_branch --track origin/some-branch
Identify the remote whose configured fetch refspec maps to
<start-point> using find_tracking_remote_for_ref() (the same lookup
"--track" uses to pick which remote to record in
branch.<name>.remote), then run "git fetch <remote> <src-ref>" for
just that ref so other remote-tracking branches are left untouched.
When <start-point> is a bare <remote> (e.g. "origin"), follow
refs/remotes/<remote>/HEAD to learn which branch to refresh. If
"git fetch" fails but the remote-tracking ref already exists locally,
warn and proceed from the existing tip; otherwise abort.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
Documentation/git-checkout.adoc | 17 +-
Documentation/git-switch.adoc | 5 +-
builtin/checkout.c | 139 +++++++++++++++-
t/t7201-co.sh | 276 ++++++++++++++++++++++++++++++++
4 files changed, 430 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-checkout.adoc b/Documentation/git-checkout.adoc
index a8b3b8c2e2..20b6cae60e 100644
--- a/Documentation/git-checkout.adoc
+++ b/Documentation/git-checkout.adoc
@@ -158,11 +158,26 @@ of it").
resets _<branch>_ to the start point instead of failing.
`-t`::
-`--track[=(direct|inherit)]`::
+`--track[=(direct|inherit|fetch)[,...]]`::
When creating a new branch, set up "upstream" configuration. See
`--track` in linkgit:git-branch[1] for details. As a convenience,
--track without -b implies branch creation.
+
+The argument is a comma-separated list. `direct` (the default) and
+`inherit` select the tracking mode and are mutually exclusive. Adding
+`fetch` requests that the remote be fetched before _<start-point>_ is
+resolved, so the new branch starts from a fresh tip: when
+_<start-point>_ is in _<remote>/<branch>_ form, only that branch is
+updated; when _<start-point>_ is a bare _<remote>_ (e.g. `origin`), the
+branch named by _<remote>/HEAD_ is updated, and the checkout fails
+with a hint to configure that symref if it is not set. The checkout
+also fails if no configured remote's fetch refspec maps to
+_<start-point>_, or if more than one does (in which case the `fetch`
+cannot be unambiguously routed). If the fetch itself fails and the
+corresponding remote-tracking ref already exists, a warning is printed
+and the checkout proceeds from the existing tip; otherwise the checkout
+is aborted.
++
If no `-b` option is given, the name of the new branch will be
derived from the remote-tracking branch, by looking at the local part of
the refspec configured for the corresponding remote, and then stripping
diff --git a/Documentation/git-switch.adoc b/Documentation/git-switch.adoc
index d6c4f229a5..a8730b1da8 100644
--- a/Documentation/git-switch.adoc
+++ b/Documentation/git-switch.adoc
@@ -155,10 +155,11 @@ variable.
attached to a terminal, regardless of `--quiet`.
`-t`::
-`--track[ (direct|inherit)]`::
+`--track[=(direct|inherit|fetch)[,...]]`::
When creating a new branch, set up "upstream" configuration.
`-c` is implied. See `--track` in linkgit:git-branch[1] for
- details.
+ details, and `--track` in linkgit:git-checkout[1] for the
+ `fetch` mode.
+
If no `-c` option is given, the name of the new branch will be derived
from the remote-tracking branch, by looking at the local part of the
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 1345e8574a..25b511f22a 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -25,10 +25,12 @@
#include "preload-index.h"
#include "read-cache.h"
#include "refs.h"
+#include "refspec.h"
#include "remote.h"
#include "repo-settings.h"
#include "resolve-undo.h"
#include "revision.h"
+#include "run-command.h"
#include "sequencer.h"
#include "setup.h"
#include "strvec.h"
@@ -62,6 +64,7 @@ struct checkout_opts {
int count_checkout_paths;
int overlay_mode;
int dwim_new_local_branch;
+ int fetch;
int discard_changes;
int accept_ref;
int accept_pathspec;
@@ -115,6 +118,129 @@ struct branch_info {
char *checkout;
};
+static void fetch_remote_for_start_point(const char *arg, int quiet)
+{
+ struct strbuf dst = STRBUF_INIT;
+ struct tracking tracking;
+ struct string_list tracking_srcs = STRING_LIST_INIT_DUP;
+ struct string_list ambiguous_remotes = STRING_LIST_INIT_DUP;
+ struct child_process cmd = CHILD_PROCESS_INIT;
+ struct object_id oid;
+ struct remote *named_remote;
+ int bare_ns;
+
+ strbuf_addf(&dst, "refs/remotes/%s", arg);
+ if (check_refname_format(dst.buf, 0))
+ die(_("cannot fetch start-point '%s': not a valid "
+ "remote-tracking name"), arg);
+
+ named_remote = remote_get(arg);
+ bare_ns = !strchr(arg, '/') ||
+ (named_remote && remote_is_configured(named_remote, 1));
+ if (bare_ns) {
+ char *head_path = xstrfmt("refs/remotes/%s/HEAD", arg);
+ const char *head_target =
+ refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
+ head_path,
+ RESOLVE_REF_READING |
+ RESOLVE_REF_NO_RECURSE,
+ &oid, NULL);
+ if (head_target &&
+ starts_with(head_target, dst.buf) &&
+ head_target[dst.len] == '/' &&
+ !check_refname_format(head_target, 0)) {
+ strbuf_reset(&dst);
+ strbuf_addstr(&dst, head_target);
+ bare_ns = 0;
+ }
+ free(head_path);
+ }
+
+ memset(&tracking, 0, sizeof(tracking));
+ tracking.spec.dst = dst.buf;
+ tracking.srcs = &tracking_srcs;
+ find_tracking_remote_for_ref(&tracking, &ambiguous_remotes);
+
+ if (tracking.matches > 1) {
+ int status = die_message(_("cannot fetch start-point '%s': "
+ "fetch refspecs of multiple remotes "
+ "map to '%s'"), arg, dst.buf);
+ advise_ambiguous_fetch_refspec(dst.buf, &ambiguous_remotes);
+ exit(status);
+ }
+
+ if (!tracking.matches) {
+ if (bare_ns && named_remote &&
+ remote_is_configured(named_remote, 1))
+ die(_("cannot fetch start-point '%s': "
+ "'refs/remotes/%s/HEAD' is not set; run "
+ "'git remote set-head %s --auto' to set it"),
+ arg, arg, arg);
+ die(_("cannot fetch start-point '%s': no configured remote's "
+ "fetch refspec matches it"), arg);
+ }
+
+ strvec_push(&cmd.args, "fetch");
+ if (quiet)
+ strvec_push(&cmd.args, "--quiet");
+ strvec_pushl(&cmd.args, tracking.remote,
+ tracking_srcs.items[0].string, NULL);
+ cmd.git_cmd = 1;
+ if (run_command(&cmd)) {
+ if (!refs_read_ref(get_main_ref_store(the_repository),
+ dst.buf, &oid))
+ warning(_("failed to fetch start-point '%s'; "
+ "using existing '%s'"), arg, dst.buf);
+ else
+ die(_("failed to fetch start-point '%s'"), arg);
+ }
+
+ string_list_clear(&tracking_srcs, 0);
+ string_list_clear(&ambiguous_remotes, 0);
+ strbuf_release(&dst);
+}
+
+static int parse_opt_checkout_track(const struct option *opt,
+ const char *arg, int unset)
+{
+ struct checkout_opts *opts = opt->value;
+ struct string_list tokens = STRING_LIST_INIT_DUP;
+ struct string_list_item *item;
+ int saw_direct = 0;
+ int ret = 0;
+
+ opts->fetch = 0;
+ if (unset) {
+ opts->track = BRANCH_TRACK_NEVER;
+ return 0;
+ }
+ opts->track = BRANCH_TRACK_EXPLICIT;
+ if (!arg)
+ return 0;
+
+ string_list_split(&tokens, arg, ",", -1);
+ for_each_string_list_item(item, &tokens) {
+ if (!strcmp(item->string, "fetch"))
+ opts->fetch = 1;
+ else if (!strcmp(item->string, "direct"))
+ saw_direct = 1;
+ else if (!strcmp(item->string, "inherit"))
+ opts->track = BRANCH_TRACK_INHERIT;
+ else {
+ ret = error(_("option `%s' expects \"%s\", \"%s\", "
+ "or \"%s\""),
+ "--track", "direct", "inherit", "fetch");
+ goto out;
+ }
+ }
+ if (saw_direct && opts->track == BRANCH_TRACK_INHERIT)
+ ret = error(_("option `%s' cannot combine \"%s\" and \"%s\""),
+ "--track", "direct", "inherit");
+out:
+ string_list_clear(&tokens, 0);
+ return ret;
+}
+
static void branch_info_release(struct branch_info *info)
{
free(info->name);
@@ -1733,10 +1859,10 @@ static struct option *add_common_switch_branch_options(
{
struct option options[] = {
OPT_BOOL('d', "detach", &opts->force_detach, N_("detach HEAD at named commit")),
- OPT_CALLBACK_F('t', "track", &opts->track, "(direct|inherit)",
+ OPT_CALLBACK_F('t', "track", opts, "(direct|inherit|fetch)[,...]",
N_("set branch tracking configuration"),
PARSE_OPT_OPTARG,
- parse_opt_tracking_mode),
+ parse_opt_checkout_track),
OPT__FORCE(&opts->force, N_("force checkout (throw away local modifications)"),
PARSE_OPT_NOCOMPLETE),
OPT_STRING(0, "orphan", &opts->new_orphan_branch, N_("new-branch"), N_("new unborn branch")),
@@ -1941,8 +2067,13 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
opts->dwim_new_local_branch &&
opts->track == BRANCH_TRACK_UNSPECIFIED &&
!opts->new_branch;
- int n = parse_branchname_arg(argc, argv, dwim_ok, which_command,
- &new_branch_info, opts, &rev);
+ int n;
+
+ if (opts->fetch)
+ fetch_remote_for_start_point(argv[0], opts->quiet);
+
+ n = parse_branchname_arg(argc, argv, dwim_ok, which_command,
+ &new_branch_info, opts, &rev);
argv += n;
argc -= n;
} else if (!opts->accept_ref && opts->from_treeish) {
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 7613b1d2a4..1e321b1512 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -870,4 +870,280 @@ test_expect_success 'tracking info copied with autoSetupMerge=inherit' '
test_cmp_config "" --default "" branch.main2.merge
'
+test_expect_success 'setup upstream for --track=fetch tests' '
+ git checkout main &&
+ git init fetch_upstream &&
+ test_commit -C fetch_upstream u_main &&
+ git remote add fetch_upstream fetch_upstream &&
+ git fetch fetch_upstream &&
+ git -C fetch_upstream checkout -b fetch_new &&
+ test_commit -C fetch_upstream u_new
+'
+
+test_expect_success 'checkout --track=fetch -b picks up branch created upstream after clone' '
+ git checkout main &&
+ test_must_fail git rev-parse --verify refs/remotes/fetch_upstream/fetch_new &&
+ git checkout --track=fetch -b local_new fetch_upstream/fetch_new &&
+ test_cmp_rev refs/remotes/fetch_upstream/fetch_new HEAD &&
+ test_cmp_config fetch_upstream branch.local_new.remote &&
+ test_cmp_config refs/heads/fetch_new branch.local_new.merge
+'
+
+test_expect_success 'checkout --track=fetch <remote>/<branch> leaves other tracking branches untouched' '
+ git checkout main &&
+ git -C fetch_upstream checkout -b fetch_target &&
+ test_commit -C fetch_upstream u_target_pre &&
+ git -C fetch_upstream checkout -b fetch_other &&
+ test_commit -C fetch_upstream u_other_pre &&
+ git fetch fetch_upstream &&
+ other_before=$(git rev-parse refs/remotes/fetch_upstream/fetch_other) &&
+ git -C fetch_upstream checkout fetch_target &&
+ test_commit -C fetch_upstream u_target_post &&
+ git -C fetch_upstream checkout fetch_other &&
+ test_commit -C fetch_upstream u_other_post &&
+ git checkout --track=fetch -b local_target fetch_upstream/fetch_target &&
+ test_cmp_rev refs/remotes/fetch_upstream/fetch_target HEAD &&
+ test "$(git rev-parse refs/remotes/fetch_upstream/fetch_other)" = "$other_before"
+'
+
+test_expect_success 'checkout --track=fetch with bare remote name fetches only <remote>/HEAD target' '
+ git checkout main &&
+ git -C fetch_upstream checkout main &&
+ git remote set-head fetch_upstream main &&
+ git -C fetch_upstream checkout -b fetch_unrelated &&
+ test_commit -C fetch_upstream u_unrelated_pre &&
+ git fetch fetch_upstream fetch_unrelated &&
+ unrelated_before=$(git rev-parse refs/remotes/fetch_upstream/fetch_unrelated) &&
+ git -C fetch_upstream checkout main &&
+ test_commit -C fetch_upstream u_main_post &&
+ git -C fetch_upstream checkout fetch_unrelated &&
+ test_commit -C fetch_upstream u_unrelated_post &&
+ git checkout --track=fetch -b local_from_remote fetch_upstream &&
+ test_cmp_rev refs/remotes/fetch_upstream/main HEAD &&
+ test "$(git rev-parse refs/remotes/fetch_upstream/fetch_unrelated)" = "$unrelated_before"
+'
+
+test_expect_success 'checkout --track=fetch aborts and does not create branch when no existing ref' '
+ git checkout main &&
+ test_might_fail git branch -D bogus &&
+ test_must_fail git checkout --track=fetch -b bogus fetch_upstream/does_not_exist &&
+ test_must_fail git rev-parse --verify refs/heads/bogus
+'
+
+test_expect_success 'checkout --track=fetch warns and proceeds when fetch fails but ref exists' '
+ git checkout main &&
+ git -C fetch_upstream checkout -b fetch_offline &&
+ test_commit -C fetch_upstream u_offline &&
+ git fetch fetch_upstream fetch_offline &&
+ saved_url=$(git config remote.fetch_upstream.url) &&
+ test_when_finished "git config remote.fetch_upstream.url \"$saved_url\"" &&
+ git config remote.fetch_upstream.url ./does-not-exist &&
+ git checkout --track=fetch -b local_offline fetch_upstream/fetch_offline 2>err &&
+ test_grep "failed to fetch" err &&
+ test_cmp_rev refs/remotes/fetch_upstream/fetch_offline HEAD
+'
+
+test_expect_success 'checkout --track=fetch resolves through configured fetch refspec' '
+ git checkout main &&
+ git remote add fetch_custom ./fetch_upstream &&
+ test_when_finished "git remote remove fetch_custom" &&
+ git config --replace-all remote.fetch_custom.fetch \
+ "+refs/heads/*:refs/remotes/custom-ns/*" &&
+ git -C fetch_upstream checkout -b fetch_refspec &&
+ test_commit -C fetch_upstream u_refspec &&
+ test_must_fail git rev-parse --verify refs/remotes/custom-ns/fetch_refspec &&
+ git checkout --track=fetch -b local_refspec custom-ns/fetch_refspec &&
+ test_cmp_rev refs/remotes/custom-ns/fetch_refspec HEAD
+'
+
+test_expect_success 'checkout --track=fetch on namespace bare name follows <ns>/HEAD' '
+ git checkout main &&
+ git remote add fetch_ns ./fetch_upstream &&
+ test_when_finished "git remote remove fetch_ns" &&
+ test_when_finished "git update-ref -d refs/remotes/ns_alias/HEAD" &&
+ git config --replace-all remote.fetch_ns.fetch \
+ "+refs/heads/*:refs/remotes/ns_alias/*" &&
+ git fetch fetch_ns &&
+ git symbolic-ref refs/remotes/ns_alias/HEAD refs/remotes/ns_alias/main &&
+ git -C fetch_upstream checkout main &&
+ test_commit -C fetch_upstream u_ns_post &&
+ git checkout --track=fetch -b local_ns ns_alias &&
+ test_cmp_rev refs/remotes/ns_alias/main HEAD &&
+ test_cmp_config fetch_ns branch.local_ns.remote &&
+ test_cmp_config refs/heads/main branch.local_ns.merge
+'
+
+test_expect_success '--track=fetch on bare hierarchical remote name follows <ns>/HEAD' '
+ git checkout main &&
+ git remote add nested/bare ./fetch_upstream &&
+ test_when_finished "git remote remove nested/bare" &&
+ test_when_finished "git update-ref -d refs/remotes/nested/bare/HEAD" &&
+ git fetch nested/bare &&
+ git symbolic-ref refs/remotes/nested/bare/HEAD \
+ refs/remotes/nested/bare/main &&
+ git -C fetch_upstream checkout main &&
+ test_commit -C fetch_upstream u_nested_bare_post &&
+ git checkout --track=fetch -b local_nested_bare nested/bare &&
+ test_cmp_rev refs/remotes/nested/bare/main HEAD
+'
+
+test_expect_success 'checkout --track=fetch handles hierarchical remote name' '
+ git checkout main &&
+ git remote add nested/remote ./fetch_upstream &&
+ test_when_finished "git remote remove nested/remote" &&
+ git -C fetch_upstream checkout -b fetch_hier &&
+ test_commit -C fetch_upstream u_hier &&
+ test_must_fail git rev-parse --verify refs/remotes/nested/remote/fetch_hier &&
+ git checkout --track=fetch -b local_hier nested/remote/fetch_hier &&
+ test_cmp_rev refs/remotes/nested/remote/fetch_hier HEAD
+'
+
+test_expect_success 'checkout --track=fetch dies on bare remote name with no <ns>/HEAD' '
+ git checkout main &&
+ git remote add fetch_nohead ./fetch_upstream &&
+ test_when_finished "git remote remove fetch_nohead" &&
+ test_might_fail git symbolic-ref -d refs/remotes/fetch_nohead/HEAD &&
+ test_must_fail git checkout --track=fetch -b local_nohead fetch_nohead 2>err &&
+ test_grep "refs/remotes/fetch_nohead/HEAD" err &&
+ test_grep "git remote set-head fetch_nohead --auto" err &&
+ test_must_fail git rev-parse --verify refs/heads/local_nohead
+'
+
+test_expect_success 'checkout --track=fetch on bare unknown name does not suggest set-head' '
+ git checkout main &&
+ test_must_fail git rev-parse --verify refs/remotes/no_such_ns/HEAD &&
+ test_must_fail git config --get remote.no_such_ns.url &&
+ test_must_fail git checkout --track=fetch -b local_unknown no_such_ns 2>err &&
+ test_grep "no configured remote" err &&
+ test_grep ! "set-head" err &&
+ test_must_fail git rev-parse --verify refs/heads/local_unknown
+'
+
+test_expect_success 'checkout --track=fetch rejects <ns>/HEAD pointing outside namespace' '
+ git checkout main &&
+ git remote add fetch_crossns ./fetch_upstream &&
+ test_when_finished "git remote remove fetch_crossns" &&
+ test_when_finished "git update-ref -d refs/remotes/fetch_crossns/HEAD" &&
+ git fetch fetch_crossns &&
+ git symbolic-ref refs/remotes/fetch_crossns/HEAD \
+ refs/remotes/fetch_upstream/u_main &&
+ test_must_fail git checkout --track=fetch -b local_crossns fetch_crossns 2>err &&
+ test_grep "refs/remotes/fetch_crossns/HEAD" err &&
+ test_must_fail git rev-parse --verify refs/heads/local_crossns
+'
+
+test_expect_success 'checkout --track=fetch dies on ambiguous fetch refspec match' '
+ git checkout main &&
+ git remote add fetch_ambig_a ./fetch_upstream &&
+ git remote add fetch_ambig_b ./fetch_upstream &&
+ test_when_finished "git remote remove fetch_ambig_a" &&
+ test_when_finished "git remote remove fetch_ambig_b" &&
+ git config --replace-all remote.fetch_ambig_a.fetch \
+ "+refs/heads/*:refs/remotes/ambig_ns/*" &&
+ git config --replace-all remote.fetch_ambig_b.fetch \
+ "+refs/heads/*:refs/remotes/ambig_ns/*" &&
+ git -C fetch_upstream checkout -b fetch_ambig &&
+ test_commit -C fetch_upstream u_ambig &&
+ test_must_fail git checkout --track=fetch -b local_ambig ambig_ns/fetch_ambig 2>err &&
+ test_grep "fetch_ambig_a" err &&
+ test_grep "fetch_ambig_b" err &&
+ test_grep "tracking namespaces" err &&
+ test_must_fail git rev-parse --verify refs/heads/local_ambig
+'
+
+test_expect_success 'checkout --track=fetch rejects invalid refname components' '
+ git checkout main &&
+ test_must_fail git checkout --track=fetch -b local_invalid "foo..bar" 2>err &&
+ test_grep "valid" err &&
+ test_must_fail git rev-parse --verify refs/heads/local_invalid
+'
+
+test_expect_success 'checkout --track=fetch,inherit rejects invalid refname components' '
+ git checkout main &&
+ test_must_fail git checkout --track=fetch,inherit -b local_invalid \
+ "foo..bar" 2>err &&
+ test_grep "valid" err &&
+ test_must_fail git rev-parse --verify refs/heads/local_invalid
+'
+
+test_expect_success 'checkout --track=inherit,direct is rejected' '
+ test_must_fail git checkout --track=inherit,direct -b bad fetch_upstream/fetch_new 2>err &&
+ test_grep "cannot combine" err
+'
+
+test_expect_success 'checkout --track=direct,inherit is rejected' '
+ test_must_fail git checkout --track=direct,inherit -b bad fetch_upstream/fetch_new 2>err &&
+ test_grep "cannot combine" err
+'
+
+test_expect_success 'checkout --track=fetch then --track=direct drops fetch (last-one-wins)' '
+ git checkout main &&
+ git -C fetch_upstream checkout -b fetch_lastwin &&
+ test_commit -C fetch_upstream u_lastwin &&
+ test_must_fail git rev-parse --verify refs/remotes/fetch_upstream/fetch_lastwin &&
+ test_must_fail git checkout --track=fetch --track=direct \
+ -b local_lastwin fetch_upstream/fetch_lastwin &&
+ test_must_fail git rev-parse --verify refs/remotes/fetch_upstream/fetch_lastwin
+'
+
+test_expect_success 'checkout --track=fetch then --no-track drops fetch' '
+ git checkout main &&
+ git -C fetch_upstream checkout -b fetch_notrack &&
+ test_commit -C fetch_upstream u_notrack &&
+ test_must_fail git rev-parse --verify refs/remotes/fetch_upstream/fetch_notrack &&
+ test_must_fail git checkout --track=fetch --no-track \
+ -b local_notrack fetch_upstream/fetch_notrack &&
+ test_must_fail git rev-parse --verify refs/remotes/fetch_upstream/fetch_notrack
+'
+
+test_expect_success 'checkout --track=fetch,inherit fetches remote-tracking start-point' '
+ git checkout main &&
+ git -C fetch_upstream checkout -b fetch_inherit &&
+ test_commit -C fetch_upstream u_inherit &&
+ test_must_fail git rev-parse --verify refs/remotes/fetch_upstream/fetch_inherit &&
+ git checkout --track=fetch,inherit -b local_inherit \
+ fetch_upstream/fetch_inherit &&
+ test_cmp_rev refs/remotes/fetch_upstream/fetch_inherit HEAD
+'
+
+test_expect_success 'checkout --track=fetch,inherit errors when start-point does not map to a remote' '
+ git checkout main &&
+ test_must_fail git checkout --track=fetch,inherit -b bad main 2>err &&
+ test_grep "no configured remote" err &&
+ test_must_fail git rev-parse --verify refs/heads/bad
+'
+
+test_expect_success 'checkout --track=fetch on local start-point errors' '
+ git checkout main &&
+ test_must_fail git checkout --track=fetch -b bad main 2>err &&
+ test_grep "no configured remote" err &&
+ test_must_fail git rev-parse --verify refs/heads/bad
+'
+
+test_expect_success 'checkout --track=bogus reports an error' '
+ git checkout main &&
+ test_must_fail git checkout --track=bogus -b bogus_branch fetch_upstream/fetch_new 2>err &&
+ test_grep "expects" err
+'
+
+test_expect_success 'checkout -q --track=fetch silences the fetch output' '
+ git checkout main &&
+ git -C fetch_upstream checkout -b fetch_quiet &&
+ test_commit -C fetch_upstream u_quiet &&
+ test_must_fail git rev-parse --verify refs/remotes/fetch_upstream/fetch_quiet &&
+ git checkout -q --track=fetch -b local_quiet \
+ fetch_upstream/fetch_quiet 2>err &&
+ test_grep ! "-> fetch_upstream/fetch_quiet" err &&
+ test_cmp_rev refs/remotes/fetch_upstream/fetch_quiet HEAD
+'
+
+test_expect_success 'switch --track=fetch -c picks up branch created upstream after clone' '
+ git checkout main &&
+ git -C fetch_upstream checkout -b fetch_switch &&
+ test_commit -C fetch_upstream u_switch &&
+ test_must_fail git rev-parse --verify refs/remotes/fetch_upstream/fetch_switch &&
+ git switch --track=fetch -c local_switch fetch_upstream/fetch_switch &&
+ test_cmp_rev refs/remotes/fetch_upstream/fetch_switch HEAD
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH v2 10/11] git-gui: adapt blame/browser parsing for bare operation
From: Johannes Sixt @ 2026-05-23 20:31 UTC (permalink / raw)
To: Mark Levedahl; +Cc: egg_mushroomcow, bootaina702, git
In-Reply-To: <20260520202411.108764-11-mlevedahl@gmail.com>
Am 20.05.26 um 22:24 schrieb Mark Levedahl:
> git-gui's blame and browser subcommands do not work with bare
> repositories, but they should per commit c52c94524b ("git-gui: Allow
> blame/browser subcommands on bare repositories", 2007-07-17). Assuming
> that commit worked, something changed since reintroducing a hard-coded
> dependency upon a worktree.
>
> The basic issue goes back to 3e45ee1ef2 ("git-gui: Smarter command line
> parsing for browser, blame", 2007-05-08), which seeks to implement
> command line parsing similar to git blame. That commit introduces
> depencies upon the worktree to decide which argument is rev or path.
>
> Looking at builtin/blame.c in git around line 1120:
>
> * (1) if dashdash_pos != 0, it is either
> * "blame [revisions] -- <path>" or
> * "blame -- <path> <rev>"
> *
> * (2) otherwise, it is one of the two:
> * "blame [revisions] <path>"
> * "blame <path> <rev>"
>
> shows the clear intent: rev and path may be swapped in input so both
> meanings must be tried, but -- may be used to designate which is the
> path forcing or precluding trying the swapped arguments.
Please do not use this code comment as recipe for our own argument
parseing. In particular, that <path> can occur for <rev> goes back to
the initial implementation of git pickaxe in cee7f245dcae ("git-pickaxe:
blame rewritten.", 2006-10-19). Since acca687fa9db ("git-pickaxe: retire
pickaxe", 2006-11-08), the documentation of git-blame states that <file>
is always last (but the implementation was not adjusted accordingly).
In general, Git's argument parsing requires revisions before pathspec.
To disambiguate, '--' can be used. If it is not used, arguments are
check whether they are files or refs, and as soon as one argument is
identified as file unambiguously, all later arguments must also be files.
We should follow this pattern, and to do that, we could just delegate
argument processing to `git rev-parse`.
>
> With a worktree, git gui correctly swaps the arguments if the given path
> exists in the worktree. git blame does this using the git repository.
> But, git-gui sometimes interprets the -- to have an exactly opposite
> meaning:
>
> git blame Makefile gitgui-0.19.0 works
> git gui blame Makefile gitgui-0.19.0 works
Git gui shows something, but ignores the ref, so doesn't quite work.
>
> git blame -- Makefile gitgui-0.19.0 works
> git gui blame -- Makefile gitgui-0.19.0 works
Ditto.
>
> git blame Makefile -- gitgui-0.19.0 fails (correctly)
> git gui blame Makefile -- gitgui-0.19.0 works (should fail)
Ditto.
>
> git blame gitgui-0.19.0 -- Makefile works (correctly)
> git gui blame gitgui-0.19.0 -- Makefile fails (should work)
Yes, there's a bug in the argument parser that -- isn't skipped, but
treated as the file name.
>
> It is possible to patch the code to operate without a worktree, but this
> will make the commands operate differently with and without a worktree,
> won't fix the parsing issues above, and won't address the issues that
> can arise when using a worktree to help decisions on a different rev
> with file/directory conflicts, etc.
Before this patch 'git gui blame' can show contents uncommitted changes,
but with this patch this isn't possible. I see you have just sent a
patch that may fix this, but I havn't looked at it, yet.
>
> So, let's rework the parser so that it uses -- as does git blame, and
> uses git ls-tree to query the given revision for existence and type of
> path rather than basing this upon a possibly unrelated worktree. Also,
> abort early when the given path is not found, or does not match the need
> (file or directory). This fixes some current cases where git-gui will
> open a window with no content, possibly also with an error message.
There is no desire to make 'git gui blame' work the same with and
without a working tree.
If we invoke git to help argument parsing, then it should be
'rev-parse', not 'ls-tree'.
>
> This does not change whether or how git-gui uses staged and unstaged
> content in the current worktree for blame display.
>
> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
> ---
> wm deiconify .
> switch -- $subcommand {
> browser {
> - if {$jump_spec ne {}} usage
Let's keep this line, which diagnoses an incorrect --line= argument.
> - 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 {
> blame::new $head $path $jump_spec
> }
> }
-- Hannes
^ permalink raw reply
* Re: [PATCH 0/3] line-log: integrate -L with the standard log output pipeline
From: Michael Montalbo @ 2026-05-24 2:00 UTC (permalink / raw)
To: D. Ben Knoble; +Cc: Michael Montalbo via GitGitGadget, git, Junio C Hamano
In-Reply-To: <CALnO6CBh7nDCwT=u1xSN2c6_x88t_gNfAaT_B4PzYKr=5i_bNA@mail.gmail.com>
On Fri, May 22, 2026 at 11:46 AM D. Ben Knoble <ben.knoble@gmail.com> wrote:
>
> Hi Michael,
>
Hi Ben,
Thanks for the thorough review!
> On Tue, Apr 28, 2026 at 12:06 AM Michael Montalbo via GitGitGadget <gitgitgadget@gmail.com> wrote:
>>
>> Since its introduction, git log -L has short-circuited from
>> log_tree_commit() into its own output function, bypassing log_tree_diff()
>> and log_tree_diff_flush(). This skips no_free save/restore,
>> always_show_header, diff_free() cleanup, and means that pickaxe (-S, -G,
>> --find-object) and --diff-filter cannot suppress commits whose pairs are all
>> filtered out, because show_log() runs before diffcore_std().
>>
>> This series restructures the flow so that -L goes through the same
>> log_tree_diff() -> log_tree_diff_flush() path as normal single-parent and
>> merge diffs, then uses that to enable several non-patch diff formats.
>
>
> Cleanup by itself to shrink the number of concepts in the code is already a good thing IMO, so getting additional features out of it is even nicer.
>
>> Patch 1: revision: move -L setup before output_format-to-diff derivation
>>
>> Preparatory reorder in setup_revisions(). The -L block sets a default
>> DIFF_FORMAT_PATCH when no format is requested; move it before the derivation
>> of revs->diff from output_format so the default is visible to that check. No
>> behavior change on its own.
>
>
> Straightforward, nice.
>
>>
>> Patch 2: line-log: integrate -L output with the standard log-tree pipeline
>>
>> Rename line_log_print() to line_log_queue_pairs(), stripping it down to only
>> queue pre-computed filepairs. log_tree_diff_flush() handles show_log(),
>> diffcore_std(), and diff_flush(). This fixes pickaxe and --diff-filter
>> suppression, and aligns the commit/diff separator with the rest of log
>> output. Also rejects --full-diff, which is meaningless when filepairs are
>> pre-computed.
>
>
> At first I questioned the removal of the DIFF_FORMAT_NO_OUTPUT conditional in line_log_queue_pairs, but now that it only queues pairs it shouldn't be checking output formats. Good.
>
> I also noted that log_tree_diff() returns the result of log_tree_diff_flush() in the -L case, which is a bit different from the other patterns. I think the difference is that the other cases have some conditional logic around the log_tree_diff_flush cases (?) but I'm not sure. Perhaps that branch should also be looking at opt->loginfo ?
>
Good catch. In practice I think they agree, since `log_tree_diff_flush()`
returns 1 exactly when it calls `show_log()` which consumes loginfo,
but matching the existing convention is cleaner. Will update.
> Finally, I wonder if in describing the removal of the early return:
>
> > - Remove the early return in log_tree_commit() that bypassed
> > no_free save/restore, always_show_header, and diff_free().
>
> we might want to be more explicit that this is _because_ line-level diff is now handled in the regular pipeline?
>
Agreed, will reword to: "Remove the early return in log_tree_commit()
that is no longer needed now that -L output flows through
log_tree_diff() and log_tree_diff_flush(); this restores no_free
save/restore, always_show_header, and diff_free() cleanup."
> [I suppose we could, in theory, split the rejection of --full-diff to a separate prep commit, idk.j]
>
It felt natural to me to put alongside the integration since
--full-diff is not yet implementable with pre-computed filepairs.
Happy to split it out if you feel strongly though.
>> Patch 3: line-log: allow non-patch diff formats with -L
>>
>> Expand the allowlist to accept --raw, --name-only, --name-status, and
>> --summary. These only read filepair metadata already set by the line-log
>> machinery. Diff stat formats (--stat, --numstat, --shortstat, --dirstat)
>> remain blocked because they call compute_diffstat() on full blob content and
>> would show whole-file statistics rather than range-scoped ones.
>
>
> Short and sweet.
>
> The stat formats are kind of like --full-diff, and I think they should probably all be rejected or all allowed: since the stats are based on the full-diff, it makes sense to enable them if we can also make -L + --full-diff semantically sensible.
>
> Otherwise, we'd need to find a way to make the stat formats scoped for -L.
>
I am working on a follow up series that takes the second path
you suggest: it adds a line-range filter in `diffcore_std()` that
clips insertions and deletions to the tracked ranges before
`compute_diffstat()` runs, so `--stat`, `--numstat`, etc. report
range-scoped numbers. That series builds on top of these three patches,
which is why stats remain blocked here.
For `--full-diff`, thinking about it more, the semantics would actually
be well-defined: "filter commits by line range, but show the full
diff for those commits." Right now, there might be a higher
implementation barrier, though. The line-log machinery fuses
commit filtering with diff generation, so there is no separate
"full diff" to fall back to for display. I will soften the rejection to
"not yet supported" rather than "incompatible," since it could
be wired up if someone separates the two concerns.
>>
>>
>> Michael Montalbo (3):
>> revision: move -L setup before output_format-to-diff derivation
>> line-log: integrate -L output with the standard log-tree pipeline
>> line-log: allow non-patch diff formats with -L
>>
>> Documentation/line-range-options.adoc | 10 +-
>> line-log.c | 30 ++----
>> line-log.h | 2 +-
>> log-tree.c | 9 +-
>> revision.c | 25 +++--
>> t/t4211-line-log.sh | 99 ++++++++++++++++---
>> t/t4211/sha1/expect.parallel-change-f-to-main | 1 -
>> .../sha256/expect.parallel-change-f-to-main | 1 -
>> 8 files changed, 120 insertions(+), 57 deletions(-)
>>
>>
>> base-commit: 9f223ef1c026d91c7ac68cc0211bde255dda6199
>> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2094%2Fmmontalbo%2Fmm%2Fline-log-use-log-tree-diff-flush-v1
>> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2094/mmontalbo/mm/line-log-use-log-tree-diff-flush-v1
>> Pull-Request: https://github.com/gitgitgadget/git/pull/2094
>> --
>> gitgitgadget
>
>
> A few other comments:
>
> - Tests should use test_grep; some do, but some don't.
> - There is one occurrence of "sed | grep" that I wonder if we want to rewrite to avoid issues with exit status one side of the pipe?
>
Will fix both these issues.
> Thanks for working on this!
>
> [Apologies for the unusual review format; this was easier for me at the moment than digging up the individual patches, and I don't think _most_ of the review would benefit from spreading out across multiple mails.]
>
Thank you too! This review format worked fine for me :)
> --
> D. Ben Knoble
^ permalink raw reply
* [PATCH] completion: hide dotfiles for selected path completion
From: Zakariyah Ali via GitGitGadget @ 2026-05-24 2:36 UTC (permalink / raw)
To: git; +Cc: Zakariyah Ali, Zakariyah Ali
From: Zakariyah Ali <zakariyahali100@gmail.com>
Signed-off-by: Zakariyah Ali <zakariyahali100@gmail.com>
---
completion: hide dotfiles for selected path completion
The completion helper for index paths uses git ls-files rather than
shell filename completion. As a result, leading-dot paths such as a
tracked .gitignore were offered even when the user had not started the
path with ..
Hide leading-dot path components for git rm, git mv, and git ls-files
when completing an empty path component. Explicit dot completion is
still preserved, so git rm . can still complete .gitignore.
This removes the existing TODO expectations in t/t9902-completion.sh and
adds coverage for explicit dot completion.
Validation:
* git diff --check -- contrib/completion/git-completion.bash
t/t9902-completion.sh
* bash -n contrib/completion/git-completion.bash
* ./t9902-completion.sh
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2311%2Falibaba0010%2Fcompletion-hide-dotfiles-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2311/alibaba0010/completion-hide-dotfiles-v1
Pull-Request: https://github.com/git/git/pull/2311
contrib/completion/git-completion.bash | 36 +++++++++++++++++---------
t/t9902-completion.sh | 10 ++-----
2 files changed, 26 insertions(+), 20 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index a8e7c6ddbf..e8f8fab125 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -638,25 +638,33 @@ __git_ls_files_helper ()
}
-# __git_index_files accepts 1 or 2 arguments:
+# __git_index_files accepts 1 to 4 arguments:
# 1: Options to pass to ls-files (required).
# 2: A directory path (optional).
# If provided, only files within the specified directory are listed.
# Sub directories are never recursed. Path must have a trailing
# slash.
# 3: List only paths matching this path component (optional).
+# 4: Hide paths whose first component starts with a dot if this is
+# "hide-dotfiles" and the third argument is empty (optional).
__git_index_files ()
{
- local root="$2" match="$3"
+ local root="$2" match="$3" hide_dotfiles="${4-}"
+ local hide_dotfiles_awk=0
+ if [ "$hide_dotfiles" = "hide-dotfiles" ] && [ -z "$match" ]; then
+ hide_dotfiles_awk=1
+ fi
__git_ls_files_helper "$root" "$1" "${match:-?}" |
- awk -F / -v pfx="${2//\\/\\\\}" '{
+ awk -F / -v pfx="${2//\\/\\\\}" -v hide_dotfiles="$hide_dotfiles_awk" '{
paths[$1] = 1
}
END {
for (p in paths) {
if (substr(p, 1, 1) != "\"") {
# No special characters, easy!
+ if (hide_dotfiles == 1 && substr(p, 1, 1) == ".")
+ continue
print pfx p
continue
}
@@ -675,8 +683,10 @@ __git_index_files ()
# We have seen the same directory unquoted,
# skip it.
continue
- else
- print pfx p
+
+ if (hide_dotfiles == 1 && substr(p, 1, 1) == ".")
+ continue
+ print pfx p
}
}
function dequote(p, bs_idx, out, esc, esc_idx, dec) {
@@ -721,13 +731,15 @@ __git_index_files ()
}'
}
-# __git_complete_index_file requires 1 argument:
+# __git_complete_index_file accepts 1 or 2 arguments:
# 1: the options to pass to ls-file
+# 2: Hide paths whose first component starts with a dot if this is
+# "hide-dotfiles" and the current word is empty (optional).
#
# The exception is --committable, which finds the files appropriate commit.
__git_complete_index_file ()
{
- local dequoted_word pfx="" cur_
+ local dequoted_word pfx="" cur_ hide_dotfiles="${2-}"
__git_dequote "$cur"
@@ -740,7 +752,7 @@ __git_complete_index_file ()
cur_="$dequoted_word"
esac
- __gitcomp_file_direct "$(__git_index_files "$1" "$pfx" "$cur_")"
+ __gitcomp_file_direct "$(__git_index_files "$1" "$pfx" "$cur_" "$hide_dotfiles")"
}
# Lists branches from the local repository.
@@ -2164,7 +2176,7 @@ _git_ls_files ()
# XXX ignore options like --modified and always suggest all cached
# files.
- __git_complete_index_file "--cached"
+ __git_complete_index_file "--cached" hide-dotfiles
}
_git_ls_remote ()
@@ -2397,9 +2409,9 @@ _git_mv ()
if [ $(__git_count_arguments "mv") -gt 0 ]; then
# We need to show both cached and untracked files (including
# empty directories) since this may not be the last argument.
- __git_complete_index_file "--cached --others --directory"
+ __git_complete_index_file "--cached --others --directory" hide-dotfiles
else
- __git_complete_index_file "--cached"
+ __git_complete_index_file "--cached" hide-dotfiles
fi
}
@@ -3219,7 +3231,7 @@ _git_rm ()
;;
esac
- __git_complete_index_file "--cached"
+ __git_complete_index_file "--cached" hide-dotfiles
}
_git_shortlog ()
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 28f61f08fb..02aaf71876 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -2811,17 +2811,15 @@ test_expect_success 'complete files' '
touch untracked &&
- : TODO .gitignore should not be here &&
test_completion "git rm " <<-\EOF &&
- .gitignore
modified
EOF
+ test_completion "git rm ." ".gitignore" &&
+
test_completion "git clean " "untracked" &&
- : TODO .gitignore should not be here &&
test_completion "git mv " <<-\EOF &&
- .gitignore
modified
EOF
@@ -2832,9 +2830,7 @@ test_expect_success 'complete files' '
mkdir untracked-dir &&
- : TODO .gitignore should not be here &&
test_completion "git mv modified " <<-\EOF &&
- .gitignore
dir
modified
untracked
@@ -2843,9 +2839,7 @@ test_expect_success 'complete files' '
test_completion "git commit " "modified" &&
- : TODO .gitignore should not be here &&
test_completion "git ls-files " <<-\EOF &&
- .gitignore
dir
modified
EOF
base-commit: 9b7fa37559a1b95ee32e32858b0d038b4cf583e5
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH v2 11/11] git-gui: add gui and pick as explicit subcommands
From: Johannes Sixt @ 2026-05-24 7:00 UTC (permalink / raw)
To: Mark Levedahl; +Cc: egg_mushroomcow, bootaina702, git
In-Reply-To: <20260520202411.108764-12-mlevedahl@gmail.com>
Am 20.05.26 um 22:24 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 discovery 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 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.
Good.
> @@ -1174,7 +1184,7 @@ proc unset_gitdir_vars {} {
>
> # find repository.
> set _gitdir {}
> -if {$_gitdir eq {}} {
> +if {[is_enabled gitdir_discovery]} {
This makes a factually unconditional branch into a conditional one.
> if {[catch {
> set _gitdir [git rev-parse --absolute-git-dir]
> } err]} {
> @@ -1186,7 +1196,7 @@ if {$_gitdir eq {}} {
> }
>
> set picked 0
> -if {$_gitdir eq {}} {
> +if {$_gitdir eq {} && [is_enabled picker]} {
> unset_gitdir_vars
> load_config 1
> apply_config
> @@ -1202,6 +1212,12 @@ if {$_gitdir eq {}} {
> set picked 1
> }
>
> +if {$_gitdir eq {}} {
> + catch {wm withdraw .}
> + error_popup [strcat [mc "Git directory not found:"] "\n\n$err"]
I wondered where this $err is filled in, and it can only be the error
from a failed gitdir discovery. Good.
> + exit 1
> +}
> +
> # find worktree, continue without if not required
> if {[catch {
> set _gitworktree [git rev-parse --show-toplevel]
This looks good!
-- Hannes
^ permalink raw reply
* Re: [PATCH v2 00/11] Improve git gui operation without a worktree
From: Johannes Sixt @ 2026-05-24 7:16 UTC (permalink / raw)
To: Mark Levedahl; +Cc: egg_mushroomcow, bootaina702, git
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
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox