* kh/name-rev-custom-format
From: Kristoffer Haugsbakk @ 2026-05-11 13:22 UTC (permalink / raw)
To: Junio C Hamano, git
In-Reply-To: <xmqq4iketzh0.fsf@gitster.g>
On Mon, May 11, 2026, at 05:08, Junio C Hamano wrote:
> * kh/name-rev-custom-format (2026-05-07) 5 commits
> - format-rev: introduce builtin for on-demand pretty formatting
> - name-rev: make dedicated --annotate-stdin --name-only test
> - name-rev: factor code for sharing with a new command
> - name-rev: run clang-format before factoring code
> - name-rev: wrap both blocks in braces
>
> A new builtin "git format-rev" is introduced for pretty formatting
> one revision expression per line or commit object names found in
> running text.
>
> Will merge to 'next'.
> source: <V4_CV_format-rev.6aa@msgid.xyz>
I’m very glad that it ready for `next`. But... please hold off on
merging it to `next` for the next version. I managed to make an
AsciiDoc mistake.
Sorry about that.
^ permalink raw reply
* Re: [PATCH v7] checkout: extend --track with a "fetch" mode to refresh start-point
From: Phillip Wood @ 2026-05-11 13:16 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget, git
Cc: Ramsay Jones, D. Ben Knoble, Kristoffer Haugsbakk, Marc Branchaud,
Harald Nordgren
In-Reply-To: <pull.2281.v7.git.git.1778280727849.gitgitgadget@gmail.com>
Hi Harald
On 08/05/2026 23:52, Harald Nordgren via GitGitGadget wrote:
> From: Harald Nordgren <haraldnordgren@gmail.com>
>
> +static void fetch_remote_for_start_point(const char *arg)
> +{
> + char *remote_name = NULL;
> + char *src_ref = NULL;
> + struct child_process cmd = CHILD_PROCESS_INIT;
> + struct strbuf dst_ref = STRBUF_INIT;
> + int have_existing_ref = 0;
> +
> + if (resolve_fetch_target(arg, &remote_name, &src_ref))
> + return;
> +
> + if (src_ref) {
> + const char *short_src = src_ref;
> + struct object_id oid;
> +
> + skip_prefix(short_src, "refs/heads/", &short_src);
> + strbuf_addf(&dst_ref, "refs/remotes/%s/%s", remote_name, short_src);
> + if (!refs_read_ref(get_main_ref_store(the_repository),
> + dst_ref.buf, &oid))
> + have_existing_ref = 1;
src_ref is the name of the branch on the remote server, not the name of
the remote tracking ref which is given by arg. If arg is a remote name
then we need to resolve refs/remotes/$arg/HEAD to find the branch to
check, otherwise we should be checking refs/remotes/$arg
I've only given this version a quick scan through, but I didn't notice
any other issues.
Thanks
Phillip
> + }
> +
> + strvec_pushl(&cmd.args, "fetch", remote_name, NULL);
> + if (src_ref)
> + strvec_push(&cmd.args, src_ref);
> + cmd.git_cmd = 1;
> + if (run_command(&cmd)) {
> + if (have_existing_ref)
> + warning(_("failed to fetch start-point '%s'; "
> + "using existing '%s'"),
> + arg, dst_ref.buf);
> + else
> + die(_("failed to fetch start-point '%s'"), arg);
> + }
> +
> + free(remote_name);
> + free(src_ref);
> + strbuf_release(&dst_ref);
> +}
> +
> +static int parse_opt_checkout_track(const struct option *opt,
> + const char *arg, int unset)
> +{
> + struct checkout_opts *opts = opt->value;
> + struct string_list tokens = STRING_LIST_INIT_DUP;
> + struct string_list_item *item;
> + int saw_direct = 0, saw_inherit = 0;
> + int ret = 0;
> +
> + opts->fetch = 0;
> +
> + if (unset) {
> + opts->track = BRANCH_TRACK_NEVER;
> + return 0;
> + }
> +
> + opts->track = BRANCH_TRACK_EXPLICIT;
> + if (!arg)
> + return 0;
> +
> + string_list_split(&tokens, arg, ",", -1);
> + for_each_string_list_item(item, &tokens) {
> + if (!strcmp(item->string, "fetch")) {
> + opts->fetch = 1;
> + } else if (!strcmp(item->string, "direct")) {
> + saw_direct = 1;
> + opts->track = BRANCH_TRACK_EXPLICIT;
> + } else if (!strcmp(item->string, "inherit")) {
> + saw_inherit = 1;
> + opts->track = BRANCH_TRACK_INHERIT;
> + } else {
> + ret = error(_("option `%s' expects \"%s\", \"%s\", "
> + "or \"%s\""),
> + "--track", "direct", "inherit", "fetch");
> + goto out;
> + }
> + }
> +
> + if (saw_direct && saw_inherit)
> + ret = error(_("option `%s' cannot combine \"%s\" and \"%s\""),
> + "--track", "direct", "inherit");
> +
> +out:
> + string_list_clear(&tokens, 0);
> + return ret;
> +}
> +
> static void branch_info_release(struct branch_info *info)
> {
> free(info->name);
> @@ -1237,7 +1391,6 @@ static int git_checkout_config(const char *var, const char *value,
> opts->dwim_new_local_branch = git_config_bool(var, value);
> return 0;
> }
> -
> if (starts_with(var, "submodule."))
> return git_default_submodule_config(var, value, NULL);
>
> @@ -1734,10 +1887,10 @@ static struct option *add_common_switch_branch_options(
> {
> struct option options[] = {
> OPT_BOOL('d', "detach", &opts->force_detach, N_("detach HEAD at named commit")),
> - OPT_CALLBACK_F('t', "track", &opts->track, "(direct|inherit)",
> + OPT_CALLBACK_F('t', "track", opts, "(direct|inherit|fetch)[,...]",
> N_("set branch tracking configuration"),
> PARSE_OPT_OPTARG,
> - parse_opt_tracking_mode),
> + parse_opt_checkout_track),
> OPT__FORCE(&opts->force, N_("force checkout (throw away local modifications)"),
> PARSE_OPT_NOCOMPLETE),
> OPT_STRING(0, "orphan", &opts->new_orphan_branch, N_("new-branch"), N_("new unborn branch")),
> @@ -1942,8 +2095,13 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
> opts->dwim_new_local_branch &&
> opts->track == BRANCH_TRACK_UNSPECIFIED &&
> !opts->new_branch;
> - int n = parse_branchname_arg(argc, argv, dwim_ok, which_command,
> - &new_branch_info, opts, &rev);
> + int n;
> +
> + if (opts->fetch)
> + fetch_remote_for_start_point(argv[0]);
> +
> + n = parse_branchname_arg(argc, argv, dwim_ok, which_command,
> + &new_branch_info, opts, &rev);
> argv += n;
> argc -= n;
> } else if (!opts->accept_ref && opts->from_treeish) {
> diff --git a/t/t7201-co.sh b/t/t7201-co.sh
> index 9bcf7c0b40..19ac6a1a2e 100755
> --- a/t/t7201-co.sh
> +++ b/t/t7201-co.sh
> @@ -801,4 +801,136 @@ test_expect_success 'tracking info copied with autoSetupMerge=inherit' '
> test_cmp_config "" --default "" branch.main2.merge
> '
>
> +test_expect_success 'setup upstream for --track=fetch tests' '
> + git checkout main &&
> + git init fetch_upstream &&
> + test_commit -C fetch_upstream u_main &&
> + git remote add fetch_upstream fetch_upstream &&
> + git fetch fetch_upstream &&
> + git -C fetch_upstream checkout -b fetch_new &&
> + test_commit -C fetch_upstream u_new
> +'
> +
> +test_expect_success 'checkout --track=fetch -b picks up branch created upstream after clone' '
> + git checkout main &&
> + test_must_fail git rev-parse --verify refs/remotes/fetch_upstream/fetch_new &&
> + git checkout --track=fetch -b local_new fetch_upstream/fetch_new &&
> + test_cmp_rev refs/remotes/fetch_upstream/fetch_new HEAD &&
> + test_cmp_config fetch_upstream branch.local_new.remote &&
> + test_cmp_config refs/heads/fetch_new branch.local_new.merge
> +'
> +
> +test_expect_success 'checkout --track=fetch <remote>/<branch> leaves other tracking branches untouched' '
> + git checkout main &&
> + git -C fetch_upstream checkout -b fetch_target &&
> + test_commit -C fetch_upstream u_target_pre &&
> + git -C fetch_upstream checkout -b fetch_other &&
> + test_commit -C fetch_upstream u_other_pre &&
> + git fetch fetch_upstream &&
> + other_before=$(git rev-parse refs/remotes/fetch_upstream/fetch_other) &&
> + git -C fetch_upstream checkout fetch_target &&
> + test_commit -C fetch_upstream u_target_post &&
> + git -C fetch_upstream checkout fetch_other &&
> + test_commit -C fetch_upstream u_other_post &&
> + git checkout --track=fetch -b local_target fetch_upstream/fetch_target &&
> + test_cmp_rev refs/remotes/fetch_upstream/fetch_target HEAD &&
> + test "$(git rev-parse refs/remotes/fetch_upstream/fetch_other)" = "$other_before"
> +'
> +
> +test_expect_success 'checkout --track=fetch with bare remote name fetches only <remote>/HEAD target' '
> + git checkout main &&
> + git -C fetch_upstream checkout main &&
> + git remote set-head fetch_upstream main &&
> + git -C fetch_upstream checkout -b fetch_unrelated &&
> + test_commit -C fetch_upstream u_unrelated_pre &&
> + git fetch fetch_upstream fetch_unrelated &&
> + unrelated_before=$(git rev-parse refs/remotes/fetch_upstream/fetch_unrelated) &&
> + git -C fetch_upstream checkout main &&
> + test_commit -C fetch_upstream u_main_post &&
> + git -C fetch_upstream checkout fetch_unrelated &&
> + test_commit -C fetch_upstream u_unrelated_post &&
> + git checkout --track=fetch -b local_from_remote fetch_upstream &&
> + test_cmp_rev refs/remotes/fetch_upstream/main HEAD &&
> + test "$(git rev-parse refs/remotes/fetch_upstream/fetch_unrelated)" = "$unrelated_before"
> +'
> +
> +test_expect_success 'checkout --track=fetch aborts and does not create branch when no existing ref' '
> + git checkout main &&
> + test_might_fail git branch -D bogus &&
> + test_must_fail git checkout --track=fetch -b bogus fetch_upstream/does_not_exist &&
> + test_must_fail git rev-parse --verify refs/heads/bogus
> +'
> +
> +test_expect_success 'checkout --track=fetch warns and proceeds when fetch fails but ref exists' '
> + git checkout main &&
> + git -C fetch_upstream checkout -b fetch_offline &&
> + test_commit -C fetch_upstream u_offline &&
> + git fetch fetch_upstream fetch_offline &&
> + saved_url=$(git config remote.fetch_upstream.url) &&
> + test_when_finished "git config remote.fetch_upstream.url \"$saved_url\"" &&
> + git config remote.fetch_upstream.url ./does-not-exist &&
> + git checkout --track=fetch -b local_offline fetch_upstream/fetch_offline 2>err &&
> + test_grep "failed to fetch" err &&
> + test_cmp_rev refs/remotes/fetch_upstream/fetch_offline HEAD
> +'
> +
> +test_expect_success 'checkout --track=fetch resolves through configured fetch refspec' '
> + git checkout main &&
> + git -C fetch_upstream checkout -b fetch_refspec &&
> + test_commit -C fetch_upstream u_refspec &&
> + git fetch fetch_upstream fetch_refspec &&
> + git remote add fetch_custom ./fetch_upstream &&
> + test_when_finished "git remote remove fetch_custom" &&
> + git config --replace-all remote.fetch_custom.fetch \
> + "+refs/heads/*:refs/remotes/custom-ns/*" &&
> + git fetch fetch_custom &&
> + test_commit -C fetch_upstream u_refspec_post &&
> + git checkout --track=fetch -b local_refspec custom-ns/fetch_refspec &&
> + test_cmp_rev refs/remotes/custom-ns/fetch_refspec HEAD
> +'
> +
> +test_expect_success 'checkout --track=inherit,direct is rejected' '
> + test_must_fail git checkout --track=inherit,direct -b bad fetch_upstream/fetch_new 2>err &&
> + test_grep "cannot combine" err
> +'
> +
> +test_expect_success 'checkout --track=fetch then --track=direct drops fetch (last-one-wins)' '
> + git checkout main &&
> + git -C fetch_upstream checkout -b fetch_lastwin &&
> + test_commit -C fetch_upstream u_lastwin &&
> + test_must_fail git rev-parse --verify refs/remotes/fetch_upstream/fetch_lastwin &&
> + test_must_fail git checkout --track=fetch --track=direct \
> + -b local_lastwin fetch_upstream/fetch_lastwin &&
> + test_must_fail git rev-parse --verify refs/remotes/fetch_upstream/fetch_lastwin
> +'
> +
> +test_expect_success 'checkout --track=fetch,inherit fetches and inherits' '
> + git checkout main &&
> + git -C fetch_upstream checkout -b fetch_inherit &&
> + test_commit -C fetch_upstream u_inherit &&
> + git fetch fetch_upstream fetch_inherit &&
> + git checkout -b base_inherit fetch_upstream/fetch_inherit &&
> + test_commit -C fetch_upstream u_inherit2 &&
> + git checkout main &&
> + git checkout --track=fetch,inherit -b local_inherit base_inherit &&
> + test_cmp_rev refs/remotes/fetch_upstream/fetch_inherit HEAD &&
> + test_cmp_config fetch_upstream branch.local_inherit.remote &&
> + test_cmp_config refs/heads/fetch_inherit branch.local_inherit.merge
> +'
> +
> +test_expect_success 'checkout --track=bogus reports an error' '
> + git checkout main &&
> + test_must_fail git checkout --track=bogus -b bogus_branch fetch_upstream/fetch_new 2>err &&
> + test_grep "expects" err
> +'
> +
> +test_expect_success 'switch --track=fetch -c picks up branch created upstream after clone' '
> + git checkout main &&
> + git -C fetch_upstream checkout -b fetch_switch &&
> + test_commit -C fetch_upstream u_switch &&
> + test_must_fail git rev-parse --verify refs/remotes/fetch_upstream/fetch_switch &&
> + git switch --track=fetch -c local_switch fetch_upstream/fetch_switch &&
> + test_cmp_rev refs/remotes/fetch_upstream/fetch_switch HEAD
> +'
> +
> test_done
>
> base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
^ permalink raw reply
* Re: [PATCH v2 6/8] promisor-remote: trust known remotes matching acceptFromServerUrl
From: Toon Claes @ 2026-05-11 13:10 UTC (permalink / raw)
To: Christian Couder, git
Cc: Junio C Hamano, Patrick Steinhardt, Taylor Blau, Karthik Nayak,
Elijah Newren, Christian Couder, Christian Couder
In-Reply-To: <20260427124108.3524129-7-christian.couder@gmail.com>
Christian Couder <christian.couder@gmail.com> writes:
> +static bool match_one_url(const struct url_info *pi, const struct url_info *ui)
> +{
> + const char *pat = pi->url;
> + const char *url = ui->url;
> + char *p_str, *u_str;
> + bool res;
> +
> + /*
> + * Schemes must match exactly. They are case-folded by
> + * url_normalize(), so strncmp() suffices.
> + */
> + if (pi->scheme_len != ui->scheme_len || strncmp(pat, url, pi->scheme_len))
> + return false;
> +
> + /*
> + * Ports must match exactly. url_normalize() strips default
> + * ports (like 443 for https), so length and content
> + * comparisons are sufficient.
> + */
> + if (pi->port_len != ui->port_len ||
> + strncmp(pat + pi->port_off, url + ui->port_off, pi->port_len))
> + return false;
> +
> + /*
> + * Match host and path separately to prevent a '*' in the host
> + * portion of the pattern from matching across the '/'
> + * boundary into the path. Use WM_PATHNAME for the host so '*'
> + * cannot cross '/' there, and 0 for the path so '*' can still
> + * match multi-level paths.
> + */
> +
> + p_str = xstrndup(pat + pi->host_off, pi->host_len);
> + u_str = xstrndup(url + ui->host_off, ui->host_len);
> + res = !wildmatch(p_str, u_str, WM_PATHNAME);
> + free(p_str);
> + free(u_str);
> +
> + if (!res)
I feel it's a bit confusing your negating the result from wildmatch()
to negate it here again? Maybe keep using the int return value, or
rename the variable to 'matches' ?
> + return false;
> +
> + p_str = xstrndup(pat + pi->path_off, pi->path_len);
> + u_str = xstrndup(url + ui->path_off, ui->path_len);
> + res = !wildmatch(p_str, u_str, 0);
> + free(p_str);
> + free(u_str);
> +
> + return res;
> +}
--
Cheers,
Toon
^ permalink raw reply
* Re: [PATCH v2 7/8] promisor-remote: auto-configure unknown remotes
From: Toon Claes @ 2026-05-11 13:06 UTC (permalink / raw)
To: Christian Couder, git
Cc: Junio C Hamano, Patrick Steinhardt, Taylor Blau, Karthik Nayak,
Elijah Newren, Christian Couder, Christian Couder
In-Reply-To: <20260427124108.3524129-8-christian.couder@gmail.com>
Christian Couder <christian.couder@gmail.com> writes:
> Previous commits have introduced the `promisor.acceptFromServerUrl`
> config variable to allowlist some URLs advertised by a server through
> the "promisor-remote" protocol capability.
>
> However the new `promisor.acceptFromServerUrl` mechanism, like the old
> `promisor.acceptFromServer` mechanism, still requires a remote to
> already exist in the client's local configuration before it can be
> accepted. This places a significant manual burden on users to
> pre-configure these remotes, and creates friction for administrators
> who have to troubleshoot or manually provision these setups for their
> teams.
>
> To eliminate this burden, let's automatically create a new `[remote]`
> section in the client's config when a server advertises an unknown
> remote whose URL matches a `promisor.acceptFromServerUrl` glob pattern.
>
> Concretely, let's add four helpers:
>
> - sanitize_remote_name(): turn an arbitrary URL-derived string into a
> valid remote name by replacing non-alphanumeric characters,
> collapsing runs of '-', and prepending "promisor-auto-".
>
> - promisor_remote_name_from_url(): normalize the URL and extract
> host+port+path to build a human-readable base name, then pass it
> through sanitize_remote_name().
>
> - configure_auto_promisor_remote(): write the remote.*.url,
> remote.*.promisor and remote.*.advertisedAs keys to the repo
> config.
>
> - handle_matching_allowed_url(): pick the final name (user-supplied
> alias or auto-generated), handle collisions by appending "-1",
> "-2", etc., then call configure_auto_promisor_remote().
>
> Let's also add should_accept_new_remote_url() which reuses the
> url_matches_accept_list() helper introduced in a previous commit to
> find a matching pattern, then delegates to handle_matching_allowed_url()
> to create the remote.
>
> And then let's call should_accept_new_remote_url() from the '!item'
> (unknown remote) branch of should_accept_remote(), setting
> `reload_config` so that the newly-written config is picked up.
>
> Finally let's document all that by:
>
> - expanding the `promisor.acceptFromServerUrl` entry to describe
> auto-creation, the optional "name=" prefix syntax, the
> "promisor-auto-*" generation rules, and numeric-suffix collision
> handling, and by
> - adding a "remote.<name>.advertisedAs" entry to "remote.adoc".
>
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
> Documentation/config/promisor.adoc | 26 +++-
> Documentation/config/remote.adoc | 9 ++
> promisor-remote.c | 202 +++++++++++++++++++++++++-
> t/t5710-promisor-remote-capability.sh | 104 +++++++++++++
> 4 files changed, 332 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/config/promisor.adoc b/Documentation/config/promisor.adoc
> index efc066c3f2..ae1686a6e0 100644
> --- a/Documentation/config/promisor.adoc
> +++ b/Documentation/config/promisor.adoc
> @@ -54,7 +54,8 @@ promisor.acceptFromServer::
> promisor.acceptFromServerUrl::
> A glob pattern to specify which server-advertised URLs a
> client is allowed to act on. When a URL matches, the client
> - will accept the advertised remote as a promisor remote and may
> + will accept the advertised remote as a promisor remote, may
> + automatically create a new remote configuration for it and may
> automatically accept field updates (such as authentication
> tokens) from the server, even if `promisor.acceptFromServer`
> is set to `none` (the default).
> @@ -66,9 +67,10 @@ this option in _ANY_ config file read by Git.
> Be _VERY_ careful with these patterns: `*` matches any sequence of
> characters within the 'host' and 'path' parts of a URL (but cannot
> cross part boundaries). An overly broad pattern is a major security
> -risk, as a matching URL allows a server to update fields (such as
> -authentication tokens) on known remotes without further confirmation.
> -To minimize security risks, follow these guidelines:
> +risk, as a matching URL allows a server to auto-configure new remotes
> +and to update fields (such as authentication tokens) on known remotes
> +without further confirmation. To minimize security risks, follow these
> +guidelines:
> +
> 1. Start with a secure protocol scheme, like `https://` or `ssh://`.
> +
> @@ -99,6 +101,22 @@ are resolved. The port must also match exactly (e.g.,
> `https://example.com:8080/*` will not match a URL advertised on
> port 9999).
> +
> +The glob pattern can optionally be prefixed with a remote name and an
> +equals sign (e.g., `cdn=https://cdn.example.com/*`). If such a prefix
> +is provided, accepted remotes will be saved under that name. If no
> +such prefix is provided, a safe remote name will be automatically
> +generated by sanitizing the URL and prefixing it with
> +`promisor-auto-`.
> ++
> +If a remote with the chosen name already exists but points to a
> +different URL, Git will append a numeric suffix (e.g., `-1`, `-2`) to
> +the name to prevent overwriting existing configurations. You should
> +make sure that this doesn't happen often though, as remotes will be
> +rejected if the numeric suffix increases too much. In all cases, the
> +original name advertised by the server is recorded in the
> +`remote.<name>.advertisedAs` configuration variable for tracing and
> +debugging purposes.
> ++
> For the security implications of accepting a promisor remote, see the
> documentation of `promisor.acceptFromServer`. For details on the
> protocol, see linkgit:gitprotocol-v2[5].
> diff --git a/Documentation/config/remote.adoc b/Documentation/config/remote.adoc
> index 91e46f66f5..6e2bbdf457 100644
> --- a/Documentation/config/remote.adoc
> +++ b/Documentation/config/remote.adoc
> @@ -91,6 +91,15 @@ remote.<name>.promisor::
> When set to true, this remote will be used to fetch promisor
> objects.
>
> +remote.<name>.advertisedAs::
> + When a promisor remote is automatically configured using
> + information advertised by a server through the
> + `promisor-remote` protocol capability (see
> + `promisor.acceptFromServerUrl`), the server's originally
> + advertised name is saved in this variable. This is for
> + information, tracing and debugging purposes. Users should not
> + typically modify or create such configuration entries.
> +
> remote.<name>.partialclonefilter::
> The filter that will be applied when fetching from this promisor remote.
> Changing or clearing this value will only affect fetches for new commits.
> diff --git a/promisor-remote.c b/promisor-remote.c
> index 72d5b94bf7..8c8a798fdb 100644
> --- a/promisor-remote.c
> +++ b/promisor-remote.c
> @@ -816,10 +816,197 @@ static struct allowed_url *url_matches_accept_list(
> return NULL;
> }
>
> -static int should_accept_remote(enum accept_promisor accept,
> +/*
> + * Sanitize the buffer to make it a valid remote name coming from the
> + * server by:
> + *
> + * - replacing any non alphanumeric character with a '-'
> + * - stripping any leading '-',
> + * - condensing multiple '-' into one,
> + * - prepending "promisor-auto-",
> + * - validating the result.
> + */
> +static int sanitize_remote_name(struct strbuf *buf, const char *url)
> +{
> + char prev = '-';
> + for (size_t i = 0; i < buf->len; ) {
> + if (!isalnum(buf->buf[i]))
> + buf->buf[i] = '-';
> + if (prev == '-' && buf->buf[i] == '-') {
> + strbuf_remove(buf, i, 1);
> + } else {
> + prev = buf->buf[i];
> + i++;
> + }
> + }
> +
> + strbuf_strip_suffix(buf, "-");
> +
> + if (!buf->len) {
> + warning(_("couldn't generate a valid remote name from "
> + "advertised url '%s', ignoring this remote"), url);
> + return -1;
> + }
> +
> + strbuf_insertstr(buf, 0, "promisor-auto-");
> +
> + if (!valid_remote_name(buf->buf)) {
> + warning(_("generated remote name '%s' from advertised url '%s' "
> + "is invalid, ignoring this remote"), buf->buf, url);
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +static char *promisor_remote_name_from_url(const char *url)
> +{
> + struct url_info url_info = { 0 };
> + char *normalized = url_normalize(url, &url_info);
> + struct strbuf buf = STRBUF_INIT;
> +
> + if (!normalized) {
> + warning(_("couldn't normalize advertised url '%s', "
> + "ignoring this remote"), url);
> + return NULL;
> + }
> +
> + if (url_info.host_len) {
> + strbuf_add(&buf, normalized + url_info.host_off, url_info.host_len);
> + strbuf_addch(&buf, '-');
> + }
> +
> + if (url_info.port_len) {
> + strbuf_add(&buf, normalized + url_info.port_off, url_info.port_len);
> + strbuf_addch(&buf, '-');
If the url doesn't have a path, this could lead to the name being
`example-com-8443`. But we have a MAX_REMOTES_WITH_SIMILAR_NAMES at 20,
would this be an issue for a second remote without configured name?
As far as I can tell from handle_matching_allowed_url(), it's no issue,
because the numeric `-%d` suffix is added and we never atoi() the number
from existing remotes in the config.
> + }
> +
> + if (url_info.path_len) {
> + strbuf_add(&buf, normalized + url_info.path_off, url_info.path_len);
> + strbuf_trim_trailing_dir_sep(&buf);
> + strbuf_strip_suffix(&buf, ".git");
> + }
> +
> + free(normalized);
> +
> + if (sanitize_remote_name(&buf, url)) {
> + strbuf_release(&buf);
> + return NULL;
> + }
> +
> + return strbuf_detach(&buf, NULL);
> +}
> +
> +static void configure_auto_promisor_remote(struct repository *repo,
> + const char *name,
> + const char *url,
> + const char *advertised_as,
> + bool reuse)
> +{
> + char *key;
> +
> + if (!reuse) {
> + fprintf(stderr, _("Auto-creating promisor remote '%s' for URL '%s'\n"),
> + name, url);
> +
> + key = xstrfmt("remote.%s.url", name);
> + repo_config_set_gently(repo, key, url);
> + free(key);
> + }
> +
> + /* NB: when reusing, this promotes an existing non-promisor remote */
> + key = xstrfmt("remote.%s.promisor", name);
> + repo_config_set_gently(repo, key, "true");
> + free(key);
> +
> + if (advertised_as) {
> + key = xstrfmt("remote.%s.advertisedAs", name);
> + repo_config_set_gently(repo, key, advertised_as);
> + free(key);
> + }
> +}
> +
> +#define MAX_REMOTES_WITH_SIMILAR_NAMES 20
> +
> +/* Return the allocated local name, or NULL on failure */
> +static char *handle_matching_allowed_url(struct repository *repo,
> + char *allowed_name,
> + const char *remote_url,
> + const char *remote_name)
> +{
> + char *name;
> + char *basename = allowed_name ?
> + xstrdup(allowed_name) :
> + promisor_remote_name_from_url(remote_url);
> + int i = 0;
> + bool reuse = false;
> +
> + if (!basename)
> + return NULL;
> +
> + name = xstrdup(basename);
> +
> + while (i < MAX_REMOTES_WITH_SIMILAR_NAMES) {
> + char *url_key = xstrfmt("remote.%s.url", name);
> + const char *existing_url;
> + int exists = !repo_config_get_string_tmp(repo, url_key, &existing_url);
> +
> + free(url_key);
> +
> + if (!exists)
> + break; /* Free to use */
> +
> + if (!strcmp(existing_url, remote_url)) {
> + reuse = true;
> + break; /* Same URL, so safe to reuse */
> + }
> +
> + i++;
> + free(name);
> + name = xstrfmt("%s-%d", basename, i);
> + }
> +
> + if (i < MAX_REMOTES_WITH_SIMILAR_NAMES) {
> + configure_auto_promisor_remote(repo, name,
> + remote_url, remote_name,
> + reuse);
> + } else {
> + warning(_("too many remotes accepted with name like '%s-X', "
> + "ignoring this remote"), basename);
> + FREE_AND_NULL(name);
> + }
> +
> + free(basename);
> + return name;
> +}
> +
> +static int should_accept_new_remote_url(struct repository *repo,
> + struct string_list *accept_urls,
> + struct promisor_info *advertised)
> +{
> + struct allowed_url *allowed = url_matches_accept_list(accept_urls,
> + advertised->url);
> + if (allowed) {
> + char *name = handle_matching_allowed_url(repo,
> + allowed->remote_name,
> + advertised->url,
> + advertised->name);
> + if (name) {
> + free((char *)advertised->local_name);
> + advertised->local_name = name;
> + return 1;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int should_accept_remote(struct repository *repo,
> + enum accept_promisor accept,
> struct promisor_info *advertised,
> struct string_list *accept_urls,
> - struct string_list *config_info)
> + struct string_list *config_info,
> + bool *reload_config)
> {
> struct promisor_info *p;
> struct string_list_item *item;
> @@ -837,9 +1024,13 @@ static int should_accept_remote(enum accept_promisor accept,
> /* Get config info for that promisor remote */
> item = string_list_lookup(config_info, remote_name);
>
> - if (!item)
> + if (!item) {
> /* We don't know about that remote */
> - return 0;
> + int res = should_accept_new_remote_url(repo, accept_urls, advertised);
> + if (res)
> + *reload_config = true;
> + return res;
> + }
>
> p = item->util;
>
> @@ -1097,7 +1288,8 @@ static void filter_promisor_remote(struct repository *repo,
> string_list_sort(&config_info);
> }
>
> - if (should_accept_remote(accept, advertised, &accept_urls, &config_info)) {
> + if (should_accept_remote(repo, accept, advertised, &accept_urls,
> + &config_info, &reload_config)) {
> if (!store_info)
> store_info = store_info_new(repo);
> if (promisor_store_advertised_fields(advertised, store_info))
> diff --git a/t/t5710-promisor-remote-capability.sh b/t/t5710-promisor-remote-capability.sh
> index 0659b2ac15..549acff23f 100755
> --- a/t/t5710-promisor-remote-capability.sh
> +++ b/t/t5710-promisor-remote-capability.sh
> @@ -458,6 +458,107 @@ test_expect_success "clone with 'None', URL allowlisted, but client has differen
> initialize_server 1 "$oid"
> '
>
> +test_expect_success "clone with URL allowlisted and no remote already configured" '
> + git -C server config promisor.advertise true &&
> + test_when_finished "rm -rf client" &&
> + test_when_finished "rm -f full_names" &&
> +
> + GIT_NO_LAZY_FETCH=0 git clone \
> + -c promisor.acceptfromserver=None \
> + -c promisor.acceptFromServerUrl="$ENCODED_TRASH_DIRECTORY_URL/*" \
> + --no-local --filter="blob:limit=5k" server client &&
So promisor.acceptFromServerUrl only works if promisor.acceptFromServer
is "none"? I mean which one should precedence? If
promisor.acceptFromServer is set to "all", the promisor remote is
accepted by the client, but not saved to the config. Is that
intentional? Should we document that?
> + # Check that exactly one remote has been auto-created, identified
> + # by "remote.<name>.advertisedAs" == "lop".
> + git -C client config get --all --show-names --regexp \
> + "remote\..*\.advertisedas" >full_names &&
> + test_line_count = 1 full_names &&
> + REMOTE_NAME=$(sed "s/^remote\.\(.*\)\.advertisedas .*$/\1/" full_names) &&
> +
> + # Check ".url" and ".promisor" values
> + printf "%s\n" "$TRASH_DIRECTORY_URL/lop" "true" >expect &&
> + git -C client config "remote.$REMOTE_NAME.url" >actual &&
> + git -C client config "remote.$REMOTE_NAME.promisor" >>actual &&
> + test_cmp expect actual &&
> +
> + # Check that the largest object is still missing on the server
> + check_missing_objects server 1 "$oid"
> +'
> +
> +test_expect_success "clone with named URL allowlisted and no pre-configured remote" '
> + git -C server config promisor.advertise true &&
> + test_when_finished "rm -rf client" &&
> +
> + GIT_NO_LAZY_FETCH=0 git clone \
> + -c promisor.acceptfromserver=None \
> + -c promisor.acceptFromServerUrl="cdn=$ENCODED_TRASH_DIRECTORY_URL/*" \
> + --no-local --filter="blob:limit=5k" server client &&
> +
> + # Check that a remote has been auto-created with the right "cdn" name and fields.
> + printf "%s\n" "$TRASH_DIRECTORY_URL/lop" "true" "lop" >expect &&
> + git -C client config "remote.cdn.url" >actual &&
> + git -C client config "remote.cdn.promisor" >>actual &&
> + git -C client config "remote.cdn.advertisedAs" >>actual &&
> + test_cmp expect actual &&
> +
> + # Check that the largest object is still missing on the server
> + check_missing_objects server 1 "$oid"
> +'
> +
> +test_expect_success "clone with URL allowlisted but colliding name" '
> + git -C server config promisor.advertise true &&
> + test_when_finished "rm -rf client" &&
> +
> + GIT_NO_LAZY_FETCH=0 git clone -c remote.cdn.promisor=true \
> + -c remote.cdn.fetch="+refs/heads/*:refs/remotes/lop/*" \
> + -c remote.cdn.url="https://example.com/cdn" \
> + -c promisor.acceptfromserver=None \
> + -c promisor.acceptFromServerUrl="cdn=$ENCODED_TRASH_DIRECTORY_URL/*" \
> + --no-local --filter="blob:limit=5k" server client &&
> +
> + # Check that a remote has been auto-created with the right "cdn-1" name and fields.
> + printf "%s\n" "$TRASH_DIRECTORY_URL/lop" "true" "lop" >expect &&
> + git -C client config "remote.cdn-1.url" >actual &&
> + git -C client config "remote.cdn-1.promisor" >>actual &&
> + git -C client config "remote.cdn-1.advertisedAs" >>actual &&
> + test_cmp expect actual &&
> +
> + # Check that the original "cdn" remote was not overwritten.
> + printf "%s\n" "https://example.com/cdn" "true" >expect &&
> + git -C client config "remote.cdn.url" >actual &&
> + git -C client config "remote.cdn.promisor" >>actual &&
> + test_cmp expect actual &&
> +
> + # Check that the largest object is still missing on the server
> + check_missing_objects server 1 "$oid"
> +'
> +
> +test_expect_success "clone with URL allowlisted and reusable remote" '
> + git -C server config promisor.advertise true &&
> + test_when_finished "rm -rf client" &&
> +
> + GIT_NO_LAZY_FETCH=0 git clone \
> + -c remote.cdn.fetch="+refs/heads/*:refs/remotes/lop/*" \
> + -c remote.cdn.url="$TRASH_DIRECTORY_URL/lop" \
> + -c promisor.acceptfromserver=None \
> + -c promisor.acceptFromServerUrl="cdn=$ENCODED_TRASH_DIRECTORY_URL/*" \
> + --no-local --filter="blob:limit=5k" server client &&
> +
> + # Check that the existing "cdn" remote has been properly updated.
> + printf "%s\n" "$TRASH_DIRECTORY_URL/lop" "true" "lop" "+refs/heads/*:refs/remotes/lop/*" >expect &&
> + git -C client config "remote.cdn.url" >actual &&
> + git -C client config "remote.cdn.promisor" >>actual &&
> + git -C client config "remote.cdn.advertisedAs" >>actual &&
> + git -C client config "remote.cdn.fetch" >>actual &&
> + test_cmp expect actual &&
> +
> + # Check that no new "cdn-1" remote has been created.
> + test_must_fail git -C client config "remote.cdn-1.url" &&
> +
> + # Check that the largest object is still missing on the server
> + check_missing_objects server 1 "$oid"
> +'
> +
> test_expect_success "clone with invalid promisor.acceptFromServerUrl" '
> git -C server config promisor.advertise true &&
> test_when_finished "rm -rf client" &&
> @@ -472,6 +573,9 @@ test_expect_success "clone with invalid promisor.acceptFromServerUrl" '
> # Check that a warning was emitted
> test_grep "invalid remote name '\''bad name'\''" err &&
>
> + # Check that no remote was auto-created
> + test_must_fail git -C client config get --regexp "remote\..*\.advertisedas" &&
> +
> # Check that the largest object is not missing on the server
> check_missing_objects server 0 "" &&
>
> --
> 2.54.0.19.gb68b9497aa
>
>
--
Cheers,
Toon
^ permalink raw reply
* [PATCH v4 2/2] commit-reach: early exit paint_down_to_common for single merge-base
From: Kristofer Karlsson via GitGitGadget @ 2026-05-11 12:59 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Kristofer Karlsson, Kristofer Karlsson
In-Reply-To: <pull.2109.v4.git.1778504352.gitgitgadget@gmail.com>
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(-)
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;
struct commit_list *result = NULL, *r;
if (repo_get_merge_bases_many_dirty(the_repository, rev[0],
rev_nr - 1, rev + 1,
- 0, &result) < 0) {
+ flags, &result) < 0) {
commit_list_free(result);
return -1;
}
diff --git a/commit-reach.c b/commit-reach.c
index 766ba1156a..5a52be90a6 100644
--- a/commit-reach.c
+++ b/commit-reach.c
@@ -97,6 +97,14 @@ static int paint_down_to_common(struct repository *r,
if (!(commit->object.flags & RESULT)) {
commit->object.flags |= RESULT;
tail = commit_list_append(commit, tail);
+ /*
+ * The queue is generation-ordered; no
+ * remaining common ancestor can be a
+ * descendant of this one.
+ */
+ if (!(mb_flags & MERGE_BASE_FIND_ALL) &&
+ generation < GENERATION_NUMBER_INFINITY)
+ break;
}
/* Mark parents of a found merge stale */
flags |= STALE;
@@ -247,7 +255,8 @@ static int remove_redundant_no_gen(struct repository *r,
min_generation = curr_generation;
}
if (paint_down_to_common(r, array[i], filled,
- work, min_generation, 0, &common)) {
+ work, min_generation,
+ MERGE_BASE_FIND_ALL, &common)) {
clear_commit_marks(array[i], all_flags);
clear_commit_marks_many(filled, work, all_flags);
commit_list_free(common);
@@ -477,7 +486,8 @@ int repo_get_merge_bases_many(struct repository *r,
struct commit **twos,
struct commit_list **result)
{
- return get_merge_bases_many_0(r, one, n, twos, 1, 0, result);
+ return get_merge_bases_many_0(r, one, n, twos, 1,
+ MERGE_BASE_FIND_ALL, result);
}
int repo_get_merge_bases_many_dirty(struct repository *r,
@@ -495,7 +505,8 @@ int repo_get_merge_bases(struct repository *r,
struct commit *two,
struct commit_list **result)
{
- return get_merge_bases_many_0(r, one, 1, &two, 1, 0, result);
+ return get_merge_bases_many_0(r, one, 1, &two, 1,
+ MERGE_BASE_FIND_ALL, result);
}
/*
@@ -540,7 +551,7 @@ int repo_in_merge_bases_many(struct repository *r, struct commit *commit,
struct commit_list *bases = NULL;
int ret = 0, i;
timestamp_t generation, max_generation = GENERATION_NUMBER_ZERO;
- enum merge_base_flags mb_flags = 0;
+ enum merge_base_flags mb_flags = MERGE_BASE_FIND_ALL;
if (ignore_missing_commits)
mb_flags |= MERGE_BASE_IGNORE_MISSING_COMMITS;
diff --git a/commit-reach.h b/commit-reach.h
index a3f2cd80eb..3f3a563d8a 100644
--- a/commit-reach.h
+++ b/commit-reach.h
@@ -19,9 +19,14 @@ int repo_get_merge_bases_many(struct repository *r,
struct commit_list **result);
enum merge_base_flags {
MERGE_BASE_IGNORE_MISSING_COMMITS = (1 << 0),
+ MERGE_BASE_FIND_ALL = (1 << 1),
};
-/* To be used only when object flags after this call no longer matter */
+/*
+ * To be used only when object flags after this call no longer matter.
+ * Without MERGE_BASE_FIND_ALL and with generation numbers available,
+ * returns after finding the first merge-base, skipping the STALE drain.
+ */
int repo_get_merge_bases_many_dirty(struct repository *r,
struct commit *one, size_t n,
struct commit **twos,
diff --git a/t/t6600-test-reach.sh b/t/t6600-test-reach.sh
index dc0421ed2f..51c23b7683 100755
--- a/t/t6600-test-reach.sh
+++ b/t/t6600-test-reach.sh
@@ -882,4 +882,44 @@ test_expect_success 'rev-list --maximal-only matches merge-base --independent' '
test_cmp expect.sorted actual.sorted
'
+# The following tests verify the early-exit optimisation in
+# paint_down_to_common when merge-base is invoked without --all.
+# Each test checks all four commit-graph configurations.
+
+merge_base_all_modes () {
+ test_when_finished rm -rf .git/objects/info/commit-graph &&
+ git merge-base "$@" >actual &&
+ test_cmp expect actual &&
+ cp commit-graph-full .git/objects/info/commit-graph &&
+ git merge-base "$@" >actual &&
+ test_cmp expect actual &&
+ cp commit-graph-half .git/objects/info/commit-graph &&
+ git merge-base "$@" >actual &&
+ test_cmp expect actual &&
+ cp commit-graph-no-gdat .git/objects/info/commit-graph &&
+ git merge-base "$@" >actual &&
+ test_cmp expect actual
+}
+
+test_expect_success 'merge-base without --all (unique base)' '
+ git rev-parse commit-5-3 >expect &&
+ merge_base_all_modes commit-5-7 commit-8-3
+'
+
+test_expect_success 'merge-base without --all is one of --all results' '
+ test_when_finished rm -rf .git/objects/info/commit-graph &&
+
+ cp commit-graph-full .git/objects/info/commit-graph &&
+ git merge-base --all commit-5-7 commit-4-8 commit-6-6 commit-8-3 >all &&
+ git merge-base commit-5-7 commit-4-8 commit-6-6 commit-8-3 >single &&
+ test_line_count = 1 single &&
+ grep -F -f single all &&
+
+ cp commit-graph-half .git/objects/info/commit-graph &&
+ git merge-base --all commit-5-7 commit-4-8 commit-6-6 commit-8-3 >all &&
+ git merge-base commit-5-7 commit-4-8 commit-6-6 commit-8-3 >single &&
+ test_line_count = 1 single &&
+ grep -F -f single all
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v4 1/2] commit-reach: introduce merge_base_flags enum
From: Kristofer Karlsson via GitGitGadget @ 2026-05-11 12:59 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Kristofer Karlsson, Kristofer Karlsson
In-Reply-To: <pull.2109.v4.git.1778504352.gitgitgadget@gmail.com>
From: Kristofer Karlsson <krka@spotify.com>
Replace the boolean ignore_missing_commits parameter in
paint_down_to_common() with an enum merge_base_flags, and thread
the flags through merge_bases_many(), get_merge_bases_many_0(),
and the public repo_get_merge_bases_many_dirty() API.
This makes callsites with boolean parameters easier to read and
prepares the function for additional flags in a subsequent commit.
No functional change: the single caller that used
ignore_missing_commits (repo_in_merge_bases_many) now sets
MERGE_BASE_IGNORE_MISSING_COMMITS in the flags word, and all
other callers pass 0.
Signed-off-by: Kristofer Karlsson <krka@spotify.com>
---
builtin/merge-base.c | 3 ++-
commit-reach.c | 23 +++++++++++++++--------
commit-reach.h | 5 +++++
3 files changed, 22 insertions(+), 9 deletions(-)
diff --git a/builtin/merge-base.c b/builtin/merge-base.c
index c7ee97fa6a..9b50b4660e 100644
--- a/builtin/merge-base.c
+++ b/builtin/merge-base.c
@@ -14,7 +14,8 @@ static int show_merge_base(struct commit **rev, size_t rev_nr, int show_all)
struct commit_list *result = NULL, *r;
if (repo_get_merge_bases_many_dirty(the_repository, rev[0],
- rev_nr - 1, rev + 1, &result) < 0) {
+ rev_nr - 1, rev + 1,
+ 0, &result) < 0) {
commit_list_free(result);
return -1;
}
diff --git a/commit-reach.c b/commit-reach.c
index d3a9b3ed6f..766ba1156a 100644
--- a/commit-reach.c
+++ b/commit-reach.c
@@ -54,7 +54,7 @@ static int paint_down_to_common(struct repository *r,
struct commit *one, int n,
struct commit **twos,
timestamp_t min_generation,
- int ignore_missing_commits,
+ enum merge_base_flags mb_flags,
struct commit_list **result)
{
struct prio_queue queue = { compare_commits_by_gen_then_commit_date };
@@ -118,7 +118,7 @@ static int paint_down_to_common(struct repository *r,
* corrupt commits would already have been
* dispatched with a `die()`.
*/
- if (ignore_missing_commits)
+ if (mb_flags & MERGE_BASE_IGNORE_MISSING_COMMITS)
return 0;
return error(_("could not parse commit %s"),
oid_to_hex(&p->object.oid));
@@ -136,6 +136,7 @@ static int paint_down_to_common(struct repository *r,
static int merge_bases_many(struct repository *r,
struct commit *one, int n,
struct commit **twos,
+ enum merge_base_flags mb_flags,
struct commit_list **result)
{
struct commit_list *list = NULL, **tail = result;
@@ -165,7 +166,7 @@ static int merge_bases_many(struct repository *r,
oid_to_hex(&twos[i]->object.oid));
}
- if (paint_down_to_common(r, one, n, twos, 0, 0, &list)) {
+ if (paint_down_to_common(r, one, n, twos, 0, mb_flags, &list)) {
commit_list_free(list);
return -1;
}
@@ -425,6 +426,7 @@ static int get_merge_bases_many_0(struct repository *r,
size_t n,
struct commit **twos,
int cleanup,
+ enum merge_base_flags mb_flags,
struct commit_list **result)
{
struct commit_list *list, **tail = result;
@@ -432,7 +434,7 @@ static int get_merge_bases_many_0(struct repository *r,
size_t cnt, i;
int ret;
- if (merge_bases_many(r, one, n, twos, result) < 0)
+ if (merge_bases_many(r, one, n, twos, mb_flags, result) < 0)
return -1;
for (i = 0; i < n; i++) {
if (one == twos[i])
@@ -475,16 +477,17 @@ int repo_get_merge_bases_many(struct repository *r,
struct commit **twos,
struct commit_list **result)
{
- return get_merge_bases_many_0(r, one, n, twos, 1, result);
+ return get_merge_bases_many_0(r, one, n, twos, 1, 0, result);
}
int repo_get_merge_bases_many_dirty(struct repository *r,
struct commit *one,
size_t n,
struct commit **twos,
+ enum merge_base_flags mb_flags,
struct commit_list **result)
{
- return get_merge_bases_many_0(r, one, n, twos, 0, result);
+ return get_merge_bases_many_0(r, one, n, twos, 0, mb_flags, result);
}
int repo_get_merge_bases(struct repository *r,
@@ -492,7 +495,7 @@ int repo_get_merge_bases(struct repository *r,
struct commit *two,
struct commit_list **result)
{
- return get_merge_bases_many_0(r, one, 1, &two, 1, result);
+ return get_merge_bases_many_0(r, one, 1, &two, 1, 0, result);
}
/*
@@ -537,6 +540,10 @@ int repo_in_merge_bases_many(struct repository *r, struct commit *commit,
struct commit_list *bases = NULL;
int ret = 0, i;
timestamp_t generation, max_generation = GENERATION_NUMBER_ZERO;
+ enum merge_base_flags mb_flags = 0;
+
+ if (ignore_missing_commits)
+ mb_flags |= MERGE_BASE_IGNORE_MISSING_COMMITS;
if (repo_parse_commit(r, commit))
return ignore_missing_commits ? 0 : -1;
@@ -555,7 +562,7 @@ int repo_in_merge_bases_many(struct repository *r, struct commit *commit,
if (paint_down_to_common(r, commit,
nr_reference, reference,
- generation, ignore_missing_commits, &bases))
+ generation, mb_flags, &bases))
ret = -1;
else if (commit->object.flags & PARENT2)
ret = 1;
diff --git a/commit-reach.h b/commit-reach.h
index 6012402dfc..a3f2cd80eb 100644
--- a/commit-reach.h
+++ b/commit-reach.h
@@ -17,10 +17,15 @@ int repo_get_merge_bases_many(struct repository *r,
struct commit *one, size_t n,
struct commit **twos,
struct commit_list **result);
+enum merge_base_flags {
+ MERGE_BASE_IGNORE_MISSING_COMMITS = (1 << 0),
+};
+
/* To be used only when object flags after this call no longer matter */
int repo_get_merge_bases_many_dirty(struct repository *r,
struct commit *one, size_t n,
struct commit **twos,
+ enum merge_base_flags mb_flags,
struct commit_list **result);
int get_octopus_merge_bases(struct commit_list *in, struct commit_list **result);
--
gitgitgadget
^ permalink raw reply related
* [PATCH v4 0/2] [RFC] commit-reach: skip STALE drain when only one merge-base needed
From: Kristofer Karlsson via GitGitGadget @ 2026-05-11 12:59 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Kristofer Karlsson
In-Reply-To: <pull.2109.v3.git.1778498532730.gitgitgadget@gmail.com>
Context for what this is all about.
I am working with a very large git monorepo and have been investigating
performance issues. After some digging I ended up looking more deeply into
git merge-base. I saw it had an --all parameter but the default is to only
return a single merge-base. Looking through the code and adding debug
timing, I realized that although the total time to compute the merge-base
was high, a very small amount of time was spent finding the initial
merge-base value that was later returned.
The optimization is actually quite dramatic in a large repo - runtime went
down from 5000ms to 50ms, so it's roughly a 100x optimization. This comes
from an exploding frontier of STALE commits to drain.
Thus, my idea is simply to return early from the function once we know what
will be returned. This only works if we find a candidate that we know will
not be pruned later - but fortunately if we have a commit graph with
generations we will visit commits in order such that it will actually not be
pruned.
CC: Derrick Stolee stolee@gmail.com
Changes since v1 (thanks Junio for the review):
* Dropped the has_gens variable entirely. If a commit has a finite
generation then it is in the commit-graph, and so are all its ancestors —
no additional check is needed to know the queue ordering is sound.
Without a commit-graph every commit gets INFINITY and the guard never
fires. This also avoids the misleading interaction with callers that pass
non-zero min_generation without having generation data.
* Simplified the early exit guard from three conditions to two: !find_all
&& generation < GENERATION_NUMBER_INFINITY.
* Fixed multi-line comment style per CodingGuidelines.
* Replaced "dominate" with concrete reasoning about queue ordering.
* Did not extract a helper function: after the simplifications above the
inner block is four lines and reads naturally inline. The right boundary
for a helper is not obvious (it could absorb just the result marking, or
also the RESULT flag check, or also the PARENT1|PARENT2 test) and each
level requires more local state passed by pointer. Happy to extract one
if preferred.
Changes since v2 (thanks Patrick for the suggestion):
* Split into two commits: the first is a pure refactoring that introduces
enum merge_base_flags and replaces the boolean ignore_missing_commits
parameter, the second adds the new MERGE_BASE_FIND_ALL flag and the early
exit optimization.
* Replaced the boolean find_all and ignore_missing_commits parameters in
paint_down_to_common() with a single enum merge_base_flags mb_flags,
reducing the function from 8 to 7 parameters. The enum is threaded
through merge_bases_many(), get_merge_bases_many_0(), and the public
repo_get_merge_bases_many_dirty() API.
* Named the enum merge_base_flags rather than paint_down_to_common_flags
since the flags express caller intent and are threaded through multiple
layers including the public API.
* Used mb_flags as the parameter name to avoid shadowing the existing local
int flags (commit object flags) inside paint_down_to_common().
Kristofer Karlsson (2):
commit-reach: introduce merge_base_flags enum
commit-reach: early exit paint_down_to_common for single merge-base
builtin/merge-base.c | 4 +++-
commit-reach.c | 36 +++++++++++++++++++++++++++---------
commit-reach.h | 12 +++++++++++-
t/t6600-test-reach.sh | 40 ++++++++++++++++++++++++++++++++++++++++
4 files changed, 81 insertions(+), 11 deletions(-)
base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2109%2Fspkrka%2Fmerge-base-early-exit-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2109/spkrka/merge-base-early-exit-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/2109
Range-diff vs v3:
1: e4dada892f ! 1: 12d9e1c85f commit-reach: early exit paint_down_to_common for single merge-base
@@ Metadata
Author: Kristofer Karlsson <krka@spotify.com>
## Commit message ##
- commit-reach: early exit paint_down_to_common for single merge-base
+ commit-reach: introduce merge_base_flags enum
- 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.
+ Replace the boolean ignore_missing_commits parameter in
+ paint_down_to_common() with an enum merge_base_flags, and thread
+ the flags through merge_bases_many(), get_merge_bases_many_0(),
+ and the public repo_get_merge_bases_many_dirty() API.
- Introduce enum merge_base_flags with MERGE_BASE_FIND_ALL and
- MERGE_BASE_IGNORE_MISSING_COMMITS, replacing the two boolean
- parameters in paint_down_to_common(). Thread the flags through
- merge_bases_many(), get_merge_bases_many_0(), and the public
- repo_get_merge_bases_many_dirty() API. git merge-base (without
- --all) passes 0, triggering the early exit.
+ This makes callsites with boolean parameters easier to read and
+ prepares the function for additional flags in a subsequent commit.
- 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
+ No functional change: the single caller that used
+ ignore_missing_commits (repo_in_merge_bases_many) now sets
+ MERGE_BASE_IGNORE_MISSING_COMMITS in the flags word, and all
+ other callers pass 0.
Signed-off-by: Kristofer Karlsson <krka@spotify.com>
## builtin/merge-base.c ##
-@@
-
- 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;
+@@ builtin/merge-base.c: static int show_merge_base(struct commit **rev, size_t rev_nr, int show_all)
struct commit_list *result = NULL, *r;
if (repo_get_merge_bases_many_dirty(the_repository, rev[0],
- rev_nr - 1, rev + 1, &result) < 0) {
+ rev_nr - 1, rev + 1,
-+ flags, &result) < 0) {
++ 0, &result) < 0) {
commit_list_free(result);
return -1;
}
@@ commit-reach.c: static int paint_down_to_common(struct repository *r,
struct commit_list **result)
{
struct prio_queue queue = { compare_commits_by_gen_then_commit_date };
-@@ commit-reach.c: static int paint_down_to_common(struct repository *r,
- if (!(commit->object.flags & RESULT)) {
- commit->object.flags |= RESULT;
- tail = commit_list_append(commit, tail);
-+ /*
-+ * The queue is generation-ordered; no
-+ * remaining common ancestor can be a
-+ * descendant of this one.
-+ */
-+ if (!(mb_flags & MERGE_BASE_FIND_ALL) &&
-+ generation < GENERATION_NUMBER_INFINITY)
-+ break;
- }
- /* Mark parents of a found merge stale */
- flags |= STALE;
@@ commit-reach.c: static int paint_down_to_common(struct repository *r,
* corrupt commits would already have been
* dispatched with a `die()`.
@@ commit-reach.c: static int merge_bases_many(struct repository *r,
commit_list_free(list);
return -1;
}
-@@ commit-reach.c: static int remove_redundant_no_gen(struct repository *r,
- min_generation = curr_generation;
- }
- if (paint_down_to_common(r, array[i], filled,
-- work, min_generation, 0, &common)) {
-+ work, min_generation,
-+ MERGE_BASE_FIND_ALL, &common)) {
- clear_commit_marks(array[i], all_flags);
- clear_commit_marks_many(filled, work, all_flags);
- commit_list_free(common);
@@ commit-reach.c: static int get_merge_bases_many_0(struct repository *r,
size_t n,
struct commit **twos,
@@ commit-reach.c: int repo_get_merge_bases_many(struct repository *r,
struct commit_list **result)
{
- return get_merge_bases_many_0(r, one, n, twos, 1, result);
-+ return get_merge_bases_many_0(r, one, n, twos, 1,
-+ MERGE_BASE_FIND_ALL, result);
++ return get_merge_bases_many_0(r, one, n, twos, 1, 0, result);
}
int repo_get_merge_bases_many_dirty(struct repository *r,
@@ commit-reach.c: int repo_get_merge_bases(struct repository *r,
struct commit_list **result)
{
- return get_merge_bases_many_0(r, one, 1, &two, 1, result);
-+ return get_merge_bases_many_0(r, one, 1, &two, 1,
-+ MERGE_BASE_FIND_ALL, result);
++ return get_merge_bases_many_0(r, one, 1, &two, 1, 0, result);
}
/*
@@ commit-reach.c: int repo_in_merge_bases_many(struct repository *r, struct commit
struct commit_list *bases = NULL;
int ret = 0, i;
timestamp_t generation, max_generation = GENERATION_NUMBER_ZERO;
-+ enum merge_base_flags mb_flags = MERGE_BASE_FIND_ALL;
++ enum merge_base_flags mb_flags = 0;
+
+ if (ignore_missing_commits)
+ mb_flags |= MERGE_BASE_IGNORE_MISSING_COMMITS;
@@ commit-reach.h: int repo_get_merge_bases_many(struct repository *r,
struct commit *one, size_t n,
struct commit **twos,
struct commit_list **result);
--/* To be used only when object flags after this call no longer matter */
+enum merge_base_flags {
-+ MERGE_BASE_FIND_ALL = (1 << 0),
-+ MERGE_BASE_IGNORE_MISSING_COMMITS = (1 << 1),
++ MERGE_BASE_IGNORE_MISSING_COMMITS = (1 << 0),
+};
+
-+/*
-+ * To be used only when object flags after this call no longer matter.
-+ * Without MERGE_BASE_FIND_ALL and with generation numbers available,
-+ * returns after finding the first merge-base, skipping the STALE drain.
-+ */
+ /* To be used only when object flags after this call no longer matter */
int repo_get_merge_bases_many_dirty(struct repository *r,
struct commit *one, size_t n,
struct commit **twos,
@@ commit-reach.h: int repo_get_merge_bases_many(struct repository *r,
struct commit_list **result);
int get_octopus_merge_bases(struct commit_list *in, struct commit_list **result);
-
- ## t/t6010-merge-base.sh ##
-@@ t/t6010-merge-base.sh: test_expect_success 'merge-base --octopus --all for complex tree' '
- test_cmp expected actual
- '
-
-+# The following tests verify that "git merge-base" (without --all)
-+# returns the same result with and without a commit-graph.
-+# This exercises the early-exit optimisation in paint_down_to_common
-+# that skips the STALE drain when generation numbers are available.
-+
-+test_expect_success 'setup for commit-graph tests' '
-+ git init graph-repo &&
-+ (
-+ cd graph-repo &&
-+
-+ # Build a forked DAG:
-+ #
-+ # L1---L2 (left)
-+ # /
-+ # S
-+ # \
-+ # R1---R2 (right)
-+ #
-+ test_commit GS &&
-+ git checkout -b left &&
-+ test_commit L1 &&
-+ test_commit L2 &&
-+ git checkout GS &&
-+ git checkout -b right &&
-+ test_commit GR1 &&
-+ test_commit GR2
-+ )
-+'
-+
-+test_expect_success 'merge-base without commit-graph' '
-+ (
-+ cd graph-repo &&
-+ rm -f .git/objects/info/commit-graph &&
-+ git merge-base left right >actual &&
-+ git rev-parse GS >expected &&
-+ test_cmp expected actual
-+ )
-+'
-+
-+test_expect_success 'merge-base with commit-graph' '
-+ (
-+ cd graph-repo &&
-+ git commit-graph write --reachable &&
-+ git merge-base left right >actual &&
-+ git rev-parse GS >expected &&
-+ test_cmp expected actual
-+ )
-+'
-+
-+test_expect_success 'merge-base --all with commit-graph' '
-+ (
-+ cd graph-repo &&
-+ git merge-base --all left right >actual &&
-+ git rev-parse GS >expected &&
-+ test_cmp expected actual
-+ )
-+'
-+
-+test_expect_success 'merge-base agrees with --all for single result' '
-+ (
-+ cd graph-repo &&
-+ git commit-graph write --reachable &&
-+ git merge-base left right >actual.single &&
-+ git merge-base --all left right >actual.all &&
-+ test_cmp actual.all actual.single
-+ )
-+'
-+
-+test_expect_success 'setup for deep chain commit-graph test' '
-+ git init deep-repo &&
-+ (
-+ cd deep-repo &&
-+
-+ # Build a deep forked DAG:
-+ #
-+ # L1--L2--...--L20 (left)
-+ # /
-+ # S
-+ # \
-+ # R1--R2--...--R20 (right)
-+ #
-+ test_commit DS &&
-+ git checkout -b left &&
-+ for i in $(test_seq 1 20)
-+ do
-+ test_commit DL$i || return 1
-+ done &&
-+ git checkout DS &&
-+ git checkout -b right &&
-+ for i in $(test_seq 1 20)
-+ do
-+ test_commit DR$i || return 1
-+ done
-+ )
-+'
-+
-+test_expect_success 'deep chain: merge-base matches with and without commit-graph' '
-+ (
-+ cd deep-repo &&
-+ rm -f .git/objects/info/commit-graph &&
-+ git merge-base left right >actual.no-graph &&
-+ git rev-parse DS >expected &&
-+ test_cmp expected actual.no-graph &&
-+ git commit-graph write --reachable &&
-+ git merge-base left right >actual.graph &&
-+ test_cmp expected actual.graph
-+ )
-+'
-+
-+test_expect_success 'deep chain: --all and non---all agree with commit-graph' '
-+ (
-+ cd deep-repo &&
-+ git commit-graph write --reachable &&
-+ git merge-base left right >actual.single &&
-+ git merge-base --all left right >actual.all &&
-+ test_cmp actual.all actual.single
-+ )
-+'
-+
- test_done
-
- ## t/t6600-test-reach.sh ##
-@@ t/t6600-test-reach.sh: test_expect_success 'rev-list --maximal-only matches merge-base --independent' '
- test_cmp expect.sorted actual.sorted
- '
-
-+# The following tests verify the early-exit optimisation in
-+# paint_down_to_common when merge-base is invoked without --all.
-+# Each test checks all four commit-graph configurations.
-+
-+merge_base_all_modes () {
-+ test_when_finished rm -rf .git/objects/info/commit-graph &&
-+ git merge-base "$@" >actual &&
-+ test_cmp expect actual &&
-+ cp commit-graph-full .git/objects/info/commit-graph &&
-+ git merge-base "$@" >actual &&
-+ test_cmp expect actual &&
-+ cp commit-graph-half .git/objects/info/commit-graph &&
-+ git merge-base "$@" >actual &&
-+ test_cmp expect actual &&
-+ cp commit-graph-no-gdat .git/objects/info/commit-graph &&
-+ git merge-base "$@" >actual &&
-+ test_cmp expect actual
-+}
-+
-+test_expect_success 'merge-base without --all (unique base)' '
-+ git rev-parse commit-5-3 >expect &&
-+ merge_base_all_modes commit-5-7 commit-8-3
-+'
-+
-+test_expect_success 'merge-base without --all is one of --all results' '
-+ test_when_finished rm -rf .git/objects/info/commit-graph &&
-+
-+ cp commit-graph-full .git/objects/info/commit-graph &&
-+ git merge-base --all commit-5-7 commit-4-8 commit-6-6 commit-8-3 >all &&
-+ git merge-base commit-5-7 commit-4-8 commit-6-6 commit-8-3 >single &&
-+ test_line_count = 1 single &&
-+ grep -F -f single all &&
-+
-+ cp commit-graph-half .git/objects/info/commit-graph &&
-+ git merge-base --all commit-5-7 commit-4-8 commit-6-6 commit-8-3 >all &&
-+ git merge-base commit-5-7 commit-4-8 commit-6-6 commit-8-3 >single &&
-+ test_line_count = 1 single &&
-+ grep -F -f single all
-+'
-+
- test_done
-: ---------- > 2: 19f1605067 commit-reach: early exit paint_down_to_common for single merge-base
--
gitgitgadget
^ permalink raw reply
* [PATCH 2/2] run-command: honor "gc.auto" for auto-maintenance
From: Patrick Steinhardt @ 2026-05-11 12:29 UTC (permalink / raw)
To: git
Cc: Jean-Christophe Manciot, Mikael Magnusson, Jeff King, Taylor Blau,
Derrick Stolee
In-Reply-To: <20260511-pks-maintenance-fix-lock-with-detach-v1-0-ccd7d62c9a40@pks.im>
The "gc.auto" configuration has traditionally been used to turn off
running git-gc(1) as part of our auto-maintenance. We have eventually
switched over to git-maintenance(1) in a95ce12430 (maintenance: replace
run_auto_gc(), 2020-09-17), and with 1942d48380 (maintenance: optionally
skip --auto process, 2020-08-28) we have introduced "maintenance.auto"
to control whether or not to run auto-maintenance.
At that point though we still shelled out to git-gc(1) internally. So
if "gc.auto=0" was set we would still _execute_ git-maintenance(1), but
the command would have exited fast because git-gc(1) itself knew to
honor the config key.
This has recently changed though, as we have adapted the default
maintenance strategy to not use git-gc(1) anymore. The consequence is
that "gc.auto=0" doesn't have an effect anymore, which is a somewhat
surprising change in behaviour for our users.
Adapt `run_auto_maintenance()` so that it knows to also read "gc.auto",
similar to how it also reads both "maintenance.autoDetach" and
"gc.autoDetach".
Reported-by: Jean-Christophe Manciot <actionmystique@gmail.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
run-command.c | 6 ++++--
t/t7900-maintenance.sh | 37 ++++++++++++++++++++++++++-----------
2 files changed, 30 insertions(+), 13 deletions(-)
diff --git a/run-command.c b/run-command.c
index c146a56532..1e7b789010 100644
--- a/run-command.c
+++ b/run-command.c
@@ -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;
/*
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index df0bbc1669..1f70462678 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -60,17 +60,32 @@ test_expect_success 'run [--auto|--quiet] with gc strategy' '
test_subcommand git gc --no-quiet --no-detach --skip-foreground-tasks <run-no-quiet.txt
'
-test_expect_success 'maintenance.auto config option' '
- GIT_TRACE2_EVENT="$(pwd)/default" git commit --quiet --allow-empty -m 1 &&
- test_subcommand git maintenance run --auto --quiet --detach <default &&
- GIT_TRACE2_EVENT="$(pwd)/true" \
- git -c maintenance.auto=true \
- commit --quiet --allow-empty -m 2 &&
- test_subcommand git maintenance run --auto --quiet --detach <true &&
- GIT_TRACE2_EVENT="$(pwd)/false" \
- git -c maintenance.auto=false \
- commit --quiet --allow-empty -m 3 &&
- test_subcommand ! git maintenance run --auto --quiet --detach <false
+for cfg in maintenance.auto gc.auto
+do
+ test_expect_success "$cfg config option" '
+ GIT_TRACE2_EVENT="$(pwd)/default" git commit --quiet --allow-empty -m 1 &&
+ test_subcommand git maintenance run --auto --quiet --detach <default &&
+ GIT_TRACE2_EVENT="$(pwd)/true" \
+ git -c $cfg=true commit --quiet --allow-empty -m 2 &&
+ test_subcommand git maintenance run --auto --quiet --detach <true &&
+ GIT_TRACE2_EVENT="$(pwd)/false" \
+ git -c $cfg=false commit --quiet --allow-empty -m 3 &&
+ test_subcommand ! git maintenance run --auto --quiet --detach <false
+ '
+done
+
+test_expect_success "maintenance.auto overrides gc.auto" '
+ test_when_finished "rm -f trace" &&
+
+ test_config maintenance.auto false &&
+ test_config gc.auto true &&
+ GIT_TRACE2_EVENT="$(pwd)/trace" git commit --quiet --allow-empty -m 1 &&
+ test_subcommand ! git maintenance run --auto --quiet --detach <trace &&
+
+ test_config maintenance.auto true &&
+ test_config gc.auto false &&
+ GIT_TRACE2_EVENT="$(pwd)/trace" git commit --quiet --allow-empty -m 1 &&
+ test_subcommand git maintenance run --auto --quiet --detach <trace
'
for cfg in maintenance.autoDetach gc.autoDetach
--
2.54.0.545.g6539524ca2.dirty
^ permalink raw reply related
* [PATCH 0/2] builtin/maintenance: fix locking and respect "gc.auto"
From: Patrick Steinhardt @ 2026-05-11 12:29 UTC (permalink / raw)
To: git
Cc: Jean-Christophe Manciot, Mikael Magnusson, Jeff King, Taylor Blau,
Derrick Stolee
Hi,
this patch series addresses the issues reported in [1]. The series is
built on top of Git 2.54.0.
Thanks!
Patrick
[1]: <CAKcFC3arsYExb5dCMQspo4V9UFDadFaj8Q4PUsMWZJw_eYrMzA@mail.gmail.com>
---
Patrick Steinhardt (2):
builtin/maintenance: fix locking with "--detach"
run-command: honor "gc.auto" for auto-maintenance
builtin/gc.c | 26 ++++++++++++--
lockfile.c | 9 +++++
lockfile.h | 10 ++++++
run-command.c | 6 ++--
setup.c | 31 +++++++++++-----
setup.h | 1 +
t/t7900-maintenance.sh | 95 ++++++++++++++++++++++++++++++++++++++++++++------
7 files changed, 154 insertions(+), 24 deletions(-)
---
base-commit: 13ef77ce6e222bef3ab145642e6ef1486075211c
change-id: 20260511-pks-maintenance-fix-lock-with-detach-a608e9b6adeb
^ permalink raw reply
* [PATCH 1/2] builtin/maintenance: fix locking with "--detach"
From: Patrick Steinhardt @ 2026-05-11 12:29 UTC (permalink / raw)
To: git
Cc: Jean-Christophe Manciot, Mikael Magnusson, Jeff King, Taylor Blau,
Derrick Stolee
In-Reply-To: <20260511-pks-maintenance-fix-lock-with-detach-v1-0-ccd7d62c9a40@pks.im>
When running git-maintenance(1), we create a lockfile that is supposed
to keep other maintenance processes from running at the same time. This
lockfile is broken though in case the "--detach" flag is passed: the
lockfile is created by the parent process and will be cleaned up either
manually or on exit. But when detaching, the parent will exit before all
of the background maintenance tasks have been ran, and consequently the
lock only covers a smaller part of the whole maintenance process.
Fix this bug by introducing two new functions:
- `daemonize_without_exit()` is the same as `daemonize()`, but doesn't
call exit(3p) for the parent process.
- `lock_file_reassign_owner()` reassigns the owner of its owned
tempfiles so that they don't get unlinked anymore when the previous
owner exits.
Together this allows us to reassign ownership of the lockfile after we
have daemonized so that the lockfile is now owned by the child process.
Reported-by: Jean-Christophe Manciot <actionmystique@gmail.com>
Helped-by: Jeff King <peff@peff.net>
Helped-by: Taylor Blau <me@ttaylorr.com>
Helped-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/gc.c | 26 ++++++++++++++++++++--
lockfile.c | 9 ++++++++
lockfile.h | 10 +++++++++
setup.c | 31 +++++++++++++++++++--------
setup.h | 1 +
t/t7900-maintenance.sh | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++
6 files changed, 124 insertions(+), 11 deletions(-)
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);
+ } else {
+ /*
+ * Failure to daemonize is ok, we'll continue in
+ * foreground.
+ */
+ }
+
trace2_region_leave("maintenance", "detach", the_repository);
}
diff --git a/lockfile.c b/lockfile.c
index 7add2f136a..96aab3c885 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -356,3 +356,12 @@ int rollback_lock_file(struct lock_file *lk)
delete_tempfile(&lk->pid_tempfile);
return delete_tempfile(&lk->tempfile);
}
+
+void lock_file_reassign_owner(struct lock_file *lk, pid_t owner)
+{
+ if (!is_lock_file_locked(lk))
+ BUG("cannot reassign ownership of unlocked lockfile");
+ lk->tempfile->owner = owner;
+ if (lk->pid_tempfile)
+ lk->pid_tempfile->owner = owner;
+}
diff --git a/lockfile.h b/lockfile.h
index e7233f28de..0b10b624fa 100644
--- a/lockfile.h
+++ b/lockfile.h
@@ -341,4 +341,14 @@ static inline int commit_lock_file_to(struct lock_file *lk, const char *path)
*/
int rollback_lock_file(struct lock_file *lk);
+/*
+ * Reassign ownership of the lockfile to a different process.
+ *
+ * This is intended for use after `fork(2)`-ing. The parent transfers ownership
+ * to the daemonized child so that its atexit handler does not unlink the lock
+ * that should outlive it, and the child claims the inherited tempfiles so that
+ * they are cleaned up when the daemon exits.
+ */
+void lock_file_reassign_owner(struct lock_file *lk, pid_t owner);
+
#endif /* LOCKFILE_H */
diff --git a/setup.c b/setup.c
index 7ec4427368..34deb6e985 100644
--- a/setup.c
+++ b/setup.c
@@ -2156,20 +2156,18 @@ void sanitize_stdfds(void)
close(fd);
}
-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
+}
+
struct template_dir_cb_data {
char *path;
int initialized;
diff --git a/setup.h b/setup.h
index 80bc6e5f07..396af8d808 100644
--- a/setup.h
+++ b/setup.h
@@ -150,6 +150,7 @@ int path_inside_repo(const char *prefix, const char *path);
void sanitize_stdfds(void);
int daemonize(void);
+pid_t daemonize_without_exit(void);
/*
* GIT_REPO_VERSION is the version we write by default. The
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 4700beacc1..df0bbc1669 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -1438,6 +1438,64 @@ test_expect_success '--no-detach causes maintenance to not run in background' '
)
'
+test_expect_success PIPE '--detach holds maintenance lock until daemonized child exits' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+
+ git config maintenance.auto false &&
+ git config core.lockfilepid true &&
+
+ git remote add origin /does/not/exist &&
+ git config set remote.origin.uploadpack "cat fifo-uploadpack" &&
+
+ mkfifo fifo-uploadpack fifo-maint &&
+
+ # Open the maintenance FIFO, as otherwise spawning
+ # git-maintenance(1) would block. Note that we need to open it
+ # as read-write, as otherwise we would block here already.
+ exec 9<>fifo-maint &&
+
+ { git maintenance run --task=prefetch --detach 7>&9 & } &&
+ parent="$!" &&
+
+ # Reap the parent process so that the exec call below will not
+ # get SIGCHLD.
+ wait "$parent" &&
+
+ # Open the git-upload-pack(1) FIFO for writing, which will
+ # block until the upload-pack script opens it for reading. Once
+ # exec returns, we know that the daemonized child is alive and
+ # pinned.
+ exec 8>fifo-uploadpack &&
+
+ test_path_is_file .git/objects/maintenance.lock &&
+ test_path_is_file .git/objects/"maintenance~pid.lock" &&
+
+ # Verify that the maintenance.lock still exists, and
+ # that it was created by the parent process, not the
+ # child.
+ echo "pid $parent" >expect &&
+ test_cmp expect .git/objects/"maintenance~pid.lock" &&
+
+ # Reopen the maintenance FIFO as read-only so that
+ # git-maintenance(1) is the only writer. This will cause it to
+ # close the FIFO once the process exits.
+ exec 9<&- &&
+ exec 9<fifo-maint &&
+
+ # Close the FIFO used by git-upload-pack(1) to unblock it and
+ # then wait until the maintenance FIFO is closed by
+ # git-maintenance(1), indicating that it has exited.
+ exec 8>&- &&
+ cat <&9 &&
+
+ test_path_is_missing .git/objects/maintenance.lock &&
+ test_path_is_missing .git/objects/"maintenance~pid.lock"
+ )
+'
+
test_expect_success '--detach causes maintenance to run in background' '
test_when_finished "rm -rf repo" &&
git init repo &&
--
2.54.0.545.g6539524ca2.dirty
^ permalink raw reply related
* [PATCH] sequencer: remove todo_add_branch_context.commit
From: Abhinav Gupta via GitGitGadget @ 2026-05-11 12:21 UTC (permalink / raw)
To: git; +Cc: Abhinav Gupta, Abhinav Gupta
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.
Signed-off-by: Abhinav Gupta <mail@abhinavg.net>
---
sequencer: remove todo_add_branch_context.commit
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2111%2Fabhinav%2Fsequencer-todoctx-rm-commit-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2111/abhinav/sequencer-todoctx-rm-commit-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2111
sequencer.c | 2 --
1 file changed, 2 deletions(-)
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);
}
}
base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH v3] commit-reach: early exit paint_down_to_common for single merge-base
From: Patrick Steinhardt @ 2026-05-11 12:04 UTC (permalink / raw)
To: Kristofer Karlsson via GitGitGadget; +Cc: git, Kristofer Karlsson
In-Reply-To: <pull.2109.v3.git.1778498532730.gitgitgadget@gmail.com>
On Mon, May 11, 2026 at 11:22:12AM +0000, Kristofer Karlsson via GitGitGadget wrote:
> Changes since v2 (thanks Patrick for the suggestion):
>
> * Replaced the boolean find_all and ignore_missing_commits parameters
> in paint_down_to_common() with a single enum merge_base_flags
> mb_flags, reducing the function from 8 to 7 parameters. The enum is
> defined in commit-reach.h with MERGE_BASE_FIND_ALL and
> MERGE_BASE_IGNORE_MISSING_COMMITS.
>
> * Named the enum merge_base_flags rather than
> paint_down_to_common_flags since the flags express caller intent and
> are threaded through multiple layers including the public
> repo_get_merge_bases_many_dirty() API.
>
> * Used mb_flags as the parameter name to avoid shadowing the existing
> local int flags (commit object flags) inside paint_down_to_common().
Thanks for making these changes. I think it would make sense to split
this up into two commits though, where the first commit only introduces
the new enum (without the new flag) and the second commit then adds the
new flag and the performance optimization.
That'd help quite a bit with the review as it splits up the changes into
a refactoring-only change without any intended semantic change, and
another commit that then does result in a user-visible change.
Patrick
^ permalink raw reply
* [PATCH v3] commit-reach: early exit paint_down_to_common for single merge-base
From: Kristofer Karlsson via GitGitGadget @ 2026-05-11 11:22 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Kristofer Karlsson, Kristofer Karlsson
In-Reply-To: <pull.2109.v2.git.1778480348118.gitgitgadget@gmail.com>
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.
Introduce enum merge_base_flags with MERGE_BASE_FIND_ALL and
MERGE_BASE_IGNORE_MISSING_COMMITS, replacing the two boolean
parameters in paint_down_to_common(). Thread the flags through
merge_bases_many(), get_merge_bases_many_0(), and the public
repo_get_merge_bases_many_dirty() API. 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>
---
[RFC] commit-reach: skip STALE drain when only one merge-base needed
Context for what this is all about.
I am working with a very large git monorepo and have been investigating
performance issues. After some digging I ended up looking more deeply
into git merge-base. I saw it had an --all parameter but the default is
to only return a single merge-base. Looking through the code and adding
debug timing, I realized that although the total time to compute the
merge-base was high, a very small amount of time was spent finding the
initial merge-base value that was later returned.
The optimization is actually quite dramatic in a large repo - runtime
went down from 5000ms to 50ms, so it's roughly a 100x optimization. This
comes from an exploding frontier of STALE commits to drain.
Thus, my idea is simply to return early from the function once we know
what will be returned. This only works if we find a candidate that we
know will not be pruned later - but fortunately if we have a commit
graph with generations we will visit commits in order such that it will
actually not be pruned.
CC: Derrick Stolee stolee@gmail.com
Changes since v1 (thanks Junio for the review):
* Dropped the has_gens variable entirely. If a commit has a finite
generation then it is in the commit-graph, and so are all its
ancestors — no additional check is needed to know the queue ordering
is sound. Without a commit-graph every commit gets INFINITY and the
guard never fires. This also avoids the misleading interaction with
callers that pass non-zero min_generation without having generation
data.
* Simplified the early exit guard from three conditions to two:
!find_all && generation < GENERATION_NUMBER_INFINITY.
* Fixed multi-line comment style per CodingGuidelines.
* Replaced "dominate" with concrete reasoning about queue ordering.
* Did not extract a helper function: after the simplifications above
the inner block is four lines and reads naturally inline. The right
boundary for a helper is not obvious (it could absorb just the result
marking, or also the RESULT flag check, or also the PARENT1|PARENT2
test) and each level requires more local state passed by pointer.
Happy to extract one if preferred.
Changes since v2 (thanks Patrick for the suggestion):
* Replaced the boolean find_all and ignore_missing_commits parameters
in paint_down_to_common() with a single enum merge_base_flags
mb_flags, reducing the function from 8 to 7 parameters. The enum is
defined in commit-reach.h with MERGE_BASE_FIND_ALL and
MERGE_BASE_IGNORE_MISSING_COMMITS.
* Named the enum merge_base_flags rather than
paint_down_to_common_flags since the flags express caller intent and
are threaded through multiple layers including the public
repo_get_merge_bases_many_dirty() API.
* Used mb_flags as the parameter name to avoid shadowing the existing
local int flags (commit object flags) inside paint_down_to_common().
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2109%2Fspkrka%2Fmerge-base-early-exit-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2109/spkrka/merge-base-early-exit-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/2109
Range-diff vs v2:
1: f7b5c267f3 ! 1: e4dada892f commit-reach: early exit paint_down_to_common for single merge-base
@@ Commit message
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 find_all is false 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.
+ 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 find_all parameter to repo_get_merge_bases_many_dirty() and
- thread it through to paint_down_to_common(). git merge-base
- (without --all) passes show_all=0, triggering the early exit.
+ Introduce enum merge_base_flags with MERGE_BASE_FIND_ALL and
+ MERGE_BASE_IGNORE_MISSING_COMMITS, replacing the two boolean
+ parameters in paint_down_to_common(). Thread the flags through
+ merge_bases_many(), get_merge_bases_many_0(), and the public
+ repo_get_merge_bases_many_dirty() API. git merge-base (without
+ --all) passes 0, triggering the early exit.
On a 2.2M-commit merge-heavy monorepo with commit-graph:
@@ Commit message
Signed-off-by: Kristofer Karlsson <krka@spotify.com>
## builtin/merge-base.c ##
-@@ builtin/merge-base.c: static int show_merge_base(struct commit **rev, size_t rev_nr, int show_all)
+@@
+
+ 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;
struct commit_list *result = NULL, *r;
if (repo_get_merge_bases_many_dirty(the_repository, rev[0],
- rev_nr - 1, rev + 1, &result) < 0) {
+ rev_nr - 1, rev + 1,
-+ show_all, &result) < 0) {
++ flags, &result) < 0) {
commit_list_free(result);
return -1;
}
## commit-reach.c ##
@@ commit-reach.c: static int paint_down_to_common(struct repository *r,
+ struct commit *one, int n,
struct commit **twos,
timestamp_t min_generation,
- int ignore_missing_commits,
-+ int find_all,
+- int ignore_missing_commits,
++ enum merge_base_flags mb_flags,
struct commit_list **result)
{
struct prio_queue queue = { compare_commits_by_gen_then_commit_date };
@@ commit-reach.c: static int paint_down_to_common(struct repository *r,
+ * remaining common ancestor can be a
+ * descendant of this one.
+ */
-+ if (!find_all &&
++ if (!(mb_flags & MERGE_BASE_FIND_ALL) &&
+ generation < GENERATION_NUMBER_INFINITY)
+ break;
}
/* Mark parents of a found merge stale */
flags |= STALE;
+@@ commit-reach.c: static int paint_down_to_common(struct repository *r,
+ * corrupt commits would already have been
+ * dispatched with a `die()`.
+ */
+- if (ignore_missing_commits)
++ if (mb_flags & MERGE_BASE_IGNORE_MISSING_COMMITS)
+ return 0;
+ return error(_("could not parse commit %s"),
+ oid_to_hex(&p->object.oid));
@@ commit-reach.c: static int paint_down_to_common(struct repository *r,
static int merge_bases_many(struct repository *r,
struct commit *one, int n,
struct commit **twos,
-+ int find_all,
++ enum merge_base_flags mb_flags,
struct commit_list **result)
{
struct commit_list *list = NULL, **tail = result;
@@ commit-reach.c: static int merge_bases_many(struct repository *r,
}
- if (paint_down_to_common(r, one, n, twos, 0, 0, &list)) {
-+ if (paint_down_to_common(r, one, n, twos, 0, 0, find_all, &list)) {
++ if (paint_down_to_common(r, one, n, twos, 0, mb_flags, &list)) {
commit_list_free(list);
return -1;
}
@@ commit-reach.c: static int remove_redundant_no_gen(struct repository *r,
}
if (paint_down_to_common(r, array[i], filled,
- work, min_generation, 0, &common)) {
-+ work, min_generation, 0, 1, &common)) {
++ work, min_generation,
++ MERGE_BASE_FIND_ALL, &common)) {
clear_commit_marks(array[i], all_flags);
clear_commit_marks_many(filled, work, all_flags);
commit_list_free(common);
@@ commit-reach.c: static int get_merge_bases_many_0(struct repository *r,
size_t n,
struct commit **twos,
int cleanup,
-+ int find_all,
++ enum merge_base_flags mb_flags,
struct commit_list **result)
{
struct commit_list *list, **tail = result;
@@ commit-reach.c: static int get_merge_bases_many_0(struct repository *r,
int ret;
- if (merge_bases_many(r, one, n, twos, result) < 0)
-+ if (merge_bases_many(r, one, n, twos, find_all, result) < 0)
++ if (merge_bases_many(r, one, n, twos, mb_flags, result) < 0)
return -1;
for (i = 0; i < n; i++) {
if (one == twos[i])
@@ commit-reach.c: int repo_get_merge_bases_many(struct repository *r,
struct commit_list **result)
{
- return get_merge_bases_many_0(r, one, n, twos, 1, result);
-+ return get_merge_bases_many_0(r, one, n, twos, 1, 1, result);
++ return get_merge_bases_many_0(r, one, n, twos, 1,
++ MERGE_BASE_FIND_ALL, result);
}
int repo_get_merge_bases_many_dirty(struct repository *r,
struct commit *one,
size_t n,
struct commit **twos,
-+ int find_all,
++ enum merge_base_flags mb_flags,
struct commit_list **result)
{
- return get_merge_bases_many_0(r, one, n, twos, 0, result);
-+ return get_merge_bases_many_0(r, one, n, twos, 0, find_all, result);
++ return get_merge_bases_many_0(r, one, n, twos, 0, mb_flags, result);
}
int repo_get_merge_bases(struct repository *r,
@@ commit-reach.c: int repo_get_merge_bases(struct repository *r,
struct commit_list **result)
{
- return get_merge_bases_many_0(r, one, 1, &two, 1, result);
-+ return get_merge_bases_many_0(r, one, 1, &two, 1, 1, result);
++ return get_merge_bases_many_0(r, one, 1, &two, 1,
++ MERGE_BASE_FIND_ALL, result);
}
/*
@@ commit-reach.c: int repo_in_merge_bases_many(struct repository *r, struct commit *commit,
+ struct commit_list *bases = NULL;
+ int ret = 0, i;
+ timestamp_t generation, max_generation = GENERATION_NUMBER_ZERO;
++ enum merge_base_flags mb_flags = MERGE_BASE_FIND_ALL;
++
++ if (ignore_missing_commits)
++ mb_flags |= MERGE_BASE_IGNORE_MISSING_COMMITS;
+
+ if (repo_parse_commit(r, commit))
+ return ignore_missing_commits ? 0 : -1;
+@@ commit-reach.c: int repo_in_merge_bases_many(struct repository *r, struct commit *commit,
if (paint_down_to_common(r, commit,
nr_reference, reference,
- generation, ignore_missing_commits, &bases))
-+ generation, ignore_missing_commits, 1, &bases))
++ generation, mb_flags, &bases))
ret = -1;
else if (commit->object.flags & PARENT2)
ret = 1;
@@ commit-reach.h: int repo_get_merge_bases_many(struct repository *r,
struct commit **twos,
struct commit_list **result);
-/* To be used only when object flags after this call no longer matter */
++enum merge_base_flags {
++ MERGE_BASE_FIND_ALL = (1 << 0),
++ MERGE_BASE_IGNORE_MISSING_COMMITS = (1 << 1),
++};
++
+/*
+ * To be used only when object flags after this call no longer matter.
-+ * When find_all is false and generation numbers are available, returns
-+ * after finding the first merge-base, skipping the STALE drain.
++ * Without MERGE_BASE_FIND_ALL and with generation numbers available,
++ * returns after finding the first merge-base, skipping the STALE drain.
+ */
int repo_get_merge_bases_many_dirty(struct repository *r,
struct commit *one, size_t n,
struct commit **twos,
-+ int find_all,
++ enum merge_base_flags mb_flags,
struct commit_list **result);
int get_octopus_merge_bases(struct commit_list *in, struct commit_list **result);
builtin/merge-base.c | 4 +-
commit-reach.c | 36 +++++++++----
commit-reach.h | 12 ++++-
t/t6010-merge-base.sh | 119 ++++++++++++++++++++++++++++++++++++++++++
t/t6600-test-reach.sh | 40 ++++++++++++++
5 files changed, 200 insertions(+), 11 deletions(-)
diff --git a/builtin/merge-base.c b/builtin/merge-base.c
index c7ee97fa6a..a87011c6cd 100644
--- a/builtin/merge-base.c
+++ b/builtin/merge-base.c
@@ -11,10 +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;
struct commit_list *result = NULL, *r;
if (repo_get_merge_bases_many_dirty(the_repository, rev[0],
- rev_nr - 1, rev + 1, &result) < 0) {
+ rev_nr - 1, rev + 1,
+ flags, &result) < 0) {
commit_list_free(result);
return -1;
}
diff --git a/commit-reach.c b/commit-reach.c
index d3a9b3ed6f..5a52be90a6 100644
--- a/commit-reach.c
+++ b/commit-reach.c
@@ -54,7 +54,7 @@ static int paint_down_to_common(struct repository *r,
struct commit *one, int n,
struct commit **twos,
timestamp_t min_generation,
- int ignore_missing_commits,
+ enum merge_base_flags mb_flags,
struct commit_list **result)
{
struct prio_queue queue = { compare_commits_by_gen_then_commit_date };
@@ -97,6 +97,14 @@ static int paint_down_to_common(struct repository *r,
if (!(commit->object.flags & RESULT)) {
commit->object.flags |= RESULT;
tail = commit_list_append(commit, tail);
+ /*
+ * The queue is generation-ordered; no
+ * remaining common ancestor can be a
+ * descendant of this one.
+ */
+ if (!(mb_flags & MERGE_BASE_FIND_ALL) &&
+ generation < GENERATION_NUMBER_INFINITY)
+ break;
}
/* Mark parents of a found merge stale */
flags |= STALE;
@@ -118,7 +126,7 @@ static int paint_down_to_common(struct repository *r,
* corrupt commits would already have been
* dispatched with a `die()`.
*/
- if (ignore_missing_commits)
+ if (mb_flags & MERGE_BASE_IGNORE_MISSING_COMMITS)
return 0;
return error(_("could not parse commit %s"),
oid_to_hex(&p->object.oid));
@@ -136,6 +144,7 @@ static int paint_down_to_common(struct repository *r,
static int merge_bases_many(struct repository *r,
struct commit *one, int n,
struct commit **twos,
+ enum merge_base_flags mb_flags,
struct commit_list **result)
{
struct commit_list *list = NULL, **tail = result;
@@ -165,7 +174,7 @@ static int merge_bases_many(struct repository *r,
oid_to_hex(&twos[i]->object.oid));
}
- if (paint_down_to_common(r, one, n, twos, 0, 0, &list)) {
+ if (paint_down_to_common(r, one, n, twos, 0, mb_flags, &list)) {
commit_list_free(list);
return -1;
}
@@ -246,7 +255,8 @@ static int remove_redundant_no_gen(struct repository *r,
min_generation = curr_generation;
}
if (paint_down_to_common(r, array[i], filled,
- work, min_generation, 0, &common)) {
+ work, min_generation,
+ MERGE_BASE_FIND_ALL, &common)) {
clear_commit_marks(array[i], all_flags);
clear_commit_marks_many(filled, work, all_flags);
commit_list_free(common);
@@ -425,6 +435,7 @@ static int get_merge_bases_many_0(struct repository *r,
size_t n,
struct commit **twos,
int cleanup,
+ enum merge_base_flags mb_flags,
struct commit_list **result)
{
struct commit_list *list, **tail = result;
@@ -432,7 +443,7 @@ static int get_merge_bases_many_0(struct repository *r,
size_t cnt, i;
int ret;
- if (merge_bases_many(r, one, n, twos, result) < 0)
+ if (merge_bases_many(r, one, n, twos, mb_flags, result) < 0)
return -1;
for (i = 0; i < n; i++) {
if (one == twos[i])
@@ -475,16 +486,18 @@ int repo_get_merge_bases_many(struct repository *r,
struct commit **twos,
struct commit_list **result)
{
- return get_merge_bases_many_0(r, one, n, twos, 1, result);
+ return get_merge_bases_many_0(r, one, n, twos, 1,
+ MERGE_BASE_FIND_ALL, result);
}
int repo_get_merge_bases_many_dirty(struct repository *r,
struct commit *one,
size_t n,
struct commit **twos,
+ enum merge_base_flags mb_flags,
struct commit_list **result)
{
- return get_merge_bases_many_0(r, one, n, twos, 0, result);
+ return get_merge_bases_many_0(r, one, n, twos, 0, mb_flags, result);
}
int repo_get_merge_bases(struct repository *r,
@@ -492,7 +505,8 @@ int repo_get_merge_bases(struct repository *r,
struct commit *two,
struct commit_list **result)
{
- return get_merge_bases_many_0(r, one, 1, &two, 1, result);
+ return get_merge_bases_many_0(r, one, 1, &two, 1,
+ MERGE_BASE_FIND_ALL, result);
}
/*
@@ -537,6 +551,10 @@ int repo_in_merge_bases_many(struct repository *r, struct commit *commit,
struct commit_list *bases = NULL;
int ret = 0, i;
timestamp_t generation, max_generation = GENERATION_NUMBER_ZERO;
+ enum merge_base_flags mb_flags = MERGE_BASE_FIND_ALL;
+
+ if (ignore_missing_commits)
+ mb_flags |= MERGE_BASE_IGNORE_MISSING_COMMITS;
if (repo_parse_commit(r, commit))
return ignore_missing_commits ? 0 : -1;
@@ -555,7 +573,7 @@ int repo_in_merge_bases_many(struct repository *r, struct commit *commit,
if (paint_down_to_common(r, commit,
nr_reference, reference,
- generation, ignore_missing_commits, &bases))
+ generation, mb_flags, &bases))
ret = -1;
else if (commit->object.flags & PARENT2)
ret = 1;
diff --git a/commit-reach.h b/commit-reach.h
index 6012402dfc..41607d8952 100644
--- a/commit-reach.h
+++ b/commit-reach.h
@@ -17,10 +17,20 @@ int repo_get_merge_bases_many(struct repository *r,
struct commit *one, size_t n,
struct commit **twos,
struct commit_list **result);
-/* To be used only when object flags after this call no longer matter */
+enum merge_base_flags {
+ MERGE_BASE_FIND_ALL = (1 << 0),
+ MERGE_BASE_IGNORE_MISSING_COMMITS = (1 << 1),
+};
+
+/*
+ * To be used only when object flags after this call no longer matter.
+ * Without MERGE_BASE_FIND_ALL and with generation numbers available,
+ * returns after finding the first merge-base, skipping the STALE drain.
+ */
int repo_get_merge_bases_many_dirty(struct repository *r,
struct commit *one, size_t n,
struct commit **twos,
+ enum merge_base_flags mb_flags,
struct commit_list **result);
int get_octopus_merge_bases(struct commit_list *in, struct commit_list **result);
diff --git a/t/t6010-merge-base.sh b/t/t6010-merge-base.sh
index 44c726ea39..f6c85d4f53 100755
--- a/t/t6010-merge-base.sh
+++ b/t/t6010-merge-base.sh
@@ -305,4 +305,123 @@ test_expect_success 'merge-base --octopus --all for complex tree' '
test_cmp expected actual
'
+# The following tests verify that "git merge-base" (without --all)
+# returns the same result with and without a commit-graph.
+# This exercises the early-exit optimisation in paint_down_to_common
+# that skips the STALE drain when generation numbers are available.
+
+test_expect_success 'setup for commit-graph tests' '
+ git init graph-repo &&
+ (
+ cd graph-repo &&
+
+ # Build a forked DAG:
+ #
+ # L1---L2 (left)
+ # /
+ # S
+ # \
+ # R1---R2 (right)
+ #
+ test_commit GS &&
+ git checkout -b left &&
+ test_commit L1 &&
+ test_commit L2 &&
+ git checkout GS &&
+ git checkout -b right &&
+ test_commit GR1 &&
+ test_commit GR2
+ )
+'
+
+test_expect_success 'merge-base without commit-graph' '
+ (
+ cd graph-repo &&
+ rm -f .git/objects/info/commit-graph &&
+ git merge-base left right >actual &&
+ git rev-parse GS >expected &&
+ test_cmp expected actual
+ )
+'
+
+test_expect_success 'merge-base with commit-graph' '
+ (
+ cd graph-repo &&
+ git commit-graph write --reachable &&
+ git merge-base left right >actual &&
+ git rev-parse GS >expected &&
+ test_cmp expected actual
+ )
+'
+
+test_expect_success 'merge-base --all with commit-graph' '
+ (
+ cd graph-repo &&
+ git merge-base --all left right >actual &&
+ git rev-parse GS >expected &&
+ test_cmp expected actual
+ )
+'
+
+test_expect_success 'merge-base agrees with --all for single result' '
+ (
+ cd graph-repo &&
+ git commit-graph write --reachable &&
+ git merge-base left right >actual.single &&
+ git merge-base --all left right >actual.all &&
+ test_cmp actual.all actual.single
+ )
+'
+
+test_expect_success 'setup for deep chain commit-graph test' '
+ git init deep-repo &&
+ (
+ cd deep-repo &&
+
+ # Build a deep forked DAG:
+ #
+ # L1--L2--...--L20 (left)
+ # /
+ # S
+ # \
+ # R1--R2--...--R20 (right)
+ #
+ test_commit DS &&
+ git checkout -b left &&
+ for i in $(test_seq 1 20)
+ do
+ test_commit DL$i || return 1
+ done &&
+ git checkout DS &&
+ git checkout -b right &&
+ for i in $(test_seq 1 20)
+ do
+ test_commit DR$i || return 1
+ done
+ )
+'
+
+test_expect_success 'deep chain: merge-base matches with and without commit-graph' '
+ (
+ cd deep-repo &&
+ rm -f .git/objects/info/commit-graph &&
+ git merge-base left right >actual.no-graph &&
+ git rev-parse DS >expected &&
+ test_cmp expected actual.no-graph &&
+ git commit-graph write --reachable &&
+ git merge-base left right >actual.graph &&
+ test_cmp expected actual.graph
+ )
+'
+
+test_expect_success 'deep chain: --all and non---all agree with commit-graph' '
+ (
+ cd deep-repo &&
+ git commit-graph write --reachable &&
+ git merge-base left right >actual.single &&
+ git merge-base --all left right >actual.all &&
+ test_cmp actual.all actual.single
+ )
+'
+
test_done
diff --git a/t/t6600-test-reach.sh b/t/t6600-test-reach.sh
index dc0421ed2f..51c23b7683 100755
--- a/t/t6600-test-reach.sh
+++ b/t/t6600-test-reach.sh
@@ -882,4 +882,44 @@ test_expect_success 'rev-list --maximal-only matches merge-base --independent' '
test_cmp expect.sorted actual.sorted
'
+# The following tests verify the early-exit optimisation in
+# paint_down_to_common when merge-base is invoked without --all.
+# Each test checks all four commit-graph configurations.
+
+merge_base_all_modes () {
+ test_when_finished rm -rf .git/objects/info/commit-graph &&
+ git merge-base "$@" >actual &&
+ test_cmp expect actual &&
+ cp commit-graph-full .git/objects/info/commit-graph &&
+ git merge-base "$@" >actual &&
+ test_cmp expect actual &&
+ cp commit-graph-half .git/objects/info/commit-graph &&
+ git merge-base "$@" >actual &&
+ test_cmp expect actual &&
+ cp commit-graph-no-gdat .git/objects/info/commit-graph &&
+ git merge-base "$@" >actual &&
+ test_cmp expect actual
+}
+
+test_expect_success 'merge-base without --all (unique base)' '
+ git rev-parse commit-5-3 >expect &&
+ merge_base_all_modes commit-5-7 commit-8-3
+'
+
+test_expect_success 'merge-base without --all is one of --all results' '
+ test_when_finished rm -rf .git/objects/info/commit-graph &&
+
+ cp commit-graph-full .git/objects/info/commit-graph &&
+ git merge-base --all commit-5-7 commit-4-8 commit-6-6 commit-8-3 >all &&
+ git merge-base commit-5-7 commit-4-8 commit-6-6 commit-8-3 >single &&
+ test_line_count = 1 single &&
+ grep -F -f single all &&
+
+ cp commit-graph-half .git/objects/info/commit-graph &&
+ git merge-base --all commit-5-7 commit-4-8 commit-6-6 commit-8-3 >all &&
+ git merge-base commit-5-7 commit-4-8 commit-6-6 commit-8-3 >single &&
+ test_line_count = 1 single &&
+ grep -F -f single all
+'
+
test_done
base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH v2] ignore: note info/exclude lives in GIT_COMMON_DIR, not GIT_DIR
From: Phillip Wood @ 2026-05-11 10:30 UTC (permalink / raw)
To: D. Ben Knoble, git
Cc: brian m . carlson, Patrick Steinhardt, Taylor Blau, Caleb White,
Calvin Wan, Junio C Hamano, Elijah Newren, Andrew Berry,
Jeff King, Derrick Stolee, Phillip Wood
In-Reply-To: <d58b6e921d3005c6170fc6c47f175214acb3fa68.1778249267.git.ben.knoble+github@gmail.com>
On 08/05/2026 15:14, D. Ben Knoble wrote:
> gitignore(5) says that the per-repository ignore file is
> $GIT_DIR/info/exclude, but in a worktree that is not the case:
>
> git rev-parse --git-path info/exclude
> /path/to/main/worktree/.git/info/exclude
> git rev-parse --git-common-dir
> /path/to/main/worktree/.git
>
> We actually use $GIT_COMMON_DIR/info/exclude. Adjust the documentation
> to say so.
Thanks for making the documentation match reality. Are there some more
instances than need to be changed? If I run
git grep -n GIT_DIR/info origin/master Documentation/gitignore.adoc
I see
origin/master:Documentation/gitignore.adoc:10:$XDG_CONFIG_HOME/git/ignore, $GIT_DIR/info/exclude, .gitignore
origin/master:Documentation/gitignore.adoc:37: * Patterns read from `$GIT_DIR/info/exclude`.
origin/master:Documentation/gitignore.adoc:53: the `$GIT_DIR/info/exclude` file.
origin/master:Documentation/gitignore.adoc:100: such as $GIT_DIR/info/exclude and core.excludesFile, are treated as if
origin/master:Documentation/gitignore.adoc:149:`$GIT_DIR/info/exclude`. Patterns in the exclude file are used in addition to
origin/master:Documentation/gitignore.adoc:150:those in `$GIT_DIR/info/exclude`.
We also have a ton of other files under Documentation that mention
$GIT_DIR/info/... all of which, apart from the release notes, I
think should probably be using $GIT_COMMON_DIR but we can always do
that separately
Thanks
Phillip
> Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail.com>
> ---
>
> Notes (benknoble/commits):
> Changes in v2:
>
> Only adjust the documentation.
>
> brian points out that a more general extension would allow using more
> info/ files as "per-worktree," which I don't have the impetus to
> implement myself.
>
> Phillip and Junio asked for a concrete use case:
>
> A colleague is developing a tool for managing the "skill files" of
> various LLM tools (Claude, Windsurf, etc.). The files have
> requirements that make it hard to generically ignore them (e.g.,
> filenames and front-matter have to match), but different tasks
> (corresponding to worktrees) may want different active skills, so it
> is desirable to ignore the files. Think of this like node_modules.
>
> Unfortunately, since per-worktree ignores don't work, the current
> solution is to put a .gitignore file in the corresponding directory
> with the installed skills that ignores itself and the installed
> skills.
>
> Since overall reactions seem fairly negative (or require a more general
> extension, which I think is probably the right course but not simply
> implemented), I've opted to adjust the docs. They originally confused
> me, as I was surprised when my colleague reported that per-worktree
> ignores didn't work (the docs imply they should by use of $GIT_DIR).
>
> Link to v1: <e3ee0a11b566dd2cc605447c111ae4620bce0fe6.1777050300.git.ben.knoble+github@gmail.com>
>
> v1 notes:
>
> Discussed briefly at https://lore.kernel.org/git/CALnO6CCXmA+ATT7CuyWkU6P8qmLCCpMi5Ppr1c78s0heznpVyw@mail.gmail.com/T
>
> This is based on next (4f69b47b94 (Merge branch 'ps/test-set-e-clean'
> into next, 2026-04-23)) but cleanly applies to master (94f057755b (Git
> 2.54, 2026-04-19)) and seen (50541634cb (Merge branch
> 'js/parseopt-subcommand-autocorrection' into seen, 2026-04-23)).
>
> Documentation/gitignore.adoc | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/gitignore.adoc b/Documentation/gitignore.adoc
> index a3d24e5c34..c423b650de 100644
> --- a/Documentation/gitignore.adoc
> +++ b/Documentation/gitignore.adoc
> @@ -7,7 +7,7 @@ gitignore - Specifies intentionally untracked files to ignore
>
> SYNOPSIS
> --------
> -$XDG_CONFIG_HOME/git/ignore, $GIT_DIR/info/exclude, .gitignore
> +$XDG_CONFIG_HOME/git/ignore, $GIT_COMMON_DIR/info/exclude, .gitignore
>
> DESCRIPTION
> -----------
> @@ -34,7 +34,7 @@ precedence, the last matching pattern decides the outcome):
> includes such `.gitignore` files in its repository, containing patterns for
> files generated as part of the project build.
>
> - * Patterns read from `$GIT_DIR/info/exclude`.
> + * Patterns read from `$GIT_COMMON_DIR/info/exclude`.
>
> * Patterns read from the file specified by the configuration
> variable `core.excludesFile`.
>
> base-commit: 4f69b47b940100b02630f745a52f9d9850f122b2
^ permalink raw reply
* Re: [PATCH] ci: enable EXPENSIVE for contributor builds
From: Patrick Steinhardt @ 2026-05-11 10:02 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Schindelin via GitGitGadget, git, Derrick Stolee,
Torsten Bögershausen, Jeff King, Johannes Schindelin
In-Reply-To: <xmqq33zys62a.fsf@gitster.g>
On Mon, May 11, 2026 at 05:29:01PM +0900, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
>
> > So with this change we now run the tests for all "official" branches,
> > and on pull requests. Which raises the question: are there any events
> > that happen regularly that are excluded by this? Because if not I think
> > it might be sensible to just enable this unconditionally, also because
> > that would make jobs on GitLab CI run expensive tests, as well.
>
> The simplicity certainly is tempting.
>
> We could instead do the "let's enable only on linux-test-vars" kind
> of "optimization", which is on the other side of the extreme, but
> that is only valid if the kind of bugs that can be revealed only by
> EXPENSIVE tests, which may not be caught by others, is expected to
> be pretty much platform or configuration agnostic. I somehow doubt
> that it is the case.
Yeah, it's probably not.
> In any case, I think spending on more machine cycles is certainly
> cheaper than human resources for things like this.
Agreed.
Patrick
^ permalink raw reply
* Re: [PATCH] config: retry acquiring config.lock for 100ms
From: Patrick Steinhardt @ 2026-05-11 10:01 UTC (permalink / raw)
To: Jörg Thalheim; +Cc: Junio C Hamano, git
In-Reply-To: <91335804a092b09757331cac72092a3835020b3a@thalheim.io>
On Mon, May 11, 2026 at 09:06:00AM +0000, Jörg Thalheim wrote:
> May 11, 2026 at 4:32 AM, "Junio C Hamano" <gitster@pobox.com mailto:gitster@pobox.com?to=%22Junio%20C%20Hamano%22%20%3Cgitster%40pobox.com%3E > wrote:
> > Patrick Steinhardt <ps@pks.im> writes:
> > > > This bites in practice when running `git worktree add -b` concurrently
> > > > against the same repository. Each invocation makes several writes to
> > > > ".git/config" to set up branch tracking, and tooling that creates
> > > > worktrees in parallel sees intermittent failures. Worse, `git worktree
> > > > add` does not propagate the failed config write to its exit code: the
> > > > worktree is created and the command exits 0, but tracking
> > > > configuration is silently dropped.
> > > >
> > > This very much sounds like a bug that is worth fixing independently.
> > >
> > > >
> > > > The lock is held only for the duration of rewriting a small file, so
> > > > retrying for 100 ms papers over any realistic contention while still
> > > > failing fast if a stale lock has been left behind by a crashed
> > > > process. This mirrors what we already do for individual reference
> > > > locks (4ff0f01cb7 (refs: retry acquiring reference locks for 100ms,
> > > > 2017-08-21)).
> > > >
> > > Famous last words :) Experience tells me that any timeout value that
> > > isn't excessive will eventually be hit in some production system. Which
> > > raises the question whether we want to make the timeout configurable,
> > > similar to "core.filesRefLockTimeout" and "core.packedRefsTimeout".
> > > ...
> > > Honestly though, I'm not really sure what to make with this. We could
> > > of course also add some validation that the configuration we want to set
> > > hasn't been modified meanwhile. But that would now lead to a situation
> > > where we have to update every single caller in our tree to make use of
> > > the new mechanism, which would be a bunch of work.
> > >
> > > And adding the timeout doesn't really change the status quo, either. We
> > > already have the case that we'll happily overwrite changes made by
> > > concurrent processes. The only thing that changes is that we make it
> > > more likely for concurrent changes to succeed.
> > >
> > We haven't heard any response to these points raised in the message
> > I am responding to. Should I still keep the patch in my tree,
> > hoping that a responses may come some day? I am tempted to discard
> > the topic as it has been quite a while since we last looked at it.
>
> I am not really sure what you want me to do here.
In general, the idea here is to engage in a discussion that can
ultimately lead to one of two outcomes:
- The discussion surfaces an area the author hasn't thought about, so
the patch is adapted accordingly.
- The discussion shows that the author already did think about the
issue, but hasn't documented the assumptions. In this case, it
should be the commit message that gets adapted.
> I don't see how git can have this value configurable, given it's about
> reading the configuration itself. Is the user supposed via command
> line?
This is a fair point indeed. But if it's not possible to change via the
configuration itself, then the next-best thing might be to introduce an
environment variable that allows configuring it.
The other aspect that wasn't discussed in the commit message is how
concurrent writes are handled, both when they are non-conflicting
(updating different keys) and when they are conflicting (updating the
same key). After spending some more time in the code I think it's
ultimately nothing we have to worry about too much, as we only start
reading the configuration after we've locked it.
So in the semantically non-conflicting case there isn't really much of a
race, because things already work as expected. But in the semantically
conflicting case it's a bit different, as the latter writer will
overwrite the result of the former one. In theory it would be possible
to detect such conflicts by:
- Reading the configuration file.
- Taking the lock.
- Rereading the configuration to check for conflicts.
But even that is racy as the first writer might have succeeded before we
read the configuration the first time. So I'm not sure whether we can do
anything about that in the first place, as the race basically exists in
the outer loop controlled by the caller.
So there probably isn't much we can do about that, and unless I missed
something I think your timeout is sensible. But ideally, such nuances
would be discussed as part of the commit message so that reviewers and
future readers are made aware of them.
Thanks!
Patrick
^ permalink raw reply
* [PATCH v6 5/5] branch: add --all-remotes flag
From: Harald Nordgren via GitGitGadget @ 2026-05-11 9:44 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Johannes Sixt, Harald Nordgren,
Harald Nordgren
In-Reply-To: <pull.2285.v6.git.git.1778492691.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
Combined with --forked or --prune-merged, --all-remotes acts on
every configured remote, in addition to any explicit <remote>
arguments. Used alone, it errors out.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
Documentation/git-branch.adoc | 9 ++++++--
builtin/branch.c | 41 +++++++++++++++++++++++++----------
t/t3200-branch.sh | 40 ++++++++++++++++++++++++++++++++++
3 files changed, 76 insertions(+), 14 deletions(-)
diff --git a/Documentation/git-branch.adoc b/Documentation/git-branch.adoc
index 9807d3c218..e5fe82de39 100644
--- a/Documentation/git-branch.adoc
+++ b/Documentation/git-branch.adoc
@@ -24,8 +24,8 @@ git branch (-m|-M) [<old-branch>] <new-branch>
git branch (-c|-C) [<old-branch>] <new-branch>
git branch (-d|-D) [-r] <branch-name>...
git branch --edit-description [<branch-name>]
-git branch --forked <remote>...
-git branch [-f] --prune-merged <remote>...
+git branch --forked (<remote>... | --all-remotes)
+git branch [-f] --prune-merged (<remote>... | --all-remotes)
DESCRIPTION
-----------
@@ -226,6 +226,11 @@ 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`.
+`--all-remotes`::
+ With `--forked` or `--prune-merged`, act on every
+ configured remote in addition to any explicit _<remote>_
+ arguments.
+
`-v`::
`-vv`::
`--verbose`::
diff --git a/builtin/branch.c b/builtin/branch.c
index 7b356e250e..5f771d2f32 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -703,6 +703,13 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
free_worktrees(worktrees);
}
+static int collect_remote_name(struct remote *remote, void *cb_data)
+{
+ struct string_list *remote_names = cb_data;
+ string_list_insert(remote_names, remote->name);
+ return 0;
+}
+
static void parse_forked_args(int argc, const char **argv,
struct string_list *remote_names,
struct string_list *tracking_refs)
@@ -792,7 +799,7 @@ static void collect_default_branch_refs(const struct string_list *remote_names,
}
}
-static void collect_forked_set(int argc, const char **argv,
+static void collect_forked_set(int argc, const char **argv, int all_remotes,
struct string_list *protected_default_refs,
struct string_list *out)
{
@@ -805,6 +812,8 @@ static void collect_forked_set(int argc, const char **argv,
};
parse_forked_args(argc, argv, &remote_names, &tracking_refs);
+ if (all_remotes)
+ for_each_remote(collect_remote_name, &remote_names);
refs_for_each_branch_ref(get_main_ref_store(the_repository),
collect_forked_branch, &cb);
@@ -818,15 +827,15 @@ static void collect_forked_set(int argc, const char **argv,
string_list_clear(&tracking_refs, 0);
}
-static int list_forked_branches(int argc, const char **argv)
+static int list_forked_branches(int argc, const char **argv, int all_remotes)
{
struct string_list out = STRING_LIST_INIT_DUP;
struct string_list_item *item;
- if (!argc)
- die(_("--forked requires at least one <remote>"));
+ if (!argc && !all_remotes)
+ die(_("--forked requires at least one <remote> or --all-remotes"));
- collect_forked_set(argc, argv, NULL, &out);
+ collect_forked_set(argc, argv, all_remotes, NULL, &out);
for_each_string_list_item(item, &out)
puts(item->string);
@@ -849,8 +858,8 @@ static struct commit *resolve_remote_head(const char *remote_name)
return commit;
}
-static int prune_merged_branches(int argc, const char **argv, int force,
- int quiet)
+static int prune_merged_branches(int argc, const char **argv,
+ int all_remotes, int force, int quiet)
{
struct string_list candidates = STRING_LIST_INIT_DUP;
struct string_list protected_default_refs = STRING_LIST_INIT_DUP;
@@ -861,10 +870,11 @@ static int prune_merged_branches(int argc, const char **argv, int force,
int n_not_merged = 0;
int ret = 0;
- if (!argc)
- die(_("--prune-merged requires at least one <remote>"));
+ if (!argc && !all_remotes)
+ die(_("--prune-merged requires at least one <remote> or --all-remotes"));
- collect_forked_set(argc, argv, &protected_default_refs, &candidates);
+ collect_forked_set(argc, argv, all_remotes, &protected_default_refs,
+ &candidates);
for_each_string_list_item(item, &candidates) {
const char *short_name = item->string;
@@ -1001,6 +1011,7 @@ int cmd_branch(int argc,
unset_upstream = 0, show_current = 0, edit_description = 0;
int forked = 0;
int prune_merged = 0;
+ int all_remotes = 0;
const char *new_upstream = NULL;
int noncreate_actions = 0;
/* possible options */
@@ -1058,6 +1069,9 @@ int cmd_branch(int argc,
N_("list local branches forked from the given <remote>s")),
OPT_BOOL(0, "prune-merged", &prune_merged,
N_("delete local branches forked from the given <remote>s that are merged into their upstream")),
+ OPT_BOOL_F(0, "all-remotes", &all_remotes,
+ N_("with --forked or --prune-merged, act on every configured remote"),
+ PARSE_OPT_NONEG),
OPT__FORCE(&force, N_("force creation, move/rename, deletion"), PARSE_OPT_NOCOMPLETE),
OPT_MERGED(&filter, N_("print only branches that are merged")),
OPT_NO_MERGED(&filter, N_("print only branches that are not merged")),
@@ -1101,6 +1115,9 @@ int cmd_branch(int argc,
argc = parse_options(argc, argv, prefix, options, builtin_branch_usage,
0);
+ if (all_remotes && !forked && !prune_merged)
+ die(_("--all-remotes requires --forked or --prune-merged"));
+
if (!delete && !rename && !copy && !edit_description && !new_upstream &&
!show_current && !unset_upstream && !forked && !prune_merged &&
argc == 0)
@@ -1154,10 +1171,10 @@ int cmd_branch(int argc,
quiet, 0, NULL);
goto out;
} else if (forked) {
- ret = list_forked_branches(argc, argv);
+ ret = list_forked_branches(argc, argv, all_remotes);
goto out;
} else if (prune_merged) {
- ret = prune_merged_branches(argc, argv, force, quiet);
+ ret = prune_merged_branches(argc, argv, all_remotes, force, quiet);
goto out;
} else if (show_current) {
print_current_branch_name();
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index fabff84f16..efededd1f0 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -1771,6 +1771,27 @@ test_expect_success '--forked requires at least one <remote>' '
test_grep "at least one <remote>" err
'
+test_expect_success '--forked --all-remotes covers every configured remote' '
+ git -C forked branch --forked --all-remotes >actual &&
+ cat >expect <<-\EOF &&
+ local-foreign
+ local-one
+ local-two
+ main
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success '--forked --all-remotes still validates explicit <remote>' '
+ test_must_fail git -C forked branch --forked nope --all-remotes 2>err &&
+ test_grep "neither a configured remote nor a remote-tracking branch" err
+'
+
+test_expect_success '--all-remotes alone is rejected' '
+ test_must_fail git -C forked branch --all-remotes 2>err &&
+ test_grep "requires --forked or --prune-merged" err
+'
+
test_expect_success '--prune-merged: setup' '
test_create_repo pm-upstream &&
test_commit -C pm-upstream base &&
@@ -1955,4 +1976,23 @@ test_expect_success 'branch -d still deletes a pruneMerged=false branch' '
test_must_fail git -C pm-optout-d rev-parse --verify refs/heads/one
'
+test_expect_success '--prune-merged --all-remotes covers every configured remote' '
+ test_when_finished "rm -rf pm-allremotes" &&
+ git clone pm-upstream pm-allremotes &&
+ test_create_repo pm-other &&
+ test_commit -C pm-other other-base &&
+ git -C pm-other branch foreign other-base &&
+ git -C pm-allremotes remote add other ../pm-other &&
+ git -C pm-allremotes fetch other &&
+ git -C pm-allremotes branch one --track origin/one &&
+ git -C pm-allremotes branch foreign --track other/foreign &&
+
+ git -C pm-allremotes update-ref -d refs/remotes/origin/one &&
+ git -C pm-allremotes update-ref -d refs/remotes/other/foreign &&
+ git -C pm-allremotes branch --force --prune-merged --all-remotes &&
+
+ test_must_fail git -C pm-allremotes rev-parse --verify refs/heads/one &&
+ test_must_fail git -C pm-allremotes rev-parse --verify refs/heads/foreign
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v6 4/5] branch: add branch.<name>.pruneMerged opt-out
From: Harald Nordgren via GitGitGadget @ 2026-05-11 9:44 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Johannes Sixt, Harald Nordgren,
Harald Nordgren
In-Reply-To: <pull.2285.v6.git.git.1778492691.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
Setting branch.<name>.pruneMerged=false exempts that branch from
--prune-merged (and from fetch --prune-merged), even with --force.
Useful for keeping a topic branch around between rounds.
Explicit deletion via 'git branch -d' is unaffected.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
Documentation/config/branch.adoc | 7 ++++++
Documentation/git-branch.adoc | 10 ++++----
builtin/branch.c | 31 +++++++++++++++++++++----
t/t3200-branch.sh | 40 ++++++++++++++++++++++++++++++++
4 files changed, 79 insertions(+), 9 deletions(-)
diff --git a/Documentation/config/branch.adoc b/Documentation/config/branch.adoc
index a4db9fa5c8..4662ef35c1 100644
--- a/Documentation/config/branch.adoc
+++ b/Documentation/config/branch.adoc
@@ -102,3 +102,10 @@ for details).
`git branch --edit-description`. Branch description is
automatically added to the `format-patch` cover letter or
`request-pull` summary.
+
+`branch.<name>.pruneMerged`::
+ If set to `false`, branch _<name>_ is exempt from
+ `git branch --prune-merged`.
+ Useful for topic branches you intend to develop further after
+ an initial round has been merged upstream. Defaults to true.
+ Explicit deletion via `git branch -d` is unaffected.
diff --git a/Documentation/git-branch.adoc b/Documentation/git-branch.adoc
index a5e869270d..9807d3c218 100644
--- a/Documentation/git-branch.adoc
+++ b/Documentation/git-branch.adoc
@@ -216,15 +216,15 @@ Each _<remote>_ may be either the name of a configured remote
Delete the local branches that `--forked` would list for
the same _<remote>_ arguments, but only when the branch's
push destination remote-tracking branch (the branch `git push`
- would update; see `branch_get_push` semantics) no longer
- resolves locally. In other words: the branch was pushed
- under some name on _<remote>_, and that name has since
- been pruned upstream.
+ would update) no longer resolves locally. In other words:
+ the branch was pushed under some name on _<remote>_, and
+ 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`.
`-v`::
`-vv`::
diff --git a/builtin/branch.c b/builtin/branch.c
index d2f07cddd8..7b356e250e 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -869,15 +869,18 @@ static int prune_merged_branches(int argc, const char **argv, int force,
for_each_string_list_item(item, &candidates) {
const char *short_name = item->string;
struct strbuf full = STRBUF_INIT;
+ struct strbuf key = STRBUF_INIT;
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);
if (branch_checked_out(full.buf)) {
strbuf_release(&full);
+ strbuf_release(&key);
continue;
}
strbuf_release(&full);
@@ -887,18 +890,38 @@ static int prune_merged_branches(int argc, const char **argv, int force,
if (upstream &&
string_list_has_string(&protected_default_refs, upstream)) {
const char *leaf = strrchr(upstream, '/');
- if (leaf && !strcmp(leaf + 1, short_name))
+ if (leaf && !strcmp(leaf + 1, short_name)) {
+ strbuf_release(&key);
continue;
+ }
}
push_ref = branch ? branch_get_push(branch, NULL) : NULL;
- if (!push_ref)
+ if (!push_ref) {
+ strbuf_release(&key);
continue;
+ }
if (refs_ref_exists(get_main_ref_store(the_repository),
- push_ref))
+ push_ref)) {
+ strbuf_release(&key);
+ continue;
+ }
+ if (string_list_has_string(&protected_default_refs, push_ref)) {
+ strbuf_release(&key);
continue;
- if (string_list_has_string(&protected_default_refs, push_ref))
+ }
+
+ strbuf_addf(&key, "branch.%s.prunemerged", short_name);
+ if (!repo_config_get_bool(the_repository, key.buf, &opt_out) &&
+ !opt_out) {
+ if (!quiet)
+ fprintf(stderr, _("Skipping '%s' "
+ "(branch.%s.pruneMerged is false)\n"),
+ short_name, short_name);
+ strbuf_release(&key);
continue;
+ }
+ strbuf_release(&key);
ALLOC_GROW(head_rev_overrides, deletable.nr + 1, alloc);
remote_name = push_ref + strlen("refs/remotes/");
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index ca3d06a1ec..fabff84f16 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -1915,4 +1915,44 @@ test_expect_success '--prune-merged spares branches whose push ref is the defaul
git -C pm-pushdefault rev-parse --verify refs/heads/topic
'
+test_expect_success '--prune-merged honours branch.<name>.pruneMerged=false' '
+ test_when_finished "rm -rf pm-optout" &&
+ git clone pm-upstream pm-optout &&
+ git -C pm-optout branch one --track origin/one &&
+ git -C pm-optout branch two --track origin/two &&
+ git -C pm-optout config branch.one.pruneMerged false &&
+
+ git -C pm-optout update-ref -d refs/remotes/origin/one &&
+ git -C pm-optout update-ref -d refs/remotes/origin/two &&
+ git -C pm-optout branch --prune-merged origin 2>err &&
+
+ git -C pm-optout rev-parse --verify refs/heads/one &&
+ test_must_fail git -C pm-optout rev-parse --verify refs/heads/two &&
+ test_grep "Skipping .one." err
+'
+
+test_expect_success '--prune-merged --force still honours pruneMerged=false' '
+ test_when_finished "rm -rf pm-optout-force" &&
+ git clone pm-upstream pm-optout-force &&
+ git -C pm-optout-force checkout -b one --track origin/one &&
+ test_commit -C pm-optout-force unpushed &&
+ git -C pm-optout-force checkout - &&
+ git -C pm-optout-force config branch.one.pruneMerged false &&
+
+ git -C pm-optout-force update-ref -d refs/remotes/origin/one &&
+ git -C pm-optout-force branch --force --prune-merged origin &&
+
+ git -C pm-optout-force rev-parse --verify refs/heads/one
+'
+
+test_expect_success 'branch -d still deletes a pruneMerged=false branch' '
+ test_when_finished "rm -rf pm-optout-d" &&
+ git clone pm-upstream pm-optout-d &&
+ git -C pm-optout-d branch one --track origin/one &&
+ git -C pm-optout-d config branch.one.pruneMerged false &&
+
+ git -C pm-optout-d branch -d one &&
+ test_must_fail git -C pm-optout-d rev-parse --verify refs/heads/one
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v6 3/5] branch: add --prune-merged <remote>
From: Harald Nordgren via GitGitGadget @ 2026-05-11 9:44 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Johannes Sixt, Harald Nordgren,
Harald Nordgren
In-Reply-To: <pull.2285.v6.git.git.1778492691.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
Delete the local branches that --forked <remote> would list,
refusing any whose tip is not reachable from the remote's default
branch. With --force, delete unconditionally. The currently
checked-out branch in any worktree is always preserved.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
Documentation/git-branch.adoc | 15 +++
builtin/branch.c | 193 +++++++++++++++++++++++++++++++---
t/t3200-branch.sh | 144 +++++++++++++++++++++++++
3 files changed, 335 insertions(+), 17 deletions(-)
diff --git a/Documentation/git-branch.adoc b/Documentation/git-branch.adoc
index 5773104cd3..a5e869270d 100644
--- a/Documentation/git-branch.adoc
+++ b/Documentation/git-branch.adoc
@@ -25,6 +25,7 @@ git branch (-c|-C) [<old-branch>] <new-branch>
git branch (-d|-D) [-r] <branch-name>...
git branch --edit-description [<branch-name>]
git branch --forked <remote>...
+git branch [-f] --prune-merged <remote>...
DESCRIPTION
-----------
@@ -211,6 +212,20 @@ Each _<remote>_ may be either the name of a configured remote
`refs/remotes/origin/*` ref) or a specific remote-tracking branch
(e.g. `origin/master`). Multiple _<remote>_ arguments are unioned.
+`--prune-merged`::
+ Delete the local branches that `--forked` would list for
+ the same _<remote>_ arguments, but only when the branch's
+ push destination remote-tracking branch (the branch `git push`
+ would update; see `branch_get_push` semantics) no longer
+ resolves locally. In other words: the branch was pushed
+ under some name on _<remote>_, and 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.
+
`-v`::
`-vv`::
`--verbose`::
diff --git a/builtin/branch.c b/builtin/branch.c
index 1941f8a9ad..d2f07cddd8 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -21,6 +21,7 @@
#include "branch.h"
#include "path.h"
#include "string-list.h"
+#include "strvec.h"
#include "column.h"
#include "utf8.h"
#include "ref-filter.h"
@@ -192,15 +193,29 @@ 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 kinds, int force, int warn_only,
int *n_not_merged)
{
struct commit *rev = lookup_commit_reference(the_repository, oid);
+ int merged;
+
if (!force && !rev) {
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 (warn_only) {
warning(_("the branch '%s' is not fully merged"),
branchname);
@@ -227,7 +242,9 @@ static void delete_branch_config(const char *branchname)
strbuf_release(&buf);
}
-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 force, int kinds,
int quiet, int warn_only, int *n_not_merged)
{
struct commit *head_rev = NULL;
@@ -317,8 +334,10 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
}
if (!(flags & (REF_ISSYMREF|REF_ISBROKEN)) &&
- 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,
+ kinds, force, warn_only, n_not_merged)) {
if (!warn_only)
ret = 1;
goto next;
@@ -753,36 +772,169 @@ static int collect_forked_branch(const struct reference *ref, void *cb_data)
return 0;
}
-static int list_forked_branches(int argc, const char **argv)
+static void collect_default_branch_refs(const struct string_list *remote_names,
+ struct string_list *out)
+{
+ struct ref_store *refs = get_main_ref_store(the_repository);
+ struct string_list_item *item;
+
+ for_each_string_list_item(item, remote_names) {
+ struct strbuf head = STRBUF_INIT;
+ const char *target;
+
+ strbuf_addf(&head, "refs/remotes/%s/HEAD", item->string);
+ target = refs_resolve_ref_unsafe(refs, head.buf,
+ RESOLVE_REF_NO_RECURSE,
+ NULL, NULL);
+ if (target && starts_with(target, "refs/remotes/"))
+ string_list_insert(out, target);
+ strbuf_release(&head);
+ }
+}
+
+static void collect_forked_set(int argc, const char **argv,
+ struct string_list *protected_default_refs,
+ struct string_list *out)
{
struct string_list remote_names = STRING_LIST_INIT_NODUP;
struct string_list tracking_refs = STRING_LIST_INIT_DUP;
- struct string_list out = STRING_LIST_INIT_DUP;
- struct string_list_item *item;
struct forked_cb cb = {
.remote_names = &remote_names,
.tracking_refs = &tracking_refs,
- .out = &out,
+ .out = out,
};
- if (!argc)
- die(_("--forked requires at least one <remote>"));
-
parse_forked_args(argc, argv, &remote_names, &tracking_refs);
refs_for_each_branch_ref(get_main_ref_store(the_repository),
collect_forked_branch, &cb);
- string_list_sort(&out);
- for_each_string_list_item(item, &out)
- puts(item->string);
+ string_list_sort(out);
+
+ if (protected_default_refs)
+ collect_default_branch_refs(&remote_names, protected_default_refs);
string_list_clear(&remote_names, 0);
string_list_clear(&tracking_refs, 0);
+}
+
+static int list_forked_branches(int argc, const char **argv)
+{
+ struct string_list out = STRING_LIST_INIT_DUP;
+ struct string_list_item *item;
+
+ if (!argc)
+ die(_("--forked requires at least one <remote>"));
+
+ collect_forked_set(argc, argv, NULL, &out);
+ for_each_string_list_item(item, &out)
+ puts(item->string);
+
string_list_clear(&out, 0);
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;
+
+ if (!argc)
+ die(_("--prune-merged requires at least one <remote>"));
+
+ collect_forked_set(argc, argv, &protected_default_refs, &candidates);
+
+ for_each_string_list_item(item, &candidates) {
+ const char *short_name = item->string;
+ struct strbuf full = STRBUF_INIT;
+ 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)) {
+ strbuf_release(&full);
+ continue;
+ }
+ strbuf_release(&full);
+
+ branch = branch_get(short_name);
+ upstream = branch ? branch_get_upstream(branch, NULL) : NULL;
+ if (upstream &&
+ string_list_has_string(&protected_default_refs, upstream)) {
+ const char *leaf = strrchr(upstream, '/');
+ if (leaf && !strcmp(leaf + 1, short_name))
+ continue;
+ }
+
+ push_ref = branch ? branch_get_push(branch, NULL) : NULL;
+ if (!push_ref)
+ continue;
+ if (refs_ref_exists(get_main_ref_store(the_repository),
+ push_ref))
+ continue;
+ 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,
+ FILTER_REFS_BRANCHES, quiet,
+ 1, &n_not_merged);
+
+ if (n_not_merged && !quiet)
+ fprintf(stderr,
+ Q_("Skipped %d branch that is not fully merged; "
+ "re-run with --force to delete it anyway.\n",
+ "Skipped %d branches that are not fully merged; "
+ "re-run with --force to delete them anyway.\n",
+ n_not_merged),
+ n_not_merged);
+
+ strvec_clear(&deletable);
+ free(head_rev_overrides);
+ string_list_clear(&candidates, 0);
+ string_list_clear(&protected_default_refs, 0);
+ return ret;
+}
+
static GIT_PATH_FUNC(edit_description, "EDIT_DESCRIPTION")
static int edit_branch_description(const char *branch_name)
@@ -825,6 +977,7 @@ int cmd_branch(int argc,
int delete = 0, rename = 0, copy = 0, list = 0,
unset_upstream = 0, show_current = 0, edit_description = 0;
int forked = 0;
+ int prune_merged = 0;
const char *new_upstream = NULL;
int noncreate_actions = 0;
/* possible options */
@@ -880,6 +1033,8 @@ int cmd_branch(int argc,
N_("edit the description for the branch")),
OPT_BOOL(0, "forked", &forked,
N_("list local branches forked from the given <remote>s")),
+ OPT_BOOL(0, "prune-merged", &prune_merged,
+ N_("delete local branches forked from the given <remote>s that are merged into their upstream")),
OPT__FORCE(&force, N_("force creation, move/rename, deletion"), PARSE_OPT_NOCOMPLETE),
OPT_MERGED(&filter, N_("print only branches that are merged")),
OPT_NO_MERGED(&filter, N_("print only branches that are not merged")),
@@ -924,7 +1079,8 @@ int cmd_branch(int argc,
0);
if (!delete && !rename && !copy && !edit_description && !new_upstream &&
- !show_current && !unset_upstream && !forked && argc == 0)
+ !show_current && !unset_upstream && !forked && !prune_merged &&
+ argc == 0)
list = 1;
if (filter.with_commit || filter.no_commit ||
@@ -933,7 +1089,7 @@ int cmd_branch(int argc,
noncreate_actions = !!delete + !!rename + !!copy + !!new_upstream +
!!show_current + !!list + !!edit_description +
- !!unset_upstream + !!forked;
+ !!unset_upstream + !!forked + !!prune_merged;
if (noncreate_actions > 1)
usage_with_options(builtin_branch_usage, options);
@@ -971,12 +1127,15 @@ int cmd_branch(int argc,
if (delete) {
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,
quiet, 0, NULL);
goto out;
} else if (forked) {
ret = list_forked_branches(argc, argv);
goto out;
+ } else if (prune_merged) {
+ ret = prune_merged_branches(argc, argv, force, quiet);
+ goto out;
} else if (show_current) {
print_current_branch_name();
ret = 0;
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 24a3ec44ee..ca3d06a1ec 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -1771,4 +1771,148 @@ test_expect_success '--forked requires at least one <remote>' '
test_grep "at least one <remote>" err
'
+test_expect_success '--prune-merged: setup' '
+ test_create_repo pm-upstream &&
+ test_commit -C pm-upstream base &&
+ git -C pm-upstream branch one base &&
+ git -C pm-upstream branch two base
+'
+
+test_expect_success '--prune-merged deletes branches whose push ref is gone' '
+ test_when_finished "rm -rf pm-clean" &&
+ git clone pm-upstream pm-clean &&
+ git -C pm-clean branch one --track origin/one &&
+ git -C pm-clean branch two --track origin/two &&
+
+ git -C pm-clean update-ref -d refs/remotes/origin/one &&
+ git -C pm-clean branch --prune-merged origin &&
+
+ test_must_fail git -C pm-clean rev-parse --verify refs/heads/one &&
+ git -C pm-clean rev-parse --verify refs/heads/two
+'
+
+test_expect_success '--prune-merged spares in-flight branches whose push ref still exists' '
+ test_when_finished "rm -rf pm-inflight" &&
+ git clone pm-upstream pm-inflight &&
+ git -C pm-inflight branch one --track origin/one &&
+
+ git -C pm-inflight branch --prune-merged origin &&
+
+ git -C pm-inflight rev-parse --verify refs/heads/one
+'
+
+test_expect_success '--prune-merged skips branches with unpushed commits' '
+ test_when_finished "rm -rf pm-unmerged" &&
+ git clone pm-upstream pm-unmerged &&
+ git -C pm-unmerged checkout -b one --track origin/one &&
+ test_commit -C pm-unmerged unpushed &&
+ git -C pm-unmerged checkout - &&
+
+ git -C pm-unmerged update-ref -d refs/remotes/origin/one &&
+ git -C pm-unmerged branch --prune-merged origin 2>err &&
+ test_grep "not fully merged" err &&
+ test_grep "Skipped 1 branch" err &&
+ test_grep "re-run with --force" err &&
+ test_grep ! "If you are sure you want to delete it" err &&
+ git -C pm-unmerged rev-parse --verify refs/heads/one
+'
+
+test_expect_success '--prune-merged --force deletes branches with unpushed commits' '
+ test_when_finished "rm -rf pm-force" &&
+ git clone pm-upstream pm-force &&
+ git -C pm-force checkout -b one --track origin/one &&
+ test_commit -C pm-force unpushed &&
+ git -C pm-force checkout - &&
+
+ git -C pm-force update-ref -d refs/remotes/origin/one &&
+ git -C pm-force branch --force --prune-merged origin &&
+
+ 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 &&
+ # 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^ &&
+
+ git -C pm-head-indep branch --prune-merged origin &&
+
+ test_must_fail git -C pm-head-indep 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 - &&
+
+ 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 &&
+
+ test_must_fail git -C pm-no-head rev-parse --verify refs/heads/one
+'
+
+test_expect_success '--prune-merged never deletes the checked-out branch' '
+ test_when_finished "rm -rf pm-head" &&
+ git clone pm-upstream pm-head &&
+ git -C pm-head checkout -b one --track origin/one &&
+
+ git -C pm-head update-ref -d refs/remotes/origin/one &&
+ git -C pm-head branch --force --prune-merged origin &&
+
+ git -C pm-head rev-parse --verify refs/heads/one
+'
+
+test_expect_success '--prune-merged deletes when push ref differs from upstream' '
+ test_when_finished "rm -rf pm-pushdiff" &&
+ git clone pm-upstream pm-pushdiff &&
+ git -C pm-pushdiff config push.default current &&
+ git -C pm-pushdiff branch --track topic-a origin/one &&
+
+ git -C pm-pushdiff branch --force --prune-merged origin &&
+
+ test_must_fail git -C pm-pushdiff rev-parse --verify refs/heads/topic-a
+'
+
+test_expect_success '--prune-merged spares the local default branch' '
+ test_when_finished "rm -rf pm-default" &&
+ git clone pm-upstream pm-default &&
+ git -C pm-default config push.default current &&
+ git -C pm-default checkout --detach &&
+ git -C pm-default branch --prune-merged origin &&
+ git -C pm-default rev-parse --verify refs/heads/main
+'
+
+test_expect_success '--prune-merged protects only the default branch by name, not by upstream' '
+ test_when_finished "rm -rf pm-default-alias" &&
+ git clone pm-upstream pm-default-alias &&
+ git -C pm-default-alias config push.default current &&
+ git -C pm-default-alias branch --track trunk origin/main &&
+ git -C pm-default-alias checkout --detach &&
+ git -C pm-default-alias branch --force --prune-merged origin &&
+ git -C pm-default-alias rev-parse --verify refs/heads/main &&
+ test_must_fail git -C pm-default-alias rev-parse --verify refs/heads/trunk
+'
+
+test_expect_success '--prune-merged spares branches whose push ref is the default branch' '
+ test_when_finished "rm -rf pm-pushdefault" &&
+ git clone pm-upstream pm-pushdefault &&
+ git -C pm-pushdefault branch --track topic origin/one &&
+ git -C pm-pushdefault config --add remote.origin.push refs/heads/topic:refs/heads/main &&
+ git -C pm-pushdefault update-ref -d refs/remotes/origin/one &&
+ git -C pm-pushdefault update-ref -d refs/remotes/origin/main &&
+ git -C pm-pushdefault checkout --detach &&
+ git -C pm-pushdefault branch --prune-merged origin &&
+ git -C pm-pushdefault rev-parse --verify refs/heads/topic
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v6 2/5] branch: let delete_branches warn instead of error on bulk refusal
From: Harald Nordgren via GitGitGadget @ 2026-05-11 9:44 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Johannes Sixt, Harald Nordgren,
Harald Nordgren
In-Reply-To: <pull.2285.v6.git.git.1778492691.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
Add two new parameters to delete_branches() and the helper
check_branch_commit():
* warn_only switches the per-branch refusal from a hard error
("error: the branch 'X' is not fully merged" plus a four-line
hint about 'git branch -D X') to a one-line warning, and
causes the function to skip those branches without setting its
exit code. Each refused branch is still skipped from deletion.
* n_not_merged, when non-NULL, is incremented for each branch
refused on the not-merged path, so a bulk caller can summarize
rather than print per-branch advice.
All existing call sites pass 0 / NULL and so are unaffected. Both
parameters are wired up so a bulk-deletion caller can suppress
the noise normally appropriate for a one-shot 'git branch -d'.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
builtin/branch.c | 29 ++++++++++++++++++++---------
1 file changed, 20 insertions(+), 9 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index b3289a8875..1941f8a9ad 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -192,7 +192,8 @@ 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,
- int kinds, int force)
+ int kinds, int force, int warn_only,
+ int *n_not_merged)
{
struct commit *rev = lookup_commit_reference(the_repository, oid);
if (!force && !rev) {
@@ -200,10 +201,18 @@ static int check_branch_commit(const char *branchname, const char *refname,
return -1;
}
if (!force && !branch_merged(kinds, branchname, rev, head_rev)) {
- error(_("the branch '%s' is not fully merged"), branchname);
- advise_if_enabled(ADVICE_FORCE_DELETE_BRANCH,
- _("If you are sure you want to delete it, "
- "run 'git branch -D %s'"), branchname);
+ if (warn_only) {
+ warning(_("the branch '%s' is not fully merged"),
+ branchname);
+ } else {
+ error(_("the branch '%s' is not fully merged"),
+ branchname);
+ advise_if_enabled(ADVICE_FORCE_DELETE_BRANCH,
+ _("If you are sure you want to delete it, "
+ "run 'git branch -D %s'"), branchname);
+ }
+ if (n_not_merged)
+ (*n_not_merged)++;
return -1;
}
return 0;
@@ -219,7 +228,7 @@ static void delete_branch_config(const char *branchname)
}
static int delete_branches(int argc, const char **argv, int force, int kinds,
- int quiet)
+ int quiet, int warn_only, int *n_not_merged)
{
struct commit *head_rev = NULL;
struct object_id oid;
@@ -309,8 +318,9 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
if (!(flags & (REF_ISSYMREF|REF_ISBROKEN)) &&
check_branch_commit(bname.buf, name, &oid, head_rev, kinds,
- force)) {
- ret = 1;
+ force, warn_only, n_not_merged)) {
+ if (!warn_only)
+ ret = 1;
goto next;
}
@@ -961,7 +971,8 @@ int cmd_branch(int argc,
if (delete) {
if (!argc)
die(_("branch name required"));
- ret = delete_branches(argc, argv, delete > 1, filter.kind, quiet);
+ ret = delete_branches(argc, argv, delete > 1, filter.kind,
+ quiet, 0, NULL);
goto out;
} else if (forked) {
ret = list_forked_branches(argc, argv);
--
gitgitgadget
^ permalink raw reply related
* [PATCH v6 1/5] branch: add --forked <remote>
From: Harald Nordgren via GitGitGadget @ 2026-05-11 9:44 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Johannes Sixt, Harald Nordgren,
Harald Nordgren
In-Reply-To: <pull.2285.v6.git.git.1778492691.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
List local branches whose configured upstream falls within any of
the given <remote> arguments. <remote> may be either a configured
remote name (matching all of its remote-tracking branches) or a
single remote-tracking branch. Multiple <remote> arguments are
unioned.
This is the building block for --prune-merged, which deletes the
listed branches.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
Documentation/git-branch.adoc | 12 ++++
builtin/branch.c | 110 +++++++++++++++++++++++++++++++++-
t/t3200-branch.sh | 54 +++++++++++++++++
3 files changed, 174 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-branch.adoc b/Documentation/git-branch.adoc
index c0afddc424..5773104cd3 100644
--- a/Documentation/git-branch.adoc
+++ b/Documentation/git-branch.adoc
@@ -24,6 +24,7 @@ git branch (-m|-M) [<old-branch>] <new-branch>
git branch (-c|-C) [<old-branch>] <new-branch>
git branch (-d|-D) [-r] <branch-name>...
git branch --edit-description [<branch-name>]
+git branch --forked <remote>...
DESCRIPTION
-----------
@@ -199,6 +200,17 @@ This option is only applicable in non-verbose mode.
Print the name of the current branch. In detached `HEAD` state,
nothing is printed.
+`--forked`::
+ List local branches that fork from any of the given _<remote>_
+ arguments, that is, those whose configured upstream
+ (`branch.<name>.merge`) is one of those remotes' remote-tracking
+ branches.
++
+Each _<remote>_ may be either the name of a configured remote
+(e.g. `origin`, meaning any branch tracking a
+`refs/remotes/origin/*` ref) or a specific remote-tracking branch
+(e.g. `origin/master`). Multiple _<remote>_ arguments are unioned.
+
`-v`::
`-vv`::
`--verbose`::
diff --git a/builtin/branch.c b/builtin/branch.c
index 1572a4f9ef..b3289a8875 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -38,6 +38,7 @@ static const char * const builtin_branch_usage[] = {
N_("git branch [<options>] (-c | -C) [<old-branch>] <new-branch>"),
N_("git branch [<options>] [-r | -a] [--points-at]"),
N_("git branch [<options>] [-r | -a] [--format]"),
+ N_("git branch [<options>] --forked <remote>..."),
NULL
};
@@ -673,6 +674,105 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
free_worktrees(worktrees);
}
+static void parse_forked_args(int argc, const char **argv,
+ struct string_list *remote_names,
+ struct string_list *tracking_refs)
+{
+ int i;
+
+ for (i = 0; i < argc; i++) {
+ const char *arg = argv[i];
+ struct remote *remote;
+ struct object_id oid;
+ char *full_ref = NULL;
+
+ remote = remote_get(arg);
+ if (remote && remote_is_configured(remote, 0)) {
+ string_list_insert(remote_names, remote->name);
+ continue;
+ }
+
+ if (repo_dwim_ref(the_repository, arg, strlen(arg), &oid,
+ &full_ref, 0) == 1 &&
+ starts_with(full_ref, "refs/remotes/")) {
+ string_list_insert(tracking_refs, full_ref);
+ free(full_ref);
+ continue;
+ }
+ free(full_ref);
+
+ die(_("'%s' is neither a configured remote nor a "
+ "remote-tracking branch"), arg);
+ }
+}
+
+static int branch_is_forked(const char *short_name,
+ const struct string_list *remote_names,
+ const struct string_list *tracking_refs)
+{
+ struct branch *branch = branch_get(short_name);
+ const char *upstream;
+
+ if (!branch || !branch->remote_name)
+ return 0;
+
+ if (string_list_has_string(remote_names, branch->remote_name))
+ return 1;
+
+ upstream = branch_get_upstream(branch, NULL);
+ if (upstream && string_list_has_string(tracking_refs, upstream))
+ return 1;
+
+ return 0;
+}
+
+struct forked_cb {
+ const struct string_list *remote_names;
+ const struct string_list *tracking_refs;
+ struct string_list *out;
+};
+
+static int collect_forked_branch(const struct reference *ref, void *cb_data)
+{
+ struct forked_cb *cb = cb_data;
+
+ if (ref->flags & REF_ISSYMREF)
+ return 0;
+ if (branch_is_forked(ref->name, cb->remote_names, cb->tracking_refs))
+ string_list_append(cb->out, ref->name);
+ return 0;
+}
+
+static int list_forked_branches(int argc, const char **argv)
+{
+ struct string_list remote_names = STRING_LIST_INIT_NODUP;
+ struct string_list tracking_refs = STRING_LIST_INIT_DUP;
+ struct string_list out = STRING_LIST_INIT_DUP;
+ struct string_list_item *item;
+ struct forked_cb cb = {
+ .remote_names = &remote_names,
+ .tracking_refs = &tracking_refs,
+ .out = &out,
+ };
+
+ if (!argc)
+ die(_("--forked requires at least one <remote>"));
+
+ parse_forked_args(argc, argv, &remote_names, &tracking_refs);
+
+ refs_for_each_branch_ref(get_main_ref_store(the_repository),
+ collect_forked_branch, &cb);
+
+ string_list_sort(&out);
+ for_each_string_list_item(item, &out)
+ puts(item->string);
+
+ string_list_clear(&remote_names, 0);
+ string_list_clear(&tracking_refs, 0);
+ string_list_clear(&out, 0);
+ return 0;
+}
+
static GIT_PATH_FUNC(edit_description, "EDIT_DESCRIPTION")
static int edit_branch_description(const char *branch_name)
@@ -714,6 +814,7 @@ int cmd_branch(int argc,
/* possible actions */
int delete = 0, rename = 0, copy = 0, list = 0,
unset_upstream = 0, show_current = 0, edit_description = 0;
+ int forked = 0;
const char *new_upstream = NULL;
int noncreate_actions = 0;
/* possible options */
@@ -767,6 +868,8 @@ int cmd_branch(int argc,
OPT_BOOL(0, "create-reflog", &reflog, N_("create the branch's reflog")),
OPT_BOOL(0, "edit-description", &edit_description,
N_("edit the description for the branch")),
+ OPT_BOOL(0, "forked", &forked,
+ N_("list local branches forked from the given <remote>s")),
OPT__FORCE(&force, N_("force creation, move/rename, deletion"), PARSE_OPT_NOCOMPLETE),
OPT_MERGED(&filter, N_("print only branches that are merged")),
OPT_NO_MERGED(&filter, N_("print only branches that are not merged")),
@@ -811,7 +914,7 @@ int cmd_branch(int argc,
0);
if (!delete && !rename && !copy && !edit_description && !new_upstream &&
- !show_current && !unset_upstream && argc == 0)
+ !show_current && !unset_upstream && !forked && argc == 0)
list = 1;
if (filter.with_commit || filter.no_commit ||
@@ -820,7 +923,7 @@ int cmd_branch(int argc,
noncreate_actions = !!delete + !!rename + !!copy + !!new_upstream +
!!show_current + !!list + !!edit_description +
- !!unset_upstream;
+ !!unset_upstream + !!forked;
if (noncreate_actions > 1)
usage_with_options(builtin_branch_usage, options);
@@ -860,6 +963,9 @@ int cmd_branch(int argc,
die(_("branch name required"));
ret = delete_branches(argc, argv, delete > 1, filter.kind, quiet);
goto out;
+ } else if (forked) {
+ ret = list_forked_branches(argc, argv);
+ goto out;
} else if (show_current) {
print_current_branch_name();
ret = 0;
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index e7829c2c4b..24a3ec44ee 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -1717,4 +1717,58 @@ test_expect_success 'errors if given a bad branch name' '
test_cmp expect actual
'
+test_expect_success '--forked: setup' '
+ test_create_repo forked-upstream &&
+ test_commit -C forked-upstream base &&
+ git -C forked-upstream branch one base &&
+ git -C forked-upstream branch two base &&
+
+ test_create_repo forked-other &&
+ test_commit -C forked-other other-base &&
+ git -C forked-other branch foreign other-base &&
+
+ git clone forked-upstream forked &&
+ git -C forked remote add other ../forked-other &&
+ git -C forked fetch other &&
+ git -C forked branch --track local-one origin/one &&
+ git -C forked branch --track local-two origin/two &&
+ git -C forked branch --track local-foreign other/foreign &&
+ git -C forked branch detached
+'
+
+test_expect_success '--forked <remote-name> lists branches tracking that remote' '
+ git -C forked branch --forked origin >actual &&
+ cat >expect <<-\EOF &&
+ local-one
+ local-two
+ main
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success '--forked <remote-tracking-branch> lists only matching branches' '
+ git -C forked branch --forked origin/one >actual &&
+ echo local-one >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success '--forked unions multiple <remote> arguments' '
+ git -C forked branch --forked origin/one other >actual &&
+ cat >expect <<-\EOF &&
+ local-foreign
+ local-one
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success '--forked rejects unknown remote/ref' '
+ test_must_fail git -C forked branch --forked nope 2>err &&
+ test_grep "neither a configured remote nor a remote-tracking branch" err
+'
+
+test_expect_success '--forked requires at least one <remote>' '
+ test_must_fail git -C forked branch --forked 2>err &&
+ test_grep "at least one <remote>" err
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v6 0/5] branch: prune-merged
From: Harald Nordgren via GitGitGadget @ 2026-05-11 9:44 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Johannes Sixt, Harald Nordgren
In-Reply-To: <pull.2285.v5.git.git.1778482708.gitgitgadget@gmail.com>
* --prune-merged now measures merged-ness against the remote's default
branch instead of the candidate's upstream — so the decision no longer
depends on which branch happens to be checked out locally.
* delete_branches() / check_branch_commit() gained a per-candidate override
that lets a caller substitute a different "what counts as merged"
reference (or skip the check). branch -d callers pass NULL and keep their
existing semantics.
* prune_merged_branches() resolves each candidate's push-remote HEAD and
threads it through, so --prune-merged --all-remotes measures each
candidate against its own remote rather than a single global reference.
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 | 32 +++
builtin/branch.c | 344 +++++++++++++++++++++++++++++--
t/t3200-branch.sh | 278 +++++++++++++++++++++++++
4 files changed, 647 insertions(+), 14 deletions(-)
base-commit: 7760f83b59750c27df653c5c46d0f80e44cfe02c
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2285%2FHaraldNordgren%2Ffetch-prune-local-branches-v6
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2285/HaraldNordgren/fetch-prune-local-branches-v6
Pull-Request: https://github.com/git/git/pull/2285
Range-diff vs v5:
1: 77e67d4b8b = 1: fb9817b220 branch: add --forked <remote>
2: 807c9f981f = 2: 42a2f93d44 branch: let delete_branches warn instead of error on bulk refusal
3: 77beb620d7 ! 3: 604ecb8965 branch: add --prune-merged <remote>
@@ Commit message
branch: add --prune-merged <remote>
Delete the local branches that --forked <remote> would list,
- refusing any whose tip is not reachable from its upstream
- remote-tracking branch. With --force, delete unconditionally.
- The currently checked-out branch in any worktree is always
- preserved.
+ refusing any whose tip is not reachable from the remote's default
+ branch. With --force, delete unconditionally. The currently
+ checked-out branch in any worktree is always preserved.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
@@ Documentation/git-branch.adoc: Each _<remote>_ may be either the name of a confi
+ under some name on _<remote>_, and that name has since
+ been pruned upstream.
++
-+By default, the local tip must also be reachable from the
-+upstream remote-tracking branch (see `--no-merged`); branches with
-+unpushed commits are refused. With `--force` (or `-f`), delete
-+them regardless. The currently checked-out branch in any worktree
-+is always preserved.
++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.
+
`-v`::
`-vv`::
@@ builtin/branch.c
#include "column.h"
#include "utf8.h"
#include "ref-filter.h"
+@@ 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 kinds, int force, int warn_only,
+ int *n_not_merged)
+ {
+ struct commit *rev = lookup_commit_reference(the_repository, oid);
++ int merged;
++
+ if (!force && !rev) {
+ 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 (warn_only) {
+ warning(_("the branch '%s' is not fully merged"),
+ branchname);
+@@ builtin/branch.c: static void delete_branch_config(const char *branchname)
+ strbuf_release(&buf);
+ }
+
+-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 force, int kinds,
+ int quiet, int warn_only, int *n_not_merged)
+ {
+ struct commit *head_rev = NULL;
+@@ builtin/branch.c: static int delete_branches(int argc, const char **argv, int force, int kinds,
+ }
+
+ if (!(flags & (REF_ISSYMREF|REF_ISBROKEN)) &&
+- 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,
++ kinds, force, warn_only, n_not_merged)) {
+ if (!warn_only)
+ ret = 1;
+ goto next;
@@ builtin/branch.c: static int collect_forked_branch(const struct reference *ref, void *cb_data)
return 0;
}
@@ 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, force,
++ ret = delete_branches(deletable.nr, deletable.v,
++ head_rev_overrides, 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,
usage_with_options(builtin_branch_usage, options);
@@ builtin/branch.c: int cmd_branch(int argc,
+ if (delete) {
+ 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,
+ quiet, 0, NULL);
+ goto out;
} else if (forked) {
ret = list_forked_branches(argc, argv);
goto out;
@@ 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 &&
++ # 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^ &&
++
++ git -C pm-head-indep branch --prune-merged origin &&
++
++ test_must_fail git -C pm-head-indep 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 - &&
++
++ 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 &&
++
++ test_must_fail git -C pm-no-head rev-parse --verify refs/heads/one
++'
++
+test_expect_success '--prune-merged never deletes the checked-out branch' '
+ test_when_finished "rm -rf pm-head" &&
+ git clone pm-upstream pm-head &&
4: cf69fb5767 ! 4: 717fc6758e branch: add branch.<name>.pruneMerged opt-out
@@ Documentation/git-branch.adoc: Each _<remote>_ may be either the name of a confi
+ the branch was pushed under some name on _<remote>_, and
+ that name has since been pruned upstream.
+
--By default, the local tip must also be reachable from the
--upstream remote-tracking branch (see `--no-merged`); branches with
--unpushed commits are refused. With `--force` (or `-f`), delete
--them regardless. The currently checked-out branch in any worktree
--is always preserved.
-+The local tip must also be reachable from the upstream
-+remote-tracking branch; branches with unpushed commits are refused.
-+With `--force` (or `-f`), 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`.
+ 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`.
`-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);
- strvec_push(&deletable, short_name);
- }
+ ALLOC_GROW(head_rev_overrides, deletable.nr + 1, alloc);
+ remote_name = push_ref + strlen("refs/remotes/");
## t/t3200-branch.sh ##
@@ t/t3200-branch.sh: test_expect_success '--prune-merged spares branches whose push ref is the defaul
5: f2cee8c79b ! 5: be25572957 branch: add --all-remotes flag
@@ Documentation/git-branch.adoc: git branch (-m|-M) [<old-branch>] <new-branch>
DESCRIPTION
-----------
-@@ Documentation/git-branch.adoc: With `--force` (or `-f`), 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: 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`.
+`--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 int list_forked_branches(int argc, const char **argv)
- return 0;
+@@ builtin/branch.c: static struct commit *resolve_remote_head(const char *remote_name)
+ return commit;
}
-static int prune_merged_branches(int argc, const char **argv, int force,
--
gitgitgadget
^ permalink raw reply
* Re: [PATCH] config: retry acquiring config.lock for 100ms
From: Jörg Thalheim @ 2026-05-11 9:06 UTC (permalink / raw)
To: Junio C Hamano, Patrick Steinhardt; +Cc: git
In-Reply-To: <xmqqcxz2vfpa.fsf@gitster.g>
I am not really sure what you want me to do here.
I don't see how git can have this value configurable, given it's about reading the configuration itself.
Is the user supposed via command line?
Meanwhile the project I was facing this issue, added the required file lock in its own code,
which has since then worked perfectly to fix my use case: https://github.com/raine/workmux/issues/116
May 11, 2026 at 4:32 AM, "Junio C Hamano" <gitster@pobox.com mailto:gitster@pobox.com?to=%22Junio%20C%20Hamano%22%20%3Cgitster%40pobox.com%3E > wrote:
>
> Patrick Steinhardt <ps@pks.im> writes:
>
> >
> > >
> > > This bites in practice when running `git worktree add -b` concurrently
> > > against the same repository. Each invocation makes several writes to
> > > ".git/config" to set up branch tracking, and tooling that creates
> > > worktrees in parallel sees intermittent failures. Worse, `git worktree
> > > add` does not propagate the failed config write to its exit code: the
> > > worktree is created and the command exits 0, but tracking
> > > configuration is silently dropped.
> > >
> > This very much sounds like a bug that is worth fixing independently.
> >
> > >
> > > The lock is held only for the duration of rewriting a small file, so
> > > retrying for 100 ms papers over any realistic contention while still
> > > failing fast if a stale lock has been left behind by a crashed
> > > process. This mirrors what we already do for individual reference
> > > locks (4ff0f01cb7 (refs: retry acquiring reference locks for 100ms,
> > > 2017-08-21)).
> > >
> > Famous last words :) Experience tells me that any timeout value that
> > isn't excessive will eventually be hit in some production system. Which
> > raises the question whether we want to make the timeout configurable,
> > similar to "core.filesRefLockTimeout" and "core.packedRefsTimeout".
> > ...
> > Honestly though, I'm not really sure what to make with this. We could
> > of course also add some validation that the configuration we want to set
> > hasn't been modified meanwhile. But that would now lead to a situation
> > where we have to update every single caller in our tree to make use of
> > the new mechanism, which would be a bunch of work.
> >
> > And adding the timeout doesn't really change the status quo, either. We
> > already have the case that we'll happily overwrite changes made by
> > concurrent processes. The only thing that changes is that we make it
> > more likely for concurrent changes to succeed.
> >
> We haven't heard any response to these points raised in the message
> I am responding to. Should I still keep the patch in my tree,
> hoping that a responses may come some day? I am tempted to discard
> the topic as it has been quite a while since we last looked at it.
>
> Thanks.
>
^ permalink raw reply
* [PATCH] fetch: add fetch.pruneLocalBranches config
From: Harald Nordgren @ 2026-05-11 8:44 UTC (permalink / raw)
To: gitster; +Cc: git, gitgitgadget, haraldnordgren, j6t, kristofferhaugsbakk
In-Reply-To: <xmqq7bpas6k0.fsf@gitster.g>
> Existing call sites are about "branch -d <other>" that allows the
> other branch to be deleted if it is part of HEAD or if it is part of
> its tracking branch, but should "branch --prune-merged" pay
> attention to what branch happens to be checked out the same way (not
> a rherotical question to hint that I do not think it should---I do
> not have a strong opinion on this either way)?
This is a very good question! My opion is that it should work the same way
regardless of which branch you are on, it should always compare against the
remote's default branch.
I this explains some weirdness I saw today when running it from non-main
and prune didn't get triggered.
I will look into making that change.
Harald
^ permalink raw reply
* Re: [PATCH 1/1] shallow: fix relative deepen on non-shallow repositories
From: Junio C Hamano @ 2026-05-11 8:30 UTC (permalink / raw)
To: René Scharfe; +Cc: Samo Pogačnik, git, owen
In-Reply-To: <ac1aac76-17bc-469b-8dc1-d3a384f5c6af@web.de>
René Scharfe <l.s.r@web.de> writes:
> Perhaps, but no warning has been given for deepening a non-shallow repo
> since the introduction of this option by cccf74e2da (fetch, upload-pack:
> --deepen=N extends shallow boundary by N commits, 2016-06-12).
>
> The best place for such a warning would be close to the user, in fetch,
> no? And in its own patch.
Yeah, the lack of warning may or may not be considered a bug, but I
agree that it is totally orthogonal to the problem the patch is
trying to address.
Thanks.
^ 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