Git development
 help / color / mirror / Atom feed
* Re: [PATCH 4/9] run-command: add support for timeout in command finisher
From: Siddh Raman Pant @ 2026-05-22  5:46 UTC (permalink / raw)
  To: j6t@kdbg.org, gitster@pobox.com
  Cc: git@vger.kernel.org, newren@gmail.com, ps@pks.im,
	oswald.buddenhagen@gmx.de, code@khaugsbakk.name
In-Reply-To: <xmqqv7cgxq0o.fsf@gitster.g>

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

On Fri, May 22 2026 at 05:40:47 +0530, Junio C Hamano wrote:
> If a protocol builds its own way to declare "this backend is slow,
> so please do not consider less than 3 seconds of nonaction something
> to worry about but kill it off if you waited more than that" to make
> the receiving/waiting end responsible for managing timeout, that
> might be workable, but it certainly feels like a kludge.  The
> protocol can instead allow an "error - for your particular request,
> we couldn't come up with an answer within a reasonable time limit"
> response (in practice, "within time limit" does not have to be the
> only reason for such an error) to be returned, I think.

I think we are confusing two different commits here.

The response read deadline is the next commit. This commit is about
force-killing a process if it doesn't respond to the initial
termination signal.

Thanks,
Siddh

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

^ permalink raw reply

* Re: [PATCH 0/8] setup: centralize object database creation
From: Patrick Steinhardt @ 2026-05-22  6:03 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqqzn4xp0c.fsf@gitster.g>

On Fri, May 22, 2026 at 09:32:35AM +0900, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > 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.
> 
> FWIW, this merge needs the following merge-fix squashed into it,
> for the topic to build standalone.
> 
> commit ce350f62ceb26f3276ea3b7ad78b7f8cb4c35cf7
> Author: Junio C Hamano <gitster@pobox.com>
> Date:   Wed May 13 12:20:29 2026 +0900
> 
>     merge-fix/ps/setup-wo-the-repository
>     
>     with  js/objects-larger-than-4gb-on-windows
> 
> diff --git a/t/helper/test-synthesize.c b/t/helper/test-synthesize.c
> index 1f28ecf0f2..3fa534fbdf 100644
> --- a/t/helper/test-synthesize.c
> +++ b/t/helper/test-synthesize.c
> @@ -506,7 +506,7 @@ static int cmd__synthesize__pack(int argc, const char **argv,
>  		OPT_END()
>  	};
>  
> -	setup_git_directory_gently(&non_git);
> +	setup_git_directory_gently(the_repository, &non_git);
>  	repo = the_repository;
>  	algo = unsafe_hash_algo(repo->hash_algo);

Oh, right, I should have mentioned this. I do have the same fixup on top
of the merge, thanks.

Patrick

^ permalink raw reply

* Re: [PATCH 8/8] setup: construct object database in `apply_repository_format()`
From: Patrick Steinhardt @ 2026-05-22  6:03 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqq4ik0zls3.fsf@gitster.g>

On Fri, May 22, 2026 at 02:59:24AM +0900, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > With the preceding changes we now always construct the repository's
> > object database before applying the repository format. Remove this
> > duplication by constructing it in `apply_repository_format()` instead.
> >
> > Note that we create the object database _after_ having set up the
> > repository's hash algorithm, but _before_ setting the compat hash
> > algorithm. This is intentional:
> >
> >   - Constructing the object database may require knowledge of its
> >     intended object format.
> >
> >   - Setting up the compatibility hash requires the object database to be
> >     initialized already, because we immediately read the loose object
> >     map.
> >
> > The first point is sensible, the second maybe a little less so. Ideally,
> > it should be the responsibility of the object database itself to
> > initialize any data structures required for the compatibility hash. But
> > this would require further changes, so this is kept as-is for now.
> 
> Yeah, I guess it is a good place to stop, instead of solving the
> chicken-and-egg problem in one go.
> 
> > Further note that this requires us to move handling of the environment
> > variables GIT_OBJECT_DIRECTORY and GIT_ALTERNATE_OBJECT_DIRECTORIES into
> > the repository format, as well. This allows the caller more flexibility
> > around whether or not those environment variables are being honored, as
> > we do do want to respect them in "setup.c", but not in "repository.c".
> 
> It seems that we really really really want to do so ;-).  "do do
> want to" -> "do want to" or even "want to", perhaps.

Fixed locally, thanks! :)

Patrick

^ permalink raw reply

* Re: [PATCH 1/8] t0001: plug test gaps for git-init(1) with GIT_OBJECT_DIRECTORY
From: Patrick Steinhardt @ 2026-05-22  6:06 UTC (permalink / raw)
  To: Kristoffer Haugsbakk; +Cc: git
In-Reply-To: <741c2a26-7380-4d8e-aa91-fb237e9f10dc@app.fastmail.com>

On Thu, May 21, 2026 at 07:51:59PM +0200, Kristoffer Haugsbakk wrote:
> On Thu, May 21, 2026, at 09:42, Patrick Steinhardt wrote:
> > 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
> 
> Should this be s/, there/; there/ ? Depends on if this is a list of
> three items or if “This is” is a subclause that is supposed to point at
> “there's many”.

That reads a bit better.

> > 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.
> 
> Isn’t it more that “the upcoming changes *would have* broken” them if
> not for this change? This seems to refer to a an alternative commit
> history where this change does not exist?

Grammar is hard :) But yeah, this of course refers to an alternative
commit history I had at one point in time that did break this.

Fixed locally, will wait a bit before sending out the next version.

Patrick

^ permalink raw reply

* Re: [PATCH 4/9] run-command: add support for timeout in command finisher
From: Siddh Raman Pant @ 2026-05-22  5:59 UTC (permalink / raw)
  To: j6t@kdbg.org, peff@peff.net
  Cc: git@vger.kernel.org, gitster@pobox.com, newren@gmail.com,
	ps@pks.im, oswald.buddenhagen@gmx.de, code@khaugsbakk.name
In-Reply-To: <20260522051048.GA862219@coredump.intra.peff.net>

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

On Fri, May 22 2026 at 10:40:48 +0530, Jeff King wrote:
> On Thu, May 21, 2026 at 04:36:05PM +0200, Johannes Sixt wrote:
> 
> > Am 21.05.26 um 11:59 schrieb Siddh Raman Pant:
> > > The timeout is for the failure path, where the external helper has
> > > already stopped following that protocol or is blocked on something
> > > outside git's control. Since git starts the helper and puts it on the
> > > log/grep path, git also needs a bounded way to recover when that helper
> > > does not make progress. Otherwise an optional note source can prevent
> > > the main git command from completing.
> > 
> > That Git communicates with a process that looks like it stopped is the
> > normal case, for example:
> > 
> > - Output is sent to the pager. The user can take their time to study the
> > output. All the while, git waits patiently for the user to advance the
> > pager.
> > 
> > - Git fetch transfers large amounts of data across the network. Most of
> > the time it waits for data to arrive and does nothing. The peer process
> > looks like it hangs. Git does not decide to kill the connection at any
> > time. It is the user's decision to do so.
> > 
> > If the notes provider hangs, then it is not on Git to decide when it has
> > waited long enough.
> 
> Yeah, I agree with your point of view. If I understand this patch series
> correctly, it is about adding an external process to map commit ids to
> note data. So I can think of some existing features that are quite close
> to that in nature, none of which use timeouts:
> 
>   - textconv filters and external diffs which process data in the middle
>     of a git-log invocation
> 
>   - long-lived clean/smudge filters map blobs to arbitrarily large text
> 
>   - cat-file's batch mode maps object ids to user-specified data about
>     that object
> 
> As you note, it's up to the command to be well-behaved. Git should
> notice and respond appropriately if the command closes the pipe, of
> course. Sometimes a timeout can help with a poorly behaved command, but
> IMHO it is not worth the cost of non-determinism that it brings.
> 
> Moreover, the bits touching run-command here make me nervous, especially
> after the challenges we saw in the child-cleanup topic that was reverted
> just after v2.54. There is often a shell interposed between Git and the
> sub-command, and we don't always know how the shell will react to
> signals. Using SIGKILL will eventually get us _something_ to wait() on,
> but it might not even be the process we care about!
> 
> I don't really care much about this external-notes feature one way or
> the other, but if we are going to do it, I don't see any reason why it
> would not behave like all of the other similar parts of Git.
> 
> -Peff

Okay, since the consensus here is pretty clear, I will remove this
commit and send a v2.

Thanks,
Siddh

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

^ permalink raw reply

* Re: [PATCH 16/18] odb/source-loose: wire up `write_object_stream()` callback
From: Patrick Steinhardt @ 2026-05-22  6:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqq8q9czm8r.fsf@gitster.g>

On Fri, May 22, 2026 at 02:49:24AM +0900, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > -int odb_source_loose_write_stream(struct odb_source_loose *loose,
> > +/*
> > + * Write the given stream into the loose object source. The only difference to
> > + * the generic implementation of this function is that we don't perform an
> 
> "difference to" -> "difference from"???

I guess this is a difference between American and British English. "to"
is more popular in British English, but basically not used at all in
American English. Will adapt, thanks.

Patrick

^ permalink raw reply

* [PATCH] compat/mingw: Allow SIGKILL to kill in mingw_kill.
From: Siddh Raman Pant @ 2026-05-22  6:16 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Kristoffer Haugsbakk, Elijah Newren,
	Patrick Steinhardt

mingw_kill() only allows SIGTERM for killing a process.

Let's also allow the natural SIGKILL for the same so that callers don't
have to do ifdef soup for special Windows handling.

Signed-off-by: Siddh Raman Pant <siddh.raman.pant@oracle.com>
---
 compat/mingw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index aa7525f419cb..00a994aa9f47 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -2250,7 +2250,7 @@ int mingw_execvp(const char *cmd, char *const *argv)
 
 int mingw_kill(pid_t pid, int sig)
 {
-	if (pid > 0 && sig == SIGTERM) {
+	if (pid > 0 && (sig == SIGTERM || sig == SIGKILL)) {
 		HANDLE h = OpenProcess(PROCESS_TERMINATE, FALSE, pid);
 
 		if (TerminateProcess(h, -1)) {
-- 
2.53.0


^ permalink raw reply related

* Re: [PATCH v10 1/4] branch: add --forked <branch>
From: Johannes Sixt @ 2026-05-22  6:18 UTC (permalink / raw)
  To: Harald Nordgren
  Cc: Kristoffer Haugsbakk, Phillip Wood, git,
	Harald Nordgren via GitGitGadget
In-Reply-To: <f2df15983067ce39b6c33ab81115863d5c3567f4.1779403204.git.gitgitgadget@gmail.com>

Am 22.05.26 um 00:40 schrieb Harald Nordgren via GitGitGadget:
> diff --git a/Documentation/git-branch.adoc b/Documentation/git-branch.adoc
> index c0afddc424..3a421f6663 100644
> --- a/Documentation/git-branch.adoc
> +++ b/Documentation/git-branch.adoc
> @@ -24,6 +24,7 @@ git branch (-m|-M) [<old-branch>] <new-branch>
>  git branch (-c|-C) [<old-branch>] <new-branch>
>  git branch (-d|-D) [-r] <branch-name>...
>  git branch --edit-description [<branch-name>]
> +git branch --forked <branch>...

I would have preferred that this option is another filter of --list
mode, not its own mode of operation. Consequently, each --forked option
would take only a single argument (which can contain globs), and can be
given multiple times.

>  
>  DESCRIPTION
>  -----------
> @@ -199,6 +200,12 @@ This option is only applicable in non-verbose mode.
>  	Print the name of the current branch. In detached `HEAD` state,
>  	nothing is printed.
>  
> +`--forked`::
> +	List local branches whose configured upstream matches any
> +	of the given _<branch>_ arguments. Each argument is either
> +	a ref (e.g. `origin/master`, `master`) or a shell-style
> +	glob (e.g. `'origin/*'`). Multiple arguments are unioned.

So this could perhaps read:

`--forked`::
	List only branches whose configured upstream matches
	_<branch>_. The argument can contain a shell-style glob
	 (e.g. `'origin/*'`). The option can be repeated to
	widen the filter.

Note that there is no reason to say "local branches". ("... are unioned"
sounds strange, so this is may attempt to express the same in a
different way.)

The icing on the cake would now be that

    git branch --merged origin/main --forked origin/*

provides the list of branches forked from origin that have already been
integrated.

-- Hannes


^ permalink raw reply

* Re: [PATCH] compat/mingw: Allow SIGKILL to kill in mingw_kill.
From: Junio C Hamano @ 2026-05-22  6:32 UTC (permalink / raw)
  To: Siddh Raman Pant, Johannes Sixt, Johannes Schindelin
  Cc: git, Kristoffer Haugsbakk, Elijah Newren, Patrick Steinhardt
In-Reply-To: <20260522061652.50078-1-siddh.raman.pant@oracle.com>

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

> mingw_kill() only allows SIGTERM for killing a process.
>
> Let's also allow the natural SIGKILL for the same so that callers don't
> have to do ifdef soup for special Windows handling.
>
> Signed-off-by: Siddh Raman Pant <siddh.raman.pant@oracle.com>
> ---
>  compat/mingw.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

I do not do windows, so I'd like to ask those much more clueful than
I am to see if they see any downsides.

The current code only handles TERM (to terminate) or 0 (to probe)
and everything else results in EINVAL, so the updated behaviour is
to pretend as if TERM is sent and do whatever PROCESS_TERMINATE
does, instead of doing nothing and erroring with EINVAL.  Which does
sound like an improvement over the status quo.

What I am wondering is if there are different kind of "kill" in the
Windows land, just like there are distinction between TERM and KILL.
For example, the program ought to be able to block TERM but not
KILL.  There are other termination-inducing signals like SIGQUIT but
until we start using them in our code, this emulation layer does not
have to know about them, I think.

Thanks.

> diff --git a/compat/mingw.c b/compat/mingw.c
> index aa7525f419cb..00a994aa9f47 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -2250,7 +2250,7 @@ int mingw_execvp(const char *cmd, char *const *argv)
>  
>  int mingw_kill(pid_t pid, int sig)
>  {
> -	if (pid > 0 && sig == SIGTERM) {
> +	if (pid > 0 && (sig == SIGTERM || sig == SIGKILL)) {
>  		HANDLE h = OpenProcess(PROCESS_TERMINATE, FALSE, pid);
>  
>  		if (TerminateProcess(h, -1)) {

^ permalink raw reply

* Re: [PATCH v10 1/4] branch: add --forked <branch>
From: Junio C Hamano @ 2026-05-22  6:36 UTC (permalink / raw)
  To: Johannes Sixt
  Cc: Harald Nordgren, Kristoffer Haugsbakk, Phillip Wood, git,
	Harald Nordgren via GitGitGadget
In-Reply-To: <273103d7-c816-4cde-9e89-b630c37b0749@kdbg.org>

Johannes Sixt <j6t@kdbg.org> writes:

> The icing on the cake would now be that
>
>     git branch --merged origin/main --forked origin/*
>
> provides the list of branches forked from origin that have already been
> integrated.

Yup, that is very nice.  Also with "--merged" replaced with
"--not-merged", i.e., "our work building on top of origin's, and
still need to be finished", would give us a good list to work on.

^ permalink raw reply

* [PATCH] doc: clarify push.default=simple in triangular workflows
From: Ivan Baluta via GitGitGadget @ 2026-05-22  6:58 UTC (permalink / raw)
  To: git; +Cc: Ivan Baluta, ivanbaluta

From: ivanbaluta <ivanbaluta.dev@gmail.com>

The documentation for 'simple' push mode currently focuses on the
centralized workflow. However, the implementation in builtin/push.c
falls back to 'current' behavior when pushing to a remote different
from the upstream (a triangular workflow).

Clarify this in the manual to align the documentation with the
long-standing implementation and prevent user confusion.

Signed-off-by: ivanbaluta <ivanbaluta.dev@gmail.com>
---
    doc: clarify push.default=simple in triangular workflows

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2115%2Fivanbaluta%2Fdoc-push-simple-triangular-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2115/ivanbaluta/doc-push-simple-triangular-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2115

 Documentation/config/push.adoc | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/config/push.adoc b/Documentation/config/push.adoc
index d9112b2260..979e40c3a4 100644
--- a/Documentation/config/push.adoc
+++ b/Documentation/config/push.adoc
@@ -45,6 +45,9 @@ If you are working on a centralized workflow (pushing to the same repository you
 pull from, which is typically `origin`), then you need to configure an upstream
 branch with the same name.
 +
+In a triangular workflow (pushing to a remote different from the upstream),
+`simple` behaves like `current`.
++
 This mode is the default since Git 2.0, and is the safest option suited for
 beginners.
 

base-commit: 59ff4886a579f4bc91e976fe18590b9ae02c7a08
-- 
gitgitgadget

^ permalink raw reply related

* Re: [PATCH v3] git-jump: pick a mode automatically when invoked without arguments
From: Greg Hurrell @ 2026-05-22  7:33 UTC (permalink / raw)
  To: Jeff King, Greg Hurrell
  Cc: git, Erik Cervin Edin, Junio C Hamano,
	Gregory Luke Hurrell Stewart
In-Reply-To: <20260522052821.GC861761@coredump.intra.peff.net>

On Fri, May 22, 2026, at 7:28 AM, Jeff King wrote:
> 
> My impression of the "auto" feature is: I am too lazy to type, so just
> take me to the interesting bits. And interesting in my experience with
> git-jump is either "I am merging, take me to the conflict" or "I am
> writing new code, take me to what I already did". Limiting the second
> case just to whitespace violations (assuming there is at least one)
> would probably be more confusing than helpful.

> If sounds like Greg has been living with "auto" and finding it useful
> for a while. So I'm mostly inclined to take the patch as-is, and people
> can experiment with it and suggest changes after using it in practice.

Yes, the "take me to the interesting bits" is very much the mental model
I've been operating with, using the simplest definition of "interesting"
("merge conflicts", followed by "changes in the worktree"). I think that
starting simple, but leaving the door open to possibly introducing more
subtleties in the future makes the most sense.

- Greg

^ permalink raw reply

* I discovered a minor issue with `git fetch`.
From: SURA @ 2026-05-22  7:45 UTC (permalink / raw)
  To: git

Hello everyone

The child processes spawned by `git fetch` can become zombie processes.
In most scenarios, these zombie processes are reaped by Process 1, so
this typically doesn't cause any problems.

However, within a Docker container, the application service itself is
sometimes designated as Process 1 (for instance, a service written in
Go). Since these application services lack the capability to reap
zombie processes, the zombies will gradually exhaust the available PID
resources.

Here are the simple steps to reproduce this issue:
1. `git clone https://github.com/SURA907/pid-1.git`
2. `cd pid-1`
3. `docker build -t pid-1 .`
4. `docker run -d --name pid-1 pid-1:latest`
5. `docker exec -it pid-1 /bin/bash`
6. `mkdir repo && cd repo && git init --bare`
7. `ps -ef`
------
UID PID PPID C STIME TTY TIME CMD
root 1 0 0 07:16 ? 00:00:00 tail -f /dev/null
root 7 0 0 07:16 pts/0 00:00:00 /bin/bash
root 13 0 0 07:16 pts/1 00:00:00 /bin/bash
root 29 7 0 07:17 pts/0 00:00:00 ps -ef
------

8. `git fetch https://github.com/git/git.git`
9. `ps -ef` (Run this command from a separate terminal session
connected to the container)
------
UID PID PPID C STIME TTY TIME CMD
root 1 0 0 07:16 ? 00:00:00 tail -f /dev/null
root 7 0 0 07:16 pts/0 00:00:00 /bin/bash
root 13 0 0 07:16 pts/1 00:00:00 /bin/bash
root 30 13 1 07:17 pts/1 00:00:00 git fetch https://github.com/git/git.git
root 31 30 0 07:17 pts/1 00:00:00 /usr/local/libexec/git-core/git
remote-https https://github.com/git/git.git
https://github.com/git/git.git
root 32 31 2 07:17 pts/1 00:00:00
/usr/local/libexec/git-core/git-remote-https
https://github.com/git/git.git https://github.com/git/git.git
root 36 30 30 07:17 pts/1 00:00:00 /usr/local/libexec/git-core/git
index-pack --stdin -v --fix-thin --keep=fetch-pack 30 on sura-pc
--pack_header=2,399455
root 38 7 0 07:17 pts/0 00:00:00 ps -ef
------

10. ps -ef (after fetch ends)
------
UID PID PPID C STIME TTY TIME CMD
root 1 0 0 07:16 ? 00:00:00 tail -f /dev/null
root 7 0 0 07:16 pts/0 00:00:00 /bin/bash
root 13 0 0 07:16 pts/1 00:00:00 /bin/bash
root 52 1 0 07:19 ? 00:00:00 [git] <defunct>
root 53 7 0 07:19 pts/0 00:00:00 ps -ef
------

A zombie process has appeared. It appears to originate from a `fetch`
subprocess that terminates very quickly; despite several attempts, I
have been unable to successfully capture it.

This issue was discovered within a legacy service. A few days after
upgrading to Git 2.53.0, the system's PID resources were exhausted by
zombie processes. This is likely the result of recent changes, as this
problem did not exist in earlier versions (2.4x).

To be honest, this is not an urgent matter; I have already deployed
`tini` as the init process (PID 1) to prevent the service from
becoming unavailable.

^ permalink raw reply

* Re: [PATCH v10 2/4] branch: add --prune-merged <branch>
From: Harald Nordgren @ 2026-05-22  7:59 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Harald Nordgren via GitGitGadget, git, Kristoffer Haugsbakk,
	Johannes Sixt, Phillip Wood
In-Reply-To: <xmqq1pf4w3x5.fsf@gitster.g>

> Please discard this version.  I had unnecessary draft comments that
> I used as reference in it.

I'm taking this to mean starting over from v9 and implementing the
'origin/*' idea again from there. Correct?


Harald

^ permalink raw reply

* Re: [PATCH] compat/mingw: Allow SIGKILL to kill in mingw_kill.
From: Siddh Raman Pant @ 2026-05-22  8:03 UTC (permalink / raw)
  To: j6t@kdbg.org, gitster@pobox.com, Johannes.Schindelin@gmx.de
  Cc: git@vger.kernel.org, code@khaugsbakk.name, newren@gmail.com,
	ps@pks.im
In-Reply-To: <xmqqwlwwt0mj.fsf@gitster.g>

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

On Fri, May 22 2026 at 12:02:52 +0530, Junio C Hamano wrote:
> I do not do windows, so I'd like to ask those much more clueful than
> I am to see if they see any downsides.
> 
> The current code only handles TERM (to terminate) or 0 (to probe)
> and everything else results in EINVAL, so the updated behaviour is
> to pretend as if TERM is sent and do whatever PROCESS_TERMINATE
> does, instead of doing nothing and erroring with EINVAL.  Which does
> sound like an improvement over the status quo.
> 
> What I am wondering is if there are different kind of "kill" in the
> Windows land, just like there are distinction between TERM and KILL.
> For example, the program ought to be able to block TERM but not
> KILL.  There are other termination-inducing signals like SIGQUIT but
> until we start using them in our code, this emulation layer does not
> have to know about them, I think.

From what I can see from the docs, there is no SIGTERM on Windows
either. So I did this change since the SIGTERM handling just looks
like a compatibility change in our code.

The docs at [1] says:
	The SIGILL and SIGTERM signals aren't generated under Windows.
	They're included for ANSI compatibility. Therefore, you can set
	signal handlers for these signals by using signal, and you can
	also explicitly generate these signals by calling raise.

Our helper uses TerminateProcess(). The docs at [2] says:
	The TerminateProcess function is used to unconditionally cause
	a process to exit.
	[...]
	A process cannot prevent itself from being terminated.

which is like SIGKILL.

So currently SIGTERM on Windows is behaving like a SIGKILL.

Thanks,
Siddh

[1] https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/signal
[2] https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-terminateprocess

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

^ permalink raw reply

* Re: [PATCH v2 01/11] git-gui: guard set/unset of GIT_DIR and GIT_WORK_TREE
From: Johannes Sixt @ 2026-05-22  8:06 UTC (permalink / raw)
  To: Mark Levedahl, git; +Cc: egg_mushroomcow, bootaina702
In-Reply-To: <20260520202411.108764-2-mlevedahl@gmail.com>

Am 20.05.26 um 22:24 schrieb Mark Levedahl:
> git-gui unconditionally exports _gitdir as GIT_DIR, and _gitworktree as
> GIT_WORK_TREE, to the environment, and furthermore unconditionally
> unsets these environment variables in many

"many cases"?

> 
> git gui must have a repository, so _gitdir can never be empty and its
> export is always valid if repository discovery completes successfully.

_gitdir cannot be empty, so we should be able to drop the conditionals
around 'set env(GIT_DIR) $_gitdir'.

> 
> git gui might not find a worktree, so _gitworktree can be empty. While
> having no worktree is valid for blame/browser subcommands, exporting
> GIT_WORK_TREE=<empty> is not valid. Rather, an empty GIT_WORK_TREE
> raises errors in git builtins, for instance 'git branch --show-current'
> as used by git, and causes breakage. This is one cause of git blame /
> git browser not working without a worktree.
> 
> A user may set GIT_DIR and/or GIT_WORK_TREE to override git's normal
> discovery rules, including repository configuration of core.worktree
> and/or worktree specific gitdirs. It is always safe to export the
> absolute pathnames of the discovered values, even though they may not be
> needed. However, the gitdir might not be found from the worktree without
> GIT_DIR being set. Furthermore, the worktree defined by the discovered
> gitdir might be overridden by GIT_WORK_TREE set before git-gui started.
> So, it is also sometimes necessary that one or both of these variables
> is set.

While all you say here is true, the actual reason for the dance is more
like the simpler: provide a clean slate for the new process and return
to the old state after it has been started.

> 
> So, let's provide two procs, one to unset GIT_DIR / GIT_WORK_TREE if
> they are set, one to set GIT_DIR and, if not empty, GIT_WORK_TREE,  so
> all call sites do the same thing, and problems with _gitworktree == {}
> are avoided.

That being said, I propose the two patches below (pasted here for
review), after which we do not need these functions anymore IMHO
because the call sites are one-liners around GIT_DIR anyway.

The commits are available here:

git fetch https://github.com/j6t/git-gui.git js/unset-git-work-tree
https://github.com/j6t/git-gui/tree/js/unset-git-work-tree

------ 8< ------
Subject: [PATCH 1/2] git-gui: remove unnecessary 'cd $_gitworktree' from do_gitk

In the procedure that invokes Gitk, we have a 'cd $_gitworktree'. Such
a change of the current directory is not necessary, because

- if we have a working tree, then the startup routine has already
  changed the current directory to the root of the working tree, which
  *is* $_gitworktree; or

- if we are in a bare repository, then there is no point in changing
  the current directory anywhere. (And $_gitworktree is empty.)

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 git-gui.sh | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/git-gui.sh b/git-gui.sh
index 23fe76e498bd..8d2b02b13fa0 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -2021,11 +2021,7 @@ proc do_gitk {revs {is_submodule false}} {
 
 		set pwd [pwd]
 
-		if {!$is_submodule} {
-			if {![is_bare]} {
-				cd $_gitworktree
-			}
-		} else {
+		if {$is_submodule} {
 			cd $current_diff_path
 			if {$revs eq {--}} {
 				set s $file_states($current_diff_path)
-- 
2.54.0.215.g4fe990ec16

------ 8< ------
Subject: [PATCH 2/2] git-gui: operate git commands without GIT_WORK_TREE

The manual page of the git command states about the --git-dir option:

   Specifying the location of the ".git" directory using this option
   (or GIT_DIR environment variable) turns off the repository
   discovery [...], and tells Git that you are at the top level of
   the working tree.

Use this to our advantage:

- Set GIT_DIR in the environment to the value that was discovered, so
  that the invoked git commands operate on the same repository
  database that Git GUI uses even after it changes the working
  directory.

- After changing the working directory to the top level of the working
  tree, ensure that GIT_WORK_TREE is not set, because, as per
  documentation, all git invocations from then on will assume that the
  current working directory is also the top level working tree.

- Remove the now obsolete GIT_WORK_TREE dance when subordinate Gitk or
  Git GUI are invoked for a submodule.

Do keep the state of GIT_WORK_TREE if we are in a bare repository,
because Git GUI is not interested in the worktree at all, as no commit
mode is possible in a bare repository.

This avoids cases where an empty GIT_WORK_TREE was exported into the
environment, most notably by a call of `git gui blame HEAD file` in a
bare repository. (Although, this particular error is currently masked
by an earlier failure in `rev-parse --show-toplevel`, which requires a
working tree.)

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 git-gui.sh | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/git-gui.sh b/git-gui.sh
index 8d2b02b13fa0..3819f8be2211 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1183,6 +1183,7 @@ if {$_prefix ne {}} {
 		exit 1
 	}
 	set _gitworktree [pwd]
+	catch {unset env(GIT_WORK_TREE)}
 	unset cdup
 } elseif {![is_enabled bare]} {
 	if {[is_bare]} {
@@ -1199,6 +1200,7 @@ if {$_prefix ne {}} {
 		exit 1
 	}
 	set _gitworktree [pwd]
+	catch {unset env(GIT_WORK_TREE)}
 }
 set _reponame [file split [file normalize $_gitdir]]
 if {[lindex $_reponame end] eq {.git}} {
@@ -1208,7 +1210,6 @@ if {[lindex $_reponame end] eq {.git}} {
 }
 
 set env(GIT_DIR) $_gitdir
-set env(GIT_WORK_TREE) $_gitworktree
 
 ######################################################################
 ##
@@ -2007,7 +2008,7 @@ proc incr_font_size {font {amt 1}} {
 
 proc do_gitk {revs {is_submodule false}} {
 	global current_diff_path file_states current_diff_side ui_index
-	global _gitdir _gitworktree
+	global _gitdir
 
 	# -- Always start gitk through whatever we were loaded with.  This
 	#    lets us bypass using shell process on Windows systems.
@@ -2041,18 +2042,16 @@ proc do_gitk {revs {is_submodule false}} {
 				}
 				set revs $old_sha1...$new_sha1
 			}
-			# GIT_DIR and GIT_WORK_TREE for the submodule are not the ones
-			# we've been using for the main repository, so unset them.
+			# GIT_DIR for the submodule is not the one we've been using for
+			# the main repository, so unset it. (GIT_WORK_TREE is already unset.)
 			# TODO we could make life easier (start up faster?) for gitk
 			# by setting these to the appropriate values to allow gitk
 			# to skip the heuristics to find their proper value
 			unset env(GIT_DIR)
-			unset env(GIT_WORK_TREE)
 		}
 		safe_exec_bg [concat $cmd $revs "--" "--"]
 
 		set env(GIT_DIR) $_gitdir
-		set env(GIT_WORK_TREE) $_gitworktree
 		cd $pwd
 
 		if {[info exists main_status]} {
@@ -2076,12 +2075,11 @@ proc do_git_gui {} {
 		error_popup [mc "Couldn't find git gui in PATH"]
 	} else {
 		global env
-		global _gitdir _gitworktree
+		global _gitdir
 
-		# see note in do_gitk about unsetting these vars when
+		# see note in do_gitk about unsetting this variable when
 		# running tools in a submodule
 		unset env(GIT_DIR)
-		unset env(GIT_WORK_TREE)
 
 		set pwd [pwd]
 		cd $current_diff_path
@@ -2089,7 +2087,6 @@ proc do_git_gui {} {
 		safe_exec_bg [concat $exe gui]
 
 		set env(GIT_DIR) $_gitdir
-		set env(GIT_WORK_TREE) $_gitworktree
 		cd $pwd
 
 		set status_operation [$::main_status \
-- 
2.54.0.215.g4fe990ec16


^ permalink raw reply related

* Re: [PATCH v2 02/11] git-gui: return status from choose_repository::pick
From: Johannes Sixt @ 2026-05-22  8:18 UTC (permalink / raw)
  To: Mark Levedahl; +Cc: egg_mushroomcow, bootaina702, git
In-Reply-To: <20260520202411.108764-3-mlevedahl@gmail.com>

Am 20.05.26 um 22:24 schrieb Mark Levedahl:
> The repository picker (choose_repository::pick) on success always
> returns with the current directory at the root of the selected worktree,
> and with the global variable _gitdir holding the name of the git
> repository, possibly as a relative path. On failure, _gitdir = {}. If
> the selection was from the "recent" list, no validation has occurred.
> 
> There are too many side effects in this interface. Note that the picker
> only supports worktrees with a .git entry in the worktree root, so git
> repository and worktree discovery will work starting in the current
> directory on return. So, let's change pick to return a 0/1 value, 1
> meaning a worktreee + repo was selected and the current directory is the
> worktree root, and leave validation and setting of _gitdir,
> _gitworktree, and _prefix to the caller.

While the removal of side-effects from the picker is very much desired,
the new return value sounds over-engineered at this point, in particular
due to this note:

> Note: pick actually does not
> return if something was not selected, rather it terminates git-gui.
> But, let's pretend at the call site that pick returns 0/false instead.

If we need the return value later, let's postpone that part of this
commit until then.

> diff --git a/git-gui.sh b/git-gui.sh
> index 4ba25da7b6..4a736190a9 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -1151,10 +1151,16 @@ if {[catch {
>  	} err]} {
>  	load_config 1
>  	apply_config
> -	choose_repository::pick
> -	if {![file isdirectory $_gitdir]} {
> +	if {![choose_repository::pick]} {
>  		exit 1
>  	}
> +	if {[catch {
> +		set _gitdir [git rev-parse --git-dir]
> +	} err]} {
> +		catch {wm withdraw .}
> +		error_popup [strcat [mc "Unusable repo/worktree:"] " [pwd] "\n\n$err"]

There's something wrong with the quotes here, and an 'exit 1' is missing.

> +	}
> +	set _prefix {}
>  	set picked 1
>  }
-- Hannes


^ permalink raw reply

* Re: [PATCH v2 03/11] git-gui: use --absolute-git-dir
From: Johannes Sixt @ 2026-05-22  8:25 UTC (permalink / raw)
  To: Mark Levedahl, git; +Cc: egg_mushroomcow, bootaina702
In-Reply-To: <20260520202411.108764-4-mlevedahl@gmail.com>

Am 20.05.26 um 22:24 schrieb Mark Levedahl:
> 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.

Very good!

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

Note that the comment above needs some adjustment as well.

> -		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"]

-- Hannes


^ permalink raw reply

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

Am 20.05.26 um 22:24 schrieb Mark Levedahl:
> 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.

Very much so!

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

Sorry, but I cannot agree with "prefix is only known after the worktree
is found". The prefix is a property that can be known even if we haven't
asked where the top-level of the working tree is. See more below.

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

You cannot leave the _prefix empty, because it breaks `git gui browser
master dir` when invoked from a subdirectory of the working tree.

This line must remain. I see that you add it back in later patch. There
may be some motivation to move prefix discovery, but there is no
motivation to remove it at this point.

>  	} err]} {
> +		if {[is_gitvars_error $err]} {
> +			exit 1
> +		}
> +		set _gitdir {}

BTW, this line would only be needed if the 'set _prefix' line above stays.

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

-- Hannes


^ permalink raw reply

* Re: [PATCH] doc: clarify push.default=simple in triangular workflows
From: Junio C Hamano @ 2026-05-22  8:49 UTC (permalink / raw)
  To: Ivan Baluta via GitGitGadget; +Cc: git, Ivan Baluta
In-Reply-To: <pull.2115.git.1779433093971.gitgitgadget@gmail.com>

"Ivan Baluta via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: ivanbaluta <ivanbaluta.dev@gmail.com>

Just noticing, but don't you want to spell your name just like you
spell it in your e-mails?  I.e., 

    From: Ivan Baluta <ivanbaluta.dev@gmail.com>

Use the same name for your sign-off below.

> The documentation for 'simple' push mode currently focuses on the
> centralized workflow. However, the implementation in builtin/push.c
> falls back to 'current' behavior when pushing to a remote different
> from the upstream (a triangular workflow).

It is not just implementation, but that is how it was designed to
do.

Whether centralized or triangular, "simple" works as a restricted
form as "current", with the same restriction.  That is, both
"current" and "simple" push out only the current branch to a single
destination that is configured, and "simple" insists that the
destination has the same name as the local branch.

So I am not sure if this three-line patch adds much value.

I agree that it _is_ confusing that the current text singles out the
centralized workflow when describing "simple".  But the remedy may
not be to add "what happens in triangular, then?", but it may be to
clarify that the need to configure the push destination whether your
push destination is the same as or different from your upstream, no?

Something along this line, perhaps?

    `simple`;;
    push the current branch with the same name on the remote.
    +
    This mode requires that the remote repository to be pushed to is
    known.  When pushing back to the same remote you pull from, the
    current branch must also have an upstream tracking branch with the
    same name.
    +
    This mode is the default since Git 2.0, and is the safest option
    suited for beginners.

That way, the description would be more self standing and the
readers hopefully do not have to refer to another mode (`current`)
to understand what happens, no?

^ permalink raw reply

* Re: [PATCH 1/8] t0001: plug test gaps for git-init(1) with GIT_OBJECT_DIRECTORY
From: Kristoffer Haugsbakk @ 2026-05-22  9:05 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git
In-Reply-To: <ag_yUsOEO6AjT4Ky@pks.im>

On Fri, May 22, 2026, at 08:06, Patrick Steinhardt wrote:
>>[snip]
>
> That reads a bit better.
>
>>>[snip]
>>
>> Isn’t it more that “the upcoming changes *would have* broken” them if
>> not for this change? This seems to refer to a an alternative commit
>> history where this change does not exist?
>
> Grammar is hard :) But yeah, this of course refers to an alternative
> commit history I had at one point in time that did break this.
>
> Fixed locally, will wait a bit before sending out the next version.

Thank you for considering my small input, as always.

Cheers.

^ permalink raw reply

* Re: [PATCH v9 3/5] branch: add --prune-merged <remote>
From: Phillip Wood @ 2026-05-22  9:47 UTC (permalink / raw)
  To: Harald Nordgren, phillip.wood
  Cc: Harald Nordgren via GitGitGadget, git, Kristoffer Haugsbakk,
	Johannes Sixt
In-Reply-To: <CAHwyqnVhhwT80Ao+7QLUAsTnUJaN5vE=ZiaxeqF3rYxxiD_Qww@mail.gmail.com>

Hi Harald

On 21/05/2026 20:16, Harald Nordgren wrote:
>> While we want to clean up topic branches, we want to avoid cleaning up
>> branches like "master" which follow an upstream branch and therefore
>> look like they've been merged straight after they've been pulled. So I
>> think as well as checking that the local branch is merged into its
>> upstream branch, we want to check that the local branch is not pushed to
>> the upstream branch i.e. that branch@{upstream} != branch@{push}.
> 
> This one I handle already by letting the default branch be guarded.

I used "master" as an example of a branch name above. There is no 
guarantee that a remote even defines a default branch, let alone that 
there is only one local branch where the its upstream and push 
destinations match. I don't see how you can avoid checking that the 
branch pushes to a different ref than its upstream and still be safe.

I'm going to be off the list from now until the week after next, I'll 
catch up with this thread when I'm back on line.

Thanks

Phillip


> 
> Harald
> 
> On Thu, May 21, 2026 at 11:46 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
>>
>> Hi Harald
>>
>> A couple more thoughts ...
>>
>> On 18/05/2026 16:27, Phillip Wood wrote:
>>> On 13/05/2026 20:34, Harald Nordgren via GitGitGadget wrote:
>>>> From: Harald Nordgren <haraldnordgren@gmail.com>
>>>>
>>>> Delete the local branches that --forked <remote> would list, but
>>>> only those whose tip is reachable from their configured upstream
>>>> remote-tracking branch (branch.<name>.merge): the work has already
>>>> landed on the upstream it tracks, so the local copy is no longer
>>>> needed.
>>
>> While we want to clean up topic branches, we want to avoid cleaning up
>> branches like "master" which follow an upstream branch and therefore
>> look like they've been merged straight after they've been pulled. So I
>> think as well as checking that the local branch is merged into its
>> upstream branch, we want to check that the local branch is not pushed to
>> the upstream branch i.e. that branch@{upstream} != branch@{push}. That
>> should also avoid deleting newly created topic branches that match their
>> upstream (I think that's probably less likely to happen in practice as
>> I'd expect the branch to be checked out and therefore protected against
>> deletion).
>>
>> Also as this is a destructive operation (there is no way to restore a
>> deleted branch and its reflog) it would be good to have a --dry-run option.
>>
>> Thanks
>>
>> Phillip
>>
> 


^ permalink raw reply

* Re: [PATCH v10 1/4] branch: add --forked <branch>
From: Harald Nordgren @ 2026-05-22 10:49 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Johannes Sixt, Kristoffer Haugsbakk, Phillip Wood, git,
	Harald Nordgren via GitGitGadget
In-Reply-To: <xmqqse7kt0ge.fsf@gitster.g>

> Johannes Sixt <j6t@kdbg.org> writes:
>
> > The icing on the cake would now be that
> >
> >     git branch --merged origin/main --forked origin/*
> >
> > provides the list of branches forked from origin that have already been
> > integrated.
>
> Yup, that is very nice.  Also with "--merged" replaced with
> "--not-merged", i.e., "our work building on top of origin's, and
> still need to be finished", would give us a good list to work on.

This is nice, but I think this would require an overhaul of other
infra as well, maybe better to do as a follow-up?


Harald

^ permalink raw reply

* Re: [PATCH v9 3/5] branch: add --prune-merged <remote>
From: Harald Nordgren @ 2026-05-22 10:51 UTC (permalink / raw)
  To: phillip.wood
  Cc: Harald Nordgren via GitGitGadget, git, Kristoffer Haugsbakk,
	Johannes Sixt
In-Reply-To: <6ae90274-3fbb-4d2a-b0f4-cd9260e4d6b3@gmail.com>

> >> While we want to clean up topic branches, we want to avoid cleaning up
> >> branches like "master" which follow an upstream branch and therefore
> >> look like they've been merged straight after they've been pulled. So I
> >> think as well as checking that the local branch is merged into its
> >> upstream branch, we want to check that the local branch is not pushed to
> >> the upstream branch i.e. that branch@{upstream} != branch@{push}.
> >
> > This one I handle already by letting the default branch be guarded.
>
> I used "master" as an example of a branch name above. There is no
> guarantee that a remote even defines a default branch, let alone that
> there is only one local branch where the its upstream and push
> destinations match. I don't see how you can avoid checking that the
> branch pushes to a different ref than its upstream and still be safe.

Ok, I'll give it a shot!

> I'm going to be off the list from now until the week after next, I'll
> catch up with this thread when I'm back on line.

Enjoy the time off!


Harald

^ permalink raw reply

* Re: [PATCH v10 1/4] branch: add --forked <branch>
From: Johannes Sixt @ 2026-05-22 11:25 UTC (permalink / raw)
  To: Harald Nordgren
  Cc: Junio C Hamano, Kristoffer Haugsbakk, Phillip Wood, git,
	Harald Nordgren via GitGitGadget
In-Reply-To: <CAHwyqnX=zvjpy3w8qn+H7L_Ncxs5+tK5Va-Lr4ZXX=XYLs2YZQ@mail.gmail.com>

Am 22.05.26 um 12:49 schrieb Harald Nordgren:
>> Johannes Sixt <j6t@kdbg.org> writes:
>>
>>> The icing on the cake would now be that
>>>
>>>     git branch --merged origin/main --forked origin/*
>>>
>>> provides the list of branches forked from origin that have already been
>>> integrated.
>>
>> Yup, that is very nice.  Also with "--merged" replaced with
>> "--not-merged", i.e., "our work building on top of origin's, and
>> still need to be finished", would give us a good list to work on.
> 
> This is nice, but I think this would require an overhaul of other
> infra as well, maybe better to do as a follow-up?
This can certainly be done as an extension in a follow-up patch. But the
UI must still be planned accordingly, i.e., --forked can only take a
single argument. For example, in

    git branch --forked foo bar

'bar' is the pattern of branches to show. The "list" is filtered
according to '--forked foo'. That is, if 'bar' was not forked from
'foo', the output is empty.

You would have to require

    git branch --forked foo --forked bar

to list all branches forked from 'foo' or 'bar'.

In the first implementation, you can restrict uses of other options with
--forked or even with a branch pattern. But you cannot be loose by
accepting multiple branch patterns with one --forked option, because
that would later clash with --list mode.

-- Hannes


^ permalink raw reply


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