* Re: followRemoteHEAD management question
From: Matt Hunter @ 2026-06-12 6:11 UTC (permalink / raw)
To: Bence Ferdinandy, Jeff King; +Cc: git
In-Reply-To: <DJ6IBPYNOTTY.3QKEZQ28P713V@ferdinandy.com>
On Thu Jun 11, 2026 at 4:36 PM EDT, Bence Ferdinandy wrote:
> On Thu Jun 11, 2026 at 08:01, Jeff King <peff@peff.net> wrote:
>>
>> My initial thought is that it might affect clone as well as fetch. But I
>> guess this feature does not kick in for clone, as it has its own logic
>> for handling the remote-tracking HEAD. Though arguably it should be
>> possible to configure it not to create one in the first place.
>
> If memory serves well clone has set the remote/HEAD well before this and
> I think it indeed uses a different mechanism/logic.
I'm a little interested to try to look into the clone case as well, but
I think I'll save it for a later patch series and keep the scope of this
one as it is.
> Bit late to the party, but happy to review/test patches if they come.
Greatly appreciated!
>
> Best,
> Bence
The first version of my patches went out. You two are Cc'd on the cover
letter, but that didn't propagate to the patches themselves, oops.
^ permalink raw reply
* Re: [PATCH v2] update-ref: add --rename option
From: Patrick Steinhardt @ 2026-06-12 6:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqwlw4nccr.fsf@gitster.g>
On Thu, Jun 11, 2026 at 11:47:16AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
>
> > One thing that I'm missing from the commit message: what's the
> > motivation for this new mode?
>
> Maintenance of merge-fix database, a kludgy way to manage evil
> merges that are needed to deal with inter-topic semantic crashes.
>
> If you are really interested, see the appendix.
Thanks for the explanation!
Patrick
^ permalink raw reply
* Re: [PATCH v3] update-ref: add --rename option
From: Patrick Steinhardt @ 2026-06-12 6:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqq7bo4n4ge.fsf@gitster.g>
On Thu, Jun 11, 2026 at 02:37:53PM -0700, Junio C Hamano wrote:
> Add a "--rename" option to "git update-ref" with the syntax:
>
> $ git update-ref --rename <old-refname> <new-refname>
>
> It renames <old-refname> together with its reflog to <new-refname>;
> even when used on a local branch ref, the current value and the
> reflog of the ref are the only things that are renamed. Document it
> and redirect casual users to "git branch -m" if that is what they
> wanted to do.
This reads much better, thanks.
> diff --git a/Documentation/git-update-ref.adoc b/Documentation/git-update-ref.adoc
> index 37a5019a8b..3b4df23a86 100644
> --- a/Documentation/git-update-ref.adoc
> +++ b/Documentation/git-update-ref.adoc
> @@ -9,6 +9,7 @@ SYNOPSIS
> --------
> [synopsis]
> git update-ref [-m <reason>] [--no-deref] -d <ref> [<old-oid>]
> +git update-ref [-m <reason>] --rename <old-refname> <new-refname>
> git update-ref [-m <reason>] [--no-deref] [--create-reflog] <ref> <new-oid> [<old-oid>]
> git update-ref [-m <reason>] [--no-deref] --stdin [-z] [--batch-updates]
This slightly triggers my OCD, but oh, well. No need to change this.
> diff --git a/builtin/update-ref.c b/builtin/update-ref.c
> index 2d68c40ecb..65ee8af08c 100644
> --- a/builtin/update-ref.c
> +++ b/builtin/update-ref.c
> @@ -800,6 +802,32 @@ int cmd_update_ref(int argc,
> if (end_null)
> usage_with_options(git_update_ref_usage, options);
>
> + if (rename) {
> + const char *oldref, *newref;
> +
> + if (delete || argc != 2)
> + usage_with_options(git_update_ref_usage, options);
Arguably, we should also complain when either "--no-deref" or "--deref"
were given, as they don't have any effect.
A slight tangent: this is part of why I really don't like commands that
determine their mode via flags: you now have to worry about every
combination of flags and whether they even make sense. With subcommands
we at least only have to worry about the set of flags that directly
apply to that given subcommand.
Makes me wonder whether I should have a look at extending git-refs(1)
further:
git refs delete <ref> [<oldvalue>]
git refs update <ref> <newvalue> [<oldvalue>]
git refs rename <ref> <oldname> <newname>
I always wanted to do this eventually so that we have one top-level
command that knows how to do "everything refs".
Anyway, except for this nit the patch looks good to me, thanks!
Patrick
^ permalink raw reply
* [PATCH 7/7] fetch: fixup a misaligned comment
From: Matt Hunter @ 2026-06-12 5:55 UTC (permalink / raw)
To: git
In-Reply-To: <20260612055947.1499497-1-m@lfurio.us>
Signed-off-by: Matt Hunter <m@lfurio.us>
---
builtin/fetch.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index a21bb82274d4..911ac8a47221 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1792,7 +1792,7 @@ static int set_head(const struct ref *remote_refs, struct remote *remote,
strbuf_addf(&b_head, "refs/remotes/%s/HEAD", remote->name);
strbuf_addf(&b_remote_head, "refs/remotes/%s/%s", remote->name, head_name);
}
- /* make sure it's valid */
+ /* make sure it's valid */
if (!baremirror && !refs_ref_exists(refs, b_remote_head.buf)) {
result = 1;
goto cleanup;
--
2.54.0
^ permalink raw reply related
* [PATCH 6/7] fetch: add configuration option fetch.followRemoteHEAD
From: Matt Hunter @ 2026-06-12 5:55 UTC (permalink / raw)
To: git
In-Reply-To: <20260612055947.1499497-1-m@lfurio.us>
'fetch.followRemoteHEAD' is added as a generic option used by all
remotes for which 'remote.<name>.followRemoteHEAD' is undefined. If
both options are undefined, a builtin default of "create" is in effect,
matching the previous behavior.
As mentioned in the previous patch, 'fetch.followRemoteHEAD' supports
all of the values that its 'remote' counterpart does _except_
warn-if-not-$branch, due to its tighter coupling to individual remote
repositories.
Documentation and advice messages for both of the followRemoteHEAD
options are reworded to better capture the relationship between the two.
The added tests assert feature parity between the two followRemoteHEAD
options, as well as the fact that 'remote.<name>.followRemoteHEAD'
always supersedes this new configurable default.
Signed-off-by: Matt Hunter <m@lfurio.us>
---
Documentation/config/fetch.adoc | 19 ++++++
Documentation/config/remote.adoc | 21 +++----
builtin/fetch.c | 32 ++++++++--
t/t5510-fetch.sh | 105 +++++++++++++++++++++++++++++++
4 files changed, 160 insertions(+), 17 deletions(-)
diff --git a/Documentation/config/fetch.adoc b/Documentation/config/fetch.adoc
index 04ac90912d3a..f7de22a34a54 100644
--- a/Documentation/config/fetch.adoc
+++ b/Documentation/config/fetch.adoc
@@ -126,3 +126,22 @@ the new bundle URI.
The creation token values are chosen by the provider serving the specific
bundle URI. If you modify the URI at `fetch.bundleURI`, then be sure to
remove the value for the `fetch.bundleCreationToken` value before fetching.
+
+`fetch.followRemoteHEAD`::
+ When fetching using a default refspec, this option determines how to handle
+ differences between a fetched remote's `HEAD` and the local
+ `remotes/<name>/HEAD` symbolic-ref. Its value is one of
++
+--
+`create`;;
+ Create `remotes/<name>/HEAD` if a ref exists on the remote, but not locally.
+ An existing symbolic-ref will not be touched. This is the default value.
+`warn`;;
+ Display a warning if the remote advertises a different `HEAD` than what is
+ set locally. Behaves like "create" if the local symbolic-ref doesn't exist.
+`always`;;
+ Silently update `remotes/<name>/HEAD` whenever the remote advertises a new
+ value.
+`never`;;
+ Never create or modify the `remotes/<name>/HEAD` symbolic-ref.
+--
diff --git a/Documentation/config/remote.adoc b/Documentation/config/remote.adoc
index eb9c8a3c4884..761bf4ba7d14 100644
--- a/Documentation/config/remote.adoc
+++ b/Documentation/config/remote.adoc
@@ -157,15 +157,12 @@ Blank values signal to ignore all previous values, allowing a reset of
the list from broader config scenarios.
remote.<name>.followRemoteHEAD::
- How linkgit:git-fetch[1] should handle updates to `remotes/<name>/HEAD`
- when fetching using the configured refspecs of a remote.
- The default value is "create", which will create `remotes/<name>/HEAD`
- if it exists on the remote, but not locally; this will not touch an
- already existing local reference. Setting it to "warn" will print
- a message if the remote has a different value than the local one;
- in case there is no local reference, it behaves like "create".
- A variant on "warn" is "warn-if-not-$branch", which behaves like
- "warn", but if `HEAD` on the remote is `$branch` it will be silent.
- Setting it to "always" will silently update `remotes/<name>/HEAD` to
- the value on the remote. Finally, setting it to "never" will never
- change or create the local reference.
+ When fetching this remote using its default refspec, this option determines
+ how to handle differences between the remote's `HEAD` and the local
+ `remotes/<name>/HEAD` symbolic-ref. Overrides the setting for
+ `fetch.followRemoteHEAD`. See `fetch.followRemoteHEAD` for a description of
+ accepted values.
++
+In addition to the values supported by `fetch.followRemoteHEAD`, this option may
+also take on the value "warn-if-not-`$branch`", which behaves like "warn", but
+ignores the warning if the remote's `HEAD` is `remotes/<name>/$branch`.
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 3cc7efdd83a0..a21bb82274d4 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -103,6 +103,7 @@ static struct string_list negotiation_include = STRING_LIST_INIT_NODUP;
struct fetch_config {
enum display_format display_format;
+ enum follow_remote_head_settings follow_remote_head;
int all;
int prune;
int prune_tags;
@@ -173,6 +174,22 @@ static int git_fetch_config(const char *k, const char *v,
"fetch.output", v);
}
+ if (!strcmp(k, "fetch.followremotehead")) {
+ if (!v)
+ return config_error_nonbool(k);
+ else if (!strcasecmp(v, "never"))
+ fetch_config->follow_remote_head = FOLLOW_REMOTE_NEVER;
+ else if (!strcasecmp(v, "create"))
+ fetch_config->follow_remote_head = FOLLOW_REMOTE_CREATE;
+ else if (!strcasecmp(v, "warn"))
+ fetch_config->follow_remote_head = FOLLOW_REMOTE_WARN;
+ else if (!strcasecmp(v, "always"))
+ fetch_config->follow_remote_head = FOLLOW_REMOTE_ALWAYS;
+ else
+ die(_("invalid value for '%s': '%s'"),
+ "fetch.followRemoteHEAD", v);
+ }
+
return git_default_config(k, v, ctx, cb);
}
@@ -1697,11 +1714,13 @@ static const char *strip_refshead(const char *name){
static void set_head_advice_msg(const char *remote, const char *head_name)
{
const char message_advice_set_head[] =
- N_("Run 'git remote set-head %s %s' to follow the change, or set\n"
- "'remote.%s.followRemoteHEAD' configuration option to a different value\n"
- "if you do not want to see this message. Specifically running\n"
- "'git config set remote.%s.followRemoteHEAD warn-if-not-%s'\n"
- "will disable the warning until the remote changes HEAD to something else.");
+ N_("Run 'git remote set-head %s %s' to follow the change, or modify\n"
+ "either of the 'remote.%s.followRemoteHEAD' or 'fetch.followRemoteHEAD'\n"
+ "configuration options to handle the situation differently.\n\n"
+
+ "Using this specific option\n\n"
+ " git config set remote.%s.followRemoteHEAD warn-if-not-%s\n\n"
+ "will suppress the warning until the remote changes HEAD to something else.");
advise_if_enabled(ADVICE_FETCH_SET_HEAD_WARN, _(message_advice_set_head),
remote, head_name, remote, remote, head_name);
@@ -1919,6 +1938,8 @@ static int do_fetch(struct transport *transport,
if (transport->remote->follow_remote_head)
follow_remote_head = transport->remote->follow_remote_head;
+ else if (config->follow_remote_head)
+ follow_remote_head = config->follow_remote_head;
else
follow_remote_head = BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;
@@ -2477,6 +2498,7 @@ int cmd_fetch(int argc,
{
struct fetch_config config = {
.display_format = DISPLAY_FORMAT_FULL,
+ .follow_remote_head = FOLLOW_REMOTE_UNCONFIGURED,
.prune = -1,
.prune_tags = -1,
.show_forced_updates = 1,
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 43190630e714..6f0ae1bdd798 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -140,6 +140,16 @@ test_expect_success "fetch test remote HEAD change" '
)
'
+test_expect_success "fetch test default followRemoteHEAD never" '
+ git -C two update-ref --no-deref -d refs/remotes/origin/HEAD &&
+ test_config -C two fetch.followRemoteHEAD "never" &&
+ GIT_TRACE_PACKET=$PWD/trace.out git -C two fetch &&
+ # Confirm that we do not even ask for HEAD when we are
+ # not going to act on it.
+ test_grep ! "ref-prefix HEAD" trace.out &&
+ test_must_fail git -C two rev-parse --verify refs/remotes/origin/HEAD
+'
+
test_expect_success "fetch test followRemoteHEAD never" '
git -C two update-ref --no-deref -d refs/remotes/origin/HEAD &&
test_config -C two remote.origin.followRemoteHEAD "never" &&
@@ -150,6 +160,21 @@ test_expect_success "fetch test followRemoteHEAD never" '
test_must_fail git -C two rev-parse --verify refs/remotes/origin/HEAD
'
+test_expect_success "fetch test default followRemoteHEAD warn no change" '
+ git -C two rev-parse --verify refs/remotes/origin/other &&
+ git -C two remote set-head origin other &&
+ git -C two rev-parse --verify refs/remotes/origin/HEAD &&
+ git -C two rev-parse --verify refs/remotes/origin/main &&
+ test_config -C two fetch.followRemoteHEAD "warn" &&
+ git -C two fetch >output &&
+ echo "${SQ}HEAD${SQ} at ${SQ}origin${SQ} is ${SQ}main${SQ}," \
+ "but we have ${SQ}other${SQ} locally." >expect &&
+ test_cmp expect output &&
+ head=$(git -C two rev-parse refs/remotes/origin/HEAD) &&
+ branch=$(git -C two rev-parse refs/remotes/origin/other) &&
+ test "z$head" = "z$branch"
+'
+
test_expect_success "fetch test followRemoteHEAD warn no change" '
git -C two rev-parse --verify refs/remotes/origin/other &&
git -C two remote set-head origin other &&
@@ -165,6 +190,17 @@ test_expect_success "fetch test followRemoteHEAD warn no change" '
test "z$head" = "z$branch"
'
+test_expect_success "fetch test default followRemoteHEAD warn create" '
+ git -C two update-ref --no-deref -d refs/remotes/origin/HEAD &&
+ test_config -C two fetch.followRemoteHEAD "warn" &&
+ git -C two rev-parse --verify refs/remotes/origin/main &&
+ output=$(git -C two fetch) &&
+ test "z" = "z$output" &&
+ head=$(git -C two rev-parse refs/remotes/origin/HEAD) &&
+ branch=$(git -C two rev-parse refs/remotes/origin/main) &&
+ test "z$head" = "z$branch"
+'
+
test_expect_success "fetch test followRemoteHEAD warn create" '
git -C two update-ref --no-deref -d refs/remotes/origin/HEAD &&
test_config -C two remote.origin.followRemoteHEAD "warn" &&
@@ -176,6 +212,18 @@ test_expect_success "fetch test followRemoteHEAD warn create" '
test "z$head" = "z$branch"
'
+test_expect_success "fetch test default followRemoteHEAD warn detached" '
+ git -C two update-ref --no-deref -d refs/remotes/origin/HEAD &&
+ git -C two update-ref refs/remotes/origin/HEAD HEAD &&
+ HEAD=$(git -C two log --pretty="%H") &&
+ test_config -C two fetch.followRemoteHEAD "warn" &&
+ git -C two fetch >output &&
+ echo "${SQ}HEAD${SQ} at ${SQ}origin${SQ} is ${SQ}main${SQ}," \
+ "but we have a detached HEAD pointing to" \
+ "${SQ}${HEAD}${SQ} locally." >expect &&
+ test_cmp expect output
+'
+
test_expect_success "fetch test followRemoteHEAD warn detached" '
git -C two update-ref --no-deref -d refs/remotes/origin/HEAD &&
git -C two update-ref refs/remotes/origin/HEAD HEAD &&
@@ -188,6 +236,19 @@ test_expect_success "fetch test followRemoteHEAD warn detached" '
test_cmp expect output
'
+test_expect_success "fetch test default followRemoteHEAD warn quiet" '
+ git -C two rev-parse --verify refs/remotes/origin/other &&
+ git -C two remote set-head origin other &&
+ git -C two rev-parse --verify refs/remotes/origin/HEAD &&
+ git -C two rev-parse --verify refs/remotes/origin/main &&
+ test_config -C two fetch.followRemoteHEAD "warn" &&
+ output=$(git -C two fetch --quiet) &&
+ test "z" = "z$output" &&
+ head=$(git -C two rev-parse refs/remotes/origin/HEAD) &&
+ branch=$(git -C two rev-parse refs/remotes/origin/other) &&
+ test "z$head" = "z$branch"
+'
+
test_expect_success "fetch test followRemoteHEAD warn quiet" '
git -C two rev-parse --verify refs/remotes/origin/other &&
git -C two remote set-head origin other &&
@@ -229,6 +290,18 @@ test_expect_success "fetch test followRemoteHEAD warn-if-not-branch branch is di
test "z$head" = "z$branch"
'
+test_expect_success "fetch test default followRemoteHEAD always" '
+ git -C two rev-parse --verify refs/remotes/origin/other &&
+ git -C two remote set-head origin other &&
+ git -C two rev-parse --verify refs/remotes/origin/HEAD &&
+ git -C two rev-parse --verify refs/remotes/origin/main &&
+ test_config -C two fetch.followRemoteHEAD "always" &&
+ git -C two fetch &&
+ head=$(git -C two rev-parse refs/remotes/origin/HEAD) &&
+ branch=$(git -C two rev-parse refs/remotes/origin/main) &&
+ test "z$head" = "z$branch"
+'
+
test_expect_success "fetch test followRemoteHEAD always" '
git -C two rev-parse --verify refs/remotes/origin/other &&
git -C two remote set-head origin other &&
@@ -241,6 +314,28 @@ test_expect_success "fetch test followRemoteHEAD always" '
test "z$head" = "z$branch"
'
+test_expect_success 'per-remote followRemoteHEAD takes priority over fetch default' '
+ git -C two rev-parse --verify refs/remotes/origin/other &&
+ git -C two remote set-head origin other &&
+ git -C two rev-parse --verify refs/remotes/origin/HEAD &&
+ git -C two rev-parse --verify refs/remotes/origin/main &&
+ test_config -C two fetch.followRemoteHEAD "never" &&
+ test_config -C two remote.origin.followRemoteHEAD "always" &&
+ git -C two fetch &&
+ head=$(git -C two rev-parse refs/remotes/origin/HEAD) &&
+ branch=$(git -C two rev-parse refs/remotes/origin/main) &&
+ test "z$head" = "z$branch"
+'
+
+test_expect_success 'default followRemoteHEAD does not kick in with refspecs' '
+ git -C two remote set-head origin other &&
+ test_config -C two fetch.followRemoteHEAD always &&
+ git -C two fetch origin refs/heads/main:refs/remotes/origin/main &&
+ echo refs/remotes/origin/other >expect &&
+ git -C two symbolic-ref refs/remotes/origin/HEAD >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'followRemoteHEAD does not kick in with refspecs' '
git -C two remote set-head origin other &&
test_config -C two remote.origin.followRemoteHEAD always &&
@@ -250,6 +345,16 @@ test_expect_success 'followRemoteHEAD does not kick in with refspecs' '
test_cmp expect actual
'
+test_expect_success 'default followRemoteHEAD create does not overwrite dangling symref' '
+ test_when_finished "git -C two remote remove custom-head" &&
+ git -C two remote add -m does-not-exist custom-head ../one &&
+ test_config -C two fetch.followRemoteHEAD create &&
+ git -C two fetch custom-head &&
+ echo refs/remotes/custom-head/does-not-exist >expect &&
+ git -C two symbolic-ref refs/remotes/custom-head/HEAD >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'followRemoteHEAD create does not overwrite dangling symref' '
test_when_finished "git -C two remote remove custom-head" &&
git -C two remote add -m does-not-exist custom-head ../one &&
--
2.54.0
^ permalink raw reply related
* [PATCH 5/7] fetch: refactor do_fetch handling of followRemoteHEAD
From: Matt Hunter @ 2026-06-12 5:55 UTC (permalink / raw)
To: git
In-Reply-To: <20260612055947.1499497-1-m@lfurio.us>
Update enum follow_remote_head_settings to include the value
FOLLOW_REMOTE_UNCONFIGURED as the new zero-initialized value for
followRemoteHEAD. This will allow us to distinguish between the option
being unset vs. explicitly set to 'create', which is ultimately the
system default. The unnecessary indentation is removed.
The do_fetch function is likewise updated to perform its own decision
making to determine the effective followRemoteHEAD mode, falling back to
the system default if necessary. This will enable the next patch to
introduce a user-configurable fallback default option.
Function set_head now accepts this value as an argument rather than only
considering the value defined by the remote.
The use of the 'warn-if-not-$branch' value is awkward in the context of
a global default option, since the branches will differ between
individual remotes. For this reason, it's left out of this scheme and
handling of the no_warn_branch variable is untouched. Since a
remote-specific setting for followRemoteHEAD takes priority, we can
assume that if remote->no_warn_branch is set, then the remote is also
asserting FOLLOW_REMOTE_WARN as the effective operating mode, and it
will be honored by do_fetch.
Signed-off-by: Matt Hunter <m@lfurio.us>
---
builtin/fetch.c | 14 ++++++++++----
remote.h | 14 ++++++++------
2 files changed, 18 insertions(+), 10 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 9a45e1e7a44d..3cc7efdd83a0 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1729,12 +1729,12 @@ static void warn_set_head(const char *remote, const char *head_name,
strbuf_release(&buf_prefix);
}
-static int set_head(const struct ref *remote_refs, struct remote *remote)
+static int set_head(const struct ref *remote_refs, struct remote *remote,
+ int follow_remote_head)
{
int result = 0, create_only, baremirror, was_detached;
struct strbuf b_head = STRBUF_INIT, b_remote_head = STRBUF_INIT,
b_local_head = STRBUF_INIT;
- int follow_remote_head = remote->follow_remote_head;
const char *no_warn_branch = remote->no_warn_branch;
char *head_name = NULL;
struct ref *ref, *matches;
@@ -1901,6 +1901,7 @@ static int do_fetch(struct transport *transport,
struct ref_update_display_info_array display_array = { 0 };
struct strmap rejected_refs = STRMAP_INIT;
int summary_width = 0;
+ int follow_remote_head;
if (tags == TAGS_DEFAULT) {
if (transport->remote->fetch_tags == 2)
@@ -1916,6 +1917,11 @@ static int do_fetch(struct transport *transport,
goto cleanup;
}
+ if (transport->remote->follow_remote_head)
+ follow_remote_head = transport->remote->follow_remote_head;
+ else
+ follow_remote_head = BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;
+
if (rs->nr) {
refspec_ref_prefixes(rs, &transport_ls_refs_options.ref_prefixes);
} else {
@@ -1924,7 +1930,7 @@ static int do_fetch(struct transport *transport,
if (transport->remote->fetch.nr) {
refspec_ref_prefixes(&transport->remote->fetch,
&transport_ls_refs_options.ref_prefixes);
- if (transport->remote->follow_remote_head != FOLLOW_REMOTE_NEVER)
+ if (follow_remote_head != FOLLOW_REMOTE_NEVER)
do_set_head = 1;
}
if (branch && branch_has_merge_config(branch) &&
@@ -2131,7 +2137,7 @@ static int do_fetch(struct transport *transport,
* Way too many cases where this can go wrong so let's just
* ignore errors and fail silently for now.
*/
- set_head(remote_refs, transport->remote);
+ set_head(remote_refs, transport->remote, follow_remote_head);
}
cleanup:
diff --git a/remote.h b/remote.h
index 54b17e4b028b..72a54d84ad51 100644
--- a/remote.h
+++ b/remote.h
@@ -62,12 +62,14 @@ struct remote_state {
void remote_state_clear(struct remote_state *remote_state);
struct remote_state *remote_state_new(void);
- enum follow_remote_head_settings {
- FOLLOW_REMOTE_NEVER = -1,
- FOLLOW_REMOTE_CREATE = 0,
- FOLLOW_REMOTE_WARN = 1,
- FOLLOW_REMOTE_ALWAYS = 2,
- };
+#define BUILTIN_FOLLOW_REMOTE_HEAD_DFLT FOLLOW_REMOTE_CREATE
+enum follow_remote_head_settings {
+ FOLLOW_REMOTE_UNCONFIGURED = 0,
+ FOLLOW_REMOTE_NEVER,
+ FOLLOW_REMOTE_CREATE,
+ FOLLOW_REMOTE_WARN,
+ FOLLOW_REMOTE_ALWAYS,
+};
struct remote {
struct hashmap_entry ent;
--
2.54.0
^ permalink raw reply related
* [PATCH 4/7] fetch: rename function report_set_head
From: Matt Hunter @ 2026-06-12 5:55 UTC (permalink / raw)
To: git
In-Reply-To: <20260612055947.1499497-1-m@lfurio.us>
Update to the slightly more obvious name 'warn_set_head', which matches
the verbiage of the followRemoteHEAD options.
Signed-off-by: Matt Hunter <m@lfurio.us>
---
builtin/fetch.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 82969e230f5a..9a45e1e7a44d 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1707,7 +1707,7 @@ static void set_head_advice_msg(const char *remote, const char *head_name)
remote, head_name, remote, remote, head_name);
}
-static void report_set_head(const char *remote, const char *head_name,
+static void warn_set_head(const char *remote, const char *head_name,
struct strbuf *buf_prev, int updateres) {
struct strbuf buf_prefix = STRBUF_INIT;
const char *prev_head = NULL;
@@ -1787,7 +1787,7 @@ static int set_head(const struct ref *remote_refs, struct remote *remote)
if (verbosity >= 0 &&
follow_remote_head == FOLLOW_REMOTE_WARN &&
(!no_warn_branch || strcmp(no_warn_branch, head_name)))
- report_set_head(remote->name, head_name, &b_local_head, was_detached);
+ warn_set_head(remote->name, head_name, &b_local_head, was_detached);
cleanup:
free(head_name);
--
2.54.0
^ permalink raw reply related
* [PATCH 3/7] t5510: cleanup remote in followRemoteHEAD dangling ref test
From: Matt Hunter @ 2026-06-12 5:55 UTC (permalink / raw)
To: git
In-Reply-To: <20260612055947.1499497-1-m@lfurio.us>
A later patch will introduce a new test which closely mirrors this one.
Update this test to remove the 'custom-head' remote it creates.
Otherwise, the two tests will conflict with each other, as the second
one to execute will fail to create this remote (which already exists,
thanks to the first test).
Signed-off-by: Matt Hunter <m@lfurio.us>
---
t/t5510-fetch.sh | 1 +
1 file changed, 1 insertion(+)
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index eca9a973b5cb..43190630e714 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -251,6 +251,7 @@ test_expect_success 'followRemoteHEAD does not kick in with refspecs' '
'
test_expect_success 'followRemoteHEAD create does not overwrite dangling symref' '
+ test_when_finished "git -C two remote remove custom-head" &&
git -C two remote add -m does-not-exist custom-head ../one &&
test_config -C two remote.custom-head.followRemoteHEAD create &&
git -C two fetch custom-head &&
--
2.54.0
^ permalink raw reply related
* [PATCH 2/7] doc: explain fetchRemoteHEADWarn advice
From: Matt Hunter @ 2026-06-12 5:55 UTC (permalink / raw)
To: git
In-Reply-To: <20260612055947.1499497-1-m@lfurio.us>
When the user sets 'remote.<name>.followRemoteHEAD' to
'warn[-if-not-$branch]', git-fetch will report when a fetched HEAD
disagrees with the locally-configured remote's HEAD. This additional
advice instructs the user how to deal with these warnings, but was
previously undocumented in git-config.
Signed-off-by: Matt Hunter <m@lfurio.us>
---
Documentation/config/advice.adoc | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/config/advice.adoc b/Documentation/config/advice.adoc
index 257db5891817..c3c190ba6a4f 100644
--- a/Documentation/config/advice.adoc
+++ b/Documentation/config/advice.adoc
@@ -48,6 +48,10 @@ all advice messages.
to create a local branch after the fact.
diverging::
Shown when a fast-forward is not possible.
+ fetchRemoteHEADWarn::
+ Shown when linkgit:git-fetch[1] reveals that a remote `HEAD`
+ differs from what is set locally and the user has opted into
+ receiving a warning in this situation.
fetchShowForcedUpdates::
Shown when linkgit:git-fetch[1] takes a long time
to calculate forced updates after ref updates, or to warn
--
2.54.0
^ permalink raw reply related
* [PATCH 1/7] fetch: fixup set_head advice for warn-if-not-branch
From: Matt Hunter @ 2026-06-12 5:55 UTC (permalink / raw)
To: git
In-Reply-To: <20260612055947.1499497-1-m@lfurio.us>
Specifying the word 'branch' in the command is not correct - a mismatch
with both the implementation in remote.c and the documentation.
Signed-off-by: Matt Hunter <m@lfurio.us>
---
builtin/fetch.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index c1d7c672f4e0..82969e230f5a 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1700,7 +1700,7 @@ static void set_head_advice_msg(const char *remote, const char *head_name)
N_("Run 'git remote set-head %s %s' to follow the change, or set\n"
"'remote.%s.followRemoteHEAD' configuration option to a different value\n"
"if you do not want to see this message. Specifically running\n"
- "'git config set remote.%s.followRemoteHEAD warn-if-not-branch-%s'\n"
+ "'git config set remote.%s.followRemoteHEAD warn-if-not-%s'\n"
"will disable the warning until the remote changes HEAD to something else.");
advise_if_enabled(ADVICE_FETCH_SET_HEAD_WARN, _(message_advice_set_head),
--
2.54.0
^ permalink raw reply related
* [PATCH 0/7] Introduce fetch.followRemoteHEAD config option
From: Matt Hunter @ 2026-06-12 5:55 UTC (permalink / raw)
To: git; +Cc: Jeff King, Bence Ferdinandy
In-Reply-To: <DJ19CI50W6UH.17QLIBNTXBWXU@lfurio.us>
git-fetch presently offers some useful ways to control how remote HEAD
symbolic-refs are (or aren't) updated when fetching from remote
repositories. Namely this is done via the
'remote.<name>.followRemoteHEAD' configuration option.
However, this option can be somewhat painful to use if you prefer a
default other than the "create" option, and often work with multiple
different remote repositories.
This series introduces the option 'fetch.followRemoteHEAD', which
provides a configurable default in place of per-remote settings.
'fetch.followRemoteHEAD' functions exactly the same as the original
option, except that it doesn't allow warning suppression via
'warn-if-not-$branch'. Given that different remotes will vary their
HEAD and set of branches independently, setting a false-positive
globally in this way doesn't make logical sense.
While it is not mentioned by any of the patches in this series, note
also that the behavior introduced by 012bc566bad7 (remote set-head: set
followRemoteHEAD to "warn" if "always") is unaffected by this series,
and this feature continues to work for only the
'remote.<name>.followRemoteHEAD' option.
Matt Hunter (7):
fetch: fixup set_head advice for warn-if-not-branch
doc: explain fetchRemoteHEADWarn advice
t5510: cleanup remote in followRemoteHEAD dangling ref test
fetch: rename function report_set_head
fetch: refactor do_fetch handling of followRemoteHEAD
fetch: add configuration option fetch.followRemoteHEAD
fetch: fixup a misaligned comment
Documentation/config/advice.adoc | 4 ++
Documentation/config/fetch.adoc | 19 ++++++
Documentation/config/remote.adoc | 21 +++---
builtin/fetch.c | 52 +++++++++++----
remote.h | 14 ++--
t/t5510-fetch.sh | 106 +++++++++++++++++++++++++++++++
6 files changed, 186 insertions(+), 30 deletions(-)
base-commit: 1ff279f3404a482a83fb04c7457e41ab26884aea
--
2.54.0
^ permalink raw reply
* Re: [PATCH v4 06/16] midx: support custom `--base` for incremental MIDX writes
From: SZEDER Gábor @ 2026-06-12 5:34 UTC (permalink / raw)
To: Taylor Blau
Cc: git, Junio C Hamano, Jeff King, Elijah Newren, Patrick Steinhardt
In-Reply-To: <1bbb387d6b6204045d97882fd8775dbff12dedbb.1779206240.git.me@ttaylorr.com>
On Tue, May 19, 2026 at 11:57:54AM -0400, Taylor Blau wrote:
> diff --git a/t/t5334-incremental-multi-pack-index.sh b/t/t5334-incremental-multi-pack-index.sh
> index 66d6894761b..68a103d13d2 100755
> --- a/t/t5334-incremental-multi-pack-index.sh
> +++ b/t/t5334-incremental-multi-pack-index.sh
> @@ -113,6 +113,36 @@ test_expect_success 'write non-incremental MIDX layer with --no-write-chain-file
> test_grep "cannot use --no-write-chain-file without --incremental" err
> '
>
> +test_expect_success 'write MIDX layer with --base without --no-write-chain-file' '
> + test_must_fail git multi-pack-index write --bitmap --incremental \
> + --base=none 2>err &&
> + test_grep "cannot use --base without --no-write-chain-file" err
> +'
> +
> +test_expect_success 'write MIDX layer with --base=none and --no-write-chain-file' '
> + test_commit base-none &&
> + git repack -d &&
> +
> + cp "$midx_chain" "$midx_chain.bak" &&
> + layer="$(git multi-pack-index write --bitmap --incremental \
> + --no-write-chain-file --base=none)" &&
> +
> + test_cmp "$midx_chain.bak" "$midx_chain" &&
> + test_path_is_file "$midxdir/multi-pack-index-$layer.midx"
> +'
> +
> +test_expect_success 'write MIDX layer with --base=<hash> and --no-write-chain-file' '
> + test_commit base-hash &&
> + git repack -d &&
> +
> + cp "$midx_chain" "$midx_chain.bak" &&
> + layer="$(git multi-pack-index write --bitmap --incremental \
> + --no-write-chain-file --base="$(nth_line 1 "$midx_chain")")" &&
There is no 'nth_line' helper function in this test script.
> +
> + test_cmp "$midx_chain.bak" "$midx_chain" &&
> + test_path_is_file "$midxdir/multi-pack-index-$layer.midx"
> +'
> +
> for reuse in false single multi
> do
> test_expect_success "full clone (pack.allowPackReuse=$reuse)" '
^ permalink raw reply
* Re: [PATCH v2 3/3] doc: git-config: escape erroneous highlight markup
From: Jeff King @ 2026-06-12 5:16 UTC (permalink / raw)
To: Jean-Noël AVILA
Cc: Tuomas Ahola, git, Kristoffer Haugsbakk, Junio C Hamano
In-Reply-To: <5106812.31r3eYUQgx@piment-oiseau>
On Thu, Jun 11, 2026 at 10:43:44PM +0200, Jean-Noël AVILA wrote:
> Oh, this is the black magic regexp that is not considering # for keyword
> character. Should be solved by something like (and I really hate these .in
> files):
Your patch looks reasonable to me, but for some reason I get this
doc-diff output when comparing before/after (what we don't see is the
improved output from bolding those chars, since doc-diff doesn't show
any markup):
diff --git a/73bccdb573fbdf3df4abf37570be486fc0f53d4a/home/peff/share/man/man1/git-config.1 b/926d9be4ab7ee1a57d4c570ec0f8aba28b3c2af5/home/peff/share/man/man1/git-config.1
index b77d79f..b2bced2 100644
--- a/73bccdb573fbdf3df4abf37570be486fc0f53d4a/home/peff/share/man/man1/git-config.1
+++ b/926d9be4ab7ee1a57d4c570ec0f8aba28b3c2af5/home/peff/share/man/man1/git-config.1
@@ -6983,7 +6983,7 @@ CONFIGURATION FILE
status.displayCommentPrefix
If set to true, git-status(1) will insert a comment prefix before each
- output line (starting with core.commentChar, i.e. # by default). This
+ output line (starting with core.commentChar, i.e. # by default). This
was the behavior of git-status(1) in Git 1.8.4 and previous. Defaults
to false.
And I can see the extra space when looking at the rendered manpage. The
XML output looks reasonable, though:
(starting with <literal>core.commentChar</literal>, i.e. <literal>#</literal> by default
as does the HTML. So perhaps it is happening at the roff level? But that
looks like:
will insert a comment prefix before each output line (starting with
\fBcore\&.commentChar\fR, i\&.e\&.
\fB#\fR
by default)\&. This was the behavior of
So...weird. groff wants to add extra space for some reason. It happens
even if I drop the bolding, and just have "#" on a line by itself. I
guess maybe it is the trailing "." of the previous line putting groff
into "oh, I'm starting a new sentence" mode and it uses two spaces.
But I think that is all outside the scope of your fix, and this is an
existing issue that we are now just unlucky enough to hit. I'd be
tempted to ignore it and possibly fix it later.
-Peff
^ permalink raw reply related
* Re: [PATCH v3 0/3] doc: config: fix AsciiDoc glitches
From: Jeff King @ 2026-06-12 4:53 UTC (permalink / raw)
To: Tuomas Ahola
Cc: git, Kristoffer Haugsbakk, Junio C Hamano, Jean-Noël Avila
In-Reply-To: <20260611161946.12166-1-taahol@utu.fi>
On Thu, Jun 11, 2026 at 07:19:43PM +0300, Tuomas Ahola wrote:
> Tuomas Ahola (3):
> doc: config: terminate runaway lists
> doc: config/sideband: fix description list delimiter
> doc: git-config: escape erroneous highlight markup
Thanks, this v3 looks good to me.
-Peff
^ permalink raw reply
* [PATCH v3] ls-files: filter pathspec before lstat
From: Tamir Duberstein @ 2026-06-12 4:31 UTC (permalink / raw)
To: git
Cc: René Scharfe, Patrick Steinhardt, Junio C Hamano, Jeff King,
Tamir Duberstein
In-Reply-To: <20260608-ls-files-pathspec-lstat-v2-1-fb734b28422e@gmail.com>
In --deleted and --modified modes, show_files() calls lstat() for each
index entry before show_ce() applies the pathspec. prune_index() avoids
most of these calls for pathspecs with a common directory prefix, but
not for a top-level name or leading wildcard.
Match before lstat() to avoid accessing the worktree for entries that
cannot be shown. Treat this as a prefilter: do not update ps_matched,
and retain the match in show_ce() so --error-unmatch is satisfied only
by entries that the selected modes actually show.
Prefilter only a single pathspec item, bounding the added work for each
index entry. Applying match_pathspec() to multiple arguments can cost
more than the lstat() calls it avoids. In a synthetic repository with
10,000 clean files, passing every path to ls-files --modified increased
runtime from 112.5 ms to 494.1 ms when the prefilter was unconditional.
With $parent and $this exported as paths to binaries built from the
parent and this commit, on a repository with 881,290 index entries:
hyperfine --warmup 0 --runs 3 \
--command-name parent \
'$parent -c core.fsmonitor=false ls-files --deleted -- README.md >/dev/null' \
--command-name this-commit \
'$this -c core.fsmonitor=false ls-files --deleted -- README.md >/dev/null'
reported means of 65.790 seconds for the parent and 4.987 seconds for
this commit.
Link: https://lore.kernel.org/r/xmqqfr2tnfk0.fsf@gitster.g
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
A selective pathspec should let ls-files --deleted and --modified avoid
statting entries that cannot be shown. Match a single pathspec before
accessing the worktree, while preserving the existing lstat-first order
for multiple pathspecs whose matching cost grows linearly.
---
Changes in v3:
- Explain the conservative single-pathspec cutoff without referring to
prior revisions.
- Rerun the primary benchmark with the final implementation.
- Make no code changes.
- Link to v2: https://patch.msgid.link/20260608-ls-files-pathspec-lstat-v2-1-fb734b28422e@gmail.com
Changes in v2:
- Restrict early matching to one pathspec after measuring a regression
with many pathspecs.
- Add all-matching and many-pathspec performance results.
- Drop the Assisted-by trailer.
- Link to v1: https://patch.msgid.link/20260607-ls-files-pathspec-lstat-v1-1-8cf40b730146@gmail.com
---
builtin/ls-files.c | 11 +++++++++++
t/meson.build | 1 +
t/perf/p3010-ls-files.sh | 31 +++++++++++++++++++++++++++++++
t/t3010-ls-files-killed-modified.sh | 18 ++++++++++++++++++
4 files changed, 61 insertions(+)
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index e1a22b41b9..8d7158652b 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -450,6 +450,17 @@ static void show_files(struct repository *repo, struct dir_struct *dir)
continue;
if (ce_skip_worktree(ce))
continue;
+ /*
+ * match_pathspec() is linear in pathspec.nr, so prefilter only
+ * the single-pathspec case. Only entries shown by show_ce()
+ * satisfy --error-unmatch.
+ */
+ if (pathspec.nr == 1 &&
+ !match_pathspec(repo->index, &pathspec, fullname.buf,
+ fullname.len, max_prefix_len, NULL,
+ S_ISDIR(ce->ce_mode) ||
+ S_ISGITLINK(ce->ce_mode)))
+ continue;
stat_err = lstat(fullname.buf, &st);
if (stat_err && (errno != ENOENT && errno != ENOTDIR))
error_errno("cannot lstat '%s'", fullname.buf);
diff --git a/t/meson.build b/t/meson.build
index 2af8d01279..ee8086e6ef 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -1140,6 +1140,7 @@ benchmarks = [
'perf/p1500-graph-walks.sh',
'perf/p1501-rev-parse-oneline.sh',
'perf/p2000-sparse-operations.sh',
+ 'perf/p3010-ls-files.sh',
'perf/p3400-rebase.sh',
'perf/p3404-rebase-interactive.sh',
'perf/p4000-diff-algorithms.sh',
diff --git a/t/perf/p3010-ls-files.sh b/t/perf/p3010-ls-files.sh
new file mode 100755
index 0000000000..ae14449432
--- /dev/null
+++ b/t/perf/p3010-ls-files.sh
@@ -0,0 +1,31 @@
+#!/bin/sh
+
+test_description='Tests ls-files worktree performance'
+
+. ./perf-lib.sh
+
+test_perf_large_repo
+test_checkout_worktree
+
+test_expect_success 'select a zero-prefix pathspec' '
+ tracked_file=$(git ls-files | sed -n 1p) &&
+ test -n "$tracked_file" &&
+ pathspec="?${tracked_file#?}" &&
+ test_export pathspec
+'
+
+test_perf 'ls-files --deleted with pathspec' '
+ git -c core.fsmonitor=false ls-files --deleted \
+ -- "$pathspec" >/dev/null
+'
+
+test_perf 'ls-files --deleted with all-matching pathspec' '
+ git -c core.fsmonitor=false ls-files --deleted -- "*" >/dev/null
+'
+
+test_perf 'ls-files --modified with pathspec' '
+ git -c core.fsmonitor=false ls-files --modified \
+ -- "$pathspec" >/dev/null
+'
+
+test_done
diff --git a/t/t3010-ls-files-killed-modified.sh b/t/t3010-ls-files-killed-modified.sh
index 7af4532cd1..6e38e10219 100755
--- a/t/t3010-ls-files-killed-modified.sh
+++ b/t/t3010-ls-files-killed-modified.sh
@@ -124,4 +124,22 @@ test_expect_success 'validate git ls-files -m output.' '
test_cmp .expected .output
'
+test_expect_success 'worktree modes honor wildcard pathspecs' '
+ cat >.expected <<-\EOF &&
+ path2/file2
+ path3/file3
+ EOF
+ git ls-files --deleted -- "path?/file?" >.output &&
+ test_cmp .expected .output &&
+
+ cat >.expected <<-\EOF &&
+ path7
+ path8
+ EOF
+ git ls-files --modified --error-unmatch -- "path[78]" >.output &&
+ test_cmp .expected .output &&
+
+ test_must_fail git ls-files --modified --error-unmatch -- path10
+'
+
test_done
---
base-commit: 9ac3f193c05c2237e2b14ebaa1149e9fc8a1abe0
change-id: 20260607-ls-files-pathspec-lstat-885125a5d644
Best regards,
--
Tamir Duberstein <tamird@gmail.com>
^ permalink raw reply related
* [PATCH v3 3/3] commit-reach: die on contains walk errors
From: Tamir Duberstein @ 2026-06-12 3:00 UTC (permalink / raw)
To: git
Cc: Jeff King, Karthik Nayak, Junio C Hamano, Victoria Dye,
Derrick Stolee, Elijah Newren, Kristofer Karlsson,
Tamir Duberstein
In-Reply-To: <20260611-ref-filter-memoized-contains-v3-0-b26af3dba285@gmail.com>
Without generation numbers, repo_is_descendant_of() can return -1 when
it cannot read commit ancestry. commit_contains() exposes that result
through a Boolean interface, so ref-filter treats it as true. This can
include a ref for --contains or exclude it for --no-contains without
failing the command.
Die when repo_is_descendant_of() reports an error. The memoized walk
already dies when it cannot parse a commit, so callers of the
non-memoized path no longer turn a failed walk into a match.
Reported-by: Jeff King <peff@peff.net>
Link: https://lore.kernel.org/r/20260611072942.GG2191159@coredump.intra.peff.net
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
commit-reach.c | 8 +++++++-
t/t6301-for-each-ref-errors.sh | 22 ++++++++++++++++++++++
2 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/commit-reach.c b/commit-reach.c
index 572d2d47ff..af5563d70f 100644
--- a/commit-reach.c
+++ b/commit-reach.c
@@ -820,10 +820,16 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
int commit_contains(struct ref_filter *filter, struct commit *commit,
struct commit_list *list, struct contains_cache *cache)
{
+ int result;
+
if (filter->with_commit_tag_algo ||
generation_numbers_enabled(the_repository))
return contains_tag_algo(commit, list, cache) == CONTAINS_YES;
- return repo_is_descendant_of(the_repository, commit, list);
+
+ result = repo_is_descendant_of(the_repository, commit, list);
+ if (result < 0)
+ die(_("failed to check reachability"));
+ return result;
}
int can_all_from_reach_with_flag(struct object_array *from,
diff --git a/t/t6301-for-each-ref-errors.sh b/t/t6301-for-each-ref-errors.sh
index e06feb06e9..72b27c8be3 100755
--- a/t/t6301-for-each-ref-errors.sh
+++ b/t/t6301-for-each-ref-errors.sh
@@ -52,6 +52,28 @@ test_expect_success 'Missing objects are reported correctly' '
test_must_be_empty brief-err
'
+test_expect_success 'missing ancestors are reported by contains filters' '
+ test_when_finished "git update-ref -d refs/heads/missing-parent" &&
+ {
+ echo "tree $(git rev-parse HEAD^{tree})" &&
+ echo "parent $MISSING" &&
+ git cat-file commit HEAD |
+ sed -n -e "/^author /p" -e "/^committer /p" &&
+ echo &&
+ echo "missing parent"
+ } >commit &&
+ broken=$(git hash-object -t commit -w commit) &&
+ git update-ref refs/heads/missing-parent "$broken" &&
+ for option in --contains --no-contains
+ do
+ test_must_fail git for-each-ref "$option=HEAD" \
+ refs/heads/missing-parent >out 2>err &&
+ test_must_be_empty out &&
+ test_grep "parse commit $MISSING" err ||
+ return 1
+ done
+'
+
test_expect_success 'ahead-behind requires an argument' '
test_must_fail git for-each-ref \
--format="%(ahead-behind)" 2>err &&
--
2.54.0.548.gbe7bb2469c
^ permalink raw reply related
* [PATCH v3 2/3] ref-filter: memoize --contains with generations
From: Tamir Duberstein @ 2026-06-12 3:00 UTC (permalink / raw)
To: git
Cc: Jeff King, Karthik Nayak, Junio C Hamano, Victoria Dye,
Derrick Stolee, Elijah Newren, Kristofer Karlsson,
Tamir Duberstein
In-Reply-To: <20260611-ref-filter-memoized-contains-v3-0-b26af3dba285@gmail.com>
git branch and git for-each-ref run a separate reachability walk for
each ref considered by --contains and --no-contains. Refs with shared
history therefore traverse the same commits repeatedly.
git tag instead uses a depth-first walk that caches results across
refs. That walk can perform poorly without generation numbers: a
negative check may walk to the root instead of stopping at a nearby
divergence. Generation numbers let it stop below the oldest target.
Use the memoized walk for all ref-filter callers when generation
numbers are available. Keep git tag on its existing path without
generations. Caching still helps when many tags share deep history:
ffc4b8012d (tag: speed up --contains calculation, 2011-06-11) reduced
git tag --contains HEAD~200 in linux-2.6 from 15.417 to 5.329 seconds.
The new shared-history perf test improves from 0.72 to 0.03 seconds. In
a repository with 62,174 remote-tracking refs, running:
git branch -r --contains c78ae85f3ce7e
improves from 104.365 seconds to 468 milliseconds.
Link: https://lore.kernel.org/git/1445163904-24611-1-git-send-email-Karthik.188@gmail.com/
Link: https://lore.kernel.org/r/20230324191009.GA536967@coredump.intra.peff.net
Link: https://lore.kernel.org/git/20260527070510.3510836-1-krka@spotify.com/
Link: https://lore.kernel.org/r/20260608223430.GA340696@coredump.intra.peff.net
Link: https://lore.kernel.org/r/CAOLa=ZSezQOj56-TezVaAcisUyczxhJmu4VghyFBHcBB_mKJ2A@mail.gmail.com
Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
commit-reach.c | 3 ++-
t/perf/p1500-graph-walks.sh | 28 +++++++++++++++++++++++++++-
2 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/commit-reach.c b/commit-reach.c
index 1d34d66fe8..572d2d47ff 100644
--- a/commit-reach.c
+++ b/commit-reach.c
@@ -820,7 +820,8 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
int commit_contains(struct ref_filter *filter, struct commit *commit,
struct commit_list *list, struct contains_cache *cache)
{
- if (filter->with_commit_tag_algo)
+ if (filter->with_commit_tag_algo ||
+ generation_numbers_enabled(the_repository))
return contains_tag_algo(commit, list, cache) == CONTAINS_YES;
return repo_is_descendant_of(the_repository, commit, list);
}
diff --git a/t/perf/p1500-graph-walks.sh b/t/perf/p1500-graph-walks.sh
index 5b23ce5db9..d167b4f7e1 100755
--- a/t/perf/p1500-graph-walks.sh
+++ b/t/perf/p1500-graph-walks.sh
@@ -32,7 +32,16 @@ test_expect_success 'setup' '
echo "X:$line" >>test-tool-tags || return 1
done &&
- commit=$(git commit-tree $(git rev-parse HEAD^{tree})) &&
+ git rev-list --first-parent --max-count=8192 HEAD >contains-commits &&
+ test_file_not_empty contains-commits &&
+ git update-ref refs/contains-perf-base "$(tail -n 1 contains-commits)" &&
+ awk "{
+ printf \"update refs/contains-perf/%04d %s\\n\", NR, \$1
+ }" contains-commits |
+ git update-ref --stdin &&
+ git pack-refs --include "refs/contains-perf/*" &&
+
+ commit=$(git commit-tree HEAD^{tree}) &&
git update-ref refs/heads/disjoint-base $commit &&
git commit-graph write --reachable
@@ -62,6 +71,23 @@ test_perf 'contains: git tag --merged' '
xargs git tag --merged=HEAD <tags
'
+test_perf 'contains: git for-each-ref' '
+ git for-each-ref --contains=refs/contains-perf-base --stdin <refs
+'
+
+test_perf 'contains: git branch' '
+ xargs git branch --contains=refs/contains-perf-base <branches
+'
+
+test_perf 'contains: git tag' '
+ xargs git tag --contains=refs/contains-perf-base <tags
+'
+
+test_perf 'contains: synthetic shared history' '
+ git for-each-ref --contains=refs/contains-perf-base \
+ refs/contains-perf/ >/dev/null
+'
+
test_perf 'is-base check: test-tool reach (refs)' '
test-tool reach get_branch_base_for_tip <test-tool-refs
'
--
2.54.0.548.gbe7bb2469c
^ permalink raw reply related
* [PATCH v3 1/3] commit-reach: handle cycles in contains walk
From: Tamir Duberstein @ 2026-06-12 3:00 UTC (permalink / raw)
To: git
Cc: Jeff King, Karthik Nayak, Junio C Hamano, Victoria Dye,
Derrick Stolee, Elijah Newren, Kristofer Karlsson,
Tamir Duberstein
In-Reply-To: <20260611-ref-filter-memoized-contains-v3-0-b26af3dba285@gmail.com>
The memoized contains traversal used by git tag assumes that commit
ancestry is acyclic. Replacement refs can violate that assumption,
causing it to keep pushing an already active commit until memory is
exhausted.
Mark commits while they are active. If the traversal encounters an
active commit, discard the cache and retry the candidate with the
cycle-safe reachability walk. Cache the candidate's result so a later
walk that reaches it can reuse the answer. Die if the fallback cannot
read ancestry.
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
commit-reach.c | 29 +++++++++++++++++++++++++----
commit-reach.h | 3 ++-
t/t7004-tag.sh | 21 +++++++++++++++++++++
3 files changed, 48 insertions(+), 5 deletions(-)
diff --git a/commit-reach.c b/commit-reach.c
index 9b3ea46d6f..1d34d66fe8 100644
--- a/commit-reach.c
+++ b/commit-reach.c
@@ -708,7 +708,8 @@ static int in_commit_list(const struct commit_list *want, struct commit *c)
/*
* Test whether the candidate is contained in the list.
- * Do not recurse to find out, though, but return -1 if inconclusive.
+ * Do not recurse to find out, though, but return CONTAINS_UNKNOWN if
+ * inconclusive.
*/
static enum contains_result contains_test(struct commit *candidate,
const struct commit_list *want,
@@ -744,7 +745,7 @@ static void push_to_contains_stack(struct commit *candidate, struct contains_sta
}
static enum contains_result contains_tag_algo(struct commit *candidate,
- const struct commit_list *want,
+ struct commit_list *want,
struct contains_cache *cache)
{
struct contains_stack contains_stack = { 0, 0, NULL };
@@ -765,6 +766,7 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
if (result != CONTAINS_UNKNOWN)
return result;
+ *contains_cache_at(cache, candidate) = CONTAINS_IN_PROGRESS;
push_to_contains_stack(candidate, &contains_stack);
while (contains_stack.nr) {
struct contains_stack_entry *entry = &contains_stack.contains_stack[contains_stack.nr - 1];
@@ -776,8 +778,8 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
contains_stack.nr--;
}
/*
- * If we just popped the stack, parents->item has been marked,
- * therefore contains_test will return a meaningful yes/no.
+ * A parent may have just been popped and marked, or may still
+ * be active when replacement refs create a cycle.
*/
else switch (contains_test(parents->item, want, cache, cutoff)) {
case CONTAINS_YES:
@@ -787,13 +789,32 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
case CONTAINS_NO:
entry->parents = parents->next;
break;
+ case CONTAINS_IN_PROGRESS:
+ /*
+ * Partial negative answers are not safe across a cycle.
+ * Discard them and use the cycle-safe reachability walk.
+ */
+ goto cycle;
case CONTAINS_UNKNOWN:
+ *contains_cache_at(cache, parents->item) =
+ CONTAINS_IN_PROGRESS;
push_to_contains_stack(parents->item, &contains_stack);
break;
}
}
free(contains_stack.contains_stack);
return contains_test(candidate, want, cache, cutoff);
+
+cycle:
+ free(contains_stack.contains_stack);
+ clear_contains_cache(cache);
+
+ result = repo_is_descendant_of(the_repository, candidate, want);
+ if (result < 0)
+ die(_("failed to check reachability after detecting a cycle"));
+ *contains_cache_at(cache, candidate) =
+ result ? CONTAINS_YES : CONTAINS_NO;
+ return result ? CONTAINS_YES : CONTAINS_NO;
}
int commit_contains(struct ref_filter *filter, struct commit *commit,
diff --git a/commit-reach.h b/commit-reach.h
index 3f3a563d8a..f908d305b1 100644
--- a/commit-reach.h
+++ b/commit-reach.h
@@ -73,7 +73,8 @@ int ref_newer(const struct object_id *new_oid, const struct object_id *old_oid);
enum contains_result {
CONTAINS_UNKNOWN = 0,
CONTAINS_NO,
- CONTAINS_YES
+ CONTAINS_YES,
+ CONTAINS_IN_PROGRESS
};
define_commit_slab(contains_cache, enum contains_result);
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index d918005dd9..4044bab006 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1611,6 +1611,27 @@ test_expect_success 'checking that first commit is in all tags (hash)' '
test_cmp expected actual
'
+test_expect_success 'tag --contains handles cyclic replacement histories' '
+ first=$(git rev-parse HEAD~2) &&
+ second=$(git rev-parse HEAD~) &&
+ third=$(git rev-parse HEAD) &&
+ test_when_finished "
+ git replace -d $first &&
+ git replace -d $third &&
+ git tag -d cycle-a cycle-b
+ " &&
+ git tag cycle-a "$first" &&
+ git tag cycle-b "$third" &&
+ git replace --graft "$first" "$third" "$second" &&
+ git replace --graft "$third" "$first" &&
+ cat >expected <<-\EOF &&
+ cycle-a
+ cycle-b
+ EOF
+ git tag --contains="$second" --list "cycle-*" >actual &&
+ test_cmp expected actual
+'
+
# other ways of specifying the commit
test_expect_success 'checking that first commit is in all tags (tag)' '
cat >expected <<-\EOF &&
--
2.54.0.548.gbe7bb2469c
^ permalink raw reply related
* [PATCH v3 0/3] Reuse --contains traversal results
From: Tamir Duberstein @ 2026-06-12 3:00 UTC (permalink / raw)
To: git
Cc: Jeff King, Karthik Nayak, Junio C Hamano, Victoria Dye,
Derrick Stolee, Elijah Newren, Kristofer Karlsson,
Tamir Duberstein
In-Reply-To: <20260608-ref-filter-memoized-contains-v2-0-e72720344a7c@gmail.com>
git tag uses a memoized traversal for --contains, while git branch
and git for-each-ref repeat a reachability walk for each ref. Reuse
the memoized traversal when generation numbers can bound the walk.
The first patch makes the memoized traversal handle replacement
cycles. The last makes the non-memoized path report reachability
errors.
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
Changes in v3:
- Split missing-ancestor error handling into its own patch.
- Use die() for reachability errors, remove redundant cache setup, and
chain cycle-test cleanup.
- Drop the unrelated empty-target-list behavior change.
- Explain why git tag retains memoization without generation numbers.
- Add p1500 coverage for all three frontends and a shared-history
case.
- Remove correctness checks from p1500 and drop output hashes.
- Link to v2: https://patch.msgid.link/20260608-ref-filter-memoized-contains-v2-0-e72720344a7c@gmail.com
Changes in v2:
- Split cycle handling into a preparatory patch.
- Exercise cycle handling through the existing git tag path.
- Move perf result verification out of setup.
- Link to v1: https://patch.msgid.link/20260607-ref-filter-memoized-contains-v1-1-a1972dde9c76@gmail.com
---
Tamir Duberstein (3):
commit-reach: handle cycles in contains walk
ref-filter: memoize --contains with generations
commit-reach: die on contains walk errors
commit-reach.c | 40 ++++++++++++++++++++++++++++++++++------
commit-reach.h | 3 ++-
t/perf/p1500-graph-walks.sh | 28 +++++++++++++++++++++++++++-
t/t6301-for-each-ref-errors.sh | 22 ++++++++++++++++++++++
t/t7004-tag.sh | 21 +++++++++++++++++++++
5 files changed, 106 insertions(+), 8 deletions(-)
---
base-commit: 9ac3f193c05c2237e2b14ebaa1149e9fc8a1abe0
change-id: 20260607-ref-filter-memoized-contains-7cb6b3bccad1
Best regards,
--
Tamir Duberstein <tamird@gmail.com>
^ permalink raw reply
* Re: [PATCH v2 2/2] ref-filter: memoize --contains with generations
From: Tamir Duberstein @ 2026-06-12 2:40 UTC (permalink / raw)
To: Jeff King
Cc: git, Karthik Nayak, Junio C Hamano, Victoria Dye, Derrick Stolee,
Elijah Newren
In-Reply-To: <20260611082244.GH2191159@coredump.intra.peff.net>
On Thu, Jun 11, 2026 at 1:22 AM Jeff King <peff@peff.net> wrote:
>
> On Mon, Jun 08, 2026 at 07:36:35PM -0700, Tamir Duberstein wrote:
>
> > The wall-time standard deviations were 11.356 seconds and 133.8
> > milliseconds, respectively. Separate runs without redirection produced
> > the same output with SHA-256
> > 2466f6e2b72aa16b1a2126eddb81c8a1b2764ee251204ac034c191a925aa896f.
>
> Heh. Without the original repo, this sha256 hash is meaningless to us,
> isn't it? Ditto for the sha1 the earlier command.
Yeah, AI slop. Removed.
>
> > int commit_contains(struct ref_filter *filter, struct commit *commit,
> > struct commit_list *list, struct contains_cache *cache)
> > {
> > - if (filter->with_commit_tag_algo)
> > + int result;
> > +
> > + if (!list)
> > + return 1;
> > + if (filter->with_commit_tag_algo ||
> > + generation_numbers_enabled(the_repository))
> > return contains_tag_algo(commit, list, cache) == CONTAINS_YES;
> > - return repo_is_descendant_of(the_repository, commit, list);
> > +
> > + result = repo_is_descendant_of(the_repository, commit, list);
> > + if (result < 0)
> > + exit(128);
> > + return result;
>
> There's a little more going on here than I expected from the commit
> message. Is it important for us to short-circuit the empty list and just
> return 1? Or did the existing helper functions already handle that?
>
> Looking at contains_tag_algo(), I think it would actually return
> CONTAINS_NO here (though I didn't test it). So this is actually a change
> in behavior for "git tag" if that's correct. I doubt it is triggerable
> in practice, though, as we would simply never call commit_contains() in
> the first place with an empty list. But if we are going to add in this
> logic, I think it makes sense to do so as a separate commit (describing
> what it is doing and why it's not (yet) a triggerable bug).
>
> Checking the result of repo_is_descendant_of() makes sense, as discussed
> earlier. But probably that should come as its own patch, since it's an
> independent bug-fix. I'm also tempted to say it should call die()
> instead of a direct exit, though it does look like the error exit paths
> from repo_is_descendant_of() would all have produced their own messages.
I dropped the empty-list change. The error check is now a separate
patch and uses die().
>
>
> And one side note. While looking at the implementation of
> repo_is_descendant_of(), I did notice something curious: it also
> switches algorithms based on the presence of generation numbers! So it
> should also be cutting off the traversal early when possible. But I
> guess its main problem is that we call it independently for each
> candidate, so it may traverse the same (useful) stretch of history
> multiple times.
>
> So probably an alternative approach to this patch would be feeding all
> of the candidates at once, the way we do with reach_filter() via
> filter_refs(). I'm not sure if we have the right functions available for
> that (naively, --contains and --merged are inversions of each other, so
> swapping the arguments to tips_reachable_from_bases() might work, but I
> didn't think very hard on it).
I tried the suggested argument swap, but tips_reachable_from_bases()
only reports whether a tip is reachable from any base. It cannot report
which candidate refs contain a target, which is what --contains needs.
I did not find an existing batched reachability helper that returns
those per-candidate answers.
>
> I wonder if that might perform better or worse. I'm content to leave it
> for another day, though, as switching to the memoizing depth-first algo
> here is a pretty easy change.
>
> > - commit=$(git commit-tree $(git rev-parse HEAD^{tree})) &&
> > + git rev-list --first-parent --max-count=8192 HEAD >contains-commits &&
> > + test_file_not_empty contains-commits &&
> > + git update-ref refs/contains-perf-base "$(tail -n 1 contains-commits)" &&
> > + awk "{
> > + printf \"update refs/contains-perf/%04d %s\\n\", NR, \$1
> > + }" contains-commits |
> > + git update-ref --stdin &&
> > + git pack-refs --include "refs/contains-perf/*" &&
>
> My head almost exploded reading the embedded quoting in that awk
> invocation. But I can't think offhand of a better way to do it. You
> can't use test_seq because it needs both the number and the original
> string. You can do it with sed, but it probably ends up even more
> unreadable.
>
> But OK, we are making a bunch of refs based on first-parent history.
>
> > + tree=$(git rev-parse HEAD^{tree}) &&
> > + base=$(git rev-parse HEAD) &&
> > + target=$(echo target | git commit-tree "$tree" -p "$base") &&
> > + git update-ref refs/contains-diverged/target "$target" &&
> > + for i in $(test_seq 1 4)
> > + do
> > + commit=$(echo candidate-$i |
> > + git commit-tree "$tree" -p "$base") &&
> > + git update-ref refs/contains-diverged/candidate-$i "$commit" ||
> > + return 1
> > + done &&
>
> And then a few candidate refs that are not reachable from other refs, or
> from each other. OK.
>
> I think you could just write:
>
> git commit-tree HEAD^{tree} -p HEAD
>
> instead of doing separate rev-parses, but it's probably not a big deal
> either way.
>
> > +test_expect_success 'verify contains results' '
> > + git for-each-ref --contains=refs/contains-perf-base \
> > + refs/contains-perf/ >actual &&
> > + test_line_count = $(wc -l <contains-commits) actual &&
> > +
> > + echo refs/contains-diverged/target >expect &&
> > + GIT_TEST_COMMIT_GRAPH=0 \
> > + git -c core.commitGraph=false for-each-ref \
> > + --format="%(refname)" \
> > + --contains=refs/contains-diverged/target \
> > + refs/contains-diverged/ >actual &&
> > + test_cmp expect actual
> > +'
>
> This is a funny test to have in the middle of a perf script (which
> hardly anybody ever runs). If we are concerned about the correctness,
> should this be in a non-perf test script? Though I'd imagine something
> like it is already covered there.
I deleted that block rather than moving it. It only rechecked ordinary
--contains semantics already covered by t3201, t6302, and t7004; with
GIT_TEST_COMMIT_GRAPH=1, those tests exercise the newly selected
memoized path for branch and for-each-ref.
The series adds functional tests for the behavior that is actually new:
t7004 covers cyclic replacement histories, and t6301 covers unreadable
ancestry. The p1500 additions now measure performance only.
>
> There's a lot of subtlety in what we're verifying, too. In the first
> half, we are checking that all of the commits in contains-perf contain
> the base. And that base is the final element of the contains-commits
> list. Which made me wonder what happens in a branch history, since that
> list is linearized. But because we used --first-parent to generate it,
> it _is_ linear, and the results work out. So OK, I don't think it's
> wrong, but I am struggling to understand the meaning of the test.
>
> The second half is just checking that...the other refs which are not
> contained in "target" are not mentioned? OK, but why do it only with
> commit graphs off. Why not both off and on? Again, I'm not sure I
> understand what we're trying to focus on here.
>
> > +test_perf 'contains: git for-each-ref --contains' '
> > + git for-each-ref --contains=refs/contains-perf-base \
> > + refs/contains-perf/ >/dev/null
> > +'
>
> Yay, actual perf tests. Here we have a ton of matches, and they all walk
> over the same chunk of history. Should get much faster, though it's
> mostly a synthetic test.
>
> For --merged, we already have separate tests with each of for-each-ref,
> branch, and tag. Should we have the same here for --contains? And should
> we be using the input repo data, rather than our synthetic test? It is
> nice to show off the performance with the synthetic test, but ultimately
> the point of the perf suite is feeding it real workloads and looking for
> regressions.
I added p1500 cases for all three frontends using refs from the input
repository, while retaining the synthetic shared-history case.
>
> > +test_perf 'contains without generations: divergent refs' '
> > + GIT_TEST_COMMIT_GRAPH=0 \
> > + git -c core.commitGraph=false for-each-ref \
> > + --contains=refs/contains-diverged/target \
> > + refs/contains-diverged/ >/dev/null
> > +'
>
> OK, and this one should find that most of them are not contained, but
> the depth-first algorithm could walk all the way down to the roots. But
> we don't run it at all, since we disable commit graphs!
>
> So what are we trying to measure here? If it left commit graphs enabled,
> I think we could demonstrate that using the depth-first algorithm with
> generation numbers does not make anything _worse_. I.e., that
> for-each-ref and branch did not regress from the change.
The divergent-ref test did not exercise the changed path, so I removed
it.
>
> > +test_expect_success 'missing ancestors are reported by contains filters' '
> > + test_when_finished "git update-ref -d refs/heads/missing-parent" &&
> > + {
> > + echo "tree $(git rev-parse HEAD^{tree})" &&
> > + echo "parent $MISSING" &&
> > + git cat-file commit HEAD |
> > + sed -n -e "/^author /p" -e "/^committer /p" &&
> > + echo &&
> > + echo "missing parent"
> > + } >commit &&
> > + broken=$(git hash-object -t commit -w commit) &&
> > + git update-ref refs/heads/missing-parent "$broken" &&
> > + for option in --contains --no-contains
> > + do
> > + test_must_fail git for-each-ref "$option=HEAD" \
> > + refs/heads/missing-parent >out 2>err &&
> > + test_must_be_empty out &&
> > + test_grep "parse commit $MISSING" err ||
> > + return 1
> > + done
> > +'
>
> This is a great thing to test, but probably should be pulled out into
> a separate patch along with the fix to check the return code.
Done in v3.
>
> The commit construction looks OK, and is nicer than corrupting the
> repository by deleting a real object. Given that we are pulling the
> idents from an existing commit, it might be simpler to just use the
> whole commit as a template, like:
>
> git cat-file commit HEAD |
> sed "s/^parent /parent $MISSING/"
>
> but it may be a matter of taste.
>
> -Peff
^ permalink raw reply
* Re: [PATCH v2 1/2] commit-reach: handle cycles in contains walk
From: Tamir Duberstein @ 2026-06-12 2:40 UTC (permalink / raw)
To: Jeff King
Cc: git, Karthik Nayak, Junio C Hamano, Victoria Dye, Derrick Stolee,
Elijah Newren
In-Reply-To: <20260611072942.GG2191159@coredump.intra.peff.net>
On Thu, Jun 11, 2026 at 12:29 AM Jeff King <peff@peff.net> wrote:
>
> On Mon, Jun 08, 2026 at 07:36:34PM -0700, Tamir Duberstein wrote:
>
> > @@ -744,7 +745,7 @@ static void push_to_contains_stack(struct commit *candidate, struct contains_sta
> > }
> >
> > static enum contains_result contains_tag_algo(struct commit *candidate,
> > - const struct commit_list *want,
> > + struct commit_list *want,
> > struct contains_cache *cache)
>
> OK, we must lose the const here because repo_is_descendant_of() does not
> have it. We could add const to that function, though that cascades down
> to a few other helpers (see below). I'm not sure if that is making the
> world a better place, or if it is just const pedantry.
I left the signature change local rather than propagating const
through the other reachability helpers.
>
> diff --git a/commit-reach.c b/commit-reach.c
> index 5df471a313..8cede01f01 100644
> --- a/commit-reach.c
> +++ b/commit-reach.c
> @@ -563,7 +563,7 @@ int repo_get_merge_bases(struct repository *r,
> */
> int repo_is_descendant_of(struct repository *r,
> struct commit *commit,
> - struct commit_list *with_commit)
> + const struct commit_list *with_commit)
> {
> if (!with_commit)
> return 1;
> @@ -955,11 +955,12 @@ int can_all_from_reach_with_flag(struct object_array *from,
> return result;
> }
>
> -int can_all_from_reach(struct commit_list *from, struct commit_list *to,
> +int can_all_from_reach(const struct commit_list *from,
> + const struct commit_list *to,
> int cutoff_by_min_date)
> {
> struct object_array from_objs = OBJECT_ARRAY_INIT;
> - struct commit_list *from_iter = from, *to_iter = to;
> + const struct commit_list *from_iter = from, *to_iter = to;
> int result;
> timestamp_t min_commit_date = cutoff_by_min_date ? from->item->date : 0;
> timestamp_t min_generation = GENERATION_NUMBER_INFINITY;
> diff --git a/commit-reach.h b/commit-reach.h
> index 3f3a563d8a..76e82f827e 100644
> --- a/commit-reach.h
> +++ b/commit-reach.h
> @@ -37,7 +37,7 @@ int get_octopus_merge_bases(struct commit_list *in, struct commit_list **result)
>
> int repo_is_descendant_of(struct repository *r,
> struct commit *commit,
> - struct commit_list *with_commit);
> + const struct commit_list *with_commit);
> int repo_in_merge_bases(struct repository *r,
> struct commit *commit,
> struct commit *reference);
> @@ -93,7 +93,8 @@ int can_all_from_reach_with_flag(struct object_array *from,
> unsigned int assign_flag,
> timestamp_t min_commit_date,
> timestamp_t min_generation);
> -int can_all_from_reach(struct commit_list *from, struct commit_list *to,
> +int can_all_from_reach(const struct commit_list *from,
> + const struct commit_list *to,
> int commit_date_cutoff);
>
>
> > +cycle:
> > + free(contains_stack.contains_stack);
> > + clear_contains_cache(cache);
> > + init_contains_cache(cache);
> > +
> > + result = repo_is_descendant_of(the_repository, candidate, want);
> > + if (result < 0)
> > + exit(128);
>
> We are feeding the whole initial "want" list, so we should get a correct
> answer regardless of how far we got into the cycle, which would run into
> problems (e.g., if the cycle existed only on some branch of the
> history). But going back to the initial list will always be correct.
> Good.
>
> Two small points, though.
>
> One, the call to init_contains_cache() is redundant here; the clear
> function is documented as making things ready for use (it's a little
> hard to grep for, due to macros, but the docs are in commit-slab.h).
> It's probably not hurting anything.
>
> Two, the call to exit(128) is unusual for our code base (I'd guess it
> was cribbed off of the top-level exits in builtin/pull.c). We'd usually
> die() instead. Even if repo_is_descendant_of() produced its own error
> message, it may be useful to mention that we were falling back to it due
> to a cycle.
I removed the redundant initialization and replaced exit(128) with
die(), adding context that the failure occurred after detecting a
cycle.
>
> But even better is if we can return the error up the stack. We do not
> return errors from contains_tag_algo() currently, but it has only one
> caller. And that caller may also directly return the result of
> repo_is_descendant_of(). So could we just pass that along?
>
> Perhaps not. Looking at the callers of commit_contains(), they treat the
> result as a pure boolean. So probably calling die() is reasonable, and
> we already do so via parse_commit_or_die() elsewhere in the algorithm.
> That does leave a potential lurking bug for the non-tag-algo code path.
I traced the callers. Returning an error from commit_contains() would
only move the fatal check into apply_ref_filter(): its NULL return
already means “filtered out”, filter_and_format_refs() returns void, and
the branch caller ignores filter_refs()'s return value. Propagating the
error to the command would require changing that whole chain, and none
of the commands can recover from an unreadable commit.
The series therefore makes both cases fail explicitly. Patch 1 calls
die() if the cycle fallback cannot read the ancestry. Patch 3 calls
die() when the ordinary non-memoized walk returns -1.
>
> > + *contains_cache_at(cache, candidate) =
> > + result ? CONTAINS_YES : CONTAINS_NO;
> > + return result ? CONTAINS_YES : CONTAINS_NO;
>
> So we actually cache our discovered value. Cute, and it might save us
> from hitting the cycle again, though not always. E.g., two candidates A
> and B share a parent P, and the cycle starts at P but does not include A
> or B. We discover the cycle and cache the value for A, but discover it
> again for B.
>
> We do lose all of the existing non-cycle cached values when we call
> clear_contains_cache(). But we have to at least clear out all of the
> IN_PROGRESS commits. It is hard to care too much about optimizing the
> outcome for this case which we expect to happen approximately never.
> So I think doing the simplest correct thing is OK.
>
> > +test_expect_success 'tag --contains handles cyclic replacement histories' '
> > + first=$(git rev-parse HEAD~2) &&
> > + second=$(git rev-parse HEAD~) &&
> > + third=$(git rev-parse HEAD) &&
> > + test_when_finished "
> > + git replace -d $first
> > + git replace -d $third
> > + git tag -d cycle-a cycle-b
> > + " &&
>
> We usually &&-chain the commands inside test_when_finished. If they
> fail, the test harness will note this and complain (if the test was not
> otherwise failing). It's usually not a big deal either way, though
> sometimes it can catch silly mistakes (e.g., if you wrote $second
> instead of $third and the "replace -d" is quietly doing nothing at all).
Fixed in v3.
>
> I'm a little surprised that the chainlint checker doesn't catch this,
> but I guess it doesn't know to recurse into the snippet handed to
> test_when_finished. It probably is not really worth the trouble to teach
> it to do so.
>
> Otherwise the test looks good to me.
>
> -Peff
^ permalink raw reply
* Re: [RFC PATCH] MyFirstContribution: mention trimming quoted text in replies
From: Junio C Hamano @ 2026-06-11 23:48 UTC (permalink / raw)
To: Weijie Yuan; +Cc: git
In-Reply-To: <080402ff0ac8127b654dccea59a1bf643df62a5c.1781186476.git.wy@wyuan.org>
Weijie Yuan <wy@wyuan.org> writes:
> ReviewingGuidelines already advises reviewers to trim irrelevant quoted
> context when replying. Give the same advice to new contributors in
> MyFirstContribution, so our documentation is consistent about mailing
> list reply etiquette.
>
> Signed-off-by: Weijie Yuan <wy@wyuan.org>
Makes sense.
> diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
> index 607876f3d8..0e2a9313ce 100644
> --- a/Documentation/MyFirstContribution.adoc
> +++ b/Documentation/MyFirstContribution.adoc
> @@ -1453,6 +1453,11 @@ effect which had not occurred to you. It is always okay to ask for clarification
> if you aren't sure why a change was suggested, or what the reviewer is asking
> you to do.
>
> +When replying to review comments, quote only the parts of the message that are
> +relevant to your response. It is usually helpful to trim away unrelated context,
> +such as large portions of the patch that are not being discussed, while keeping
> +enough quoted text for readers to understand what you are responding to.
> +
> Make sure your email client has a plaintext email mode and it is turned on; the
> Git list rejects HTML email. Please also follow the mailing list etiquette
> outlined in the
The insertion point is well chosen, immediately following the
discussion on how to handle review comments and before the technical
details of email client configuration. The text itself is clear and
gives sound advice.
Will queue and wait for others to weigh in. Thanks.
^ permalink raw reply
* Re: [PATCH v2] log: improve --follow following renames for non-linear history
From: Junio C Hamano @ 2026-06-11 22:32 UTC (permalink / raw)
To: Miklos Vajna; +Cc: Jeff King, git
In-Reply-To: <aipTOsH8LKTSwglj@collabora.com>
Miklos Vajna <vmiklos@collabora.com> writes:
> situation when determining what path to follow for a specific commit
> with multiple previously visited children.
> ---
Missing sign-off; omitting sign-off to say that this is primarily
for requesting comments and not ready for application (often we see
RFC on the Subject line when this is done) is fine, though.
>> Can a "map" cut it?
>>
>> If a history forked at commit A, with two children commit B and
>> commit C, and you started traversing the history from a much later
>> descendant M that merges these two lines of history (i.e., M^1
>> contains B, M^2 contains C, and A==B^1==C^1), while traversing down
>> from M to B you may find that you need to follow path1 and similarly
>> somewhere between M down to C the path you are following may be
>> path2. And the traversal meets at A. The slab records path1 for B
>> and path2 for C. Wouldn't you need to be able to store both path1
>> and path2 for commit A? What path do you need to pay attention to
>> when traversing past A to its ancestors?
>
> Indeed, I focused on merge commits and their parents and I did not
> consider that slab[A] may be set to path1 when visiting one parent and
> then slab[A] may be set to path2 when visiting an other parent -- even
> if "A" itself is just a plain commit with no renames and is not a merge.
"A" in my example is a fork point. One of A's children may arrive
at A following path1 while another child may come to A following
path2. IOW, in the history below:
B---X---o---M---o---Z
/ /
A---C---Y---o
* A has the original path at "path0"; so do B and C.
* X renames "path0" to "path1"
* Y renames "path0" to "path2"
* M merges path1 coming from upper and path2 from lower history
and records the result at path "path".
* Z has "path".
You run "git log --follow Z -- path".
My answer to my (rhetorical) question (Can a "map" cut it?) actually
was "we probably can", since our "rename following" code does not
handle cases where two paths in a parent is merged into a single
path in a child, or a single path in a parent is split to form
multiple paths in a child.
So the "what path are we following?" slab would need to keep track
of a single path. From Z down to M, we follow "path". "path1" is
followed from M to X and "path2" is followed from M to Y. And from
X to A, and Y to A, we follow "path0". IOW, I did not think we need
two paths recorded for one commit.
Are any of your test cases added by this patch behave differently
with this version (vs the "single path assigned to each commit"
version you had earlier)? If so, then obviously there is some hole
in my above discussion.
One case that _could_ break down is if a rename on one track (say,
at Y) is so huge that it is not recognised as a rename. Then from Y
down to A we would probably try to track "path2" (because we fail to
notice that "path2" came from "path0") and declare that "path2"
appeared at Y from nowhere. But even then, we shouldn't propagate
"path2" down to A, so A would get only "path0" which was what we
follow going from X down to A, I think. Still no need for following
multiple paths at a fork point.
> + /* Any recorded paths for this commit? If so, restore it */
> + if (opt->diffopt.flags.follow_renames) {
> + paths = get_follow_pathspec_at(opt, commit);
> + if (!paths->nr) {
> + const char *path = pathspec_single_path(&opt->diffopt.pathspec);
> + if (path)
> + string_list_insert(paths, path);
We do not need to worry about deduplicating, as string_list_insert()
will automatically takes care of that for us, which is nice.
> + }
> + set_pathspec_to_paths(&opt->diffopt.pathspec, paths);
> + if (paths->nr > 1) {
> + /* diff_check_follow_pathspec() doesn't handle multiple paths */
> + saved_follow_renames = opt->diffopt.flags.follow_renames;
> + opt->diffopt.flags.follow_renames = 0;
> + }
> + }
Eek. That's a subtle workaround to break the built-in safety to
ensure there is only one pathspec element while following.
^ permalink raw reply
* [PATCH v3] update-ref: add --rename option
From: Junio C Hamano @ 2026-06-11 21:37 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt
In-Reply-To: <xmqqv7brz9ba.fsf@gitster.g>
Add a "--rename" option to "git update-ref" with the syntax:
$ git update-ref --rename <old-refname> <new-refname>
It renames <old-refname> together with its reflog to <new-refname>;
even when used on a local branch ref, the current value and the
reflog of the ref are the only things that are renamed. Document it
and redirect casual users to "git branch -m" if that is what they
wanted to do.
Because the "--stdin" mode wants to operate on its refs in a
reference transaction, and the API function refs_rename_ref() does
not work well as part of a transaction, it is currently not possible
to add a corresponding "rename" verb to the "--stdin" mode before
the underlying API learns to rename refs atomically inside a
transaction. It hence is left for a future refactoring.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* As a single patch topic, the range-diff relative to v2 is at the
end of the message.
- Simplified the proposed commit log message a bit.
- Dropped mention of --[no-]deref from the synopsis section.
- Reworded documentation with help from Patrick.
Documentation/git-update-ref.adoc | 9 +++++++++
builtin/update-ref.c | 32 +++++++++++++++++++++++++++++--
t/t1400-update-ref.sh | 24 +++++++++++++++++++++++
3 files changed, 63 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-update-ref.adoc b/Documentation/git-update-ref.adoc
index 37a5019a8b..3b4df23a86 100644
--- a/Documentation/git-update-ref.adoc
+++ b/Documentation/git-update-ref.adoc
@@ -9,6 +9,7 @@ SYNOPSIS
--------
[synopsis]
git update-ref [-m <reason>] [--no-deref] -d <ref> [<old-oid>]
+git update-ref [-m <reason>] --rename <old-refname> <new-refname>
git update-ref [-m <reason>] [--no-deref] [--create-reflog] <ref> <new-oid> [<old-oid>]
git update-ref [-m <reason>] [--no-deref] --stdin [-z] [--batch-updates]
@@ -39,6 +40,14 @@ the result of following the symbolic pointers.
With `-d`, it deletes the named <ref> after verifying that it
still contains <old-oid>.
+With `--rename`, it renames <old-refname> together with its reflog to
+<new-refname>. The command fails if <old-refname> does not exist, or
+if <new-refname> already exists. The command does not update any
+symbolic references pointing to the renamed reference, and neither
+does it update `.git/config` tracking settings. It is thus not
+recommended to use it for renaming local branches. Use `git branch -m`
+instead.
+
With `--stdin`, update-ref reads instructions from standard input and
performs all modifications together. Specify commands of the form:
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 2d68c40ecb..65ee8af08c 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -15,6 +15,7 @@
static const char * const git_update_ref_usage[] = {
N_("git update-ref [<options>] -d <refname> [<old-oid>]"),
N_("git update-ref [<options>] <refname> <new-oid> [<old-oid>]"),
+ N_("git update-ref [<options>] --rename <old-refname> <new-refname>"),
N_("git update-ref [<options>] --stdin [-z] [--batch-updates]"),
NULL
};
@@ -756,13 +757,14 @@ int cmd_update_ref(int argc,
{
const char *refname, *oldval;
struct object_id oid, oldoid;
- int delete = 0, no_deref = 0, read_stdin = 0, end_null = 0;
+ int delete = 0, rename = 0, no_deref = 0, read_stdin = 0, end_null = 0;
int create_reflog = 0;
unsigned int flags = 0;
struct option options[] = {
OPT_STRING( 'm', NULL, &msg, N_("reason"), N_("reason of the update")),
OPT_BOOL('d', NULL, &delete, N_("delete the reference")),
+ OPT_BOOL( 0 , "rename", &rename, N_("rename the reference")),
OPT_BOOL( 0 , "no-deref", &no_deref,
N_("update <refname> not the one it points to")),
OPT_BOOL('z', NULL, &end_null, N_("stdin has NUL-terminated arguments")),
@@ -787,7 +789,7 @@ int cmd_update_ref(int argc,
}
if (read_stdin) {
- if (delete || argc > 0)
+ if (delete || rename || argc > 0)
usage_with_options(git_update_ref_usage, options);
if (end_null)
line_termination = '\0';
@@ -800,6 +802,32 @@ int cmd_update_ref(int argc,
if (end_null)
usage_with_options(git_update_ref_usage, options);
+ if (rename) {
+ const char *oldref, *newref;
+
+ if (delete || argc != 2)
+ usage_with_options(git_update_ref_usage, options);
+
+ oldref = argv[0];
+ newref = argv[1];
+
+ if (check_refname_format(oldref, 0))
+ die("invalid ref format: %s", oldref);
+ if (check_refname_format(newref, 0))
+ die("invalid ref format: %s", newref);
+
+ if (!refs_ref_exists(get_main_ref_store(the_repository), oldref))
+ die("no ref named '%s'", oldref);
+
+ if (refs_ref_exists(get_main_ref_store(the_repository), newref))
+ die("ref '%s' already exists", newref);
+
+ if (refs_rename_ref(get_main_ref_store(the_repository),
+ oldref, newref, msg))
+ die("rename failed");
+ return 0;
+ }
+
if (delete) {
if (argc < 1 || argc > 2)
usage_with_options(git_update_ref_usage, options);
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index b2858a9061..4330cad282 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -2455,4 +2455,28 @@ test_expect_success 'dangling symref overwritten without old oid' '
test_must_fail git rev-parse --verify refs/heads/does-not-exist
'
+test_expect_success '--rename fails if old-refname does not exist' '
+ test_must_fail git update-ref --rename refs/tags/no-such-ref refs/tags/new-ref 2>err &&
+ test_grep "no ref named .refs/tags/no-such-ref." err
+'
+
+test_expect_success '--rename fails if new-refname does exist' '
+ git update-ref refs/tags/existing HEAD &&
+ git update-ref refs/tags/old-ref HEAD &&
+ test_must_fail git update-ref --rename refs/tags/old-ref refs/tags/existing 2>err &&
+ test_grep "ref .refs/tags/existing. already exists" err
+'
+
+test_expect_success '--rename moves old-refname and its reflog to new-refname' '
+ test_config core.logallrefupdates always &&
+ git update-ref -m "old tag" refs/tags/old-tag HEAD &&
+ git update-ref -m "to new" --rename refs/tags/old-tag refs/tags/new-tag 2>err &&
+ test_must_be_empty err &&
+ git show-ref --exists refs/tags/new-tag &&
+ test_must_fail git show-ref --exists refs/tags/old-tag &&
+ git log -g refs/tags/new-tag >output &&
+ test_grep "old tag" output &&
+ test_grep "to new" output
+'
+
test_done
Range-diff against v2:
1: 00cd13fda7 ! 1: a54c2d4d68 update-ref: add --rename option
@@ Commit message
$ git update-ref --rename <old-refname> <new-refname>
- It renames <old-refname> together with its reflog to <new-refname>
- (even when used on a local branch ref, the current value and the
- reflog of the ref are the only things that are renamed). As the
- command is a low-level plumbing command, attempts to rename branches
- are not warned, but we document it to draw attention of unsuspecting
- users and protect them from burning themselves.
+ It renames <old-refname> together with its reflog to <new-refname>;
+ even when used on a local branch ref, the current value and the
+ reflog of the ref are the only things that are renamed. Document it
+ and redirect casual users to "git branch -m" if that is what they
+ wanted to do.
Because the "--stdin" mode wants to operate on its refs in a
reference transaction, and the API function refs_rename_ref() does
@@ Documentation/git-update-ref.adoc: SYNOPSIS
--------
[synopsis]
git update-ref [-m <reason>] [--no-deref] -d <ref> [<old-oid>]
-+git update-ref [-m <reason>] [--no-deref] --rename <old-refname> <new-refname>
++git update-ref [-m <reason>] --rename <old-refname> <new-refname>
git update-ref [-m <reason>] [--no-deref] [--create-reflog] <ref> <new-oid> [<old-oid>]
git update-ref [-m <reason>] [--no-deref] --stdin [-z] [--batch-updates]
@@ Documentation/git-update-ref.adoc: the result of following the symbolic pointers
+With `--rename`, it renames <old-refname> together with its reflog to
+<new-refname>. The command fails if <old-refname> does not exist, or
-+if <new-refname> already exists. Because `git update-ref` does not
-+update active worktree `HEAD` symbolic references or `.git/config`
-+tracking settings when you rename a local branch in the `refs/heads/`
-+hierarchy, think twice before using this command to rename a local
-+branch (use `git branch -m` instead).
++if <new-refname> already exists. The command does not update any
++symbolic references pointing to the renamed reference, and neither
++does it update `.git/config` tracking settings. It is thus not
++recommended to use it for renaming local branches. Use `git branch -m`
++instead.
+
With `--stdin`, update-ref reads instructions from standard input and
performs all modifications together. Specify commands of the form:
--
2.55.0-rc0-119-ga57a595f62
^ permalink raw reply related
* Re: git-diff in a worktree is an order of magnitude slower?
From: brian m. carlson @ 2026-06-11 21:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, D. Ben Knoble, Git
In-Reply-To: <xmqqbjdhnfaf.fsf@gitster.g>
[-- Attachment #1: Type: text/plain, Size: 1283 bytes --]
On 2026-06-11 at 17:43:52, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > I guess the distinction goes back to c06ff4908b (Record ns-timestamps if
> > possible, but do not use it without USE_NSEC, 2009-03-04), which details
> > some reasons you might not want USE_NSEC. Feels like it ought to be a
> > run-time config, though, and maybe even something that gets auto-probed
> > by git-init.
>
> I thought for a bit but didn't think of a clean way to auto-probe if
> a filesystem loses nanosecond-precision part of .st_Xtime when
> "metadata is flushed and later read back in" with reasonable
> overhead. I do not think we want to trigger system-wide sync and/or
> dropping of buffer cache ;-)
We could have `git update-index` take options like it does for
`--untracked-cache` and `--no-untracked-cache` to control these for
people who want them. For instance, I know what operating system and
file system I'm using (Linux with btrfs), so if I know that option is
safe, I can enable it at runtime and reap the benefits.
We could even have `--test-use-nsec` to perform a `uname` and `statfs`
call to determine whether this is a known safe configuration if probing
is not possible.
--
brian m. carlson (they/them)
Toronto, Ontario, CA
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 325 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox