Git development
 help / color / mirror / Atom feed
* [PATCH v7 0/5] branch: prune-merged
From: Harald Nordgren via GitGitGadget @ 2026-05-12  8:23 UTC (permalink / raw)
  To: git; +Cc: Kristoffer Haugsbakk, Johannes Sixt, Harald Nordgren
In-Reply-To: <pull.2285.v6.git.git.1778492691.gitgitgadget@gmail.com>

 * --prune-merged now checks if a branch is merged into its own upstream
   first. If the upstream is gone, it checks against the remote's default
   branch instead. If neither exists, the branch is refused (use --force to
   delete anyway).

Harald Nordgren (5):
  branch: add --forked <remote>
  branch: let delete_branches warn instead of error on bulk refusal
  branch: add --prune-merged <remote>
  branch: add branch.<name>.pruneMerged opt-out
  branch: add --all-remotes flag

 Documentation/config/branch.adoc |   7 +
 Documentation/git-branch.adoc    |  33 +++
 builtin/branch.c                 | 332 +++++++++++++++++++++++++++++--
 t/t3200-branch.sh                | 280 ++++++++++++++++++++++++++
 4 files changed, 635 insertions(+), 17 deletions(-)


base-commit: 29bd7ed5127255713c1ac2f43b7c6f257d7b4594
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2285%2FHaraldNordgren%2Ffetch-prune-local-branches-v7
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2285/HaraldNordgren/fetch-prune-local-branches-v7
Pull-Request: https://github.com/git/git/pull/2285

Range-diff vs v6:

 1:  fb9817b220 = 1:  22fa8515df branch: add --forked <remote>
 2:  42a2f93d44 = 2:  b443f0f367 branch: let delete_branches warn instead of error on bulk refusal
 3:  604ecb8965 ! 3:  a245009893 branch: add --prune-merged <remote>
     @@ Documentation/git-branch.adoc: Each _<remote>_ may be either the name of a confi
      +	been pruned upstream.
      ++
      +As a safety check, branches with commits not yet integrated into
     -+the remote's default branch are refused. With `--force` (or `-f`),
     -+delete them regardless. The currently checked-out branch in any
     -+worktree is always preserved.
     ++their upstream remote-tracking branch are refused; if the upstream
     ++itself is gone, the remote's default branch is consulted instead.
     ++With `--force` (or `-f`), delete refused branches regardless. The
     ++currently checked-out branch in any worktree is always preserved.
      +
       `-v`::
       `-vv`::
     @@ builtin/branch.c
       #include "column.h"
       #include "utf8.h"
       #include "ref-filter.h"
     +@@ builtin/branch.c: static const char *branch_get_color(enum color_branch ix)
     + }
     + 
     + static int branch_merged(int kind, const char *name,
     +-			 struct commit *rev, struct commit *head_rev)
     ++			 struct commit *rev, struct commit *head_rev,
     ++			 int no_head_fallback)
     + {
     + 	/*
     + 	 * This checks whether the merge bases of branch and HEAD (or
     +@@ builtin/branch.c: static int branch_merged(int kind, const char *name,
     + 					 &oid, NULL)) != NULL)
     + 			reference_rev = lookup_commit_reference(the_repository,
     + 								&oid);
     ++
     ++		if (!reference_rev && no_head_fallback && upstream &&
     ++		    starts_with(upstream, "refs/remotes/")) {
     ++			const char *remote_name = upstream + strlen("refs/remotes/");
     ++			const char *slash = strchr(remote_name, '/');
     ++			if (slash) {
     ++				struct strbuf head_ref = STRBUF_INIT;
     ++				strbuf_add(&head_ref, "refs/remotes/", strlen("refs/remotes/"));
     ++				strbuf_add(&head_ref, remote_name, slash - remote_name);
     ++				strbuf_addstr(&head_ref, "/HEAD");
     ++				if (refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
     ++							    head_ref.buf,
     ++							    RESOLVE_REF_READING,
     ++							    &oid, NULL))
     ++					reference_rev = lookup_commit_reference(the_repository,
     ++										&oid);
     ++				strbuf_release(&head_ref);
     ++			}
     ++		}
     + 	}
     +-	if (!reference_rev)
     ++	if (!reference_rev) {
     ++		if (no_head_fallback) {
     ++			free(reference_name_to_free);
     ++			return 0;
     ++		}
     + 		reference_rev = head_rev;
     ++	}
     + 
     + 	merged = reference_rev ? repo_in_merge_bases(the_repository, rev,
     + 						     reference_rev) : 0;
     +@@ builtin/branch.c: static int branch_merged(int kind, const char *name,
     + 	 * any of the following code, but during the transition period,
     + 	 * a gentle reminder is in order.
     + 	 */
     +-	if (head_rev != reference_rev) {
     ++	if (!no_head_fallback && head_rev != reference_rev) {
     + 		int expect = head_rev ? repo_in_merge_bases(the_repository, rev, head_rev) : 0;
     + 		if (expect < 0)
     + 			exit(128);
      @@ builtin/branch.c: static int branch_merged(int kind, const char *name,
       
       static int check_branch_commit(const char *branchname, const char *refname,
       			       const struct object_id *oid, struct commit *head_rev,
     -+			       struct commit *head_rev_override,
     -+			       int use_head_rev_override,
     ++			       int no_head_fallback,
       			       int kinds, int force, int warn_only,
       			       int *n_not_merged)
       {
     - 	struct commit *rev = lookup_commit_reference(the_repository, oid);
     -+	int merged;
     -+
     - 	if (!force && !rev) {
     +@@ builtin/branch.c: static int check_branch_commit(const char *branchname, const char *refname,
       		error(_("couldn't look up commit object for '%s'"), refname);
       		return -1;
       	}
      -	if (!force && !branch_merged(kinds, branchname, rev, head_rev)) {
     -+	if (use_head_rev_override) {
     -+		if (!head_rev_override)
     -+			return 0;
     -+		merged = repo_in_merge_bases(the_repository, rev,
     -+					     head_rev_override);
     -+		if (merged < 0)
     -+			exit(128);
     -+	} else {
     -+		merged = branch_merged(kinds, branchname, rev, head_rev);
     -+	}
     -+	if (!force && !merged) {
     ++	if (!force && !branch_merged(kinds, branchname, rev, head_rev,
     ++				     no_head_fallback)) {
       		if (warn_only) {
       			warning(_("the branch '%s' is not fully merged"),
       				branchname);
     @@ builtin/branch.c: static void delete_branch_config(const char *branchname)
       
      -static int delete_branches(int argc, const char **argv, int force, int kinds,
      +static int delete_branches(int argc, const char **argv,
     -+			   struct commit **head_rev_overrides,
     ++			   int no_head_fallback,
      +			   int force, int kinds,
       			   int quiet, int warn_only, int *n_not_merged)
       {
     @@ builtin/branch.c: static int delete_branches(int argc, const char **argv, int fo
      -		    check_branch_commit(bname.buf, name, &oid, head_rev, kinds,
      -					force, warn_only, n_not_merged)) {
      +		    check_branch_commit(bname.buf, name, &oid, head_rev,
     -+					head_rev_overrides ? head_rev_overrides[i] : NULL,
     -+					!!head_rev_overrides,
     ++					no_head_fallback,
      +					kinds, force, warn_only, n_not_merged)) {
       			if (!warn_only)
       				ret = 1;
     @@ builtin/branch.c: static int collect_forked_branch(const struct reference *ref,
       	return 0;
       }
       
     -+static struct commit *resolve_remote_head(const char *remote_name)
     -+{
     -+	struct ref_store *refs = get_main_ref_store(the_repository);
     -+	struct strbuf head_ref = STRBUF_INIT;
     -+	struct object_id oid;
     -+	struct commit *commit = NULL;
     -+
     -+	strbuf_addf(&head_ref, "refs/remotes/%s/HEAD", remote_name);
     -+	if (refs_resolve_ref_unsafe(refs, head_ref.buf, RESOLVE_REF_READING,
     -+				    &oid, NULL))
     -+		commit = lookup_commit_reference(the_repository, &oid);
     -+	strbuf_release(&head_ref);
     -+	return commit;
     -+}
     -+
      +static int prune_merged_branches(int argc, const char **argv, int force,
      +				 int quiet)
      +{
      +	struct string_list candidates = STRING_LIST_INIT_DUP;
      +	struct string_list protected_default_refs = STRING_LIST_INIT_DUP;
      +	struct strvec deletable = STRVEC_INIT;
     -+	struct commit **head_rev_overrides = NULL;
     -+	size_t alloc = 0;
      +	struct string_list_item *item;
      +	int n_not_merged = 0;
      +	int ret = 0;
     @@ builtin/branch.c: static int collect_forked_branch(const struct reference *ref,
      +		struct branch *branch;
      +		const char *push_ref;
      +		const char *upstream;
     -+		const char *remote_name;
     -+		const char *slash;
      +
      +		strbuf_addf(&full, "refs/heads/%s", short_name);
      +		if (branch_checked_out(full.buf)) {
     @@ builtin/branch.c: static int collect_forked_branch(const struct reference *ref,
      +		if (string_list_has_string(&protected_default_refs, push_ref))
      +			continue;
      +
     -+		ALLOC_GROW(head_rev_overrides, deletable.nr + 1, alloc);
     -+		remote_name = push_ref + strlen("refs/remotes/");
     -+		slash = strchr(remote_name, '/');
     -+		if (slash) {
     -+			char *name = xstrndup(remote_name, slash - remote_name);
     -+			head_rev_overrides[deletable.nr] = resolve_remote_head(name);
     -+			free(name);
     -+		} else {
     -+			head_rev_overrides[deletable.nr] = NULL;
     -+		}
      +		strvec_push(&deletable, short_name);
      +	}
      +
      +	if (deletable.nr)
      +		ret = delete_branches(deletable.nr, deletable.v,
     -+				      head_rev_overrides, force,
     ++				      1, force,
      +				      FILTER_REFS_BRANCHES, quiet,
      +				      1, &n_not_merged);
      +
     @@ builtin/branch.c: static int collect_forked_branch(const struct reference *ref,
      +			n_not_merged);
      +
      +	strvec_clear(&deletable);
     -+	free(head_rev_overrides);
      +	string_list_clear(&candidates, 0);
      +	string_list_clear(&protected_default_refs, 0);
      +	return ret;
     @@ builtin/branch.c: int cmd_branch(int argc,
       		if (!argc)
       			die(_("branch name required"));
      -		ret = delete_branches(argc, argv, delete > 1, filter.kind,
     -+		ret = delete_branches(argc, argv, NULL, delete > 1, filter.kind,
     ++		ret = delete_branches(argc, argv, 0, delete > 1, filter.kind,
       				      quiet, 0, NULL);
       		goto out;
       	} else if (forked) {
     @@ t/t3200-branch.sh: test_expect_success '--forked requires at least one <remote>'
      +	test_must_fail git -C pm-force rev-parse --verify refs/heads/one
      +'
      +
     -+test_expect_success '--prune-merged measures merged-ness against <remote>/HEAD, not local HEAD' '
     -+	test_when_finished "rm -rf pm-head-indep" &&
     -+	git clone pm-upstream pm-head-indep &&
     -+	git -C pm-head-indep branch one --track origin/one &&
     -+	git -C pm-head-indep update-ref -d refs/remotes/origin/one &&
     ++test_expect_success '--prune-merged falls back to remote default branch when upstream is gone' '
     ++	test_when_finished "rm -rf pm-fallback" &&
     ++	git clone pm-upstream pm-fallback &&
     ++	git -C pm-fallback branch one --track origin/one &&
     ++	git -C pm-fallback update-ref -d refs/remotes/origin/one &&
      +	# Detach HEAD to an unrelated commit so the candidate is not
     -+	# reachable from local HEAD; it is still reachable from
     -+	# refs/remotes/origin/HEAD, which is what should matter.
     -+	git -C pm-head-indep commit --allow-empty -m unrelated &&
     -+	git -C pm-head-indep checkout --detach &&
     -+	git -C pm-head-indep reset --hard HEAD^ &&
     ++	# reachable from local HEAD. The upstream origin/one is now
     ++	# gone; the merged-ness check should fall back to
     ++	# refs/remotes/origin/HEAD, against which "one" is reachable.
     ++	git -C pm-fallback commit --allow-empty -m unrelated &&
     ++	git -C pm-fallback checkout --detach &&
     ++	git -C pm-fallback reset --hard HEAD^ &&
      +
     -+	git -C pm-head-indep branch --prune-merged origin &&
     ++	git -C pm-fallback branch --prune-merged origin &&
      +
     -+	test_must_fail git -C pm-head-indep rev-parse --verify refs/heads/one
     ++	test_must_fail git -C pm-fallback rev-parse --verify refs/heads/one
      +'
      +
     -+test_expect_success '--prune-merged skips merged-ness check when <remote>/HEAD is unset' '
     -+	test_when_finished "rm -rf pm-no-head" &&
     -+	git clone pm-upstream pm-no-head &&
     -+	git -C pm-no-head checkout -b one --track origin/one &&
     -+	test_commit -C pm-no-head unpushed &&
     -+	git -C pm-no-head checkout - &&
     ++test_expect_success '--prune-merged refuses when upstream and remote default are both gone' '
     ++	test_when_finished "rm -rf pm-both-gone" &&
     ++	git clone pm-upstream pm-both-gone &&
     ++	git -C pm-both-gone checkout -b one --track origin/one &&
     ++	test_commit -C pm-both-gone unpushed &&
     ++	git -C pm-both-gone checkout - &&
      +
     -+	git -C pm-no-head update-ref -d refs/remotes/origin/HEAD &&
     -+	git -C pm-no-head update-ref -d refs/remotes/origin/one &&
     -+	git -C pm-no-head branch --prune-merged origin &&
     ++	git -C pm-both-gone update-ref -d refs/remotes/origin/HEAD &&
     ++	git -C pm-both-gone update-ref -d refs/remotes/origin/one &&
     ++	git -C pm-both-gone branch --prune-merged origin 2>err &&
     ++	test_grep "not fully merged" err &&
      +
     -+	test_must_fail git -C pm-no-head rev-parse --verify refs/heads/one
     ++	git -C pm-both-gone rev-parse --verify refs/heads/one
      +'
      +
      +test_expect_success '--prune-merged never deletes the checked-out branch' '
 4:  717fc6758e ! 4:  2c3f751569 branch: add branch.<name>.pruneMerged opt-out
     @@ Documentation/git-branch.adoc: Each _<remote>_ may be either the name of a confi
      +	that name has since been pruned upstream.
       +
       As a safety check, branches with commits not yet integrated into
     - the remote's default branch are refused. With `--force` (or `-f`),
     - delete them regardless. The currently checked-out branch in any
     --worktree is always preserved.
     -+worktree is always preserved, as is any branch with
     -+`branch.<name>.pruneMerged` set to `false`.
     + their upstream remote-tracking branch are refused; if the upstream
     + itself is gone, the remote's default branch is consulted instead.
     + With `--force` (or `-f`), delete refused branches regardless. The
     +-currently checked-out branch in any worktree is always preserved.
     ++currently checked-out branch in any worktree is always preserved,
     ++as is any branch with `branch.<name>.pruneMerged` set to `false`.
       
       `-v`::
       `-vv`::
     @@ builtin/branch.c: static int prune_merged_branches(int argc, const char **argv,
       		struct branch *branch;
       		const char *push_ref;
       		const char *upstream;
     - 		const char *remote_name;
     - 		const char *slash;
      +		int opt_out = 0;
       
       		strbuf_addf(&full, "refs/heads/%s", short_name);
     @@ builtin/branch.c: static int prune_merged_branches(int argc, const char **argv,
      +		}
      +		strbuf_release(&key);
       
     - 		ALLOC_GROW(head_rev_overrides, deletable.nr + 1, alloc);
     - 		remote_name = push_ref + strlen("refs/remotes/");
     + 		strvec_push(&deletable, short_name);
     + 	}
      
       ## t/t3200-branch.sh ##
      @@ t/t3200-branch.sh: test_expect_success '--prune-merged spares branches whose push ref is the defaul
 5:  be25572957 ! 5:  f79707ce7c branch: add --all-remotes flag
     @@ Documentation/git-branch.adoc: git branch (-m|-M) [<old-branch>] <new-branch>
       
       DESCRIPTION
       -----------
     -@@ Documentation/git-branch.adoc: delete them regardless. The currently checked-out branch in any
     - worktree is always preserved, as is any branch with
     - `branch.<name>.pruneMerged` set to `false`.
     +@@ Documentation/git-branch.adoc: With `--force` (or `-f`), delete refused branches regardless. The
     + currently checked-out branch in any worktree is always preserved,
     + as is any branch with `branch.<name>.pruneMerged` set to `false`.
       
      +`--all-remotes`::
      +	With `--forked` or `--prune-merged`, act on every
     @@ builtin/branch.c: static void collect_forked_set(int argc, const char **argv,
       	for_each_string_list_item(item, &out)
       		puts(item->string);
       
     -@@ builtin/branch.c: static struct commit *resolve_remote_head(const char *remote_name)
     - 	return commit;
     +@@ builtin/branch.c: static int list_forked_branches(int argc, const char **argv)
     + 	return 0;
       }
       
      -static int prune_merged_branches(int argc, const char **argv, int force,

-- 
gitgitgadget

^ permalink raw reply

* git clone fails when using --dissociate together with a reference repository that contains a commit-graph
From: Daniel Mach @ 2026-05-12  7:35 UTC (permalink / raw)
  To: git


[-- Attachment #1.1: Type: text/plain, Size: 1403 bytes --]

Hi,

I've stumbled upon a bug that the following command failed:

$ git clone <url> <dir> --reference <old-dir> --dissociate
fatal: unable to parse commit <SHA>
warning: Clone succeeded, but checkout failed.
You can inspect what was checked out with 'git status'
and retry with 'git restore --source=HEAD :/'

Omitting --dissociate fixed the error, but it wasn't clear to me what 
might be the root cause.

$ git --version
git version 2.54.0

With the help of AI I was able to create a reproducer (see the attached 
script).
I have verified that the reproducer works and also simplified it.

AI generated report (take it with a grain of salt):
* The Bug: During dissociation, Git correctly repacks objects and 
removes the objects/info/alternates file. However, the git clone process 
has already initialized its object store including the alternate's 
commit-graph.
   After dissociation, it fails to "forget" or reload the object store, 
leading to a crash when it tries to use the commit-graph (which refers 
to the now-unlinked alternate) to perform the initial checkout.
* Proof of state: As shown in the script output, a manual git checkout 
immediately after the failure succeeds, proving the repository is 
structurally sound but the clone process itself was in an inconsistent 
state.
* Workaround: Passing -c core.commitGraph=false to the clone command 
prevents the crash.

regards,
Daniel


[-- Attachment #1.2: Type: text/html, Size: 3034 bytes --]

[-- Attachment #2: reproducer_git_clone_dissociate.sh --]
[-- Type: application/x-shellscript, Size: 1335 bytes --]

^ permalink raw reply

* [PATCH] fetch: add fetch.pruneLocalBranches config
From: Harald Nordgren @ 2026-05-12  7:35 UTC (permalink / raw)
  To: gitster; +Cc: git, gitgitgadget, haraldnordgren, j6t, kristofferhaugsbakk
In-Reply-To: <xmqqse7xr0t7.fsf@gitster.g>

> I may be misreading the above and misunderstood you, but if you mean
> that the feature now checks with remote/origin/master when I have a
> local branch that were forked from remote/origin/todo and set to
> merge new changes from there, I do not think it is a good change.

I think you are right. My latest code assumes that everyone works toward
the default branch, which is what I do 99% of the time, but yeah, it should
be more agnostic from different workflow.

I'll take another look.


Harald

^ permalink raw reply

* [PATCH] log: let --follow follow renames in merge commits
From: Miklos Vajna @ 2026-05-12  7:21 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Patrick Steinhardt, brian m. carlson

Have a repo with a subtree merge, do a 'git log --follow prefix/test.c',
the output only contains history in the outer repo, not commits that
were merged via a subtree merge.

This is inconsistent, since doing a 'git blame prefix/test.c' does find
the original commits. This works because find_rename() in blame.c is
invoked for each parent, and there diff_tree_oid() is used, which uses
try_to_follow_renames(). This means that in case a rename happens as
part of a merge commit, git blame can follow that rename.

Fix the problem in a similar way for the 'git log --follow' case: in
case log_tree_diff() finds a merge commit and it would return early,
then do some extra work in the follow_renames case first. Check each
parent, use diff_tree_oid() and if found_follow is set, then work with
that parent instead of returning.

This means that users examining the history of a repo with subtree
merges can see all commits to a file with a single 'git log --follow'
invocation, instead of one invocation for the outer repo and one for the
history before the subtree merge.

Signed-off-by: Miklos Vajna <vmiklos@collabora.com>
---

Hi Junio,

I sent this out a week ago at
<https://lore.kernel.org/git/afmfSa-p-9vuDL3E@collabora.com/T/#u>, I
didn't get any reply to it -- so I'm somewhat optimistic that the patch
itself is a good idea, seeing no negative comments.

So this is a resend, this time to you, CC'ing the list, rather than the
other way around.

Could you please review this?

Thanks,

Miklos

 log-tree.c                          | 20 ++++++++++++++++-
 t/meson.build                       |  1 +
 t/t4218-log-follow-subtree-merge.sh | 34 +++++++++++++++++++++++++++++
 3 files changed, 54 insertions(+), 1 deletion(-)
 create mode 100755 t/t4218-log-follow-subtree-merge.sh

diff --git a/log-tree.c b/log-tree.c
index 7e048701d0..bce09c7dac 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -1142,8 +1142,26 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
 				/* Show parent info for multiple diffs */
 				log->parent = parents->item;
 			}
-		} else
+		} else {
+			if (opt->diffopt.flags.follow_renames) {
+				/*
+				 * Detect a rename across one of the parents.
+				 * Check each parent till we find a follow.
+				 */
+				struct commit_list *p;
+				for (p = parents; p; p = p->next) {
+					parse_commit_or_die(p->item);
+					diff_tree_oid(get_commit_tree_oid(p->item),
+						      oid, "", &opt->diffopt);
+					diff_queue_clear(&diff_queued_diff);
+					if (opt->diffopt.found_follow) {
+						opt->diffopt.found_follow = 0;
+						break;
+					}
+				}
+			}
 			return 0;
+		}
 	}
 
 	showed_log = 0;
diff --git a/t/meson.build b/t/meson.build
index 7528e5cda5..b4ae8d76d8 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -574,6 +574,7 @@ integration_tests = [
   't4215-log-skewed-merges.sh',
   't4216-log-bloom.sh',
   't4217-log-limit.sh',
+  't4218-log-follow-subtree-merge.sh',
   't4252-am-options.sh',
   't4253-am-keep-cr-dos.sh',
   't4254-am-corrupt.sh',
diff --git a/t/t4218-log-follow-subtree-merge.sh b/t/t4218-log-follow-subtree-merge.sh
new file mode 100755
index 0000000000..7ca607cbb8
--- /dev/null
+++ b/t/t4218-log-follow-subtree-merge.sh
@@ -0,0 +1,34 @@
+#!/bin/sh
+
+test_description='Test --follow follows renames across subtree merges'
+
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+
+. ./test-lib.sh
+
+test_expect_success 'setup subtree-merged repository' '
+	git init inner &&
+	echo inner >inner/inner.txt &&
+	git -C inner add inner.txt &&
+	git -C inner commit -m "inner init" &&
+
+	git init outer &&
+	echo outer >outer/outer.txt &&
+	git -C outer add outer.txt &&
+	git -C outer commit -m "outer init" &&
+
+	git -C outer fetch ../inner master &&
+	git -C outer merge -s ours --no-commit --allow-unrelated-histories \
+		FETCH_HEAD &&
+	git -C outer read-tree --prefix=inner/ -u FETCH_HEAD &&
+	git -C outer commit -m "Merge inner repo into inner/ subdirectory"
+'
+
+test_expect_success '--follow finds the pre-merge commit through a subtree merge' '
+	git -C outer log --follow --pretty=tformat:%s inner/inner.txt >actual &&
+	echo "inner init" >expect &&
+	test_cmp expect actual
+'
+
+test_done
-- 
2.51.0


^ permalink raw reply related

* Re: [PATCH v3 2/2] config: add "worktree" and "worktree/i" includeIf conditions
From: Patrick Steinhardt @ 2026-05-12  7:14 UTC (permalink / raw)
  To: me; +Cc: git, Kristoffer Haugsbakk, Junio C Hamano
In-Reply-To: <20260403-includeif-worktree-v3-2-109ce5782b03@black-desk.cn>

On Fri, Apr 03, 2026 at 03:02:29PM +0800, Chen Linxuan via B4 Relay wrote:
> From: Chen Linxuan <me@black-desk.cn>
> 
> The includeIf mechanism already supports matching on the .git
> directory path (gitdir) and the currently checked out branch
> (onbranch).  But in multi-worktree setups the .git directory of a
> linked worktree points into the main repository's .git/worktrees/
> area, which makes gitdir patterns cumbersome when one wants to
> include config based on the working tree's checkout path instead.
> 
> Introduce two new condition keywords:
> 
>   - worktree:<pattern> matches the realpath of the current worktree's
>     working directory (i.e. repo_get_work_tree()) against a glob
>     pattern.  This is the path returned by git rev-parse
>     --show-toplevel.
> 
>   - worktree/i:<pattern> is the case-insensitive variant.

Seems sensible.

> The implementation reuses the include_by_path() helper introduced in
> the previous commit, passing the worktree path in place of the
> gitdir.  The condition never matches in bare repositories (where
> there is no worktree) or during early config reading (where no
> repository is available).

Right. This is because `repo_get_work_tree()` would return a NULL
pointer in these cases, and `include_by_path()` exits early in that
case.

> diff --git a/Documentation/config.adoc b/Documentation/config.adoc
> index 62eebe7c5450..a4f3ec905098 100644
> --- a/Documentation/config.adoc
> +++ b/Documentation/config.adoc
> @@ -146,6 +146,48 @@ refer to linkgit:gitignore[5] for details. For convenience:
>  	This is the same as `gitdir` except that matching is done
>  	case-insensitively (e.g. on case-insensitive file systems)
>  
> +`worktree`::
> +	The data that follows the keyword `worktree` and a colon is used as a
> +	glob pattern. If the working directory of the current worktree matches
> +	the pattern, the include condition is met.
> ++
> +The worktree location is the path where files are checked out (as returned
> +by `git rev-parse --show-toplevel`). This is different from `gitdir`, which
> +matches the `.git` directory path. In a linked worktree, the worktree path
> +is the directory where that worktree's files are located, not the main
> +repository's `.git` directory.
> ++
> +The pattern can contain standard globbing wildcards and two additional
> +ones, `**/` and `/**`, that can match multiple path components. Please
> +refer to linkgit:gitignore[5] for details. For convenience:
> +
> + * If the pattern starts with `~/`, `~` will be substituted with the
> +   content of the environment variable `HOME`.
> +
> + * If the pattern starts with `./`, it is replaced with the directory
> +   containing the current config file.
> +
> + * If the pattern does not start with either `~/`, `./` or `/`, `**/`
> +   will be automatically prepended. For example, the pattern `foo/bar`
> +   becomes `**/foo/bar` and would match `/any/path/to/foo/bar`.
> +
> + * If the pattern ends with `/`, `**` will be automatically added. For
> +   example, the pattern `foo/` becomes `foo/**`. In other words, it
> +   matches "foo" and everything inside, recursively.

This whole listing here is the exact same as we have for the `gitdir`
condition. Can we maybe deduplicate these into a common section?

> diff --git a/config.c b/config.c
> index 7d5dae0e8450..6d0c2d0725e4 100644
> --- a/config.c
> +++ b/config.c
> @@ -400,6 +400,12 @@ static int include_condition_is_true(const struct key_value_info *kvi,
>  		return include_by_path(kvi, opts->git_dir, cond, cond_len, 0);
>  	else if (skip_prefix_mem(cond, cond_len, "gitdir/i:", &cond, &cond_len))
>  		return include_by_path(kvi, opts->git_dir, cond, cond_len, 1);
> +	else if (skip_prefix_mem(cond, cond_len, "worktree:", &cond, &cond_len))
> +		return include_by_path(kvi, inc->repo ? repo_get_work_tree(inc->repo) : NULL,
> +				       cond, cond_len, 0);
> +	else if (skip_prefix_mem(cond, cond_len, "worktree/i:", &cond, &cond_len))
> +		return include_by_path(kvi, inc->repo ? repo_get_work_tree(inc->repo) : NULL,
> +				       cond, cond_len, 1);
>  	else if (skip_prefix_mem(cond, cond_len, "onbranch:", &cond, &cond_len))
>  		return include_by_branch(inc, cond, cond_len);
>  	else if (skip_prefix_mem(cond, cond_len, "hasconfig:remote.*.url:", &cond,

I feel like this is something that we might eventually want to convert
to be table-driven. But I think that doesn't have to happen as part of
this patch series.

> diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh
> index 6e51f892f320..8a5ba4b884d3 100755
> --- a/t/t1305-config-include.sh
> +++ b/t/t1305-config-include.sh

Just because it was explicitly mentioned: we might also want to have a
test that verifies this works with early-config parsing. We already have
a similar test for "gitdir:" in "conditional include, early config
reading".

And should we also have a "nongit" branch where we verify outside a
repository?

Other than that this series looks good to me, thanks!

Patrick

^ permalink raw reply

* Re: [PATCH v3 1/2] config: refactor include_by_gitdir() into include_by_path()
From: Patrick Steinhardt @ 2026-05-12  7:13 UTC (permalink / raw)
  To: me; +Cc: git, Kristoffer Haugsbakk, Junio C Hamano
In-Reply-To: <20260403-includeif-worktree-v3-1-109ce5782b03@black-desk.cn>

On Fri, Apr 03, 2026 at 03:02:28PM +0800, Chen Linxuan via B4 Relay wrote:
> From: Chen Linxuan <me@black-desk.cn>
> 
> The include_by_gitdir() function matches the realpath of a given
> path against a glob pattern, but its interface is tightly coupled to
> the gitdir condition: it takes a struct config_options *opts and
> extracts opts->git_dir internally.
> 
> Refactor it into a more generic include_by_path() helper that takes
> a const char *path parameter directly, and update the gitdir and
> gitdir/i callers to pass opts->git_dir explicitly.  No behavior
> change, just preparing for the addition of a new worktree condition
> that will reuse the same path-matching logic with a different path.

Good preparatory step.

Patrick

^ permalink raw reply

* Re: [GSoC PATCH v6 0/6] preserve promisor files content after repack
From: Junio C Hamano @ 2026-05-12  6:49 UTC (permalink / raw)
  To: git, Christian Couder
  Cc: Taylor Blau, LorenzoPegorari, Derrick Stolee, Patrick Steinhardt,
	Tian Yuchen, Eric Sunshine, Elijah Newren
In-Reply-To: <cover.1776384902.git.lorenzo.pegorari2002@gmail.com>

LorenzoPegorari <lorenzo.pegorari2002@gmail.com> writes:

> The goal of this patch is to solve the NEEDSWORK comment added by
> 5374a290 (fetch-pack: write fetched refs to .promisor, 14/10/2019). This
> is done by adding a helper function that takes the content of all
> .promisor files in the `repository`, and copies it inside the first
> .promisor file created by the repack.
>
> Also, I added a comment explaining what is the purpose of the content of
> the .promisor files, since this wasn't explained anywhere (I found
> information regarding this only in the message of the previously cited
> commit).
>
> Finally, I added some tests to "t7700-repack.sh" and
> "t7703-repack-geometric.sh" that check if the content of .promisor files
> are correctly copied into the .promisor files created by a repack.
>
> V6 DIFF:
>  * changed the name of the helper function to
>    `write_promisor_file_after_repack`.
>  * modified the helper function to create the ".promisor" file, so that
>    is not required anymore.
>  * modified the logic of the helper function (as suggested by Tian
>    Yuchen)
>  * modified the helper function to check for possible errors, and to
>    check if the lines of the ".promisor" files are correctly formed.
>  * fixed memory leak.
>  * improved comments.
>
> LorenzoPegorari (6):
>   pack-write: add explanation to promisor file content
>   repack-promisor add helper to fill promisor file after repack
>   repack-promisor: preserve content of promisor files after repack
>   t7700: test for promisor file content after repack
>   t7703: test for promisor file content after geometric repack
>   repack-promisor: add missing headers
>
>  Documentation/git-repack.adoc |   4 +-
>  pack-write.c                  |   9 ++
>  repack-promisor.c             | 194 ++++++++++++++++++++++++++++++----
>  t/t7700-repack.sh             |  61 +++++++++++
>  t/t7703-repack-geometric.sh   |  33 ++++++
>  5 files changed, 280 insertions(+), 21 deletions(-)


Lorenzo, it seems that not many people are reviewing this final
round, and then I noticed that the list of CC addresses lacks a big
name in the promisor remote topic, so I added Christian to the To:
line of this message.  Christian, you have no obligation to review
these patches if they do not interest you, but just in case you
weren't aware of this effort, I thought it might interest you; I am
sure we all would benefit from your expertise.

Thanks.

^ permalink raw reply

* Re: [PATCH v3 0/3] builtin/history: introduce "fixup" subcommand
From: Patrick Steinhardt @ 2026-05-12  6:41 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Elijah Newren, D. Ben Knoble, Tian Yuchen, git
In-Reply-To: <xmqq33zxp4aq.fsf@gitster.g>

On Tue, May 12, 2026 at 02:47:41PM +0900, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > this short patch series introduces a new "fixup" subcommand. This
> > command is the first one that I felt is missing in my day to day work,
> > as I end up doing fixup commits quite often.
> >
> > The flow is rather simple: the user stages some changes, and then they
> > execute `git history fixup <commit>` to amend those changes to the given
> > commit. As with the other subcommands, dependent branches will then be
> > rebased automatically.
> >
> > This is the first command that may result in merge conflicts. For now we
> > simply abort in such cases, but there are plans to introduce first-class
> > conflicts into Git. So once we have them, we'll also be able to handle
> > such cases more gracefully. I still think that the command is useful
> > even without that conflict handling.
> >
> > Changes in v3:
> >   - Some more polishing of the command's description.
> >   - Link to v2: https://patch.msgid.link/20260423-b4-pks-history-fixup-v2-0-d7571c6d36eb@pks.im
> >
> > Changes in v2:
> >   - Introduce "--empty=(keep|drop|abort)" to specify what happens with
> >     empty commits.
> >   - Adapt documentation a bit to hopefully clarify how changes are
> >     backported.
> >   - Link to v1: https://patch.msgid.link/20260422-b4-pks-history-fixup-v1-0-48d4484243de@pks.im
> 
> The iterations v2 and v3 saw no comments, unfortunately.  I just
> gave three patches in v3 a cursory look and nothing stood out as
> curious or fishy.  Shall we mark the topic for 'next' now?

I didn't plan to post another iteration, so this works for me. Thanks!

Patrick

^ permalink raw reply

* Re: [PATCH v3 0/2] includeIf: add "worktree" condition for matching working tree path
From: Junio C Hamano @ 2026-05-12  6:41 UTC (permalink / raw)
  To: git, Kristoffer Haugsbakk, Chen Linxuan; +Cc: Chen Linxuan via B4 Relay
In-Reply-To: <20260403-includeif-worktree-v3-0-109ce5782b03@black-desk.cn>

Chen Linxuan via B4 Relay <devnull+me.black-desk.cn@kernel.org>
writes:

> The `includeIf` mechanism already supports matching on the `.git`
> directory path (`gitdir`) and the currently checked out branch
> (`onbranch`).  But in multi-worktree setups the `.git` directory of a
> linked worktree points into the main repository's `.git/worktrees/`
> area, which makes `gitdir` patterns cumbersome when one wants to
> include config based on the working tree's checkout path instead.
>
> Introduce two new condition keywords:
>
>   - `worktree:<pattern>` matches the realpath of the current worktree's
>     working directory against a glob pattern.
>   - `worktree/i:<pattern>` is the case-insensitive variant.
>
> Supported pattern features: glob wildcards, `**/` and `/**`, `~`
> expansion, `./` relative paths, and trailing-`/` prefix matching.
> The condition never matches in a bare repository.
>
> Signed-off-by: Chen Linxuan <me@black-desk.cn>
> ---
> Changes in v3:
> - Apply Junio's suggestion.
> - Link to v2: https://lore.kernel.org/r/20260402-includeif-worktree-v2-0-36e339b898d7@black-desk.cn
>
> Changes in v2:
>
> - Add missing signed-off-by lines.
> - Link to v1: https://lore.kernel.org/r/20260401-includeif-worktree-v1-0-906db69f2c79@black-desk.cn

Can we have a volunteer (or two) to review these patches?  The
feature sounds like a worthwhile thing to have, and the code on the
surface looks OK-ish to me, but I am not fully back up to speed and
can use an extra set of eyeballs.

Thanks.

>
> ---
> Chen Linxuan (2):
>       config: refactor include_by_gitdir() into include_by_path()
>       config: add "worktree" and "worktree/i" includeIf conditions
>
>  Documentation/config.adoc | 50 +++++++++++++++++++++++++++++++++++
>  config.c                  | 25 ++++++++++--------
>  t/t1305-config-include.sh | 66 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 130 insertions(+), 11 deletions(-)
> ---
> base-commit: 270e10ad6dda3379ea0da7efd11e4fbf2cd7a325
> change-id: 20260401-includeif-worktree-fcb64950dfba
>
> Best regards,

^ permalink raw reply

* Re: [PATCH] sequencer: remove todo_add_branch_context.commit
From: Patrick Steinhardt @ 2026-05-12  6:36 UTC (permalink / raw)
  To: Abhinav Gupta via GitGitGadget; +Cc: git, Abhinav Gupta
In-Reply-To: <pull.2111.git.1778502113485.gitgitgadget@gmail.com>

On Mon, May 11, 2026 at 12:21:53PM +0000, Abhinav Gupta via GitGitGadget wrote:
> From: Abhinav Gupta <mail@abhinavg.net>
> 
> The 'commit' field in 'struct todo_add_branch_context' is unused.
> It's written to, but never read from.
> add_decorations_to_list() gets the commit passed to it explicitly
> as an argument.

To add some historic flavor: the struct has been introduced via
900b50c242 (rebase: add --update-refs option, 2022-07-19), and the
`commit` field was already unused back then.

> diff --git a/sequencer.c b/sequencer.c
> index b7d8dca47f..19839da1e6 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -6409,7 +6409,6 @@ struct todo_add_branch_context {
>  	size_t items_nr;
>  	size_t items_alloc;
>  	struct strbuf *buf;
> -	struct commit *commit;
>  	struct string_list refs_to_oids;
>  };
>  
> @@ -6498,7 +6497,6 @@ static int todo_list_add_update_ref_commands(struct todo_list *todo_list)
>  		ctx.items[ctx.items_nr++] = todo_list->items[i++];
>  
>  		if (item->commit) {
> -			ctx.commit = item->commit;
>  			add_decorations_to_list(item->commit, &ctx);
>  		}

Nit: while at it we could've also dropped the curly braces according to
our coding guidelines. But that alone isn't worth a reroll.

Other than that this is a welcome cleanup, thanks!

Patrick

^ permalink raw reply

* Re: [PATCH v15 00/13] fsmonitor: implement filesystem change listener for Linux
From: Junio C Hamano @ 2026-05-12  6:26 UTC (permalink / raw)
  To: Ben Knoble
  Cc: Paul Tarjan via GitGitGadget, git, Patrick Steinhardt,
	Paul Tarjan, Gábor SZEDER, Jeff King, Paul Tarjan
In-Reply-To: <487628C4-596C-4870-A652-E1670C700AD7@gmail.com>

Ben Knoble <ben.knoble@gmail.com> writes:

>> Le 15 avr. 2026 à 09:27, Paul Tarjan via GitGitGadget <gitgitgadget@gmail.com> a écrit :
>> 
>> This series implements the built-in fsmonitor daemon for Linux using the
>> inotify API, bringing it to feature parity with the existing Windows and
>> macOS implementations. It also fixes two memory leaks in the
>> platform-independent daemon code and deduplicates the IPC and settings logic
>> that is now shared between macOS and Linux.
>
> Troubleshooting a Gentoo build failure of next has me pretty
> convinced this topic is in there already. Junio should probably
> check my math, but I think that means we want to see fixes on top
> of that base now (unless we are reverting this topic from next and
> queuing a new version?).
>
> (The failure is a Gentoo-ism; we carry a patch that stops applying
> with this series. Not anything this project needs to worry about.)

So is there a verdict already, which this project may not need to
worry about?  This has been kept out of 'next' after getting
reverted but if the breakage was due to Gentoo-ism whose workaround
does not need to get upstreamed, and if there are no other reasons to
block the topic, I am inclined to mark the topic for 'next'.

Thanks.

^ permalink raw reply

* Re: [PATCH v6] t2000: consolidate second scenario into a single test block
From: Junio C Hamano @ 2026-05-12  6:15 UTC (permalink / raw)
  To: Zakariyah Ali; +Cc: git, karthik.188
In-Reply-To: <20260429103607.406339-1-zakariyahali100@gmail.com>

Zakariyah Ali <zakariyahali100@gmail.com> writes:

> The second test scenario in t2000 consists of several fragmented
> test_expect_success blocks that handle data setup, tree writes,
> execution of git-checkout-index, and final state validation.
>
> Consolidate these nine separate blocks into a single self-contained
> test block. This follows the modern Git testing standard where setup,
> execution, and validation of a single logical scenario are kept
> together.
>
> As a result of this consolidation, the show_files() helper and its
> associated test_debug calls are no longer used and have been removed.
> This also removes a dependency on the non-portable 'find -ls' command.

The patch, at first glance, looked quite messy but it turns out that
it is mostly just a lot of removals of (1) early test closure
followed by the start of the next test or (2) test_debug calls in
between.  The only thing that was slightly outside that pattern was
the computation of tree3, whose result was not even used for
test_debug in the original.

Will mark the topic for 'next'.  Thanks.

^ permalink raw reply

* [PATCH] merge: use repo_in_merge_bases for octopus up-to-date check
From: Kristofer Karlsson via GitGitGadget @ 2026-05-12  6:11 UTC (permalink / raw)
  To: git; +Cc: Kristofer Karlsson, Kristofer Karlsson

From: Kristofer Karlsson <krka@spotify.com>

The octopus merge path checks whether each remote head is already
an ancestor of HEAD by computing all merge-bases via
repo_get_merge_bases() and comparing the first result's OID to
the remote head.  This is more expensive than necessary:
repo_get_merge_bases() calls paint_down_to_common() with
min_generation=0, performs the full STALE drain, and may run
remove_redundant(), when all we need is a yes/no reachability
answer.

Replace this with repo_in_merge_bases(), which answers the
is-ancestor question directly.  When generation numbers are
available, repo_in_merge_bases() uses can_all_from_reach() -- a
DFS bounded by generation number that stops as soon as the target
is found or ruled out, without entering paint_down_to_common() at
all.  Without generation numbers, it still benefits from a tighter
min_generation floor.

Signed-off-by: Kristofer Karlsson <krka@spotify.com>
---
    merge: use repo_in_merge_bases for octopus up-to-date check
    
    While reviewing callers of repo_get_merge_bases() for a different patch,
    I noticed the octopus up-to-date loop in builtin/merge.c computes full
    merge-bases only to check whether each remote head is an ancestor of
    HEAD.
    
    The existing code calls repo_get_merge_bases(), takes the first result,
    frees the list, and compares the OID to the remote head. This is
    equivalent to an is-ancestor check, which repo_in_merge_bases() answers
    directly.
    
    Using repo_in_merge_bases() simplifies the code (-14/+4 lines) and
    avoids unnecessary work: with generation numbers it uses
    can_all_from_reach() instead of paint_down_to_common(), and without
    generation numbers it still benefits from a tighter min_generation
    floor. In practice this only matters for octopus merges on repos with
    deep history, so the main value here is the simplification.
    
    The comment "Here we have to calculate the individual merge_bases again"
    dates from 2008 (1c7b76be, "Build in merge"). At the time,
    in_merge_bases() was the same cost as computing merge bases. Stolee's
    2018 generation number work (d7c1ec3e) and the later switch to
    can_all_from_reach (6cc01743) made it significantly cheaper, but this
    call site was never updated.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2110%2Fspkrka%2Fmerge-octopus-in-merge-bases-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2110/spkrka/merge-octopus-in-merge-bases-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2110

 builtin/merge.c             | 18 ++++--------------
 t/t6408-merge-up-to-date.sh | 10 ++++++++++
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index 2cbce56f8d..862107cf41 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1735,21 +1735,11 @@ int cmd_merge(int argc,
 		struct commit_list *j;
 
 		for (j = remoteheads; j; j = j->next) {
-			struct commit_list *common_one = NULL;
-			struct commit *common_item;
-
-			/*
-			 * Here we *have* to calculate the individual
-			 * merge_bases again, otherwise "git merge HEAD^
-			 * HEAD^^" would be missed.
-			 */
-			if (repo_get_merge_bases(the_repository, head_commit,
-						 j->item, &common_one) < 0)
+			int ret = repo_in_merge_bases(the_repository,
+						      j->item, head_commit);
+			if (ret < 0)
 				exit(128);
-
-			common_item = common_one->item;
-			commit_list_free(common_one);
-			if (!oideq(&common_item->object.oid, &j->item->object.oid)) {
+			if (!ret) {
 				up_to_date = 0;
 				break;
 			}
diff --git a/t/t6408-merge-up-to-date.sh b/t/t6408-merge-up-to-date.sh
index 7763c1ba98..be0840efb6 100755
--- a/t/t6408-merge-up-to-date.sh
+++ b/t/t6408-merge-up-to-date.sh
@@ -89,4 +89,14 @@ test_expect_success 'merge fast-forward octopus' '
 	test "$expect" = "$current"
 '
 
+test_expect_success 'merge octopus already up to date' '
+
+	git reset --hard c2 &&
+	test_tick &&
+	git merge c0 c1 &&
+	expect=$(git rev-parse c2) &&
+	current=$(git rev-parse HEAD) &&
+	test "$expect" = "$current"
+'
+
 test_done

base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
-- 
gitgitgadget

^ permalink raw reply related

* Re: [PATCH 0/2] doc: log: fix --decorate description list
From: Junio C Hamano @ 2026-05-12  6:03 UTC (permalink / raw)
  To: kristofferhaugsbakk; +Cc: git, Kristoffer Haugsbakk, Jean-Noël Avila
In-Reply-To: <CV_doc_log_--decorate_list.626@msgid.xyz>

kristofferhaugsbakk@fastmail.com writes:

> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
>
> Topic name: kh/doc-log-decorate-list
>
> Topic summary: Fix formatting of the '--decorate' description list.
>
> [1/2] doc: log: fix --decorate description list
> [2/2] doc: log: use the same delimiter in description list
>
>  Documentation/git-log.adoc | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
>
> base-commit: 67ad42147a7acc2af6074753ebd03d904476118f

This does give us a solid improvement.  Let me mark the topic for
'next'.

Thanks.

^ permalink raw reply

* Re: [PATCH 1/2] builtin/maintenance: fix locking with "--detach"
From: Patrick Steinhardt @ 2026-05-12  5:59 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jean-Christophe Manciot, Mikael Magnusson, Jeff King,
	Taylor Blau, Derrick Stolee
In-Reply-To: <xmqq4ikdqvad.fsf@gitster.g>

On Tue, May 12, 2026 at 10:19:22AM +0900, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > diff --git a/builtin/gc.c b/builtin/gc.c
> > index 3a71e314c9..09cb92ac97 100644
> > --- a/builtin/gc.c
> > +++ b/builtin/gc.c
> > @@ -1810,10 +1810,32 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts,
> >  				   TASK_PHASE_FOREGROUND))
> >  			result = 1;
> >  
> > -	/* Failure to daemonize is ok, we'll continue in foreground. */
> >  	if (opts->detach > 0) {
> > +		pid_t child_pid;
> > +
> >  		trace2_region_enter("maintenance", "detach", the_repository);
> > -		daemonize();
> > +
> > +		child_pid = daemonize_without_exit();
> > +		if (!child_pid) {
> > +			/*
> > +			 * We're in the child process, so we take ownership of
> > +			 * the lockfile.
> > +			 */
> > +			lock_file_reassign_owner(&lk, getpid());
> > +		} else if (child_pid > 0) {
> > +			/*
> > +			 * We're in the parent process, so we assign ownership
> > +			 * of the lockfile to the child and then exit immediately.
> > +			 */
> > +			lock_file_reassign_owner(&lk, child_pid);
> > +			exit(0);
> 
> The point of reassigning the owner to somebody else is so that we
> won't clean them when we exit as the tempfile.c::remove_tempfile()
> function checks the "owner" is "me" and refrains from unlinking
> those that do not belong to us, so there is nothing wrong in this
> code, but this somehow felt awkward.  In a sense, child_pid here
> does not have to be what fork() returned but anything that is not
> our own pid.  Perhaps "we assign ... to the child" -> "we relinquish
> ... to prevent us removing upon exiting" would convey the intention
> better?  I dunno.

Fair. This is what I got now:

	/*
	 * We're in the parent process, so we drop ownership of
	 * the lockfile to prevent us from removing it upon
	 * exit.
	 */

> > -int daemonize(void)
> > +pid_t daemonize_without_exit(void)
> >  {
> >  #ifdef NO_POSIX_GOODIES
> >  	errno = ENOSYS;
> >  	return -1;
> >  #else
> > -	switch (fork()) {
> > -		case 0:
> > -			break;
> > -		case -1:
> > -			die_errno(_("fork failed"));
> > -		default:
> > -			exit(0);
> > -	}
> > +	pid_t pid = fork();
> > +	if (pid < 0)
> > +		return -1;
> > +	if (pid > 0)
> > +		return pid;
> > +
> >  	if (setsid() == -1)
> >  		die_errno(_("setsid failed"));
> >  	close(0);
> > @@ -2180,6 +2178,21 @@ int daemonize(void)
> >  #endif
> >  }
> >  
> > +int daemonize(void)
> > +{
> > +#ifdef NO_POSIX_GOODIES
> > +	errno = ENOSYS;
> > +	return -1;
> > +#else
> > +	pid_t pid = daemonize_without_exit();
> > +	if (pid < 0)
> > +		die_errno(_("fork failed"));
> > +	if (pid > 0)
> > +		exit(0);
> > +	return 0;
> > +#endif
> > +}
> 
> I was hoping that we can do without the #ifdef in this caller as
> daemonize_without_exit() already has exactly the same condtional
> compilation.  If the NO_POSIX_GOODIES side can just return silently
> wit ENOSYS, shouldn't the callers be also fine if we return failure
> instead of calling die_errno(_("fork failed")), I have to wonder.
> 
> But because (1) as long as we have to call die_errno() here, we must
> keep the conditional compilation in daemonize() as well as
> daemonize_without_exit(), and (2) changing what the callers get when
> fork failed here is totally outside of this topic, I would say that
> the code around here is good as-is.

Yeah, I was also pondering whether I can drop the additional ifdef. But
I eventually decided to aim for the most minimal fix that has the least
potential for additional regressions. So I aimed to keep `daemonize()`
semantically the same as before, and I aimed to only fix the issue with
the lockfile we know about.

I agree though that this is something we should probably clean up in a
subsequent series. We don't have that many callers of `daemonize()`
after all, so it shouldn't be that involved, either.

Thanks!

Patrick

^ permalink raw reply

* Re: [PATCH 2/2] run-command: honor "gc.auto" for auto-maintenance
From: Patrick Steinhardt @ 2026-05-12  5:59 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jeff King, git, Jean-Christophe Manciot, Mikael Magnusson,
	Taylor Blau, Derrick Stolee
In-Reply-To: <xmqqzf25pgm0.fsf@gitster.g>

On Tue, May 12, 2026 at 10:21:43AM +0900, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
> 
> > On Mon, May 11, 2026 at 02:29:56PM +0200, Patrick Steinhardt wrote:
> >
> >> @@ -1946,8 +1946,10 @@ int prepare_auto_maintenance(struct repository *r, int quiet,
> >>  {
> >>  	int enabled, auto_detach;
> >>  
> >> -	if (!repo_config_get_bool(r, "maintenance.auto", &enabled) &&
> >> -	    !enabled)
> >> +	if (repo_config_get_bool(r, "maintenance.auto", &enabled) &&
> >> +	    repo_config_get_bool(r, "gc.auto", &enabled))
> >> +		enabled = 1;
> >> +	if (!enabled)
> >>  		return 0;
> >
> > gc.auto isn't a bool; it's the count of loose objects after which to run
> > maintenance. So "0" works in both contexts, but will we complain if
> > gc.auto is set to 100? I think maybe not, because we fall back to
> > git_parse_int(), but it feels kind of fragile.
> >
> > The gc code uses repo_config_get_int() here.
> >
> > -Peff
> 
> Very good point.  I was about to send the same message ;-)

Ugh, true indeed. Will fix, thanks!

Patrick

^ permalink raw reply

* Re: [PATCH v2 2/2] commit: sign commit after mutating buffer
From: Junio C Hamano @ 2026-05-12  5:54 UTC (permalink / raw)
  To: brian m. carlson; +Cc: git, Kushal Das, Elijah Newren
In-Reply-To: <20260427221834.1824543-2-sandals@crustytoothpaste.net>

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> The ensure_utf8 function can mutate the buffer to change its encoding,
> so we must call it before signing the buffer so that we do not
> invalidate the signature, which is made over raw bytes.  Fix a bug which
> caused the compatibility code to not convert the compatibility buffer if
> the main buffer was invalid UTF-8.  We expect both buffers to be valid
> UTF-8 or both invalid, since the only data that would differ between
> them would be hex object IDs, which are always valid UTF-8.
>
> Add a test for this case using 0xfe and 0xff, which are never valid in
> UTF-8.
>
> Reported-by: Kushal Das <kushal@sunet.se>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
>  commit.c                 | 15 +++++++++++----
>  t/t7510-signed-commit.sh | 10 ++++++++++
>  2 files changed, 21 insertions(+), 4 deletions(-)

This iteration hasn't seen any reaction but comparing it with the
previous round and peeking at comments that the previous round
received, I guess everybody commented on the previous round is happy
with this version.

Let me mark the topic for 'next'.

Thanks.


>
> diff --git a/commit.c b/commit.c
> index 790dd2faed..e5d725fe93 100644
> --- a/commit.c
> +++ b/commit.c
> @@ -1726,6 +1726,7 @@ int commit_tree_extended(const char *msg, size_t msg_len,
>  	struct repository *r = the_repository;
>  	int result = 0;
>  	int encoding_is_utf8;
> +	bool warned = false;
>  	struct strbuf buffer = STRBUF_INIT, compat_buffer = STRBUF_INIT;
>  	struct strbuf sig = STRBUF_INIT, compat_sig = STRBUF_INIT;
>  	struct object_id *parent_buf = NULL, *compat_oid = NULL;
> @@ -1747,6 +1748,13 @@ int commit_tree_extended(const char *msg, size_t msg_len,
>  		oidcpy(&parent_buf[i++], &p->item->object.oid);
>  
>  	write_commit_tree(&buffer, msg, msg_len, tree, parent_buf, nparents, author, committer, extra);
> +
> +	/* And check the encoding. */
> +	if (encoding_is_utf8 && !ensure_utf8(&buffer)) {
> +		fprintf(stderr, _(commit_utf8_warn));
> +		warned = true;
> +	}
> +
>  	if (sign_commit && sign_buffer(&buffer, &sig, sign_commit,
>  				       SIGN_BUFFER_USE_DEFAULT_KEY)) {
>  		result = -1;
> @@ -1780,6 +1788,9 @@ int commit_tree_extended(const char *msg, size_t msg_len,
>  		free_commit_extra_headers(compat_extra);
>  		free(mapped_parents);
>  
> +		if (encoding_is_utf8 && !ensure_utf8(&compat_buffer) && !warned)
> +			fprintf(stderr, _(commit_utf8_warn));
> +
>  		if (sign_commit && sign_buffer(&compat_buffer, &compat_sig,
>  					       sign_commit,
>  					       SIGN_BUFFER_USE_DEFAULT_KEY)) {
> @@ -1818,10 +1829,6 @@ int commit_tree_extended(const char *msg, size_t msg_len,
>  		}
>  	}
>  
> -	/* And check the encoding. */
> -	if (encoding_is_utf8 && (!ensure_utf8(&buffer) || !ensure_utf8(&compat_buffer)))
> -		fprintf(stderr, _(commit_utf8_warn));
> -
>  	if (r->compat_hash_algo) {
>  		hash_object_file(r->compat_hash_algo, compat_buffer.buf, compat_buffer.len,
>  			OBJ_COMMIT, &compat_oid_buf);
> diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
> index 1201c85ba6..aa9108da54 100755
> --- a/t/t7510-signed-commit.sh
> +++ b/t/t7510-signed-commit.sh
> @@ -462,4 +462,14 @@ test_expect_success 'custom `gpg.program`' '
>  	git commit -S --allow-empty -m signed-commit
>  '
>  
> +test_expect_success GPG 'commit verifies with non-UTF-8 commit message' '
> +	printf "I hate\\376\\377UTF-8\\n" >message &&
> +	echo unusual-message >file &&
> +	git add file &&
> +	test_tick && git commit -S -F message 2>err &&
> +	git verify-commit HEAD &&
> +	grep "commit message did not conform to UTF-8" err >lines &&
> +	test_line_count = 1 lines
> +'
> +
>  test_done

^ permalink raw reply

* Re: [PATCH v4] index-pack, unpack-objects: increase input buffer from 4 KiB to 128 KiB
From: Junio C Hamano @ 2026-05-12  5:51 UTC (permalink / raw)
  To: Scott Bauersfeld via GitGitGadget
  Cc: git, Derrick Stolee, Jeff King, Scott Bauersfeld
In-Reply-To: <pull.2282.v4.git.git.1777387660841.gitgitgadget@gmail.com>

"Scott Bauersfeld via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Scott Bauersfeld <sbauersfeld@g.ucla.edu>
>
> index-pack and unpack-objects both read pack data from stdin through
> a 4 KiB static buffer. In index-pack, each fill() flushes consumed
> bytes to the pack file via write_or_die(), capping every write(2)
> at 4 KiB. unpack-objects uses the same buffer pattern for reads.
>
> On FUSE-backed filesystems every write(2) is a synchronous round
> trip through the FUSE protocol (userspace -> kernel -> userspace ->
> back), so the 4 KiB buffer turns a clone into many unnecessary tiny
> writes with noticeable latency overhead.
>
> Increase the buffer from 4 KiB to 128 KiB. Introduce a shared
> DEFAULT_IO_BUFFER_SIZE constant in git-compat-util.h (next to
> MAX_IO_SIZE) and use it in index-pack, unpack-objects, and the
> hashfile layer in csum-file (which already used 128 KiB but
> hardcoded the value).
>
> Pack file writes to a FUSE filesystem with writeback caching
> disabled during HTTPS clones of git/git (~293 MB pack):
>
>   74,958 -> 4,687 (94% fewer)
>
> Wall-clock time of git clone over HTTPS onto a FUSE passthrough
> filesystem with writeback caching disabled, 3 runs per variant:
>
>   vscode (~1.26 GB pack): 84.5s -> 75.7s avg (10% faster)
>   git/git (~306 MB pack):  22.6s -> 20.0s avg (11% faster)
>
> Signed-off-by: Scott Bauersfeld <sbauersfeld@g.ucla.edu>
> ---
>...
>     
>     Changes since v3
>     ================
>     
>      * Replaced strace-based syscall measurements with FUSE daemon write
>        logging. The earlier strace numbers (72,465 → 24,943, 65% reduction)
>        were distorted: strace -f ptrace intercepts every syscall in all
>        traced processes and added enough overhead to distort the
>        measurements. The FUSE daemon logging captures write sizes without
>        perturbing the traced processes, showing the true reduction is 94%
>        (74,958 → 4,687).
>      * Note: Why 4,687 writes instead of ~2k writes as would be expected
>        with a 128 KiB buffer size? It appears that fill() is calling xread()
>        on a pipe and the linux default buffer size for pipes is 64KiB. I
>        also tested using fcntl(F_SETPIPE_SZ) to increase the pipe's buffer
>        size to 128KiB, which does indeed reduce total pack file writes to
>        ~2.4K.

It seems that everybody was happy with v3 already, so let's merge it
down to 'next'.

Thanks.

^ permalink raw reply

* Re: [PATCH v3 0/3] builtin/history: introduce "fixup" subcommand
From: Junio C Hamano @ 2026-05-12  5:47 UTC (permalink / raw)
  To: Elijah Newren, D. Ben Knoble, Tian Yuchen; +Cc: Patrick Steinhardt, git
In-Reply-To: <20260427-b4-pks-history-fixup-v3-0-cb908f06264b@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> this short patch series introduces a new "fixup" subcommand. This
> command is the first one that I felt is missing in my day to day work,
> as I end up doing fixup commits quite often.
>
> The flow is rather simple: the user stages some changes, and then they
> execute `git history fixup <commit>` to amend those changes to the given
> commit. As with the other subcommands, dependent branches will then be
> rebased automatically.
>
> This is the first command that may result in merge conflicts. For now we
> simply abort in such cases, but there are plans to introduce first-class
> conflicts into Git. So once we have them, we'll also be able to handle
> such cases more gracefully. I still think that the command is useful
> even without that conflict handling.
>
> Changes in v3:
>   - Some more polishing of the command's description.
>   - Link to v2: https://patch.msgid.link/20260423-b4-pks-history-fixup-v2-0-d7571c6d36eb@pks.im
>
> Changes in v2:
>   - Introduce "--empty=(keep|drop|abort)" to specify what happens with
>     empty commits.
>   - Adapt documentation a bit to hopefully clarify how changes are
>     backported.
>   - Link to v1: https://patch.msgid.link/20260422-b4-pks-history-fixup-v1-0-48d4484243de@pks.im

The iterations v2 and v3 saw no comments, unfortunately.  I just
gave three patches in v3 a cursory look and nothing stood out as
curious or fishy.  Shall we mark the topic for 'next' now?


^ permalink raw reply

* Re: [PATCH v4 2/2] commit-reach: early exit paint_down_to_common for single merge-base
From: Kristofer Karlsson @ 2026-05-12  5:16 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Kristofer Karlsson via GitGitGadget, git, Patrick Steinhardt
In-Reply-To: <xmqqbjelqx2t.fsf@gitster.g>

Thank you!

As for the difference in variable name, I will attribute it to a mix
of oversight and personal preference to keep variable names short if
their scope is very small (and longer names for things like fields or
larger scope).
In fact, I might have preferred flags instead of mb_flags within
paint_down_to_common but it conflicted with the existing flags for the
commit so I had to differentiate them.

That said, I think it would also be fair to rename it to mb_flags everywhere.

On Tue, 12 May 2026 at 02:40, Junio C Hamano <gitster@pobox.com> wrote:
>
> "Kristofer Karlsson via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > From: Kristofer Karlsson <krka@spotify.com>
> >
> > Commits not in the commit-graph get GENERATION_NUMBER_INFINITY and
> > sort to the top of the priority queue.  After those, commits with
> > finite generation numbers are popped in non-increasing order.
> > When MERGE_BASE_FIND_ALL is not set the first doubly-painted commit
> > with a finite generation is therefore a best merge-base: no commit
> > still in the queue can be a descendant of it.  Skip the expensive
> > STALE drain in this case.
> >
> > Add MERGE_BASE_FIND_ALL to the merge_base_flags enum.  Callers that
> > need every merge-base (repo_get_merge_bases_many, repo_get_merge_bases,
> > repo_in_merge_bases_many, remove_redundant_no_gen) pass the flag to
> > preserve existing behavior.  git merge-base (without --all) passes 0,
> > triggering the early exit.
> >
> > On a 2.2M-commit merge-heavy monorepo with commit-graph:
> >
> >   HEAD vs ~500:   5,229ms -> 24ms
> >   HEAD vs ~1000:  4,214ms -> 39ms
> >   HEAD vs ~5000:  3,799ms -> 46ms
> >   HEAD vs ~10000: 3,827ms -> 61ms
> >
> > Signed-off-by: Kristofer Karlsson <krka@spotify.com>
> > ---
> >  builtin/merge-base.c  |  3 ++-
> >  commit-reach.c        | 19 +++++++++++++++----
> >  commit-reach.h        |  7 ++++++-
> >  t/t6600-test-reach.sh | 40 ++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 63 insertions(+), 6 deletions(-)
>
> Very nicely done and well described.
>
> > diff --git a/builtin/merge-base.c b/builtin/merge-base.c
> > index 9b50b4660e..a87011c6cd 100644
> > --- a/builtin/merge-base.c
> > +++ b/builtin/merge-base.c
> > @@ -11,11 +11,12 @@
> >
> >  static int show_merge_base(struct commit **rev, size_t rev_nr, int show_all)
> >  {
> > +     enum merge_base_flags flags = show_all ? MERGE_BASE_FIND_ALL : 0;
>
> Curious that only this variable, among 6 that this two-patch series
> introduces for the type, is called "flags" while all others are
> called "mb_flags".  No need to change it; the comment is mostly to
> show I did read the two patches with reasonable attention to the
> detail ;-).
>
> Will queue.  Thanks.

^ permalink raw reply

* Re: [PATCH v3 1/3] replay: allow callers to control what happens with empty commits
From: Junio C Hamano @ 2026-05-12  4:51 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git, Elijah Newren, D. Ben Knoble, Tian Yuchen
In-Reply-To: <20260427-b4-pks-history-fixup-v3-1-cb908f06264b@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> +	/* Handle commits that become empty */
>  	if (oideq(&replayed_base_tree->object.oid, &result->tree->object.oid) &&
> -	    !oideq(&pickme_tree->object.oid, &base_tree->object.oid))
> -		return replayed_base;
> +	    !oideq(&pickme_tree->object.oid, &base_tree->object.oid)) {
> +		switch (empty) {
> +		case REPLAY_EMPTY_COMMIT_DROP:
> +			return replayed_base;
> +		case REPLAY_EMPTY_COMMIT_KEEP:
> +			break;
> +		case REPLAY_EMPTY_COMMIT_ABORT:
> +			result->clean = error(_("commit %s became empty after replay"),
> +					      oid_to_hex(&pickme->object.oid));

OK.  merge-ort.h clearly explains what negative values in .clean
member means, so this is a good way to signal a failure up the
call chain.

> +			return NULL;
> +		}
> +	}

^ permalink raw reply

* Re: [PATCH] alias: restore support for simple dotted aliases
From: Junio C Hamano @ 2026-05-12  4:43 UTC (permalink / raw)
  To: Jonatan Holmgren; +Cc: Jeff King, git, rsch, michael.grossfeld
In-Reply-To: <d1170f92-3690-4fa4-8070-75ac9f119174@jontes.page>

Jonatan Holmgren <jonatan@jontes.page> writes:

> Sorry, that wasn't a "hey we should deprecate this" code-wise, I was 
> asking from a documentation point of view, i.e. was curious how you felt 
> about what is "advisable". Shouldn't've included that in my email

After this, the discussion went dark, but I think everything that
needs saying has been said and we are in agreement that the current
patch is a good way forward without closing doors for the future too
tightly ;-)  Let me mark the topic for 'next'.

Thanks, all.

^ permalink raw reply

* Re: [PATCH 0/3] line-log: integrate -L with the standard log output pipeline
From: Junio C Hamano @ 2026-05-12  4:01 UTC (permalink / raw)
  To: git, Michael Montalbo via GitGitGadget; +Cc: Michael Montalbo
In-Reply-To: <pull.2094.git.1777349126.gitgitgadget@gmail.com>

"Michael Montalbo via GitGitGadget" <gitgitgadget@gmail.com> writes:

> Since its introduction, git log -L has short-circuited from
> log_tree_commit() into its own output function, bypassing log_tree_diff()
> and log_tree_diff_flush(). This skips no_free save/restore,
> always_show_header, diff_free() cleanup, and means that pickaxe (-S, -G,
> --find-object) and --diff-filter cannot suppress commits whose pairs are all
> filtered out, because show_log() runs before diffcore_std().
>
> This series restructures the flow so that -L goes through the same
> log_tree_diff() -> log_tree_diff_flush() path as normal single-parent and
> merge diffs, then uses that to enable several non-patch diff formats.

This unfortunately saw no reviews and

  https://lore.kernel.org/git/pull.2094.git.1777349126.gitgitgadget@gmail.com/

does not show the previous rouns so I am assuming nobody is
interested in the topic?  

Or are people more busily writing their own patches than reviewing
others' patches?  Unfortunately that is not sustainable.




> Patch 1: revision: move -L setup before output_format-to-diff derivation
>
> Preparatory reorder in setup_revisions(). The -L block sets a default
> DIFF_FORMAT_PATCH when no format is requested; move it before the derivation
> of revs->diff from output_format so the default is visible to that check. No
> behavior change on its own.
>
> Patch 2: line-log: integrate -L output with the standard log-tree pipeline
>
> Rename line_log_print() to line_log_queue_pairs(), stripping it down to only
> queue pre-computed filepairs. log_tree_diff_flush() handles show_log(),
> diffcore_std(), and diff_flush(). This fixes pickaxe and --diff-filter
> suppression, and aligns the commit/diff separator with the rest of log
> output. Also rejects --full-diff, which is meaningless when filepairs are
> pre-computed.
>
> Patch 3: line-log: allow non-patch diff formats with -L
>
> Expand the allowlist to accept --raw, --name-only, --name-status, and
> --summary. These only read filepair metadata already set by the line-log
> machinery. Diff stat formats (--stat, --numstat, --shortstat, --dirstat)
> remain blocked because they call compute_diffstat() on full blob content and
> would show whole-file statistics rather than range-scoped ones.
>
> Michael Montalbo (3):
>   revision: move -L setup before output_format-to-diff derivation
>   line-log: integrate -L output with the standard log-tree pipeline
>   line-log: allow non-patch diff formats with -L
>
>  Documentation/line-range-options.adoc         | 10 +-
>  line-log.c                                    | 30 ++----
>  line-log.h                                    |  2 +-
>  log-tree.c                                    |  9 +-
>  revision.c                                    | 25 +++--
>  t/t4211-line-log.sh                           | 99 ++++++++++++++++---
>  t/t4211/sha1/expect.parallel-change-f-to-main |  1 -
>  .../sha256/expect.parallel-change-f-to-main   |  1 -
>  8 files changed, 120 insertions(+), 57 deletions(-)
>
>
> base-commit: 9f223ef1c026d91c7ac68cc0211bde255dda6199
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2094%2Fmmontalbo%2Fmm%2Fline-log-use-log-tree-diff-flush-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2094/mmontalbo/mm/line-log-use-log-tree-diff-flush-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/2094

^ permalink raw reply

* Re: [PATCH v3 0/8] builtin: implement, document and test url-parse
From: Junio C Hamano @ 2026-05-12  3:50 UTC (permalink / raw)
  To: Matheus Afonso Martins Moreira
  Cc: Torsten Bögershausen, Matheus Moreira via GitGitGadget, git,
	Ghanshyam Thakkar
In-Reply-To: <6c0a1601cd379bcdc87b4fe3b854166a@matheusmoreira.com>

Matheus Afonso Martins Moreira <matheus@matheusmoreira.com> writes:

>> Reviewers comment: Nicely done.
>
> Thank you!
>
>> More a question to myself, may be, about t9904 (and may be other parts)
>> I have in mind that the parser learned to handle
>>
>> file://server/share/repo
>> correctly under Windows.
>> I don't know if this needs to be addressed here or in a follow-up commit ?
>
> I'd be happy to revisit this in a follow-up. It's been a while
> since I used MSYS but I do remember the fact it rewrites paths
> internally. I wasn't sure how to handle it properly in the tests.

So the only potential thing that is missing from the series is the
above, which we are fine to postpone in a follow-up series?  I think
that is a good stopping point.  Given that this command is new, it
is fine that it has known and documented short-comings that will be
improved (of course on the other hand, we are not in any urgent need
for this new command, so we do not have to ship it half-baked).

Is everybody happy with the patches in the current shape and should
I mark it for 'next'?

Thanks.

^ permalink raw reply

* Re: [PATCH v3 0/9] pack-bitmap: fix various pseudo-merge bugs
From: Junio C Hamano @ 2026-05-12  1:49 UTC (permalink / raw)
  To: Taylor Blau; +Cc: git, Jeff King, Elijah Newren
In-Reply-To: <xmqqse7xpftn.fsf@gitster.g>

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

> Taylor Blau <me@ttaylorr.com> writes:
>
>> [Note to the maintainer: this series has been rebased onto the current
>> tip of master, which is 7760f83b597 (Merge branch
>> 'jc/neuter-sideband-fixup', 2026-05-11) at the time of writing].
>
> A note like this is very much appreciated, but please also state the
> reason why the rebase was necessary.  "Because the current tip of
> 'master' has advanced" is not a good reason.  "The previous
> synthetic base was made by merging topic X and topic Y on
> then-current 'master', but both have graduated" is a so-so ok
> reason.  "Because the updated implementation of this series uses
> facilities that appeared in recent 'master' that come from topics A
> and B, which the previous iteration did not use" and "Recent updates
> to 'master' brings in conflicting changes from topic C" are
> excellent reasons.

Forgot one important case.  "It turns out that this fix is important
so it was rebased to be applicable to an older maintenance release M"
would be very much appreciated as well.

Perhaps after coming up with a few more good reasons, we should
describe them in Documentation/SubmittingPatches somewhere.


^ 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