Git development
 help / color / mirror / Atom feed
* [PATCH 1/8] t0001: plug test gaps for git-init(1) with GIT_OBJECT_DIRECTORY
From: Patrick Steinhardt @ 2026-05-21  7:42 UTC (permalink / raw)
  To: git
In-Reply-To: <20260521-b4-pks-setup-centralize-odb-creation-v1-0-f130d2a7e8ae@pks.im>

In subsequent commits we'll rework how we set up the repository. This is
a somewhat intricate and thus fragile sequence, there's many things that
can go subtly wrong, and there are lots of interesting interactions that
one can discover.

One such discovered edge case was the interaction between git-init(1)
and the "GIT_OBJECT_DIRECTORY" enviroment variable. When set, the
behaviour is that the object directory should be created at the path
that the variable points to. This behaviour is documented as such in
its man page:

  If the object storage directory is specified via the
  GIT_OBJECT_DIRECTORY environment variable then the sha1 directories
  are created underneath; otherwise, the default $GIT_DIR/objects
  directory is used.

Curiously enough though we don't seem to have any tests that exercise
this directly, and thus a subsequent commit inadvertently broke this
expectation.

Plug this test gap.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t0001-init.sh | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index e4d32bb4d2..e89feca544 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -980,4 +980,14 @@ test_expect_success 're-init reads matching includeIf.onbranch' '
 	test_cmp expect err
 '
 
+test_expect_success 'init honors GIT_OBJECT_DIRECTORY' '
+	test_when_finished "rm -rf init-objdir custom-odb" &&
+	mkdir custom-odb &&
+	env GIT_OBJECT_DIRECTORY="$(pwd)/custom-odb" \
+		git init init-objdir &&
+	test_path_is_missing init-objdir/.git/objects/pack &&
+	test_path_is_dir custom-odb/pack &&
+	test_path_is_dir custom-odb/info
+'
+
 test_done

-- 
2.54.0.771.g3ed373ac14.dirty


^ permalink raw reply related

* [PATCH 0/8] setup: centralize object database creation
From: Patrick Steinhardt @ 2026-05-21  7:42 UTC (permalink / raw)
  To: git

Hi,

this small patch series refactors the logic for how we discover and
configure repositories. Most importantly, this involves the following
two steps:

  1. We unify the logic to apply the repository format, which is
     currently open-coded across multiple sites. These sites have
     already diverged, where some repository extensions are not
     consistently applied.

  2. We then centralize creation of the object database to happen at the
     same time we apply the repository format.

The end result is that we apply the repository format exactly once, and
that's also the point in time where we can finalize the setup of the
repo's data structures as we know about all details of the repo at that
time. Ultimately, this makes it trivial to introduce the "objectStorage"
extension, even though that's not part of this patch series.

The series is built on top of aec3f58750 (Sync with 'maint', 2026-05-21)
with ps/setup-wo-the-repository at df69f40c34 (setup: stop using
`the_repository` in `init_db()`, 2026-05-19) merged into it.

Thanks!

Patrick

---
Patrick Steinhardt (8):
      t0001: plug test gaps for git-init(1) with GIT_OBJECT_DIRECTORY
      setup: drop `setup_git_env()`
      setup: deduplicate logic to apply repository format
      repository: stop initializing the object database in `repo_set_gitdir()`
      setup: stop creating the object database in `setup_git_env()`
      setup: stop initializing object database without repository
      repository: stop reading loose object map twice on repo init
      setup: construct object database in `apply_repository_format()`

 commit-graph.c  |   4 +-
 environment.h   |   8 +---
 refs.c          |   3 +-
 repository.c    |  40 +++++------------
 repository.h    |   3 --
 setup.c         | 130 +++++++++++++++++++++++++++++++-------------------------
 setup.h         |  19 +++++++++
 t/t0001-init.sh |  10 +++++
 8 files changed, 117 insertions(+), 100 deletions(-)


---
base-commit: 3398daa441965513c48744305d33bd36404547d6
change-id: 20260519-b4-pks-setup-centralize-odb-creation-3479c53fb11d


^ permalink raw reply

* Re: [PATCH 4/9] run-command: add support for timeout in command finisher
From: Johannes Sixt @ 2026-05-21  7:21 UTC (permalink / raw)
  To: Siddh Raman Pant
  Cc: Calvin Wan, Patrick Steinhardt, Elijah Newren,
	Kristoffer Haugsbakk, Junio C Hamano, git
In-Reply-To: <f58c8c522814dce9257f64733e9fbc9bd9f446c0.1779207350.git.siddh.raman.pant@oracle.com>

Am 19.05.26 um 18:30 schrieb Siddh Raman Pant:
> A called command may not respond to the initial signal and will get
> stuck in finish_command() -> wait_or_whine().
> 
> So let's add timeout support into the finisher so that if a deadline
> occurs, we can send a force-kill signal.

This is extremely suspicious. A communication protocl with a child
program that requires to kill the child looks like a design error. A
band-aid like this timeout should not be necessary for a well-behaved
child process.

If the (your?) problem is that the child process is actually not
well-behaved, then I suggest to use a middle-man as child process that
behaves well from the point of view of the git process, but can punish
the ill-behaved downstream process when needed.

Please, do not add this infrastructure to core Git, and instead fix the
communication protocol.

> 
> The force-kill signal is in the argument because a program may trap a
> signal, so it is the responsibility of caller to pass the correct kill
> signal.
-- Hannes


^ permalink raw reply

* Re: [PATCH 0/2] builtin/maintenance: fix locking and respect "gc.auto"
From: Junio C Hamano @ 2026-05-21  5:55 UTC (permalink / raw)
  To: Patrick Steinhardt
  Cc: Jean-Christophe Manciot, Mikael Magnusson, Jeff King, Taylor Blau,
	Derrick Stolee, git
In-Reply-To: <ag6ahXA104_70g3e@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> Hi,
>
> On Mon, May 11, 2026 at 02:29:54PM +0200, Patrick Steinhardt wrote:
>> this patch series addresses the issues reported in [1]. The series is
>> built on top of Git 2.54.0.
>
> Junio: I saw that you are starting to prep for Git 2.54.1, and
> a89346e34a (Start preparing for 2.54.1, 2026-05-21) explicitly mentions
> a couple of additional topics that should land in that bugfix release.
> This topic here isn't mentioned though, but I very much think that these
> fixes should be included.

Sure.  As of https://lore.kernel.org/git/ag1MHje6-C6nmcO4@pks.im/ I
think it can be merged to 'next', which will allow me to list it in
there?

Are there other topics that should be fast-tracked?



^ permalink raw reply

* Re: [PATCH 0/2] builtin/maintenance: fix locking and respect "gc.auto"
From: Patrick Steinhardt @ 2026-05-21  5:39 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jean-Christophe Manciot, Mikael Magnusson, Jeff King, Taylor Blau,
	Derrick Stolee, git
In-Reply-To: <20260511-pks-maintenance-fix-lock-with-detach-v1-0-ccd7d62c9a40@pks.im>

Hi,

On Mon, May 11, 2026 at 02:29:54PM +0200, Patrick Steinhardt wrote:
> this patch series addresses the issues reported in [1]. The series is
> built on top of Git 2.54.0.

Junio: I saw that you are starting to prep for Git 2.54.1, and
a89346e34a (Start preparing for 2.54.1, 2026-05-21) explicitly mentions
a couple of additional topics that should land in that bugfix release.
This topic here isn't mentioned though, but I very much think that these
fixes should be included.

Thanks!

Patrick

^ permalink raw reply

* [PATCH v2] Documentation/git-range-diff: add missing notes options in synopsis
From: Siddh Raman Pant @ 2026-05-21  5:28 UTC (permalink / raw)
  To: Kristoffer Haugsbakk, git
  Cc: Patrick Steinhardt, Junio C Hamano, Elijah Newren
In-Reply-To: <72839071-153f-4306-a705-3be0dc203109@app.fastmail.com>

git-range-diff supports note options which are also mentioned later in
the help, but they are missing from the synopsis. Let's fix that.

Signed-off-by: Siddh Raman Pant <siddh.raman.pant@oracle.com>
---
v1 -> v2: Fixed typo and removed fixes line.

 Documentation/git-range-diff.adoc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/git-range-diff.adoc b/Documentation/git-range-diff.adoc
index 880557084533..5cc5e2ed5673 100644
--- a/Documentation/git-range-diff.adoc
+++ b/Documentation/git-range-diff.adoc
@@ -11,7 +11,7 @@ SYNOPSIS
 git range-diff [--color=[<when>]] [--no-color] [<diff-options>]
 	[--no-dual-color] [--creation-factor=<factor>]
 	[--left-only | --right-only] [--diff-merges=<format>]
-	[--remerge-diff]
+	[--remerge-diff] [--no-notes | --notes[=<ref>]]
 	( <range1> <range2> | <rev1>...<rev2> | <base> <rev1> <rev2> )
 	[[--] <path>...]
 
-- 
2.53.0


^ permalink raw reply related

* Re: [PATCH] Documentation/git-range-diff: add missing notes options in synopsis
From: Kristoffer Haugsbakk @ 2026-05-21  5:12 UTC (permalink / raw)
  To: Siddh Raman Pant, git; +Cc: Patrick Steinhardt, Junio C Hamano, Elijah Newren
In-Reply-To: <20260521041908.41055-1-siddh.raman.pant@oracle.com>

On Thu, May 21, 2026, at 06:19, Siddh Raman Pant wrote:
> git-range-diff supports note options which are also mentioned later in
> the help, but they are missing from synopis. Let's fix that.

s/synopis/synopsis/ or s/synopis/the synopsis/

>
> Fixes: bd3619188682 ("range-diff: pass through --notes to `git log`")

This project doesn’t use `Fixes` trailers. Mentions of commits go in the
commit message body (outside the trailers) using `git log -1
--format-reference <cmt>`.

The Linux project has uses for this structured information since there
is a lot of backporting of bugfixes. But I haven’t heard of a need for
that in this project.

> Signed-off-by: Siddh Raman Pant <siddh.raman.pant@oracle.com>
> ---
>  Documentation/git-range-diff.adoc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Okay, there is no need to update the synopsis in the source code since
git-range-diff(1) is excluded from `t/t0450-txt-doc-vs-help.sh`. So this
looks correct.

>
> diff --git a/Documentation/git-range-diff.adoc
>[snip]

^ permalink raw reply

* Re: [PATCH v2 10/11] git-gui: adapt blame/browser parsing for bare operation
From: Shroom Moo @ 2026-05-21  5:02 UTC (permalink / raw)
  To: Mark Levedahl; +Cc: git, Johannes Sixt, Aina Boot
In-Reply-To: <20260520202411.108764-11-mlevedahl@gmail.com>

On 5/21/26 4:24 AM, Mark Levedahl wrote: 
> +proc find_path_type {head path} {
> +	if {$path eq {./}} {
> +		# the root-tree exists in every rev, ls-tree gives data on the contents,
> +		# not the type of tree itself. So, if the rev exists, return {tree}
> +		if {[catch {set objtype [git ls-tree $head]}]} {
> +			set objtype {}
> +		} else {
> +			set objtype {tree}
> +		}
> +	} else {
> +		# test that the path exists in head, ls-tree gives info on the path only
> +		if {[catch {set objtype [git ls-tree {--format=%(objecttype)} $head $path]}]} {
> +			set objtype {}
> +		}
> +	}
> +	return $objtype
> +}

In v1, argument parsing relied on file exists within the worktree to 
determine if a path existed, without using ls-tree. In v2, the use of 
git ls-tree seems to actually be intended to list directory contents, 
rather than querying the type of the path itself. 

If $path is a directory (a tree object), git ls-tree outputs the 
object type for every entry within that directory, one per line. 

The variable objtype is assigned a multi-line string. When compared 
against "tree", the match fails, causing the function to return an 
empty string, which subsequently leads to an error. We can change to 
"git cat-file -t" or similiar approaches. 

Shroom


^ permalink raw reply

* Re: [PATCH v2 07/11] git-gui: try harder to find worktree from gitdir
From: Shroom Moo @ 2026-05-21  4:55 UTC (permalink / raw)
  To: Mark Levedahl; +Cc: git, Johannes Sixt, Aina Boot
In-Reply-To: <20260520202411.108764-8-mlevedahl@gmail.com>

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 {}
> +		}
> +	}

There is also an unaddressed issue: 
In [file exists {gitdir}] and [open {gitdir} r], {gitdir} is a 
literal string referring to a file named gitdir in the current 
working directory. However, in the context of a linked worktree 
(created via git worktree add), the actual file path is 
$_gitdir/gitdir (e.g., .git/worktrees/<name>/gitdir). While the 
current working directory could be anywhere (even inside the .git 
directory), $_gitdir is an absolute path pointing to that worktree's 
gitdir (e.g., /path/to/main/.git/worktrees/branch). The gitdir file 
resides within the $_gitdir directory and contains a relative path 
like ../../.git/worktrees/branch. The current code logic will never 
locate this file. 

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. 

Maybe the identification of linked worktree should not directly look 
for the gitdir file, but should check whether there is a.git file and 
its content points to... /.git/worktrees/... ? Anyways, using the 
literal {gitdir} to search in the current directory lead to risks. 

Shroom


^ permalink raw reply

* [PATCH] Documentation/git-range-diff: add missing notes options in synopsis
From: Siddh Raman Pant @ 2026-05-21  4:19 UTC (permalink / raw)
  To: git; +Cc: Patrick Steinhardt, Junio C Hamano, Kristoffer Haugsbakk,
	Elijah Newren

git-range-diff supports note options which are also mentioned later in
the help, but they are missing from synopis. Let's fix that.

Fixes: bd3619188682 ("range-diff: pass through --notes to `git log`")
Signed-off-by: Siddh Raman Pant <siddh.raman.pant@oracle.com>
---
 Documentation/git-range-diff.adoc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/git-range-diff.adoc b/Documentation/git-range-diff.adoc
index 880557084533..5cc5e2ed5673 100644
--- a/Documentation/git-range-diff.adoc
+++ b/Documentation/git-range-diff.adoc
@@ -11,7 +11,7 @@ SYNOPSIS
 git range-diff [--color=[<when>]] [--no-color] [<diff-options>]
 	[--no-dual-color] [--creation-factor=<factor>]
 	[--left-only | --right-only] [--diff-merges=<format>]
-	[--remerge-diff]
+	[--remerge-diff] [--no-notes | --notes[=<ref>]]
 	( <range1> <range2> | <rev1>...<rev2> | <base> <rev1> <rev2> )
 	[[--] <path>...]
 
-- 
2.53.0


^ permalink raw reply related

* Re: [PATCH 1/9] Documentation/git-range-diff: add missing notes options in synopsis
From: Siddh Raman Pant @ 2026-05-21  4:13 UTC (permalink / raw)
  To: gitster@pobox.com
  Cc: git@vger.kernel.org, newren@gmail.com, ps@pks.im,
	code@khaugsbakk.name
In-Reply-To: <xmqqpl2p38s4.fsf@gitster.g>

[-- Attachment #1: Type: text/plain, Size: 1219 bytes --]

On Thu, May 21 2026 at 05:58:59 +0530, Junio C Hamano wrote:
> Siddh Raman Pant <siddh.raman.pant@oracle.com> writes:
> 
> > On Wed, May 20 2026 at 05:17:51 +0530, Junio C Hamano wrote:
> > > This has nothing to do with "external notes" topic, no?
> > 
> > Yeah, but since I added the command line flag I found it doesn't
> > mention the existing flags.
> > 
> > Fixing it in the "external notes" commit would be bad, so I put it
> > before that, since it also then provides a logical place to add new
> > flags.
> 
> What I meant was that it would have been better as a standalone
> patch that is unrelated to the (now) 8-patch topic for the external
> notes.  That way, it can move faster without waiting for the rest.
> 
> Unless this patch has complex semantic or textual conflicts that
> makes it easier to manage together with the external notes series,
> that is.  I think adding [--notes=...] to one existing line (this
> patch) and adding a new line with [--[no-]external] on it (the main
> part of the topic) can be done in parallel and it is not too much to
> ask for the integrator to merge them on the receiving end.

Ok sure, I'll send it as a standalone patch.

Thanks,
Siddh

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH 7/9] notes: support an external command to display notes
From: Siddh Raman Pant @ 2026-05-21  4:12 UTC (permalink / raw)
  To: git@vger.kernel.org, calvinwan@google.com, gitster@pobox.com,
	newren@gmail.com, sandals@crustytoothpaste.net, ps@pks.im,
	code@khaugsbakk.name
In-Reply-To: <ag5b4O7-k-3QBR4W@fruit.crustytoothpaste.net>

[-- Attachment #1: Type: text/plain, Size: 2380 bytes --]

On Thu, May 21 2026 at 06:42:00 +0530, brian m. carlson wrote:
> > Assisted-by: Codex:gpt-5.5-xhigh-fast
> 
> Just a question here: was this written in whole or in part by Codex, or
> was it just used as a reference to ask questions?  I ask because the
> style of notes-external.c differs quite a bit from the style we use (for
> one, the horizontal rule comments) and we have this in

AI tools typically don't generate comments in code like in this series,
you can see by trying out for yourself. Each comment is hand-written by
me. Sorry, I'll remove those lines in v2 after this discussion.

AI was used for review, and providing the initial skeletal, which I
changed significantly.

> SubmittingPatches:
> 
>     The Developer's Certificate of Origin requires contributors to certify
>     that they know the origin of their contributions to the project and
>     that they have the right to submit it under the project's license.
>     It's not yet clear that this can be legally satisfied when submitting
>     significant amount of content that has been generated by AI tools.
> 
>     [...]
> 
>     To avoid these issues, we will reject anything that looks AI
>     generated, that sounds overly formal or bloated, that looks like AI
>     slop, that looks good on the surface but makes no sense, or that
>     senders don’t understand or cannot explain.

Please tell me why this change is a slop and doesn't make sense.

If I wanted to mislead here, I would not have used the "Assisted-by"
trailer, which is now being used in kernel land:

https://www.kernel.org/doc/html/latest/process/submitting-patches.html#using-assisted-by

There have already been commits in the git.git history having the
Assisted-by trailer.

> I'll note that it also has a lot of global variables, which are common
> in the codebase but we're trying to move away from, 

Is there a new facility to store the config without a global variable?

If the issue is the number, I can make a housing struct if you want.

> and it's more
> verbose in commenting than we'd normally see elsewhere in the codebase.

I added comments to explain the code clearly as it's being followed,
especially since this is a new feature and I wanted the intent to be
clear.

If you could tell me which comments to remove, that would be great.

Thanks,
Siddh

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH v2] git-jump: pick a mode automatically when invoked without arguments
From: Junio C Hamano @ 2026-05-21  1:30 UTC (permalink / raw)
  To: Greg Hurrell via GitGitGadget
  Cc: git, Jeff King, Greg Hurrell, Erik Cervin Edin, Greg Hurrell
In-Reply-To: <xmqqik8h36al.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

>> +mode_auto() {
>> +	if test "$(git rev-parse --is-inside-work-tree 2>/dev/null)" != "true"; then
>> +		usage >&2
>> +		exit 1
>> +	fi
>
> That looks like a basic safety measure, which is good.
>
>> +	if test -n "$(git ls-files -u "$@")"; then
>> +		mode_merge "$@"
>> +	elif ! git diff --quiet "$@"; then
>> +		mode_diff "$@"
>> +	elif ! git diff --check >/dev/null 2>&1; then
>
> Shouldn't this "diff --check" be restricted by "$@" if given?

>> +		mode_ws "$@"

If there are any unstaged changes (possibly with whitespace errors),
'git diff --quiet' would exit with non-zero, so "elif ! git diff
--quiet" would be taken and we do mode_diff.  The user cannot rely
on "auto" to trigger mode_ws to check whitespace errors in the
working tree files because of this.  If there is no unstaged
changes, 'git diff --quiet' woudl exit with zero, so the control
comes to "git diff --check", but then there is nothing mode_ws to
work on in that case, right?  So it is not clear to me in what
situation this auto selection of mode_ws would help us.

>> +	else
>> +		usage >&2
>> +		exit 1
>> +	fi
>> +}

^ permalink raw reply

* Re: [PATCH v2] git-jump: pick a mode automatically when invoked without arguments
From: Junio C Hamano @ 2026-05-21  1:22 UTC (permalink / raw)
  To: Greg Hurrell via GitGitGadget
  Cc: git, Jeff King, Greg Hurrell, Erik Cervin Edin, Greg Hurrell
In-Reply-To: <pull.2108.v2.git.1779280307112.gitgitgadget@gmail.com>

"Greg Hurrell via GitGitGadget" <gitgitgadget@gmail.com> writes:

> If none of the interesting cases listed above applies, then auto mode
> falls back to the existing usage-and-exit behavior.

If more than one interesting cases apply, what happens, and what
should happen?

> diff --git a/contrib/git-jump/README b/contrib/git-jump/README
> index 3211841305..ac35792e55 100644
> --- a/contrib/git-jump/README
> +++ b/contrib/git-jump/README
> @@ -75,8 +75,20 @@ git jump grep foo_bar
>  # arbitrary grep options
>  git jump grep -i foo_bar
>  
> +# jump to places with conflict markers or whitespace errors
> +# (as reported by # `git diff --check`)

Is "#" after "reported by" intended?

> diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump
> index 8d1d5d79a6..43d3b42a41 100755
> --- a/contrib/git-jump/git-jump
> +++ b/contrib/git-jump/git-jump
> @@ -2,10 +2,11 @@
>  
>  usage() {
>  	cat <<\EOF
> -usage: git jump [--stdout] <mode> [<args>]
> +usage: git jump [--stdout] [<mode>] [<args>]

So "git jump --stdout foo.c" should mean "git jump --stdout auto
foo.c", OK.

> @@ -16,6 +17,10 @@ grep: elements are grep hits. Arguments are given to git grep or, if
>  
>  ws: elements are whitespace errors. Arguments are given to diff --check.
>  
> +auto: select one of the other modes based on worktree state;
> +      "merge" if there are unmerged paths, "diff" if there are
> +      unstaged changes, "ws" if there are whitespace errors.
> +
>  If the optional argument `--stdout` is given, print the quickfix
>  lines to standard output instead of feeding it to the editor.
>  EOF
> @@ -82,6 +87,23 @@ mode_ws() {
>  	git diff --check "$@"
>  }
>  
> +mode_auto() {
> +	if test "$(git rev-parse --is-inside-work-tree 2>/dev/null)" != "true"; then
> +		usage >&2
> +		exit 1
> +	fi

That looks like a basic safety measure, which is good.

> +	if test -n "$(git ls-files -u "$@")"; then
> +		mode_merge "$@"
> +	elif ! git diff --quiet "$@"; then
> +		mode_diff "$@"
> +	elif ! git diff --check >/dev/null 2>&1; then

Shouldn't this "diff --check" be restricted by "$@" if given?

> +		mode_ws "$@"
> +	else
> +		usage >&2
> +		exit 1
> +	fi
> +}


^ permalink raw reply

* Re: [PATCH 7/9] notes: support an external command to display notes
From: brian m. carlson @ 2026-05-21  1:12 UTC (permalink / raw)
  To: Siddh Raman Pant
  Cc: git, Calvin Wan, Patrick Steinhardt, Elijah Newren,
	Kristoffer Haugsbakk, Junio C Hamano
In-Reply-To: <9619077369f1a567bd505b1de1e4f672a5cd1950.1779207350.git.siddh.raman.pant@oracle.com>

[-- Attachment #1: Type: text/plain, Size: 2473 bytes --]

On 2026-05-19 at 16:30:36, Siddh Raman Pant wrote:
> git notes is a very very helpful feature to show user-supplied
> information about a commit alongside its message transparently.
> 
> For distributed teams working on large git repos (huge number of
> branches/refs, files, etc.) and using the notes feature to mark
> information on git commits, a TOCTOU race can happen due to very
> large size of the repo and notes ref:
> 	- Person A updates a note for commit X.
> 	- Person A pushes the notes but it takes some time.
> 	- Person B fetches notes and doesn't find the updated note.
> 	- Person B can come to know of it only when he overwrites it
> 	  and encounters a push failure.
> 
> This problem excaberates on scale.
> 
> One solution to this is a realtime fetch or faster updation via
> external means, but unfortunately we lose the coherence in the
> display of information, and the user would end up reinventing
> git log.
> 
> So let's add support for an external command to display the notes.
> 
> We split the addition of documentation and tests from this commit for
> easier review. The new help text added in Documentation/ in the next
> commit should make the usage clear.
> 
> Assisted-by: Codex:gpt-5.5-xhigh-fast

Just a question here: was this written in whole or in part by Codex, or
was it just used as a reference to ask questions?  I ask because the
style of notes-external.c differs quite a bit from the style we use (for
one, the horizontal rule comments) and we have this in
SubmittingPatches:

    The Developer's Certificate of Origin requires contributors to certify
    that they know the origin of their contributions to the project and
    that they have the right to submit it under the project's license.
    It's not yet clear that this can be legally satisfied when submitting
    significant amount of content that has been generated by AI tools.

    [...]

    To avoid these issues, we will reject anything that looks AI
    generated, that sounds overly formal or bloated, that looks like AI
    slop, that looks good on the surface but makes no sense, or that
    senders don’t understand or cannot explain.

I'll note that it also has a lot of global variables, which are common
in the codebase but we're trying to move away from, and it's more
verbose in commenting than we'd normally see elsewhere in the codebase.
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 325 bytes --]

^ permalink raw reply

* Re: [PATCH 1/9] Documentation/git-range-diff: add missing notes options in synopsis
From: Junio C Hamano @ 2026-05-21  0:28 UTC (permalink / raw)
  To: Siddh Raman Pant
  Cc: git@vger.kernel.org, newren@gmail.com, ps@pks.im,
	code@khaugsbakk.name
In-Reply-To: <b3958381907244ca06a39e2fc116eec113a6bc85.camel@oracle.com>

Siddh Raman Pant <siddh.raman.pant@oracle.com> writes:

> On Wed, May 20 2026 at 05:17:51 +0530, Junio C Hamano wrote:
>> This has nothing to do with "external notes" topic, no?
>
> Yeah, but since I added the command line flag I found it doesn't
> mention the existing flags.
>
> Fixing it in the "external notes" commit would be bad, so I put it
> before that, since it also then provides a logical place to add new
> flags.

What I meant was that it would have been better as a standalone
patch that is unrelated to the (now) 8-patch topic for the external
notes.  That way, it can move faster without waiting for the rest.

Unless this patch has complex semantic or textual conflicts that
makes it easier to manage together with the external notes series,
that is.  I think adding [--notes=...] to one existing line (this
patch) and adding a new line with [--[no-]external] on it (the main
part of the topic) can be done in parallel and it is not too much to
ask for the integrator to merge them on the receiving end.

Thanks.




^ permalink raw reply

* [PATCH v2 11/11] git-gui: add gui and pick as explicit subcommands
From: Mark Levedahl @ 2026-05-20 20:24 UTC (permalink / raw)
  To: git; +Cc: j6t, egg_mushroomcow, bootaina702, Mark Levedahl
In-Reply-To: <20260520202411.108764-1-mlevedahl@gmail.com>

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.

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

diff --git a/git-gui.sh b/git-gui.sh
index ae609f86f1..299c1a0292 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1024,6 +1024,8 @@ proc load_config {include_global} {
 ##
 ## feature option selection
 
+enable_option picker
+enable_option gitdir_discovery
 if {[regexp {^git-(.+)$} [file tail $argv0] _junk subcommand]} {
 	unset _junk
 } else {
@@ -1035,6 +1037,9 @@ if {$subcommand eq {gui.sh}} {
 if {$subcommand eq {gui} && [llength $argv] > 0} {
 	set subcommand [lindex $argv 0]
 	set argv [lrange $argv 1 end]
+	if {$subcommand eq {gui}} {
+		disable_option picker
+	}
 }
 
 enable_option multicommit
@@ -1050,6 +1055,7 @@ blame {
 	disable_option multicommit
 	disable_option branch
 	disable_option transport
+	disable_option picker
 }
 citool {
 	enable_option singlecommit
@@ -1058,6 +1064,7 @@ citool {
 	disable_option multicommit
 	disable_option branch
 	disable_option transport
+	disable_option picker
 
 	while {[llength $argv] > 0} {
 		set a [lindex $argv 0]
@@ -1080,6 +1087,9 @@ citool {
 		set argv [lrange $argv 1 end]
 	}
 }
+pick {
+	disable_option gitdir_discovery
+}
 }
 
 ######################################################################
@@ -1174,7 +1184,7 @@ proc unset_gitdir_vars {} {
 
 # find repository.
 set _gitdir {}
-if {$_gitdir eq {}} {
+if {[is_enabled gitdir_discovery]} {
 	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"]
+	exit 1
+}
+
 # find worktree, continue without if not required
 if {[catch {
 	set _gitworktree [git rev-parse --show-toplevel]
@@ -3137,14 +3153,15 @@ blame {
 	return
 }
 citool -
-gui {
+gui -
+pick {
 	if {[llength $argv] != 0} {
 		usage
 	}
 	# fall through to setup UI for commits
 }
 default {
-	set err "[mc usage:] $argv0 \[{blame|browser|citool}\]"
+	set err "[mc usage:] $argv0 \[{blame|browser|citool|gui|pick}\]"
 	if {[tk windowingsystem] eq "win32"} {
 		wm withdraw .
 		tk_messageBox -icon error -message $err \
-- 
2.54.0.99.14


^ permalink raw reply related

* [PATCH v2 10/11] git-gui: adapt blame/browser parsing for bare operation
From: Mark Levedahl @ 2026-05-20 20:24 UTC (permalink / raw)
  To: git; +Cc: j6t, egg_mushroomcow, bootaina702, Mark Levedahl
In-Reply-To: <20260520202411.108764-1-mlevedahl@gmail.com>

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.

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 blame       -- Makefile gitgui-0.19.0    works
    git gui blame   -- Makefile gitgui-0.19.0    works

    git blame       Makefile -- gitgui-0.19.0    fails (correctly)
    git gui blame   Makefile -- gitgui-0.19.0    works (should fail)

    git blame       gitgui-0.19.0 -- Makefile    works (correctly)
    git gui blame   gitgui-0.19.0 -- Makefile    fails (should work)

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.

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.

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>
---
 git-gui.sh | 151 ++++++++++++++++++++++++++++++-----------------------
 1 file changed, 87 insertions(+), 64 deletions(-)

diff --git a/git-gui.sh b/git-gui.sh
index d373457901..ae609f86f1 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -3014,100 +3014,123 @@ proc normalize_relpath {path} {
 	}
 }
 
+proc find_path_type {head path} {
+	if {$path eq {./}} {
+		# the root-tree exists in every rev, ls-tree gives data on the contents,
+		# not the type of tree itself. So, if the rev exists, return {tree}
+		if {[catch {set objtype [git ls-tree $head]}]} {
+			set objtype {}
+		} else {
+			set objtype {tree}
+		}
+	} else {
+		# test that the path exists in head, ls-tree gives info on the path only
+		if {[catch {set objtype [git ls-tree {--format=%(objecttype)} $head $path]}]} {
+			set objtype {}
+		}
+	}
+	return $objtype
+}
+
 # -- Not a normal commit type invocation?  Do that instead!
 #
 switch -- $subcommand {
 browser -
 blame {
 	if {$subcommand eq "blame"} {
-		set subcommand_args {[--line=<num>] rev? path}
+		set subcommand_args {[--line=<num>] <[rev] [--] filename | [--] filename rev>}
+		set required_objtype blob
 	} else {
-		set subcommand_args {rev? path}
+		set subcommand_args {<[rev] [--] directory | [--] directory rev>}
+		set required_objtype tree
 	}
-	if {$argv eq {}} usage
+	set maxargs [llength $subcommand_args]
+	set nargs [llength $argv]
+	if {$nargs < 1 || $nargs > $maxargs} usage
 	set head {}
+	set althead {}
 	set path {}
+	set altpath {}
+	set canswap 1
 	set jump_spec {}
-	set is_path 0
-	foreach a $argv {
-		set p [file join $_prefix $a]
 
-		if {$is_path || [file exists $p]} {
-			if {$path ne {}} usage
-			set path [normalize_relpath $p]
-			break
-		} elseif {$a eq {--}} {
-			if {$path ne {}} {
-				if {$head ne {}} usage
-				set head $path
-				set path {}
+	# assume: [--line=num] [head] [--] path as the possible arguments, in order.
+	# head and path may need a swap later.
+	for {set iarg 0} {$iarg < $nargs} {incr iarg} {
+		set arg [lindex $argv $iarg]
+		if {$arg eq {--}} {
+			# next arg is the path, prevent or FORCE swap?
+			if {$iarg == $nargs - 2} {
+				set canswap 0
+			} elseif {$iarg == $nargs - 3} {
+				set canswap 2
+			} else {
+				usage
 			}
-			set is_path 1
-		} elseif {[regexp {^--line=(\d+)$} $a a lnum]} {
-			if {$jump_spec ne {} || $head ne {}} usage
+		} elseif {[regexp {^--line=(\d+)$} $arg arg lnum]} {
+			# --line can only be the first arg
+			if {$iarg != 0 || $maxargs < 4} usage
 			set jump_spec [list $lnum]
+		} elseif {$iarg == $nargs - 1} {
+			# assume final argument is path
+			set path [normalize_relpath [file join $_prefix $arg]]
+			set althead $arg
 		} elseif {$head eq {}} {
-			if {$head ne {}} usage
-			set head $a
-			set is_path 1
+			# assume the other argument is head
+			set head $arg
+			set altpath [normalize_relpath [file join $_prefix $arg]]
 		} else {
 			usage
 		}
 	}
-	unset is_path
-
-	if {$head ne {} && $path eq {}} {
-		if {[string index $head 0] eq {/}} {
-			set path [normalize_relpath $head]
-			set head {}
-		} else {
-			set path [normalize_relpath $_prefix$head]
-			set head {}
-		}
-	}
 
+	# no swapping allowed if head not given, use current branch (HEAD)
 	if {$head eq {}} {
 		load_current_branch
-	} else {
-		if {[regexp [string map "@@ [expr $hashlength - 1]" {^[0-9a-f]{1,@@}$}] $head]} {
-			if {[catch {
-					set head [git rev-parse --verify $head]
-				} err]} {
-				if {[tk windowingsystem] eq "win32"} {
-					tk_messageBox -icon error -title [mc Error] -message $err
-				} else {
-					puts stderr $err
-				}
-				exit 1
-			}
+		set head $current_branch
+		set canswap 0
+	}
+
+	# -- before "rev" arg means we got -- path head
+	if {$canswap == 2} {
+		set head $althead
+		set path $altpath
+		set canswap 0
+	}
+
+	set objtype [find_path_type $head $path]
+	if {$objtype eq {} && $canswap} {
+		set objtype [find_path_type $althead $altpath]
+		if {$objtype ne {}} {
+			set head $althead
+			set path $altpath
 		}
-		set current_branch $head
+	}
+	set current_branch $head
+
+	# check that path exists in head, and objtype matches need
+	if {$objtype ne $required_objtype} {
+		switch -- $required_objtype {
+			tree {set err [strcat \
+				[mc "'%s' is not a directory in rev '%s'" $path $head]]}
+			blob {set err [strcat \
+				[mc "'%s' is not a filename in rev '%s'" $path $head]]}
+		}
+		if {[tk windowingsystem] eq "win32"} {
+			catch {wm withdraw .}
+			error_popup $err
+		} else {
+			puts stderr $err
+		}
+		exit 1
 	}
 
 	wm deiconify .
 	switch -- $subcommand {
 	browser {
-		if {$jump_spec ne {}} usage
-		if {$head eq {}} {
-			if {$path ne {} && [file isdirectory $path]} {
-				set head $current_branch
-			} else {
-				set head $path
-				set path {}
-			}
-		}
 		browser::new $head $path
 	}
-	blame   {
-		if {$head eq {} && ![file exists $path]} {
-			catch {wm withdraw .}
-			tk_messageBox \
-				-icon error \
-				-type ok \
-				-title [mc "git-gui: fatal error"] \
-				-message [mc "fatal: cannot stat path %s: No such file or directory" $path]
-			exit 1
-		}
+	blame {
 		blame::new $head $path $jump_spec
 	}
 	}
-- 
2.54.0.99.14


^ permalink raw reply related

* [PATCH v2 09/11] git-gui: allow specifying path '.' to the browser
From: Mark Levedahl @ 2026-05-20 20:24 UTC (permalink / raw)
  To: git; +Cc: j6t, egg_mushroomcow, bootaina702, Mark Levedahl
In-Reply-To: <20260520202411.108764-1-mlevedahl@gmail.com>

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

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


^ permalink raw reply related

* [PATCH v2 08/11] git-gui: use HEAD as current branch when detached (bug fix)
From: Mark Levedahl @ 2026-05-20 20:24 UTC (permalink / raw)
  To: git; +Cc: j6t, egg_mushroomcow, bootaina702, Mark Levedahl
In-Reply-To: <20260520202411.108764-1-mlevedahl@gmail.com>

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.

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


^ permalink raw reply related

* [PATCH v2 03/11] git-gui: use --absolute-git-dir
From: Mark Levedahl @ 2026-05-20 20:24 UTC (permalink / raw)
  To: git; +Cc: j6t, egg_mushroomcow, bootaina702, Mark Levedahl
In-Reply-To: <20260520202411.108764-1-mlevedahl@gmail.com>

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

But, git rev-parse supports --absolute-git-dir from fac60b8925
("rev-parse: add option for absolute or relative path formatting",
2020-12-13), and included in git 2.31. git-gui requires git >= 2.36, so
this more useful form is always available. Use --absolute-git-dir to
always get an absolute path, avoiding the need for other checks, and
delete the now unneeded code to fix a relative _gitdir.

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

diff --git a/git-gui.sh b/git-gui.sh
index 4a736190a9..233c975786 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1146,7 +1146,7 @@ if {[catch {
 	&& [catch {
 		# beware that from the .git dir this sets _gitdir to .
 		# and _prefix to the empty string
-		set _gitdir [git rev-parse --git-dir]
+		set _gitdir [git rev-parse --absolute-git-dir]
 		set _prefix [git rev-parse --show-prefix]
 	} err]} {
 	load_config 1
@@ -1155,7 +1155,7 @@ if {[catch {
 		exit 1
 	}
 	if {[catch {
-		set _gitdir [git rev-parse --git-dir]
+		set _gitdir [git rev-parse --absolute-git-dir]
 	} err]} {
 		catch {wm withdraw .}
 		error_popup [strcat [mc "Unusable repo/worktree:"] " [pwd] "\n\n$err"]
@@ -1175,13 +1175,6 @@ if {$hashalgorithm eq "sha1"} {
 	exit 1
 }
 
-# we expand the _gitdir when it's just a single dot (i.e. when we're being
-# run from the .git dir itself) lest the routines to find the worktree
-# get confused
-if {$_gitdir eq "."} {
-	set _gitdir [pwd]
-}
-
 if {![file isdirectory $_gitdir]} {
 	catch {wm withdraw .}
 	error_popup [strcat [mc "Git directory not found:"] "\n\n$_gitdir"]
-- 
2.54.0.99.14


^ permalink raw reply related

* [PATCH v2 07/11] git-gui: try harder to find worktree from gitdir
From: Mark Levedahl @ 2026-05-20 20:24 UTC (permalink / raw)
  To: git; +Cc: j6t, egg_mushroomcow, bootaina702, Mark Levedahl
In-Reply-To: <20260520202411.108764-1-mlevedahl@gmail.com>

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

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

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 {}
+			}
+		}]} {
+			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]
+			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
-- 
2.54.0.99.14


^ permalink raw reply related

* [PATCH v2 06/11] git-gui: use git rev-parse for worktree discovery
From: Mark Levedahl @ 2026-05-20 20:24 UTC (permalink / raw)
  To: git; +Cc: j6t, egg_mushroomcow, bootaina702, Mark Levedahl
In-Reply-To: <20260520202411.108764-1-mlevedahl@gmail.com>

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.

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


^ permalink raw reply related

* [PATCH v2 05/11] git-gui: simplify [is_bare] to report if a worktree is known
From: Mark Levedahl @ 2026-05-20 20:24 UTC (permalink / raw)
  To: git; +Cc: j6t, egg_mushroomcow, bootaina702, Mark Levedahl
In-Reply-To: <20260520202411.108764-1-mlevedahl@gmail.com>

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

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

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

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

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


^ permalink raw reply related

* [PATCH v2 04/11] git-gui: use rev-parse exclusively to find a repository
From: Mark Levedahl @ 2026-05-20 20:24 UTC (permalink / raw)
  To: git; +Cc: j6t, egg_mushroomcow, bootaina702, Mark Levedahl
In-Reply-To: <20260520202411.108764-1-mlevedahl@gmail.com>

git-gui attempts to use env(GIT_DIR) directly as the git repository,
accepting GIT_DIR if it is a directory. Only if that fails is git
rev-parse used to discover the repository.  But, this avoids all of
git-core's validity checking on a repository, thus possibly deferring an
error to a later step, possibly unexpected. Repository validation should
be part of initial setup so that later processing does not need error
trapping for configuration errors.

Let's just invoke rev-parse so all error checking is done.

While here, let's cleanup the error handling.

Stop if an error occurs and the user set GIT_DIR or GIT_WORK_TREE.
Use of either or both of those variables is supported by git, but their
use also means the user has taken responsibility that they are correct,
so a failure is something the user must address.

Otherwise on error, continue the existing behavior and show the
repository picker. But, let's move the possible invocation of
repository_chooser::pick to a separate code block. This permits adding
separate conditions on using pick indepent of repository discovery, and
will be exploited later in the series.  Note that the picker always
returns with the current directory in the root of a worktree with the
git repository is in the .git subdirectory.  The variable "picked" is
used by git-gui to automatically execute the "Explore Working Copy" menu
item after the repository picker is run.  This is controlled by config
variable gui.autoexplore, and happens after all discovery is complete.

Remove a later check on whether _gitdir is a directory: that code
cannot be reached without rev-parse already validating the repository.

_prefix should not be set before worktree discovery: the prefix is only
known after the worktree is found, and at this point we have only
discovered the repository. This is true even when running the repository
picker: that option provides a list of prior selections, and does no
validation on the list beyond checking that the directories exist.  For
now, just initialize _prefix along with other global variables.

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

diff --git a/git-gui.sh b/git-gui.sh
index 233c975786..c61a6cbd8f 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -374,6 +374,7 @@ set _gitdir {}
 set _gitworktree {}
 set _isbare {}
 set _githtmldir {}
+set _prefix {}
 set _reponame {}
 set _shellpath {@@SHELL_PATH@@}
 
@@ -1122,6 +1123,24 @@ unset argv0dir
 ##
 ## repository setup
 
+proc is_gitvars_error {err} {
+	set havevars 0
+	set GIT_DIR {}
+	set GIT_WORK_TREE {}
+	catch {set GIT_DIR $::env(GIT_DIR); set havevars 1}
+	catch {set GIT_WORK_TREE $::env(GIT_WORK_TREE) ; set havevars 1}
+
+	if {$havevars} {
+		catch {wm withdraw .}
+		error_popup [strcat [mc "Invalid configuration:"] \
+		   "\n" "GIT_DIR: " $GIT_DIR \
+		   "\n" "GIT_WORK_TREE: " $GIT_WORK_TREE \
+			"\n\n$err"]
+		return 1
+	}
+	return 0
+}
+
 proc set_gitdir_vars {} {
 	global _gitdir _gitworktree env
 	if {$_gitdir ne {}} {
@@ -1138,17 +1157,22 @@ proc unset_gitdir_vars {} {
 	catch {unset env(GIT_WORK_TREE)}
 }
 
-set picked 0
-if {[catch {
-		set _gitdir $env(GIT_DIR)
-		set _prefix {}
-		}]
-	&& [catch {
-		# beware that from the .git dir this sets _gitdir to .
-		# and _prefix to the empty string
+# find repository.
+set _gitdir {}
+if {$_gitdir eq {}} {
+	if {[catch {
 		set _gitdir [git rev-parse --absolute-git-dir]
-		set _prefix [git rev-parse --show-prefix]
 	} err]} {
+		if {[is_gitvars_error $err]} {
+			exit 1
+		}
+		set _gitdir {}
+	}
+}
+
+set picked 0
+if {$_gitdir eq {}} {
+	unset_gitdir_vars
 	load_config 1
 	apply_config
 	if {![choose_repository::pick]} {
@@ -1160,7 +1184,6 @@ if {[catch {
 		catch {wm withdraw .}
 		error_popup [strcat [mc "Unusable repo/worktree:"] " [pwd] "\n\n$err"]
 	}
-	set _prefix {}
 	set picked 1
 }
 
@@ -1175,11 +1198,6 @@ if {$hashalgorithm eq "sha1"} {
 	exit 1
 }
 
-if {![file isdirectory $_gitdir]} {
-	catch {wm withdraw .}
-	error_popup [strcat [mc "Git directory not found:"] "\n\n$_gitdir"]
-	exit 1
-}
 # _gitdir exists, so try loading the config
 load_config 0
 apply_config
-- 
2.54.0.99.14


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox