Git development
 help / color / mirror / Atom feed
* Re: [PATCH 1/2] b4: introduce configuration for the Git project
From: Tuomas Ahola @ 2026-06-03 13:30 UTC (permalink / raw)
  To: SZEDER Gábor; +Cc: Patrick Steinhardt, Weijie Yuan, git, Junio C Hamano
In-Reply-To: <aiAK9eLvew+mgWt+@szeder.dev>

SZEDER Gábor <szeder.dev@gmail.com> wrote:
> On Wed, Jun 03, 2026 at 08:55:04AM +0200, Patrick Steinhardt wrote:
> > On Wed, Jun 03, 2026 at 10:12:22AM +0800, Weijie Yuan wrote:
> > > On Tue, Jun 02, 2026 at 08:09:55PM +0300, Tuomas Ahola wrote:
> > > > Huh?  Doesn't MyFirstContribution speak *against* shallow threading?
> > > >
> > > > 	        [...]  make sure to replace it with the correct Message-ID for your
> > > > 	**previous cover letter** - that is, if you're sending v2, use the Message-ID
> > > > 	from v1; if you're sending v3, use the Message-ID from v2.
> > > 
> > > I don't get it. Doesn't shallow threading means every following patches
> > > are replying to the cover letter? Replying to the previous one is
> > > --chain-reply-to, if I'm not mistaken.
> > 
> > Shallow threading basically means that all patches are sent as a
> > response to the current cover letter, and the current cover letter is
> > always attached to the cover letter of the _first_ version.
> 
> No, in Git shallow threading means that all patches are sent as a
> respose to the current cover letter, period.  It has nothing to do
> with whether the current cover letter is sent as a reply to the cover
> letter of the first or the previous version.
> 

That seems to be the established meaning of shallow threading, e.g. in
`git format-patch --thread=shallow`.  Unfortunately there is a slight
terminology clash here.

Indeed, in B4 the config option `b4.send-same-thread = shallow` *is*
about whether the cover letter is a reply to v1 or v(n-1).

> > So this quote is definitely at odds with the configuration I have
> > proposed. It's actually quite surprising to me that we recommend deep
> > threading -- I personally find it extremely hard to navigate as the
> > nesting eventually gets way too deep.
> 
> Deep threading means that every mail is a reply to the previous one.
> Again, it has nothing to do with the relation of the current cover
> letter and the previous cover letters.
> 
> Therefore, we do not recommend deep threading.
> 

In the usual meaning of the word that is the case.  Most certainly
we don't recommend that kind of deep threading, but that wasn't the
question we were discussing here.

^ permalink raw reply

* Re: git hook question
From: Adrian Ratiu @ 2026-06-03 13:07 UTC (permalink / raw)
  To: Jeff King, Wesley Schwengle; +Cc: git
In-Reply-To: <20260529210049.GC2628906@coredump.intra.peff.net>

On Fri, 29 May 2026, Jeff King <peff@peff.net> wrote:
> [re-adding list cc; let's let everyone benefit from the discussion]
>
> On Fri, May 29, 2026 at 04:14:33PM -0400, Wesley Schwengle wrote:
>
>> > I don't think the hooks themselves should need to be aware. If somebody
>> > is calling "git hook run pre-push" without providing arguments, they are
>> > breaking the contract to the hooks. You can get away with it if you know
>> > your particular hooks do not care about those arguments, but in the
>> > general case, what should a pre-push hook that _does_ care about the
>> > remote name do when it doesn't get any arguments? It's an error.
>> 
>> Are they? The manual says this:
>> 
>> git hook run has been designed to make it easy for tools which wrap Git to
>> configure and execute hooks using the Git hook infrastructure.  It is
>> possible to provide arguments and stdin via the command line, as well as
>> specifying parallel or series execution if the user has provided multiple
>> hooks.
>> 
>>      Assuming your wrapper wants to support a hook named
>> "mywrapper-start-tests", you can have your users specify their hooks like
>> so:
>> 
>>          [hook "setup-test-dashboard"]
>>            event = mywrapper-start-tests
>>            command = ~/mywrapper/setup-dashboard.py --tap
>> 
>>      Then, in your mywrapper tool, you can invoke any users' configured
>> hooks by running:
>> 
>>          git hook run --allow-unknown-hook-name mywrapper-start-tests \
>>            # providing something to stdin
>>            --stdin some-tempfile-123 \
>>            # execute multiple hooks in parallel
>>            --jobs 3 \
>>            # plus some arguments of your own...
>>            -- \
>>            --testname bar \
>>            baz
>> 
>> There is nothing about the contract of the hook, in fact, the way it is
>> written there isn't really a contract.
>
> This is a made-up hook, so it is up to the person defining
> mywrapper-start-tests to define that contract. And in this example,
> implicitly it takes whatever is in some-tempfile-123 on stdin, and
> --testname as an argument. What those mean would need to be communicated
> between the script invoking "git hook" and whoever is configuring hooks.
>
> I agree that is not made very clear in the documentation, though.
>
>> > So whether you are getting input as arguments or over stdin, it's
>> > probably something the hook needs to deal with (or at least think
>> > about).
>> 
>> Right. I see where this is going. That means I think the examples in the
>> manual are incorrect, no, that's harsh, it could be stated more clearly in
>> git-hook(1).
>> 
>> Examples like this:
>> 
>> > [hook "linter"]
>> >   event = pre-commit
>> >   command = ~/bin/linter --cpp20
>> 
>> seem to indicate: Any script can be run as a hook, the fact it needs to
>> respect the native hook structure isn't mentioned. This is mentioned:
>
> That example is OK-ish, in the sense that pre-commit does not take any
> arguments or receive anything on stdin. So you really can invoke
> whatever program you like (though it needs to understand how to use Git
> commands to look at what is staged in the index). So the details of
> "~/bin/linter" are doing a lot of the heavy lifting here, which is left
> unsaid.
>
> But the later example that adds "event = pre-push" is actively
> misleading. How does the ~/bin/linter script even know in which context
> it's being run? In the real world you are more likely to invoke a script
> that is aware it is a Git hook and can react accordingly.
>
> So I suspect there is a lot of room for expanding the documentation and
> explaining some of these gotchas. +cc Adrian, who wrote these docs, for
> visibility.

Yes, there is a lot of room for improvements everywhere, especially in
the documentation.

Patches are very much welcome to expand on or correct hook-related
issues. :) 

BTW the git hook command is also just a very basic tool for testing, it
needs much attention and more additions. It is obviously not
feature-complete or bug-free.

Some historical context for the curious:

This area of work was blocked for almost a decade because people tried to
find a perfect/complete solution in one go, with complex patch series
reaching even 36-38 review iterations for a single series which went
nowhere, was regressing, was hard to review, you get the idea. 

So I tried to enable a simplified incremental development approach,
reusing existing APIs & mechanisms, to allow more people to contribute
smaller patches which are also easier to review, test and so on.

P.S:
This also reminds me, I don't think it's documented anywhere that the
proc-receive hook is not using hook.[ch], so it cannot be specified via
configs yet like pre-receive and other similar server hooks.

I actually have a collegue at Collabora working on converting
proc-receive so we can remove some deprecated APIs and also clean up
some external hook_exists() calls which are now redundant because they
are handled by the unified hook.c implementation.

^ permalink raw reply

* Re: [PATCH v2 01/18] odb/source-loose: move loose source into "odb/" subsystem
From: Karthik Nayak @ 2026-06-03 13:07 UTC (permalink / raw)
  To: Patrick Steinhardt, git; +Cc: Junio C Hamano
In-Reply-To: <20260601-b4-pks-odb-source-loose-v2-1-90ff159430af@pks.im>

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

Patrick Steinhardt <ps@pks.im> writes:

> In subsequent patches we'll be turning `struct odb_source_loose` into a
> proper `struct odb_source`. As a first step towards this goal, move its

s/its/this?

> struct out of "object-file.c" and into "odb/source-loose.c".
>
> This detaches the implementation of the loose object source from the
> generic object file code, following the same convention already used by
> the "files" and "in-memory" sources.
>
> No functional changes are intended.
>

[snip]

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

^ permalink raw reply

* Re: [PATCH v2 0/8] setup: centralize object database creation
From: Karthik Nayak @ 2026-06-03 13:04 UTC (permalink / raw)
  To: Patrick Steinhardt, git; +Cc: Kristoffer Haugsbakk, Junio C Hamano
In-Reply-To: <20260526-b4-pks-setup-centralize-odb-creation-v2-0-2fa5b385c13e@pks.im>

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

Patrick Steinhardt <ps@pks.im> writes:

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

Apart from some questions/comments, the series looks good. Thanks

- Karthik

[snip]

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

^ permalink raw reply

* Re: [PATCH] git-gui: silence install recipes under "make -s"
From: Johannes Sixt @ 2026-06-03 12:58 UTC (permalink / raw)
  To: Harald Nordgren; +Cc: git, Harald Nordgren via GitGitGadget
In-Reply-To: <pull.2318.git.git.1780477489662.gitgitgadget@gmail.com>

Am 03.06.26 um 11:04 schrieb Harald Nordgren via GitGitGadget:
> From: Harald Nordgren <haraldnordgren@gmail.com>
> 
> The split install/uninstall recipes embed "echo" calls that fire
> even under "make -s", so install still prints "DEST /path" and
> "INSTALL 644 about.tcl" banners. The existing "-s" block only
> clears QUIET_GEN.

Good catch.

> Wrap the whole "ifndef V" block in the canonical "-s" guard from
> shared.mak, and drop the now-redundant narrow block.

Can we please mention shared.mak in a way that doesn't assume that this
patch was made in the Git repository?

> +ifneq ($(findstring s,$(firstword -$(MAKEFLAGS))),s)
>  ifndef V
>  	QUIET          = @
>  	QUIET_GEN      = $(QUIET)echo '   ' GEN '$@' &&
> @@ -89,6 +90,7 @@ ifndef V
>  	REMOVE_F0 = dst=
>  	REMOVE_F1 = && echo '   ' REMOVE `basename "$$dst"` && $(RM_RF) "$$dst"
>  endif
> +endif

> -ifeq ($(findstring $(firstword -$(MAKEFLAGS)),s),s)

I would have expected that the old and the new condition expressions
only differ in the ifeq vs. ifneq, but they are different in more than
that. Assuming that the new expression is correct, was the old one
incorrect?

> -QUIET_GEN =
> -endif
-- Hannes


^ permalink raw reply

* Re: [PATCH v2 5/8] setup: stop creating the object database in `setup_git_env()`
From: Karthik Nayak @ 2026-06-03 12:57 UTC (permalink / raw)
  To: Patrick Steinhardt, git; +Cc: Kristoffer Haugsbakk, Junio C Hamano
In-Reply-To: <20260526-b4-pks-setup-centralize-odb-creation-v2-5-2fa5b385c13e@pks.im>

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

Patrick Steinhardt <ps@pks.im> writes:

> In the preceding commit we have stopped creating the object database in
> `repo_set_gitdir()`. But the logic is still somewhat confusing as we
> still end up creating it conditionally in `setup_git_dir()`, which is
> called multiple times.
>
> Drop the conditional logic and instead create the object database in all
> places where we have discovered and configured a repository.
>
> This leads to even more duplication than we already had in the preceding
> commit, but an alert reader may notice that we now (almost) always call
> `odb_new()` directly before having called `apply_repository_format()`.
> The only exception to this is `setup_git_directory_gently()`, where we
> also call the function when _not_ applying the repository format. This
> will be fixed in the next commit, and once that's done we can then unify
> creation of the object database into `apply_repository_format()`.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  setup.c | 37 ++++++++++++++++++++++++++-----------
>  1 file changed, 26 insertions(+), 11 deletions(-)
>
> diff --git a/setup.c b/setup.c
> index 3bd3f6c592..0dc9fe4565 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -1035,8 +1035,7 @@ const char *read_gitfile_gently(const char *path, int *return_error_code)
>  }
>
>  static void setup_git_env_internal(struct repository *repo,
> -				   const char *git_dir,
> -				   bool skip_initializing_odb)
> +				   const char *git_dir)
>  {
>  	char *git_replace_ref_base;
>  	const char *shallow_file;
> @@ -1053,10 +1052,6 @@ static void setup_git_env_internal(struct repository *repo,
>  	repo_set_gitdir(repo, git_dir, &args);
>  	strvec_clear(&to_free);
>
> -	if (!skip_initializing_odb)
> -		repo->objects = odb_new(repo, getenv_safe(&to_free, DB_ENVIRONMENT),
> -					getenv_safe(&to_free, ALTERNATE_DB_ENVIRONMENT));
> -
>  	if (getenv(NO_REPLACE_OBJECTS_ENVIRONMENT))
>  		disable_replace_refs();
>  	replace_ref_base = getenv(GIT_REPLACE_REF_BASE_ENVIRONMENT);
> @@ -1072,10 +1067,10 @@ static void setup_git_env_internal(struct repository *repo,
>  		fetch_if_missing = 0;
>  }
>
> -static void set_git_dir_1(struct repository *repo, const char *path, bool skip_initializing_odb)
> +static void set_git_dir_1(struct repository *repo, const char *path)
>  {
>  	xsetenv(GIT_DIR_ENVIRONMENT, path, 1);
> -	setup_git_env_internal(repo, path, skip_initializing_odb);
> +	setup_git_env_internal(repo, path);
>  }
>
>  static void update_relative_gitdir(const char *name UNUSED,
> @@ -1089,7 +1084,7 @@ static void update_relative_gitdir(const char *name UNUSED,
>  	trace_printf_key(&trace_setup_key,
>  			 "setup: move $GIT_DIR to '%s'",
>  			 path);
> -	set_git_dir_1(repo, path, true);
> +	set_git_dir_1(repo, path);

Since we were providing `true`, we didn't have to initialize the odb
here. Makes sense.

>  	free(path);
>  }
>
> @@ -1102,7 +1097,7 @@ static void set_git_dir(struct repository *repo, const char *path, int make_real
>  		path = realpath.buf;
>  	}
>
> -	set_git_dir_1(repo, path, false);
> +	set_git_dir_1(repo, path);
>

Huh. I was expecting the odb to be setup here.

>  	if (!is_absolute_path(path))
>  		chdir_notify_register(NULL, update_relative_gitdir, repo);
>
> @@ -1879,8 +1874,15 @@ const char *enter_repo(struct repository *repo, const char *path, unsigned flags
>  	}
>
>  	if (is_git_directory(".")) {
> +		struct strvec to_free = STRVEC_INIT;
> +
>  		set_git_dir(repo, ".", 0);
> +		repo->objects = odb_new(repo,
> +					getenv_safe(&to_free, DB_ENVIRONMENT),
> +					getenv_safe(&to_free, ALTERNATE_DB_ENVIRONMENT));
>  		check_and_apply_repository_format(repo, NULL);
> +
> +		strvec_clear(&to_free);
>  		return path;
>  	}
>
> @@ -2032,13 +2034,19 @@ const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
>  	    startup_info->have_repository ||
>  	    /* GIT_DIR_EXPLICIT */
>  	    getenv(GIT_DIR_ENVIRONMENT)) {
> +		struct strvec to_free = STRVEC_INIT;
> +
>  		if (!repo->gitdir) {
>  			const char *gitdir = getenv(GIT_DIR_ENVIRONMENT);
>  			if (!gitdir)
>  				gitdir = DEFAULT_GIT_DIR_ENVIRONMENT;
> -			setup_git_env_internal(repo, gitdir, false);
> +			setup_git_env_internal(repo, gitdir);
>  		}
>
> +		repo->objects = odb_new(repo,
> +					getenv_safe(&to_free, DB_ENVIRONMENT),
> +					getenv_safe(&to_free, ALTERNATE_DB_ENVIRONMENT));
> +
>

Okay, now it makes sense. we move the ODB creations to layers above.

>  		if (startup_info->have_repository) {
>  			struct strbuf err = STRBUF_INIT;
>
> @@ -2048,6 +2056,8 @@ const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
>  			clear_repository_format(&repo_fmt);
>  			strbuf_release(&err);
>  		}
> +
> +		strvec_clear(&to_free);
>  	}
>  	/*
>  	 * Since precompose_string_if_needed() needs to look at
> @@ -2796,6 +2806,7 @@ int init_db(struct repository *repo,
>  	int exist_ok = flags & INIT_DB_EXIST_OK;
>  	char *original_git_dir = real_pathdup(git_dir, 1);
>  	struct repository_format repo_fmt = REPOSITORY_FORMAT_INIT;
> +	struct strvec to_free = STRVEC_INIT;
>
>  	if (real_git_dir) {
>  		struct stat st;
> @@ -2816,6 +2827,9 @@ int init_db(struct repository *repo,
>  	}
>  	startup_info->have_repository = 1;
>
> +	repo->objects = odb_new(repo, getenv_safe(&to_free, DB_ENVIRONMENT),
> +				getenv_safe(&to_free, ALTERNATE_DB_ENVIRONMENT));
> +
>  	/*
>  	 * Check to see if the repository version is right.
>  	 * Note that a newly created repository does not have
> @@ -2879,6 +2893,7 @@ int init_db(struct repository *repo,
>  	}
>
>  	clear_repository_format(&repo_fmt);
> +	strvec_clear(&to_free);
>  	free(original_git_dir);
>  	return 0;
>  }
>
> --
> 2.54.0.926.g75ba10bac6.dirty

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

^ permalink raw reply

* Re: [PATCH v2 4/8] repository: stop initializing the object database in `repo_set_gitdir()`
From: Karthik Nayak @ 2026-06-03 12:49 UTC (permalink / raw)
  To: Patrick Steinhardt, git; +Cc: Kristoffer Haugsbakk, Junio C Hamano
In-Reply-To: <20260526-b4-pks-setup-centralize-odb-creation-v2-4-2fa5b385c13e@pks.im>

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

Patrick Steinhardt <ps@pks.im> writes:

[snip]

> diff --git a/repository.c b/repository.c
> index 58a13f7c4f..2c2395105f 100644
> --- a/repository.c
> +++ b/repository.c
> @@ -181,12 +181,6 @@ void repo_set_gitdir(struct repository *repo,
>  	free(old_gitdir);
>
>  	repo_set_commondir(repo, o->commondir);
> -
> -	if (!repo->objects)
> -		repo->objects = odb_new(repo, o->object_dir, o->alternate_db);
> -	else if (!o->skip_initializing_odb)
> -		BUG("cannot reinitialize an already-initialized object directory");
> -

This always confuses me, so we were creating the odb even if
`o->skip_initializing_odb` was set to true, if `repo->objects` didn't
exist. Weird.

>  	repo->disable_ref_updates = o->disable_ref_updates;
>
>  	expand_base_dir(&repo->graft_file, o->graft_file,
> @@ -302,6 +296,8 @@ int repo_init(struct repository *repo,
>  		goto error;
>  	}
>
> +	repo->objects = odb_new(repo, NULL, NULL);
> +
>  	if (worktree)
>  		repo_set_worktree(repo, worktree);
>
> diff --git a/repository.h b/repository.h
> index c3ec0f4b79..36e2db2633 100644
> --- a/repository.h
> +++ b/repository.h
> @@ -221,12 +221,9 @@ const char *repo_get_work_tree(struct repository *repo);
>   */
>  struct set_gitdir_args {
>  	const char *commondir;
> -	const char *object_dir;
>  	const char *graft_file;
>  	const char *index_file;
> -	const char *alternate_db;
>  	bool disable_ref_updates;
> -	bool skip_initializing_odb;
>  };
>
>  void repo_set_gitdir(struct repository *repo, const char *root,
> diff --git a/setup.c b/setup.c
> index c5015923f1..3bd3f6c592 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -1045,17 +1045,18 @@ static void setup_git_env_internal(struct repository *repo,
>  	struct strvec to_free = STRVEC_INIT;
>
>  	args.commondir = getenv_safe(&to_free, GIT_COMMON_DIR_ENVIRONMENT);
> -	args.object_dir = getenv_safe(&to_free, DB_ENVIRONMENT);
>  	args.graft_file = getenv_safe(&to_free, GRAFT_ENVIRONMENT);
>  	args.index_file = getenv_safe(&to_free, INDEX_ENVIRONMENT);
> -	args.alternate_db = getenv_safe(&to_free, ALTERNATE_DB_ENVIRONMENT);
>  	if (getenv(GIT_QUARANTINE_ENVIRONMENT))
>  		args.disable_ref_updates = true;
> -	args.skip_initializing_odb = skip_initializing_odb;
>
>  	repo_set_gitdir(repo, git_dir, &args);
>  	strvec_clear(&to_free);
>
> +	if (!skip_initializing_odb)
> +		repo->objects = odb_new(repo, getenv_safe(&to_free, DB_ENVIRONMENT),
> +					getenv_safe(&to_free, ALTERNATE_DB_ENVIRONMENT));
> +

Now this makes a lot more sense.

>  	if (getenv(NO_REPLACE_OBJECTS_ENVIRONMENT))
>  		disable_replace_refs();
>  	replace_ref_base = getenv(GIT_REPLACE_REF_BASE_ENVIRONMENT);
>
> --
> 2.54.0.926.g75ba10bac6.dirty

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

^ permalink raw reply

* Re: [PATCH v2 3/8] setup: deduplicate logic to apply repository format
From: Karthik Nayak @ 2026-06-03 12:43 UTC (permalink / raw)
  To: Patrick Steinhardt, git; +Cc: Kristoffer Haugsbakk, Junio C Hamano
In-Reply-To: <20260526-b4-pks-setup-centralize-odb-creation-v2-3-2fa5b385c13e@pks.im>

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

Patrick Steinhardt <ps@pks.im> writes:

> After having discovered the repository format we then apply it to the
> repository so that it knows to use the proper repository extensions. The
> logic to apply the format is duplicated across three callsites, which
> makes it rather painfull to add new extensions.
>
> Introduce a new function `apply_repository_format()` that takes a repo
> and applies a given format to it and adapt all callsites to use it.
> While at it, rename `check_repository_format()` to clarify that it
> doesn't only _check_ the format, but that it also applies it.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  repository.c | 31 +++++++-------------
>  setup.c      | 93 ++++++++++++++++++++++++++++++++----------------------------
>  setup.h      |  9 ++++++
>  3 files changed, 70 insertions(+), 63 deletions(-)
>
> diff --git a/repository.c b/repository.c
> index db57b8308b..58a13f7c4f 100644
> --- a/repository.c
> +++ b/repository.c
> @@ -262,8 +262,8 @@ void repo_set_worktree(struct repository *repo, const char *path)
>  	trace2_def_repo(repo);
>  }
>
> -static int read_and_verify_repository_format(struct repository_format *format,
> -					     const char *commondir)
> +static int read_repository_format_from_commondir(struct repository_format *format,
> +						 const char *commondir)

Nit: The commit explicitly calls out one rename, but this one wasn't.

>  {
>  	int ret = 0;
>  	struct strbuf sb = STRBUF_INIT;
> @@ -272,11 +272,6 @@ static int read_and_verify_repository_format(struct repository_format *format,
>  	read_repository_format(format, sb.buf);
>  	strbuf_reset(&sb);
>
> -	if (verify_repository_format(format, &sb) < 0) {
> -		warning("%s", sb.buf);
> -		ret = -1;
> -	}
> -

So we remove this, so that the callee would independently verify the
format I assume.

Edit: seems like we call verify_repository_format() within
apply_repository_format() and the latter is called by the callee.

>  	strbuf_release(&sb);
>  	return ret;
>  }
> @@ -290,6 +285,8 @@ int repo_init(struct repository *repo,
>  	      const char *worktree)
>  {
>  	struct repository_format format = REPOSITORY_FORMAT_INIT;
> +	struct strbuf err = STRBUF_INIT;
> +
>  	memset(repo, 0, sizeof(*repo));
>
>  	initialize_repository(repo);
> @@ -297,21 +294,13 @@ int repo_init(struct repository *repo,
>  	if (repo_init_gitdir(repo, gitdir))
>  		goto error;
>
> -	if (read_and_verify_repository_format(&format, repo->commondir))
> +	if (read_repository_format_from_commondir(&format, repo->commondir))
>  		goto error;
>
> -	repo_set_hash_algo(repo, format.hash_algo);
> -	repo_set_compat_hash_algo(repo, format.compat_hash_algo);
> -	repo_set_ref_storage_format(repo, format.ref_storage_format,
> -				    format.ref_storage_payload);
> -	repo->repository_format_worktree_config = format.worktree_config;
> -	repo->repository_format_relative_worktrees = format.relative_worktrees;
> -	repo->repository_format_precious_objects = format.precious_objects;
> -	repo->repository_format_submodule_path_cfg = format.submodule_path_cfg;
> -
> -	/* take ownership of format.partial_clone */

I see that we now do an xstrdup for format.partial_clone, meaning we
have our own memory segment to care about. Do we have to worry about
format.partial_clone not being free'd?

> -	repo->repository_format_partial_clone = format.partial_clone;
> -	format.partial_clone = NULL;
> +	if (apply_repository_format(repo, &format, &err) < 0) {
> +		warning("%s", err.buf);
> +		goto error;
> +	}
>
>  	if (worktree)
>  		repo_set_worktree(repo, worktree);

[snip]

> diff --git a/setup.h b/setup.h
> index 9409326fe4..5ed92f53fa 100644
> --- a/setup.h
> +++ b/setup.h
> @@ -221,6 +221,15 @@ void clear_repository_format(struct repository_format *format);
>  int verify_repository_format(const struct repository_format *format,
>  			     struct strbuf *err);
>
> +/*
> + * Apply the given repository format to the repo. This initializes extensions
> + * and basic data structures required for normal operation. Returns 0 on
> + * success, a negative error code otherwise.
> + */

Nit: perhaps we should also mention that we verify the format?

> +int apply_repository_format(struct repository *repo,
> +			    const struct repository_format *format,
> +			    struct strbuf *err);
> +
>  const char *get_template_dir(const char *option_template);
>
>  #define INIT_DB_QUIET      (1 << 0)
>
> --
> 2.54.0.926.g75ba10bac6.dirty
p

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

^ permalink raw reply

* Re: Git for Windows Failing to Clone
From: Johannes Schindelin @ 2026-06-03 12:42 UTC (permalink / raw)
  To: Dylan Carlyle; +Cc: git
In-Reply-To: <CAJKusd6WJUUVhbyN_-XHkGWVYeNe_=K2U3tZoezPWFG3+OG_zw@mail.gmail.com>

Hi Dylan,

On Wed, 3 Jun 2026, Dylan Carlyle wrote:

> Thank you for filling out a Git bug report!
> Please answer the following questions to help us understand your issue.
> 
> What did you do before the bug happened? (Steps to reproduce your issue):
> 
> Ran git clone user@ip_addres:repo
> 
> What did you expect to happen? (Expected behavior):
> 
> The repo to be cloned
> 
> What happened instead? (Actual behavior):
> 
> remote: Enumerating objects: 57873, done.
> remote: Counting objects: 100% (57873/57873), done.
> remote: Compressing objects: 100% (32002/32002), done.
> fatal: pack has bad object at offset 460179591: inflate returned 1
> fatal: fetch-pack: invalid index-pack output
> 
> What's different between what you expected and what actually happened?
> 
> The clone never finishes on Windows.
> 
> Anything else you want to add:
> 
> Git version on the remote server is 2.47.3
> This works fine from Linux but fails on Windows.

I guess that this is virtually identical to
https://github.com/git-for-windows/git/issues/6265

Since it works from Linux, but not from Windows, I strongly suspect the
problem to be related to that vexing 2GB/4GB problem induced by Git's
continued use of `unsigned long` instead of `size_t`, which I am slowly
(_very_ slowly) trying to address.

You can find out whether that effort might help you, by running `git repo
structure` on the original repository (I'd expect a blob whose unpacked
size is larger than 4GB).

Ciao,
Johannes

> 
> [System Info]
> git version:
> git version 2.54.0.windows.1
> cpu: x86_64
> built from commit: 2b8a3ab140826ac423c2845ef81d4c6ac4f7bf3c
> sizeof-long: 4
> sizeof-size_t: 8
> shell-path: D:/git-sdk-64-build-installers/usr/bin/sh
> rust: disabled
> feature: fsmonitor--daemon
> 
> -- 
> Kind Regards,
> 
> Dylan Carlyle
> REFTEK Systems, Inc.
> Systems Administrator
> 
> 

^ permalink raw reply

* Re: [PATCH v3] index-pack: retain child bases in delta cache
From: Derrick Stolee @ 2026-06-03 12:24 UTC (permalink / raw)
  To: Arijit Banerjee via GitGitGadget, git
  Cc: Ævar Arnfjörð Bjarmason, Junio C Hamano, Jeff King,
	Arijit Banerjee, Arijit Banerjee
In-Reply-To: <pull.2131.v3.git.1780445118653.gitgitgadget@gmail.com>

On 6/2/26 8:05 PM, Arijit Banerjee via GitGitGadget wrote:

>      Changes since v2:
>      
>       * Addressed Jeff King's review question by releasing cached base data
>         after all direct children have been dispatched, while keeping the
>         existing subtree bookkeeping intact.
>       * Re-ran t/t5302-pack-index.sh, p5302-pack-index.sh, and end-to-end
>         full clone spot checks with the precise-release version.
...
> +static int base_data_has_remaining_direct_children(struct base_data *c)
> +{
> +	return c->ref_first <= c->ref_last ||
> +	       c->ofs_first <= c->ofs_last;
> +}
> +

I'm glad you were able to find some bookkeeping that already exists to
help with this decision.



>   static void prune_base_data(struct base_data *retain)
>   {
>   	struct list_head *pos;
> @@ -1201,8 +1207,12 @@ static void *threaded_second_pass(void *data)
>   		}
>   
>   		work_lock();
> -		if (parent)
> +		if (parent) {
>   			parent->retain_data--;
> +			if (!parent->retain_data &&
> +			    !base_data_has_remaining_direct_children(parent))
> +				free_base_data(parent);
> +		}

This appears like the correct place to do this.

>   		if (child && child->data) {
>   			/*
> @@ -1212,7 +1222,6 @@ static void *threaded_second_pass(void *data)
>   			list_add(&child->list, &work_head);
>   			base_cache_used += child->size;
>   			prune_base_data(NULL);
> -			free_base_data(child);

And still we don't want this universal free.

Thanks for re-running your performance numbers after this change. I didn't
see any significant difference in the relative changes.

I don't think we have a way of measuring "memory pressure" during the
performance test suite. Did you see any evidence that this change has the
intended effect of reducing process memory proactively instead of relying
on the cache evictions?

Thanks,
-Stolee

^ permalink raw reply

* Re: [PATCH 1/2] b4: introduce configuration for the Git project
From: Weijie Yuan @ 2026-06-03 12:23 UTC (permalink / raw)
  To: SZEDER Gábor; +Cc: Patrick Steinhardt, Tuomas Ahola, git, Junio C Hamano
In-Reply-To: <aiAK9eLvew+mgWt+@szeder.dev>

On Wed, Jun 03, 2026 at 01:07:33PM +0200, SZEDER Gábor wrote:
> No, in Git shallow threading means that all patches are sent as a
> respose to the current cover letter, period.  It has nothing to do
> with whether the current cover letter is sent as a reply to the cover
> letter of the first or the previous version.

Thanks, agree

> Deep threading means that every mail is a reply to the previous one.
> Again, it has nothing to do with the relation of the current cover
> letter and the previous cover letters.
>
> Therefore, we do not recommend deep threading.

So the same reason with Patrick?

Thanks

^ permalink raw reply

* Re: [PATCH v2 1/8] t0001: plug test gaps for git-init(1) with GIT_OBJECT_DIRECTORY
From: Karthik Nayak @ 2026-06-03 12:22 UTC (permalink / raw)
  To: Patrick Steinhardt, git; +Cc: Kristoffer Haugsbakk, Junio C Hamano
In-Reply-To: <20260526-b4-pks-setup-centralize-odb-creation-v2-1-2fa5b385c13e@pks.im>

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

Patrick Steinhardt <ps@pks.im> writes:

> 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" environment 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 would have
> broken 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
> +'
> +

I was surprised to find that such a small number of tests ever use
GIT_OBJECT_DIRECTORY. This looks good.

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

^ permalink raw reply

* [PATCH 2/2] builtin/add: use die_for_required_opt() helper
From: Siddharth Shrimali @ 2026-06-03 11:10 UTC (permalink / raw)
  To: git; +Cc: gitster, christian.couder, toon, jn.avila, r.siddharth.shrimali
In-Reply-To: <20260603111044.39116-1-r.siddharth.shrimali@gmail.com>

Clean up manual option dependency checks by replacing explicit conditional
blocks with the newly introduced die_for_required_opt() helper function.

Specifically, simplify the prerequisite check logic for both
'--ignore-missing' (which requires '--dry-run') and '--pathspec-file-nul'
(which requires '--pathspec-from-file').

Signed-off-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
---
 builtin/add.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/builtin/add.c b/builtin/add.c
index c859f66519..a5c91c6dcf 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -441,8 +441,7 @@ int cmd_add(int argc,
 	if (addremove && take_worktree_changes)
 		die(_("options '%s' and '%s' cannot be used together"), "-A", "-u");
 
-	if (!show_only && ignore_missing)
-		die(_("the option '%s' requires '%s'"), "--ignore-missing", "--dry-run");
+	die_for_required_opt(ignore_missing, "--ignore-missing", show_only, "--dry-run");
 
 	if (chmod_arg && ((chmod_arg[0] != '-' && chmod_arg[0] != '+') ||
 			  chmod_arg[1] != 'x' || chmod_arg[2]))
@@ -462,6 +461,8 @@ int cmd_add(int argc,
 		       PATHSPEC_SYMLINK_LEADING_PATH,
 		       prefix, argv);
 
+	die_for_required_opt(pathspec_file_nul, "--pathspec-file-nul",
+				!!pathspec_from_file, "--pathspec-from-file");
 	if (pathspec_from_file) {
 		if (pathspec.nr)
 			die(_("'%s' and pathspec arguments cannot be used together"), "--pathspec-from-file");
@@ -470,8 +471,6 @@ int cmd_add(int argc,
 				    PATHSPEC_PREFER_FULL |
 				    PATHSPEC_SYMLINK_LEADING_PATH,
 				    prefix, pathspec_from_file, pathspec_file_nul);
-	} else if (pathspec_file_nul) {
-		die(_("the option '%s' requires '%s'"), "--pathspec-file-nul", "--pathspec-from-file");
 	}
 
 	if (require_pathspec && pathspec.nr == 0) {
-- 
2.54.0


^ permalink raw reply related

* [PATCH 1/2] parse-options: introduce die_for_required_opt()
From: Siddharth Shrimali @ 2026-06-03 11:10 UTC (permalink / raw)
  To: git; +Cc: gitster, christian.couder, toon, jn.avila, r.siddharth.shrimali
In-Reply-To: <20260603111044.39116-1-r.siddharth.shrimali@gmail.com>

Introduce a new helper function die_for_required_opt() to check if a
given option is present without its required prerequisite option.

This provides a centralized API for handling simple option dependencies
(i.e., X requires Y), matching the style of the existing mutual-exclusion
helpers like die_for_incompatible_opt{2,3,4}().

Suggested-by: Christian Couder <christian.couder@gmail.com>
Signed-off-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
---
 parse-options.c | 7 +++++++
 parse-options.h | 3 +++
 2 files changed, 10 insertions(+)

diff --git a/parse-options.c b/parse-options.c
index a676da86f5..e100f9a0c1 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -1558,3 +1558,10 @@ void die_for_incompatible_opt4(int opt1, const char *opt1_name,
 		break;
 	}
 }
+
+void die_for_required_opt(int opt1, const char *opt1_name,
+			  int opt2, const char *opt2_name)
+{
+	if (opt1 && !opt2)
+		die(_("the option '%s' requires '%s'"), opt1_name, opt2_name);
+}
diff --git a/parse-options.h b/parse-options.h
index 0d1f738f8d..99dc53325d 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -460,6 +460,9 @@ static inline void die_for_incompatible_opt2(int opt1, const char *opt1_name,
 				  0, "");
 }
 
+void die_for_required_opt(int opt1, const char *opt1_name,
+			  int opt2, const char *opt2_name);
+
 /*
  * Use these assertions for callbacks that expect to be called with NONEG and
  * NOARG respectively, and do not otherwise handle the "unset" and "arg"
-- 
2.54.0


^ permalink raw reply related

* [PATCH 0/2] parse-options: introduce die_for_required_opt() helper
From: Siddharth Shrimali @ 2026-06-03 11:10 UTC (permalink / raw)
  To: git; +Cc: gitster, christian.couder, toon, jn.avila, r.siddharth.shrimali

Many built-in commands in Git manually check for option prerequisites 
(i.e., option X relies on option Y being present) using explicit 
conditional blocks and duplicated error message strings.

This short series comes out of a discussion with Christian about 
localization and code duplication. To address these issues, it 
introduces a centralized API helper that handles simple option 
prerequisites safely.

- Patch 1 introduces the `die_for_required_opt()` helper function 
  inside parse-options.
  
- Patch 2 cleans up `builtin/add.c` as a proof-of-concept by migrating 
  its manual prerequisite checks for '--ignore-missing' and 
  '--pathspec-file-nul' over to the new helper.

If this initial approach looks good, we can later extend the helper 
to handle more complex multi-option dependencies.

Siddharth Shrimali (2):
  parse-options: introduce die_for_required_opt()
  builtin/add: use die_for_required_opt() helper

 builtin/add.c   | 7 +++----
 parse-options.c | 7 +++++++
 parse-options.h | 3 +++
 3 files changed, 13 insertions(+), 4 deletions(-)

-- 
2.54.0


^ permalink raw reply

* Re: [PATCH 1/2] b4: introduce configuration for the Git project
From: SZEDER Gábor @ 2026-06-03 11:07 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: Weijie Yuan, Tuomas Ahola, git, Junio C Hamano
In-Reply-To: <ah_PyDwO1Sffr5yq@pks.im>

On Wed, Jun 03, 2026 at 08:55:04AM +0200, Patrick Steinhardt wrote:
> On Wed, Jun 03, 2026 at 10:12:22AM +0800, Weijie Yuan wrote:
> > On Tue, Jun 02, 2026 at 08:09:55PM +0300, Tuomas Ahola wrote:
> > > Huh?  Doesn't MyFirstContribution speak *against* shallow threading?
> > >
> > > 	        [...]  make sure to replace it with the correct Message-ID for your
> > > 	**previous cover letter** - that is, if you're sending v2, use the Message-ID
> > > 	from v1; if you're sending v3, use the Message-ID from v2.
> > 
> > I don't get it. Doesn't shallow threading means every following patches
> > are replying to the cover letter? Replying to the previous one is
> > --chain-reply-to, if I'm not mistaken.
> 
> Shallow threading basically means that all patches are sent as a
> response to the current cover letter, and the current cover letter is
> always attached to the cover letter of the _first_ version.

No, in Git shallow threading means that all patches are sent as a
respose to the current cover letter, period.  It has nothing to do
with whether the current cover letter is sent as a reply to the cover
letter of the first or the previous version.

> So this quote is definitely at odds with the configuration I have
> proposed. It's actually quite surprising to me that we recommend deep
> threading -- I personally find it extremely hard to navigate as the
> nesting eventually gets way too deep.

Deep threading means that every mail is a reply to the previous one.
Again, it has nothing to do with the relation of the current cover
letter and the previous cover letters.

Therefore, we do not recommend deep threading.

> You know -- I'll include a patch that changes the wording there to also
> use shallow nesting, mostly to kick off a discussion and arrive at a
> decision there.



^ permalink raw reply

* Re: [PATCH v2 1/3] Documentation/MyFirstContribution: recommend shallow threading
From: Weijie Yuan @ 2026-06-03 10:29 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git, Junio C Hamano, Tuomas Ahola, Ramsay Jones
In-Reply-To: <20260603-pks-b4-v2-1-a8aea0aa2c23@pks.im>

I'm afraid there will be some chaos.

As mentioned earlier, GitGitGadget now supports deep nesting of
iterations, if b4 changes while GitGitGadget doesn't, it would be
inconsistent in the archive. So, negotiation is necessary here.

As I know, b4 can generate a cover letter containing "Changes with vn",
e.g. in [PATCH v5 00/10], there would be Changes with v4, v3, v2, v1. In
this case, it is semantically correct that the cover letter of v5 is
replying-to the cover letter of v1.

But in traditional way, it seems that the norm is put a range-diff in
the cover letter. In this case, chain-reply-to makes more sence to me:
e.g. The cover letter contains the range-diff against v2, so cover
letter v3 is pointing to cover letter v2. (I don't know whether
git format-patch accepts several --range-diff or not, but if so, I
guess it might be painful to typing several refs, or copy and paste
from previous cover letter) Therefore, if git format-patch could
generate cover letter containing all the changes with v4/v3/v2/v1 as b4
does, it would be consistent, and semantically correct to pointing to
the first cover letter.

Do we need to consider backward compatibility here? ;-)

Thanks!

^ permalink raw reply

* [PATCH v2] rebase: skip branch symref aliases
From: Son Luong Ngoc via GitGitGadget @ 2026-06-03 10:27 UTC (permalink / raw)
  To: git; +Cc: Kristoffer Haugsbakk, Phillip Wood, Son Luong Ngoc,
	Son Luong Ngoc
In-Reply-To: <pull.2126.git.1779946921.gitgitgadget@gmail.com>

From: Son Luong Ngoc <sluongng@gmail.com>

git rebase --update-refs can fail after the normal rebase path has
updated the current branch when another local branch is a symref to it.
This can happen during a default-branch rename where refs/heads/main
points at refs/heads/master while users migrate.

The sequencer queues update-ref commands from local branch decorations.
Commit 106b6885c7 (rebase: ignore non-branch update-refs) filters out
decorations that are not local branches, such as HEAD and tags. A branch
symref is different: it is still a local branch decoration, but if it
resolves to another branch then that target branch is itself present in
the decoration list and will be updated as a concrete branch.

Skip branch decorations whose symrefs resolve to refs/heads/*, because
those targets are already represented by concrete branch decorations.
This prevents aliases from scheduling a second update for the same
branch. Keep symrefs to non-branch targets on the existing path.

Preserve the existing checked-out branch handling before applying these
skips. Such refs still need a todo-list comment instead of an update-ref
command, even when the checked-out ref is the branch being rebased or a
branch symref alias. Use a copy of the resolved HEAD ref so later ref
resolution does not overwrite it.

Signed-off-by: Son Luong Ngoc <sluongng@gmail.com>
---
    rebase: handle --update-refs branch symrefs
    
    Changes since v1:
    
     * Squashed the regression test and fix into a single patch.
     * Reworked the implementation per Phillip's review: skip local branch
       symrefs that are not checked out when their targets resolve to
       refs/heads/*, while preserving the existing behavior for symrefs to
       non-branch targets.
     * Preserved checked-out branch comments before applying the new symref
       skip, so worktrees still get the existing todo-list warning instead
       of an update-ref command.
     * Folded the symref coverage into the existing "--update-refs updates
       refs correctly" test, covering both an alias of HEAD and an alias of
       another branch.
     * Kept the relationship to 106b6885c7 in the commit message: non-local
       decorations are already filtered, and this patch handles the
       remaining local branch decorations that are symref aliases.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2126%2Fsluongng%2Fsl%2Frebase-update-refs-symrefs-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2126/sluongng/sl/rebase-update-refs-symrefs-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2126

Range-diff vs v1:

 1:  a550923440 < -:  ---------- t3404: add failing branch symref test
 2:  0ab0a71744 ! 1:  68f698225c rebase: skip branch symref aliases
     @@ Metadata
       ## Commit message ##
          rebase: skip branch symref aliases
      
     -    rebase --update-refs records local branch decorations before replaying
     -    commits. If a decoration is a symbolic branch such as refs/heads/main
     -    pointing at refs/heads/master, updating it later dereferences back to
     -    master and can fail because the normal rebase path already moved that
     -    branch.
     +    git rebase --update-refs can fail after the normal rebase path has
     +    updated the current branch when another local branch is a symref to it.
     +    This can happen during a default-branch rename where refs/heads/main
     +    points at refs/heads/master while users migrate.
      
     -    Resolve local branch symref decorations to their referents before
     -    queuing update-ref commands, and skip duplicates. This keeps branch
     -    aliases from scheduling a second update for the same underlying branch
     -    while still using the existing old-OID check for the single queued
     -    update.
     +    The sequencer queues update-ref commands from local branch decorations.
     +    Commit 106b6885c7 (rebase: ignore non-branch update-refs) filters out
     +    decorations that are not local branches, such as HEAD and tags. A branch
     +    symref is different: it is still a local branch decoration, but if it
     +    resolves to another branch then that target branch is itself present in
     +    the decoration list and will be updated as a concrete branch.
     +
     +    Skip branch decorations whose symrefs resolve to refs/heads/*, because
     +    those targets are already represented by concrete branch decorations.
     +    This prevents aliases from scheduling a second update for the same
     +    branch. Keep symrefs to non-branch targets on the existing path.
     +
     +    Preserve the existing checked-out branch handling before applying these
     +    skips. Such refs still need a todo-list comment instead of an update-ref
     +    command, even when the checked-out ref is the branch being rebased or a
     +    branch symref alias. Use a copy of the resolved HEAD ref so later ref
     +    resolution does not overwrite it.
      
          Signed-off-by: Son Luong Ngoc <sluongng@gmail.com>
      
     @@ sequencer.c: static int add_decorations_to_list(const struct commit *commit,
       	const struct name_decoration *decoration = get_name_decoration(&commit->object);
      -	const char *head_ref = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
      -						       "HEAD",
     -+	struct ref_store *refs = get_main_ref_store(the_repository);
     -+	const char *head_ref = refs_resolve_ref_unsafe(refs, "HEAD",
     - 						       RESOLVE_REF_READING,
     +-						       RESOLVE_REF_READING,
      -						       NULL,
      -						       NULL);
     -+						       NULL, NULL);
     -+	char *resolved_head_ref = refs_resolve_refdup(refs, "HEAD",
     -+						       RESOLVE_REF_READING,
     -+						       NULL, NULL);
     -+	struct strbuf update_ref = STRBUF_INIT;
     ++	struct ref_store *refs = get_main_ref_store(the_repository);
     ++	char *head_ref = refs_resolve_refdup(refs, "HEAD",
     ++					     RESOLVE_REF_READING,
     ++					     NULL, NULL);
       
       	while (decoration) {
       		struct todo_item *item;
       		const char *path;
     -+		const char *ref = decoration->name;
      +		const char *resolved_ref;
     -+		int is_symref = 0;
      +		int flags = 0;
       		size_t base_offset = ctx->buf->len;
       
       		/*
     -@@ sequencer.c: static int add_decorations_to_list(const struct commit *commit,
     - 		 * updated by the default rebase behavior.
     - 		 * Exclude it from the list of refs to update,
     - 		 * as well as any non-branch decorations.
     -+		 *
     -+		 * Resolve branch symrefs after checking for the current HEAD so
     -+		 * that aliases do not schedule duplicate updates for their
     -+		 * referents.
     -+		 *
     +-		 * If the branch is the current HEAD, then it will be
     +-		 * updated by the default rebase behavior.
     +-		 * Exclude it from the list of refs to update,
     +-		 * as well as any non-branch decorations.
       		 * Non-branch decorations may be present if the pretty format
       		 * includes "%d", which would have loaded all refs
       		 * into the global decoration table.
     @@ sequencer.c: static int add_decorations_to_list(const struct commit *commit,
      +			continue;
      +		}
      +
     -+		if (head_ref && !strcmp(head_ref, ref)) {
     ++		path = branch_checked_out(decoration->name);
     ++
     ++		/*
     ++		 * If the branch is the current HEAD, then it will be
     ++		 * updated by the default rebase behavior. Exclude it from
     ++		 * the list of refs to update, unless it is checked out and
     ++		 * needs a comment in the todo list.
     ++		 */
     ++		if (!path && head_ref && !strcmp(head_ref, decoration->name)) {
      +			decoration = decoration->next;
      +			continue;
      +		}
      +
     -+		strbuf_reset(&update_ref);
     -+		resolved_ref = refs_resolve_ref_unsafe(refs, ref,
     -+						       RESOLVE_REF_READING |
     -+						       RESOLVE_REF_NO_RECURSE,
     ++		resolved_ref = refs_resolve_ref_unsafe(refs, decoration->name,
     ++						       RESOLVE_REF_READING,
      +						       NULL, &flags);
     -+		if ((flags & REF_ISSYMREF) && resolved_ref) {
     -+			if (!starts_with(resolved_ref, "refs/heads/")) {
     -+				decoration = decoration->next;
     -+				continue;
     -+			}
     -+
     -+			strbuf_addstr(&update_ref, resolved_ref);
     -+			ref = update_ref.buf;
     -+			is_symref = 1;
     -+		}
     -+
     -+		if ((is_symref && resolved_head_ref &&
     -+		     !strcmp(resolved_head_ref, ref)) ||
     -+		    string_list_has_string(&ctx->refs_to_oids, ref)) {
     ++		if (!path && resolved_ref && (flags & REF_ISSYMREF) &&
     ++		    starts_with(resolved_ref, "refs/heads/")) {
       			decoration = decoration->next;
       			continue;
       		}
     @@ sequencer.c: static int add_decorations_to_list(const struct commit *commit,
       
       		/* If the branch is checked out, then leave a comment instead. */
      -		if ((path = branch_checked_out(decoration->name))) {
     -+		if ((path = branch_checked_out(ref))) {
     ++		if (path) {
       			item->command = TODO_COMMENT;
       			strbuf_commented_addf(ctx->buf, comment_line_str,
       					      "Ref %s checked out at '%s'\n",
     --					      decoration->name, path);
     -+					      ref, path);
     - 		} else {
     - 			struct string_list_item *sti;
     - 			item->command = TODO_UPDATE_REF;
     --			strbuf_addf(ctx->buf, "%s\n", decoration->name);
     -+			strbuf_addf(ctx->buf, "%s\n", ref);
     - 
     - 			sti = string_list_insert(&ctx->refs_to_oids,
     --						 decoration->name);
     --			sti->util = init_update_ref_record(decoration->name);
     -+						 ref);
     -+			sti->util = init_update_ref_record(ref);
     - 		}
     - 
     - 		item->offset_in_buf = base_offset;
      @@ sequencer.c: static int add_decorations_to_list(const struct commit *commit,
       		decoration = decoration->next;
       	}
       
     -+	strbuf_release(&update_ref);
     -+	free(resolved_head_ref);
     ++	free(head_ref);
       	return 0;
       }
       
      
       ## t/t3404-rebase-interactive.sh ##
      @@ t/t3404-rebase-interactive.sh: test_expect_success '--update-refs ignores non-branch decorations' '
     - 	test_cmp expect actual
       '
       
     --test_expect_failure '--update-refs skips branch symrefs to current branch' '
     -+test_expect_success '--update-refs skips branch symrefs to current branch' '
     - 	test_when_finished "
     - 		test_might_fail git rebase --abort &&
     - 		git checkout primary &&
     + test_expect_success '--update-refs updates refs correctly' '
     ++	test_when_finished "
     ++		test_might_fail git symbolic-ref -d refs/heads/no-conflict-branch-alias &&
     ++		test_might_fail git symbolic-ref -d refs/heads/second-alias
     ++	" &&
     + 	git checkout -B update-refs no-conflict-branch &&
     + 	git branch -f base HEAD~4 &&
     + 	git branch -f first HEAD~3 &&
     + 	git branch -f second HEAD~3 &&
     + 	git branch -f third HEAD~1 &&
     ++	git symbolic-ref refs/heads/no-conflict-branch-alias \
     ++		refs/heads/no-conflict-branch &&
     ++	git symbolic-ref refs/heads/second-alias refs/heads/second &&
     + 	test_commit extra2 fileX &&
     + 	git commit --amend --fixup=L &&
     + 
     +@@ t/t3404-rebase-interactive.sh: test_expect_success '--update-refs updates refs correctly' '
     + 
     + 	test_cmp_rev HEAD~3 refs/heads/first &&
     + 	test_cmp_rev HEAD~3 refs/heads/second &&
     ++	test_cmp_rev HEAD~3 refs/heads/second-alias &&
     + 	test_cmp_rev HEAD~1 refs/heads/third &&
     + 	test_cmp_rev HEAD refs/heads/no-conflict-branch &&
     ++	test_cmp_rev HEAD refs/heads/no-conflict-branch-alias &&
     ++	test_write_lines refs/heads/no-conflict-branch >expect &&
     ++	git symbolic-ref refs/heads/no-conflict-branch-alias >actual &&
     ++	test_cmp expect actual &&
     ++	test_write_lines refs/heads/second >expect &&
     ++	git symbolic-ref refs/heads/second-alias >actual &&
     ++	test_cmp expect actual &&
     + 
     + 	q_to_tab >expect <<-\EOF &&
     + 	Successfully rebased and updated refs/heads/update-refs.


 sequencer.c                   | 43 +++++++++++++++++++++++++----------
 t/t3404-rebase-interactive.sh | 15 ++++++++++++
 2 files changed, 46 insertions(+), 12 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 1ee4b2875b..6ab8b47108 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -6445,28 +6445,46 @@ static int add_decorations_to_list(const struct commit *commit,
 				   struct todo_add_branch_context *ctx)
 {
 	const struct name_decoration *decoration = get_name_decoration(&commit->object);
-	const char *head_ref = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
-						       "HEAD",
-						       RESOLVE_REF_READING,
-						       NULL,
-						       NULL);
+	struct ref_store *refs = get_main_ref_store(the_repository);
+	char *head_ref = refs_resolve_refdup(refs, "HEAD",
+					     RESOLVE_REF_READING,
+					     NULL, NULL);
 
 	while (decoration) {
 		struct todo_item *item;
 		const char *path;
+		const char *resolved_ref;
+		int flags = 0;
 		size_t base_offset = ctx->buf->len;
 
 		/*
-		 * If the branch is the current HEAD, then it will be
-		 * updated by the default rebase behavior.
-		 * Exclude it from the list of refs to update,
-		 * as well as any non-branch decorations.
 		 * Non-branch decorations may be present if the pretty format
 		 * includes "%d", which would have loaded all refs
 		 * into the global decoration table.
 		 */
-		if ((head_ref && !strcmp(head_ref, decoration->name)) ||
-		    (decoration->type != DECORATION_REF_LOCAL)) {
+		if (decoration->type != DECORATION_REF_LOCAL) {
+			decoration = decoration->next;
+			continue;
+		}
+
+		path = branch_checked_out(decoration->name);
+
+		/*
+		 * If the branch is the current HEAD, then it will be
+		 * updated by the default rebase behavior. Exclude it from
+		 * the list of refs to update, unless it is checked out and
+		 * needs a comment in the todo list.
+		 */
+		if (!path && head_ref && !strcmp(head_ref, decoration->name)) {
+			decoration = decoration->next;
+			continue;
+		}
+
+		resolved_ref = refs_resolve_ref_unsafe(refs, decoration->name,
+						       RESOLVE_REF_READING,
+						       NULL, &flags);
+		if (!path && resolved_ref && (flags & REF_ISSYMREF) &&
+		    starts_with(resolved_ref, "refs/heads/")) {
 			decoration = decoration->next;
 			continue;
 		}
@@ -6478,7 +6496,7 @@ static int add_decorations_to_list(const struct commit *commit,
 		memset(item, 0, sizeof(*item));
 
 		/* If the branch is checked out, then leave a comment instead. */
-		if ((path = branch_checked_out(decoration->name))) {
+		if (path) {
 			item->command = TODO_COMMENT;
 			strbuf_commented_addf(ctx->buf, comment_line_str,
 					      "Ref %s checked out at '%s'\n",
@@ -6501,6 +6519,7 @@ static int add_decorations_to_list(const struct commit *commit,
 		decoration = decoration->next;
 	}
 
+	free(head_ref);
 	return 0;
 }
 
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 58b3bb0c27..bc0b6a31f7 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1979,11 +1979,18 @@ test_expect_success '--update-refs ignores non-branch decorations' '
 '
 
 test_expect_success '--update-refs updates refs correctly' '
+	test_when_finished "
+		test_might_fail git symbolic-ref -d refs/heads/no-conflict-branch-alias &&
+		test_might_fail git symbolic-ref -d refs/heads/second-alias
+	" &&
 	git checkout -B update-refs no-conflict-branch &&
 	git branch -f base HEAD~4 &&
 	git branch -f first HEAD~3 &&
 	git branch -f second HEAD~3 &&
 	git branch -f third HEAD~1 &&
+	git symbolic-ref refs/heads/no-conflict-branch-alias \
+		refs/heads/no-conflict-branch &&
+	git symbolic-ref refs/heads/second-alias refs/heads/second &&
 	test_commit extra2 fileX &&
 	git commit --amend --fixup=L &&
 
@@ -1991,8 +1998,16 @@ test_expect_success '--update-refs updates refs correctly' '
 
 	test_cmp_rev HEAD~3 refs/heads/first &&
 	test_cmp_rev HEAD~3 refs/heads/second &&
+	test_cmp_rev HEAD~3 refs/heads/second-alias &&
 	test_cmp_rev HEAD~1 refs/heads/third &&
 	test_cmp_rev HEAD refs/heads/no-conflict-branch &&
+	test_cmp_rev HEAD refs/heads/no-conflict-branch-alias &&
+	test_write_lines refs/heads/no-conflict-branch >expect &&
+	git symbolic-ref refs/heads/no-conflict-branch-alias >actual &&
+	test_cmp expect actual &&
+	test_write_lines refs/heads/second >expect &&
+	git symbolic-ref refs/heads/second-alias >actual &&
+	test_cmp expect actual &&
 
 	q_to_tab >expect <<-\EOF &&
 	Successfully rebased and updated refs/heads/update-refs.

base-commit: c69baaf57ba26cf117c2b6793802877f19738b0d
-- 
gitgitgadget

^ permalink raw reply related

* Re: [PATCH 2/2] builtin/history: implement "drop" subcommand
From: Patrick Steinhardt @ 2026-06-03 10:06 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqbjdt25e3.fsf@gitster.g>

On Tue, Jun 02, 2026 at 08:43:48AM +0900, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
[snip]
> > +static int update_worktree(struct repository *repo,
> > +			   const struct commit *old_head,
> > +			   const struct commit *new_head,
> > +			   bool dry_run)
> > +{
> > +...
> > +
> > +out:
> > +	clear_unpack_trees_porcelain(&opts);
> > +	rollback_lock_file(&lock);
> > +	release_index(&index);
> > +	free(desc_buf[0]);
> > +	free(desc_buf[1]);
> > +	return ret;
> > +}
> 
> The function looks very familiar---anybody who wants to perform
> "checkout <other-commit>" needs to do exactly the above.  It is a
> bit surprising and disappointing that this topic needs to *invent*
> its own helper function and carry it as a file-scope static.

It certainly is. We basically have this whole dance in ~8 different
locations by now, and given the verbosity that is required for the whole
setup it's a good hint that the interface is not exactly great.

One of the functions that we might be able to reuse is `reset_head()`...
goes down the rabbit hole... ugh, this is turning out to be somewhat
painful. I'll send a v2 that does the whole exercise, but I'm not a 100%
convinced it's the right thing to do. There's various assumptions that
we have to break:

  - It assumes that the index is always clean.

  - We don't have a dry-run mode.

  - We need to stop it from updating any refs.

  - We need to introduce another field to let the caller decide which
    commit we're moving from.

So I'm 7 commits deep now adapting the function to our needs. But maybe
the end result is ultimately worth it...? We'll see.

Patrick

^ permalink raw reply

* Re: [PATCH 2/2] builtin/history: implement "drop" subcommand
From: Patrick Steinhardt @ 2026-06-03 10:06 UTC (permalink / raw)
  To: Pablo Sabater; +Cc: git
In-Reply-To: <CAN5EUNQbSN7+SDWcrh3jTD7SXrnD=e-fQ9Qj9778R7cy2q4b1g@mail.gmail.com>

On Tue, Jun 02, 2026 at 09:31:17AM +0200, Pablo Sabater wrote:
> El mar, 2 jun 2026 a las 8:16, Patrick Steinhardt (<ps@pks.im>) escribió:
[snip]
> > +       original = lookup_commit_reference_by_name(argv[0]);
> > +       if (!original) {
> > +               ret = error(_("commit cannot be found: %s"), argv[0]);
> > +               goto out;
> > +       }
> > +
> > +       if (!original->parents) {
> > +               ret = error(_("cannot drop root commit %s: "
> > +                             "it has no parent to replay onto"),
> > +                           argv[0]);
> > +               goto out;
> > +       } else if (original->parents->next) {
> > +               ret = error(_("cannot drop merge commit"));
> 
> Why the if block adds which commit context, but not on the else if block?

True indeed, will adapt.

> > diff --git a/t/t3454-history-drop.sh b/t/t3454-history-drop.sh
> > new file mode 100755
> > index 0000000000..b320ff09b3
> > --- /dev/null
> > +++ b/t/t3454-history-drop.sh
> > @@ -0,0 +1,513 @@
> > +#!/bin/sh
> > +
> > +test_description='tests for git-history drop subcommand'
> > +
> > +. ./test-lib.sh
> > +. "$TEST_DIRECTORY/lib-log-graph.sh"
> > +
> > +expect_graph () {
> > +       cat >expect &&
> > +       lib_test_cmp_graph --graph --format=%s "$@"
> > +}
> 
> This function appears exactly the same at t6016 and t4215 but named as
> check_graph. I was gonna do a cleanup for a commit series I'm working
> on to bring that function to `lib-log-graph.sh` because all these test
> files share that they import graph functions from `lib-log-graph.c`,
> maybe you could do it?

I'd rather keep this series focussed, but I wouldn't mind a follow up
that deduplicates these call sites.

> Also:
> 
> lib_test_cmp_graph () {
>         git log --graph "$@" >output &&
>         sed 's/ *$//' >output.sanitized <output &&
>         test_cmp expect output.sanitized
> }
> 
> Already uses `--graph` you can drop it from expect_graph()

True indeed, dropped the "--graph" argument.

Thanks!

Patrick

^ permalink raw reply

* Re: [PATCH v2 1/3] Documentation/MyFirstContribution: recommend shallow threading
From: Tuomas Ahola @ 2026-06-03 10:01 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git, Junio C Hamano, Weijie Yuan, Ramsay Jones
In-Reply-To: <20260603-pks-b4-v2-1-a8aea0aa2c23@pks.im>

Patrick Steinhardt <ps@pks.im> wrote:

> The "MyFirstContribution" document recommends the use of deep threading:
> every cover letter of subsequent iterations shall be linked to the cover
> letter of the preceding version. The result of this is that eventually,
> threads with many versions are getting nested so deep that it becomes
> hard to follow.
> 
> Adapt the recommendation to instead propose shallow threading: instead
> of linking the cover letter to the previous cover letter, the user is
> supposed to always link it to the first cover letter. This still makes
> it easy to follow the iterations, but has the benefit of nesting to a
> much shallower level.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  Documentation/MyFirstContribution.adoc | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
> index b9fdefce02..069020196c 100644
> --- a/Documentation/MyFirstContribution.adoc
> +++ b/Documentation/MyFirstContribution.adoc
> @@ -1227,8 +1227,8 @@ Message-ID: <foo.12345.author@example.com>
>  
>  Your Message-ID is `<foo.12345.author@example.com>`. This example will be used
>  below as well; make sure to replace it with the correct Message-ID for your
> -**previous cover letter** - that is, if you're sending v2, use the Message-ID
> -from v1; if you're sending v3, use the Message-ID from v2.
> +**first cover letter** - that is, for any subsequent version that you send,
> +always use the Message-ID from v1.
>  
>  While you're looking at the email, you should also note who is CC'd, as it's
>  common practice in the mailing list to keep all CCs on a thread. You can add
> 
> -- 
> 2.54.0.1064.gd145956f57.dirty

If we adapt this change to the guidance, let's fix also other places of the
document that talk about replying to the previous cover letter.

-----8<-----

diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
index 069020196c..bf64a211bd 100644
--- a/Documentation/MyFirstContribution.adoc
+++ b/Documentation/MyFirstContribution.adoc
@@ -790,7 +790,7 @@ We can note a few things:
   v3", etc. in place of "PATCH". For example, "[PATCH v2 1/3]" would be the first of
   three patches in the second iteration. Each iteration is sent with a new cover
   letter (like "[PATCH v2 0/3]" above), itself a reply to the cover letter of the
-  previous iteration (more on that below).
+  first iteration (more on that below).
 
 NOTE: A single-patch topic is sent with "[PATCH]", "[PATCH v2]", etc. without
 _i_/_n_ numbering (in the above thread overview, no single-patch topic appears,
@@ -1214,7 +1214,7 @@ between your last version and now, if it's something significant. You do not
 need the exact same body in your second cover letter; focus on explaining to
 reviewers the changes you've made that may not be as visible.
 
-You will also need to go and find the Message-ID of your previous cover letter.
+You will also need to go and find the Message-ID of your original cover letter.
 You can either note it when you send the first series, from the output of `git
 send-email`, or you can look it up on the
 https://lore.kernel.org/git[mailing list]. Find your cover letter in the

^ permalink raw reply related

* Re: [PATCH 1/2] b4: introduce configuration for the Git project
From: Weijie Yuan @ 2026-06-03  9:51 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: Tuomas Ahola, git, Junio C Hamano
In-Reply-To: <ah_c3kgmfRh3bXns@wyuan.org>

On Wed, Jun 03, 2026 at 03:51:21PM +0800, Weijie Yuan wrote:
> On Wed, Jun 03, 2026 at 08:55:04AM +0200, Patrick Steinhardt wrote:
> > So this quote is definitely at odds with the configuration I have
> > proposed. It's actually quite surprising to me that we recommend deep
> > threading -- I personally find it extremely hard to navigate as the
> > nesting eventually gets way too deep.
>
> Sorry I'm a little confused. The example thread at git-scm.com:
>
> https://git-scm.com/docs/MyFirstContribution#ready-to-share
>
> Isn't this actually supporting shallow nesting?
>
> > It's actually quite surprising to me that we recommend deep
> > threading -- I personally find it extremely hard to navigate as the
> > nesting eventually gets way too deep.
>
> In my understanding, deep threading == --chain-reply-to, so can you
> point out where do Git recommend deep threading? I always thought Git
> supports shallow threading.
>
> Thanks! And please forgive me if I am wrong :-)

Ah, I know you mean the deep nesting of cover letters, sorry, now I
know.

Thanks!

^ permalink raw reply

* Re: [PATCH v2 0/3] contrib/subtree: reduce recursion during split
From: Ian Jackson @ 2026-06-03  9:12 UTC (permalink / raw)
  To: Colin Stagner
  Cc: Junio C Hamano, git, Christian Heusel, george, Christian Hesse,
	Phillip Wood
In-Reply-To: <0915b5cc-5cbb-4cce-a832-147f85d4ff1f@howdoi.land>

Colin Stagner writes ("Re: [PATCH v2 0/3] contrib/subtree: reduce recursion during split"):
> On 6/1/26 17:13, Junio C Hamano wrote:
> > While I do agree that avoiding bash-isms in the main part of Git and
> > sticking to vanilla POSIX has merit, this particular one seems more
> > like an artificial limit imposed by dash than sticking to the POSIX
> > as the common denoninator, at least to me.
> 
> Correct, this topic is a workaround for an artificial limit. The limit 
> is Debian-specific and was introduced as a downstream patch in 2018 [1], 
> [2].

I don't think it is correct to say that this is Debian-specific.  The
limit is baked into dash, which is a non-distro-specific minimal POSIX
shell derived from NetBSD's ash:
  http://gondor.apana.org.au/~herbert/dash/
I don't know what other distros use it (or can use it) as their
/bin/sh.  I also haven't checked POSIX to see if the question of
maximum recursion level is discussed.

Ian.

-- 
Ian Jackson <ijackson@chiark.greenend.org.uk>   These opinions are my own.  

Pronouns: they/he.  If I emailed you from @fyvzl.net or @evade.org.uk,
that is a private address which bypasses my fierce spamfilter.

^ permalink raw reply

* [PATCH v12 6/6] branch: add --dry-run for --prune-merged
From: Harald Nordgren via GitGitGadget @ 2026-06-03  9:04 UTC (permalink / raw)
  To: git
  Cc: Kristoffer Haugsbakk, Johannes Sixt, Phillip Wood,
	Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2285.v12.git.git.1780477479.gitgitgadget@gmail.com>

From: Harald Nordgren <haraldnordgren@gmail.com>

With --dry-run, --prune-merged prints the local branches it would
delete, one "Would delete branch <name>" line per candidate, and
exits without touching any ref.

The @{push}-vs-@{upstream} and unmerged filtering still applies,
so the dry-run output is exactly the set that the live run would
delete.

--dry-run is only meaningful in combination with --prune-merged
and is rejected otherwise.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 Documentation/git-branch.adoc |  8 ++++++-
 builtin/branch.c              | 12 +++++++---
 t/t3200-branch.sh             | 44 +++++++++++++++++++++++++++++++++++
 3 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-branch.adoc b/Documentation/git-branch.adoc
index 69878549fc..c579df4fe0 100644
--- a/Documentation/git-branch.adoc
+++ b/Documentation/git-branch.adoc
@@ -25,7 +25,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 (--prune-merged <branch>)...
+git branch [--dry-run] (--prune-merged <branch>)...
 
 DESCRIPTION
 -----------
@@ -230,6 +230,12 @@ Branches refused by the "fully merged" safety check are listed as
 warnings and skipped; pass them to `git branch -D` explicitly if
 you want them gone.
 
+`--dry-run`::
+	With `--prune-merged`, print which branches would be
+	deleted and exit without touching any ref.  Useful for
+	sanity-checking a wide pattern like `'origin/*'` before
+	committing to the deletion.
+
 `-v`::
 `-vv`::
 `--verbose`::
diff --git a/builtin/branch.c b/builtin/branch.c
index e03805a8a7..1811511b9e 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -860,7 +860,7 @@ static void collect_forked_set(const struct string_list *upstreams,
 }
 
 static int prune_merged_branches(const struct string_list *upstreams,
-				 int quiet)
+				 int quiet, int dry_run)
 {
 	struct ref_store *refs = get_main_ref_store(the_repository);
 	struct string_list candidates = STRING_LIST_INIT_DUP;
@@ -917,7 +917,7 @@ static int prune_merged_branches(const struct string_list *upstreams,
 				      quiet,
 				      1, /* warn_only */
 				      1, /* no_head_fallback */
-				      0  /* dry_run */);
+				      dry_run);
 
 	strvec_clear(&deletable);
 	string_list_clear(&candidates, 0);
@@ -967,6 +967,7 @@ int cmd_branch(int argc,
 	    unset_upstream = 0, show_current = 0, edit_description = 0;
 	struct string_list forked_upstreams = STRING_LIST_INIT_DUP;
 	struct string_list prune_merged_upstreams = STRING_LIST_INIT_DUP;
+	int dry_run = 0;
 	const char *new_upstream = NULL;
 	int noncreate_actions = 0;
 	/* possible options */
@@ -1024,6 +1025,8 @@ int cmd_branch(int argc,
 			N_("list local branches whose upstream matches <branch> (repeatable)")),
 		OPT_STRING_LIST(0, "prune-merged", &prune_merged_upstreams, N_("branch"),
 			N_("delete local branches whose upstream matches <branch> and is merged (repeatable)")),
+		OPT_BOOL(0, "dry-run", &dry_run,
+			N_("with --prune-merged, only print which branches would be deleted")),
 		OPT__FORCE(&force, N_("force creation, move/rename, deletion"), PARSE_OPT_NOCOMPLETE),
 		OPT_MERGED(&filter, N_("print only branches that are merged")),
 		OPT_NO_MERGED(&filter, N_("print only branches that are not merged")),
@@ -1083,6 +1086,9 @@ int cmd_branch(int argc,
 	if (noncreate_actions > 1)
 		usage_with_options(builtin_branch_usage, options);
 
+	if (dry_run && !prune_merged_upstreams.nr)
+		die(_("--dry-run requires --prune-merged"));
+
 	if (recurse_submodules_explicit) {
 		if (!submodule_propagate_branches)
 			die(_("branch with --recurse-submodules can only be used if submodule.propagateBranches is enabled"));
@@ -1124,7 +1130,7 @@ int cmd_branch(int argc,
 		if (argc)
 			die(_("--prune-merged does not take positional arguments; "
 			      "repeat --prune-merged for each <branch>"));
-		ret = prune_merged_branches(&prune_merged_upstreams, quiet);
+		ret = prune_merged_branches(&prune_merged_upstreams, quiet, dry_run);
 		goto out;
 	} else if (show_current) {
 		print_current_branch_name();
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 9e33179590..29bfd0e109 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -2027,4 +2027,48 @@ test_expect_success 'branch -d still deletes a pruneMerged=false branch' '
 	test_must_fail git -C pm-optout-d rev-parse --verify refs/heads/one
 '
 
+test_expect_success '--prune-merged --dry-run lists but does not delete' '
+	test_when_finished "rm -rf pm-dry" &&
+	git clone pm-upstream pm-dry &&
+	git -C pm-dry remote add fork ../pm-fork &&
+	test_config -C pm-dry remote.pushDefault fork &&
+	test_config -C pm-dry push.default current &&
+	git -C pm-dry branch one one-commit &&
+	git -C pm-dry branch --set-upstream-to=origin/next one &&
+	git -C pm-dry branch two two-commit &&
+	git -C pm-dry branch --set-upstream-to=origin/next two &&
+
+	git -C pm-dry branch --dry-run --prune-merged "origin/*" >actual &&
+	test_grep "Would delete branch one " actual &&
+	test_grep "Would delete branch two " actual &&
+
+	git -C pm-dry rev-parse --verify refs/heads/one &&
+	git -C pm-dry rev-parse --verify refs/heads/two
+'
+
+test_expect_success '--prune-merged --dry-run only lists branches the live run would delete' '
+	test_when_finished "rm -rf pm-dry-mixed" &&
+	git clone pm-upstream pm-dry-mixed &&
+	git -C pm-dry-mixed remote add fork ../pm-fork &&
+	test_config -C pm-dry-mixed remote.pushDefault fork &&
+	test_config -C pm-dry-mixed push.default current &&
+	git -C pm-dry-mixed checkout -b wip origin/next &&
+	git -C pm-dry-mixed branch --set-upstream-to=origin/next wip &&
+	test_commit -C pm-dry-mixed local-only &&
+	git -C pm-dry-mixed checkout - &&
+	git -C pm-dry-mixed branch merged one-commit &&
+	git -C pm-dry-mixed branch --set-upstream-to=origin/next merged &&
+
+	git -C pm-dry-mixed branch --dry-run --prune-merged "origin/*" >out &&
+	test_grep "Would delete branch merged" out &&
+	test_grep ! "Would delete branch wip" out &&
+	git -C pm-dry-mixed rev-parse --verify refs/heads/wip &&
+	git -C pm-dry-mixed rev-parse --verify refs/heads/merged
+'
+
+test_expect_success '--dry-run without --prune-merged is rejected' '
+	test_must_fail git -C forked branch --dry-run 2>err &&
+	test_grep "requires --prune-merged" err
+'
+
 test_done
-- 
gitgitgadget

^ permalink raw reply related

* [PATCH v12 5/6] branch: add branch.<name>.pruneMerged opt-out
From: Harald Nordgren via GitGitGadget @ 2026-06-03  9:04 UTC (permalink / raw)
  To: git
  Cc: Kristoffer Haugsbakk, Johannes Sixt, Phillip Wood,
	Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2285.v12.git.git.1780477479.gitgitgadget@gmail.com>

From: Harald Nordgren <haraldnordgren@gmail.com>

Setting branch.<name>.pruneMerged=false exempts that branch from
"git branch --prune-merged". Useful for a topic branch you want
to develop further after an initial round has been merged
upstream.

Unless --quiet is given, the skip is reported per branch so the
user knows why their topic was preserved.

Explicit deletion via "git branch -d" continues to consult the
normal merge check and is not affected by this setting.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 Documentation/config/branch.adoc |  7 +++++++
 Documentation/git-branch.adoc    |  5 +++--
 builtin/branch.c                 | 14 ++++++++++++++
 t/t3200-branch.sh                | 30 ++++++++++++++++++++++++++++++
 4 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/Documentation/config/branch.adoc b/Documentation/config/branch.adoc
index a4db9fa5c8..6c1b5bb9cd 100644
--- a/Documentation/config/branch.adoc
+++ b/Documentation/config/branch.adoc
@@ -102,3 +102,10 @@ for details).
 	`git branch --edit-description`. Branch description is
 	automatically added to the `format-patch` cover letter or
 	`request-pull` summary.
+
+`branch.<name>.pruneMerged`::
+	If set to `false`, branch _<name>_ is exempt from
+	`git branch --prune-merged`.  Useful for a topic branch you
+	intend to develop further after an initial round has been
+	merged upstream.  Defaults to true.  Explicit deletion via
+	`git branch -d` is unaffected.
diff --git a/Documentation/git-branch.adoc b/Documentation/git-branch.adoc
index f7942fcd7d..69878549fc 100644
--- a/Documentation/git-branch.adoc
+++ b/Documentation/git-branch.adoc
@@ -221,9 +221,10 @@ the upstream refs refreshed.
 +
 A branch is left alone if any of the following holds:
 its upstream no longer resolves locally; it is checked out in any
-worktree; or its push destination (`<branch>@{push}`) equals its
+worktree; its push destination (`<branch>@{push}`) equals its
 upstream (`<branch>@{upstream}`), so it cannot be distinguished
-from a freshly pulled trunk that just looks "fully merged".
+from a freshly pulled trunk that just looks "fully merged"; or
+`branch.<name>.pruneMerged` is set to `false`.
 +
 Branches refused by the "fully merged" safety check are listed as
 warnings and skipped; pass them to `git branch -D` explicitly if
diff --git a/builtin/branch.c b/builtin/branch.c
index 736480b002..e03805a8a7 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -878,7 +878,9 @@ static int prune_merged_branches(const struct string_list *upstreams,
 		struct branch *branch = branch_get(short_name);
 		const char *upstream, *push;
 		struct strbuf full = STRBUF_INIT;
+		struct strbuf key = STRBUF_INIT;
 		int skip;
+		int opt_out;
 
 		strbuf_addf(&full, "refs/heads/%s", short_name);
 		skip = !!branch_checked_out(full.buf);
@@ -893,6 +895,18 @@ static int prune_merged_branches(const struct string_list *upstreams,
 		if (!push || !strcmp(push, upstream))
 			continue;
 
+		strbuf_addf(&key, "branch.%s.prunemerged", short_name);
+		if (!repo_config_get_bool(the_repository, key.buf, &opt_out) &&
+		    !opt_out) {
+			if (!quiet)
+				fprintf(stderr,
+					_("Skipping '%s' (branch.%s.pruneMerged is false)\n"),
+					short_name, short_name);
+			strbuf_release(&key);
+			continue;
+		}
+		strbuf_release(&key);
+
 		strvec_push(&deletable, short_name);
 	}
 
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index beb86987ad..9e33179590 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -1997,4 +1997,34 @@ test_expect_success '--prune-merged rejects positional arguments' '
 	test_grep "does not take positional arguments" err
 '
 
+test_expect_success '--prune-merged honours branch.<name>.pruneMerged=false' '
+	test_when_finished "rm -rf pm-optout" &&
+	git clone pm-upstream pm-optout &&
+	git -C pm-optout remote add fork ../pm-fork &&
+	test_config -C pm-optout remote.pushDefault fork &&
+	test_config -C pm-optout push.default current &&
+	git -C pm-optout branch one one-commit &&
+	git -C pm-optout branch --set-upstream-to=origin/next one &&
+	git -C pm-optout branch two two-commit &&
+	git -C pm-optout branch --set-upstream-to=origin/next two &&
+	test_config -C pm-optout branch.one.pruneMerged false &&
+
+	git -C pm-optout branch --prune-merged "origin/*" 2>err &&
+
+	git -C pm-optout rev-parse --verify refs/heads/one &&
+	test_must_fail git -C pm-optout rev-parse --verify refs/heads/two &&
+	test_grep "Skipping .one." err
+'
+
+test_expect_success 'branch -d still deletes a pruneMerged=false branch' '
+	test_when_finished "rm -rf pm-optout-d" &&
+	git clone pm-upstream pm-optout-d &&
+	git -C pm-optout-d branch one one-commit &&
+	git -C pm-optout-d branch --set-upstream-to=origin/next one &&
+	test_config -C pm-optout-d branch.one.pruneMerged false &&
+
+	git -C pm-optout-d branch -d one &&
+	test_must_fail git -C pm-optout-d rev-parse --verify refs/heads/one
+'
+
 test_done
-- 
gitgitgadget


^ 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