* [PATCH v12 0/6] branch: prune-merged
From: Harald Nordgren via GitGitGadget @ 2026-06-03 9:04 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Johannes Sixt, Phillip Wood,
Harald Nordgren
In-Reply-To: <pull.2285.v11.git.git.1779449498.gitgitgadget@gmail.com>
* Reworked --forked from a standalone action into a --list-mode filter.
* Switched --forked and --prune-merged to repeatable OPT_STRING_LIST
options.
* Dropped the bare-remote-name resolution for --forked, the argument is now
a ref or a glob.
Harald Nordgren (6):
branch: add --forked filter for --list mode
branch: let delete_branches warn instead of error on bulk refusal
branch: prepare delete_branches for a bulk caller
branch: add --prune-merged <branch>
branch: add branch.<name>.pruneMerged opt-out
branch: add --dry-run for --prune-merged
Documentation/config/branch.adoc | 7 +
Documentation/git-branch.adoc | 37 ++++
builtin/branch.c | 317 +++++++++++++++++++++++++--
ref-filter.c | 10 +-
ref-filter.h | 2 +
t/t3200-branch.sh | 354 +++++++++++++++++++++++++++++++
6 files changed, 701 insertions(+), 26 deletions(-)
base-commit: 9ac3f193c05c2237e2b14ebaa1149e9fc8a1abe0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2285%2FHaraldNordgren%2Ffetch-prune-local-branches-v12
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2285/HaraldNordgren/fetch-prune-local-branches-v12
Pull-Request: https://github.com/git/git/pull/2285
Range-diff vs v11:
1: b9fddd124a ! 1: 8834c424fb branch: add --forked <branch>
@@ Metadata
Author: Harald Nordgren <haraldnordgren@gmail.com>
## Commit message ##
- branch: add --forked <branch>
+ branch: add --forked filter for --list mode
- List local branches whose configured upstream
- (branch.<name>.merge resolved against branch.<name>.remote)
- matches any of the given <branch> arguments.
+ Add a --forked option to "git branch" list mode that keeps only
+ branches whose configured upstream matches <branch>. The argument
+ can be a ref (e.g. "origin/main", "master") or a shell-style
+ glob (e.g. "origin/*"). The option can be repeated to widen the
+ filter.
- Each <branch> is interpreted against the local repository, not
- against any specific remote:
+ Because it is a filter on list mode, --forked composes with the
+ existing list-mode filters, so
- * a literal upstream short name, e.g. "origin/main" or "master"
- for a branch whose upstream is local;
- * a wildmatch pattern, e.g. "origin/*";
- * a bare configured-remote name, e.g. "origin", which resolves
- to whatever refs/remotes/origin/HEAD points at, matching how
- "git checkout -b topic origin" picks a starting point.
+ git branch --merged origin/main --forked 'origin/*'
- The literal-vs-wildcard distinction is settled at parse time so
- the per-branch matching loop calls wildmatch() only for genuine
- wildcards. Multiple <branch> arguments are unioned. Output is
- sorted by branch name.
+ lists branches forked from origin that have already been
+ integrated into origin/main, and --no-merged inverts the question.
This is the building block for --prune-merged, which deletes the
listed branches once they have landed on their upstream.
@@ Commit message
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
## Documentation/git-branch.adoc ##
-@@ Documentation/git-branch.adoc: git branch (-m|-M) [<old-branch>] <new-branch>
- git branch (-c|-C) [<old-branch>] <new-branch>
- git branch (-d|-D) [-r] <branch-name>...
- git branch --edit-description [<branch-name>]
-+git branch --forked <branch>...
-
- DESCRIPTION
- -----------
+@@ Documentation/git-branch.adoc: git branch [--color[=<when>] | --no-color] [--show-current]
+ [--merged [<commit>]] [--no-merged [<commit>]]
+ [--contains [<commit>]] [--no-contains [<commit>]]
+ [--points-at <object>] [--format=<format>]
++ [(--forked <branch>)...]
+ [(-r|--remotes) | (-a|--all)]
+ [--list] [<pattern>...]
+ git branch [--track[=(direct|inherit)] | --no-track] [-f]
@@ Documentation/git-branch.adoc: This option is only applicable in non-verbose mode.
Print the name of the current branch. In detached `HEAD` state,
nothing is printed.
-+`--forked`::
-+ List local branches whose configured upstream
-+ (`branch.<name>.merge` resolved against `branch.<name>.remote`)
-+ matches any of the given _<branch>_ arguments.
-++
-+Each _<branch>_ is interpreted against the local repository: a literal
-+upstream like `origin/main` or a local branch like `master`, or a
-+wildmatch pattern like `'origin/*'`. A bare configured-remote name
-+(e.g. `origin`) resolves to the target of `refs/remotes/<remote>/HEAD`,
-+to match the way `git checkout -b topic origin` picks a starting
-+point. Multiple _<branch>_ arguments are unioned.
++`--forked <branch>`::
++ List only branches whose configured upstream matches
++ _<branch>_. The argument can be a ref (e.g. `origin/main`,
++ `master`) or a shell-style glob (e.g. `'origin/*'`). The
++ option can be repeated to widen the filter.
+
`-v`::
`-vv`::
@@ builtin/branch.c
+#include "wildmatch.h"
static const char * const builtin_branch_usage[] = {
- N_("git branch [<options>] [-r | -a] [--merged] [--no-merged]"),
-@@ builtin/branch.c: 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 <branch>..."),
- NULL
- };
+- N_("git branch [<options>] [-r | -a] [--merged] [--no-merged]"),
++ N_("git branch [<options>] [-r | -a] [--merged] [--no-merged] [(--forked <branch>)...]"),
+ N_("git branch [<options>] [-f] [--recurse-submodules] <branch-name> [<start-point>]"),
+ N_("git branch [<options>] [-l] [<pattern>...]"),
+ N_("git branch [<options>] [-r] (-d | -D) <branch-name>..."),
+@@ builtin/branch.c: static char *build_format(struct ref_filter *filter, int maxwidth, const char *r
+ return strbuf_detach(&fmt, NULL);
+ }
+
++static void filter_array_by_forked(struct ref_array *array,
++ const struct string_list *upstreams);
++
+ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sorting,
+- struct ref_format *format, struct string_list *output)
++ struct ref_format *format, struct string_list *output,
++ const struct string_list *forked_upstreams)
+ {
+ int i;
+ struct ref_array array;
+@@ builtin/branch.c: static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin
+
+ filter_refs(&array, filter, filter->kind);
+
++ if (forked_upstreams->nr)
++ filter_array_by_forked(&array, forked_upstreams);
++
+ if (filter->verbose)
+ maxwidth = calc_maxwidth(&array, strlen(remote_prefix));
@@ builtin/branch.c: static void copy_or_rename_branch(const char *oldname, const char *newname, int
free_worktrees(worktrees);
@@ builtin/branch.c: static void copy_or_rename_branch(const char *oldname, const c
+
+static int parse_one_forked_arg(const char *arg, struct upstream_pattern *out)
+{
-+ struct ref_store *refs = get_main_ref_store(the_repository);
-+ struct remote *remote;
+ struct object_id oid;
+ char *full_ref = NULL;
-+ struct strbuf head_ref = STRBUF_INIT;
-+ const char *resolved;
+
+ if (has_glob_specials(arg)) {
+ out->name = xstrdup(arg);
@@ builtin/branch.c: static void copy_or_rename_branch(const char *oldname, const c
+ return 0;
+ }
+
-+ remote = remote_get(arg);
-+ if (remote && remote_is_configured(remote, 0)) {
-+ strbuf_addf(&head_ref, "refs/remotes/%s/HEAD", remote->name);
-+ resolved = refs_resolve_ref_unsafe(refs, head_ref.buf,
-+ RESOLVE_REF_NO_RECURSE,
-+ NULL, NULL);
-+ if (resolved && starts_with(resolved, "refs/remotes/")) {
-+ out->name = xstrdup(short_upstream_name(resolved));
-+ out->is_wildcard = 0;
-+ strbuf_release(&head_ref);
-+ return 0;
-+ }
-+ strbuf_release(&head_ref);
-+ }
-+
+ if (repo_dwim_ref(the_repository, arg, strlen(arg), &oid,
+ &full_ref, 0) == 1 &&
+ (starts_with(full_ref, "refs/heads/") ||
@@ builtin/branch.c: static void copy_or_rename_branch(const char *oldname, const c
+ return -1;
+}
+
-+static void parse_forked_args(int argc, const char **argv,
++static void parse_forked_args(const struct string_list *args,
+ struct upstream_pattern **patterns_out,
+ size_t *nr_out)
+{
+ struct upstream_pattern *patterns;
-+ int i;
++ size_t i;
+
-+ ALLOC_ARRAY(patterns, argc);
-+ for (i = 0; i < argc; i++) {
-+ if (parse_one_forked_arg(argv[i], &patterns[i]) < 0) {
++ ALLOC_ARRAY(patterns, args->nr);
++ for (i = 0; i < args->nr; i++) {
++ const char *arg = args->items[i].string;
++ if (parse_one_forked_arg(arg, &patterns[i]) < 0) {
+ upstream_pattern_list_clear(patterns, i);
-+ die(_("'%s' is not a valid branch or pattern"),
-+ argv[i]);
++ die(_("'%s' is not a valid branch or pattern"), arg);
+ }
+ }
+ *patterns_out = patterns;
-+ *nr_out = argc;
++ *nr_out = args->nr;
+}
+
+static int upstream_matches(const char *short_upstream,
@@ builtin/branch.c: static void copy_or_rename_branch(const char *oldname, const c
+ return 0;
+}
+
-+struct forked_cb {
-+ const struct upstream_pattern *patterns;
-+ size_t nr_patterns;
-+ struct string_list *out;
-+};
-+
-+static int collect_forked_branch(const struct reference *ref, void *cb_data)
++static int branch_upstream_matches(const char *full_refname,
++ const struct upstream_pattern *patterns,
++ size_t nr_patterns)
+{
-+ struct forked_cb *cb = cb_data;
++ const char *short_name;
+ struct branch *branch;
+ const char *upstream;
+
-+ if (ref->flags & REF_ISSYMREF)
++ if (!skip_prefix(full_refname, "refs/heads/", &short_name))
+ return 0;
-+ branch = branch_get(ref->name);
++ branch = branch_get(short_name);
+ if (!branch)
+ return 0;
+ upstream = branch_get_upstream(branch, NULL);
+ if (!upstream)
+ return 0;
-+ if (upstream_matches(short_upstream_name(upstream),
-+ cb->patterns, cb->nr_patterns))
-+ string_list_append(cb->out, ref->name);
-+ return 0;
++ return upstream_matches(short_upstream_name(upstream),
++ patterns, nr_patterns);
+}
+
-+static int list_forked_branches(int argc, const char **argv)
++static void filter_array_by_forked(struct ref_array *array,
++ const struct string_list *upstreams)
+{
+ struct upstream_pattern *patterns = NULL;
+ size_t nr_patterns = 0;
-+ struct string_list out = STRING_LIST_INIT_DUP;
-+ struct string_list_item *item;
-+ struct forked_cb cb;
-+
-+ if (!argc)
-+ die(_("--forked requires at least one <branch>"));
++ int i, kept = 0;
+
-+ parse_forked_args(argc, argv, &patterns, &nr_patterns);
-+ cb.patterns = patterns;
-+ cb.nr_patterns = nr_patterns;
-+ cb.out = &out;
++ parse_forked_args(upstreams, &patterns, &nr_patterns);
+
-+ 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);
++ for (i = 0; i < array->nr; i++) {
++ struct ref_array_item *item = array->items[i];
++ if (branch_upstream_matches(item->refname,
++ patterns, nr_patterns))
++ array->items[kept++] = item;
++ else
++ free_ref_array_item(item);
++ }
++ array->nr = kept;
+
+ upstream_pattern_list_clear(patterns, nr_patterns);
-+ string_list_clear(&out, 0);
-+ return 0;
+}
+
static GIT_PATH_FUNC(edit_description, "EDIT_DESCRIPTION")
@@ builtin/branch.c: 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;
++ struct string_list forked_upstreams = STRING_LIST_INIT_DUP;
const char *new_upstream = NULL;
int noncreate_actions = 0;
/* possible options */
@@ builtin/branch.c: 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 whose upstream matches the given <branch>...")),
++ OPT_STRING_LIST(0, "forked", &forked_upstreams, N_("branch"),
++ N_("list local branches whose upstream matches <branch> (repeatable)")),
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")),
@@ builtin/branch.c: 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 ||
-@@ builtin/branch.c: int cmd_branch(int argc,
+- filter.reachable_from || filter.unreachable_from || filter.points_at.nr)
++ filter.reachable_from || filter.unreachable_from ||
++ filter.points_at.nr || forked_upstreams.nr)
+ list = 1;
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);
-
@@ builtin/branch.c: 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;
+ ref_sorting_set_sort_flags_all(sorting, REF_SORTING_ICASE, icase);
+ ref_sorting_set_sort_flags_all(
+ sorting, REF_SORTING_DETACHED_HEAD_FIRST, 1);
+- print_ref_list(&filter, sorting, &format, &output);
++ print_ref_list(&filter, sorting, &format, &output,
++ &forked_upstreams);
+ print_columns(&output, colopts, NULL);
+ string_list_clear(&output, 0);
+ ref_sorting_release(sorting);
+@@ builtin/branch.c: int cmd_branch(int argc,
+
+ out:
+ string_list_clear(&sorting_options, 0);
++ string_list_clear(&forked_upstreams, 0);
+ return ret;
+ }
+
+ ## ref-filter.c ##
+@@ ref-filter.c: static int filter_one(const struct reference *ref, void *cb_data)
+ }
+
+ /* Free memory allocated for a ref_array_item */
+-static void free_array_item(struct ref_array_item *item)
++void free_ref_array_item(struct ref_array_item *item)
+ {
+ free((char *)item->symref);
+ if (item->value) {
+@@ ref-filter.c: static int filter_and_format_one(const struct reference *ref, void *cb_data)
+
+ strbuf_release(&output);
+ strbuf_release(&err);
+- free_array_item(item);
++ free_ref_array_item(item);
+
+ /*
+ * Increment the running count of refs that match the filter. If
+@@ ref-filter.c: void ref_array_clear(struct ref_array *array)
+ int i;
+
+ for (i = 0; i < array->nr; i++)
+- free_array_item(array->items[i]);
++ free_ref_array_item(array->items[i]);
+ FREE_AND_NULL(array->items);
+ array->nr = array->alloc = 0;
+
+@@ ref-filter.c: static void reach_filter(struct ref_array *array,
+ if (is_merged == include_reached)
+ array->items[array->nr++] = array->items[i];
+ else
+- free_array_item(item);
++ free_ref_array_item(item);
+ }
+
+ clear_commit_marks_many(old_nr, to_clear, ALL_REV_FLAGS);
+@@ ref-filter.c: void pretty_print_ref(const char *name, const struct object_id *oid,
+
+ strbuf_release(&err);
+ strbuf_release(&output);
+- free_array_item(ref_item);
++ free_ref_array_item(ref_item);
+ }
+
+ static int parse_sorting_atom(const char *atom)
+
+ ## ref-filter.h ##
+@@ ref-filter.h: void filter_and_format_refs(struct ref_filter *filter, unsigned int type,
+ struct ref_format *format);
+ /* Clear all memory allocated to ref_array */
+ void ref_array_clear(struct ref_array *array);
++/* Free a single item from a ref_array */
++void free_ref_array_item(struct ref_array_item *item);
+ /* Used to verify if the given format is correct and to parse out the used atoms */
+ int verify_ref_format(struct ref_format *format);
+ /* Sort the given ref_array as per the ref_sorting provided */
## t/t3200-branch.sh ##
@@ t/t3200-branch.sh: test_expect_success 'errors if given a bad branch name' '
@@ t/t3200-branch.sh: test_expect_success 'errors if given a bad branch name' '
+ git -C forked branch --track local-trunk local-base
+'
+
-+test_expect_success '--forked <upstream-tracking-branch> lists matching branches' '
-+ git -C forked branch --forked origin/one >actual &&
++test_expect_success '--forked <upstream-tracking-branch> filters by upstream' '
++ git -C forked branch --forked origin/one --format="%(refname:short)" >actual &&
+ echo local-one >expect &&
+ test_cmp expect actual
+'
+
-+test_expect_success '--forked <glob> matches by wildmatch' '
-+ git -C forked branch --forked "origin/*" >actual &&
++test_expect_success '--forked <glob> filters by wildmatch' '
++ git -C forked branch --forked "origin/*" --format="%(refname:short)" >actual &&
+ cat >expect <<-\EOF &&
+ local-one
+ local-two
@@ t/t3200-branch.sh: test_expect_success 'errors if given a bad branch name' '
+'
+
+test_expect_success '--forked <local-branch> matches branches with local upstream' '
-+ git -C forked branch --forked local-base >actual &&
++ git -C forked branch --forked local-base --format="%(refname:short)" >actual &&
+ echo local-trunk >expect &&
+ test_cmp expect actual
+'
+
-+test_expect_success '--forked <remote> resolves via refs/remotes/<remote>/HEAD' '
-+ test_when_finished "git -C forked symbolic-ref refs/remotes/origin/HEAD refs/remotes/origin/main" &&
-+ git -C forked symbolic-ref refs/remotes/origin/HEAD refs/remotes/origin/one &&
-+ git -C forked branch --forked origin >actual &&
-+ echo local-one >expect &&
-+ test_cmp expect actual
-+'
-+
-+test_expect_success '--forked unions multiple <branch> arguments' '
-+ git -C forked branch --forked origin/one other/foreign >actual &&
++test_expect_success '--forked can be repeated to widen the filter' '
++ git -C forked branch --forked origin/one --forked other/foreign --format="%(refname:short)" >actual &&
+ cat >expect <<-\EOF &&
+ local-foreign
+ local-one
@@ t/t3200-branch.sh: test_expect_success 'errors if given a bad branch name' '
+'
+
+test_expect_success '--forked combines literal and glob arguments' '
-+ git -C forked branch --forked local-base "other/*" >actual &&
++ git -C forked branch --forked local-base --forked "other/*" --format="%(refname:short)" >actual &&
+ cat >expect <<-\EOF &&
+ local-foreign
+ local-trunk
@@ t/t3200-branch.sh: test_expect_success 'errors if given a bad branch name' '
+'
+
+test_expect_success '--forked "*/*" covers every remote-tracking upstream' '
-+ git -C forked branch --forked "*/*" >actual &&
++ git -C forked branch --forked "*/*" --format="%(refname:short)" >actual &&
+ cat >expect <<-\EOF &&
+ local-foreign
+ local-one
@@ t/t3200-branch.sh: test_expect_success 'errors if given a bad branch name' '
+ test_cmp expect actual
+'
+
++test_expect_success '--forked composes with --no-merged' '
++ test_when_finished "git -C forked checkout detached" &&
++ git -C forked checkout local-one &&
++ test_commit -C forked local-only &&
++ git -C forked branch --forked "origin/*" --no-merged origin/one \
++ --format="%(refname:short)" >actual &&
++ echo local-one >expect &&
++ test_cmp expect actual
++'
++
+test_expect_success '--forked rejects unknown branch/pattern' '
+ test_must_fail git -C forked branch --forked nope 2>err &&
+ test_grep "not a valid branch or pattern" err
+'
+
-+test_expect_success '--forked requires at least one <branch>' '
++test_expect_success '--forked requires a value' '
+ test_must_fail git -C forked branch --forked 2>err &&
-+ test_grep "at least one <branch>" err
++ test_grep "requires a value" err
+'
+
test_done
2: b666d09bf5 ! 2: 6c95e4e77c branch: let delete_branches warn instead of error on bulk refusal
@@ Commit message
so a bulk caller can report not-fully-merged branches as one-line
warnings and continue, instead of erroring with the four-line "use
'git branch -D'" advice that the standalone "git branch -d" path
- emits. Default callers pass 0 and are unaffected.
+ emits. Default callers pass 0 and are unaffected.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
@@ builtin/branch.c: int cmd_branch(int argc,
+ ret = delete_branches(argc, argv, delete > 1, filter.kind,
+ quiet, 0);
goto out;
- } else if (forked) {
- ret = list_forked_branches(argc, argv);
+ } else if (show_current) {
+ print_current_branch_name();
3: 6e6580270e ! 3: 004a96f7a4 branch: prepare delete_branches for a bulk caller
@@ Metadata
## Commit message ##
branch: prepare delete_branches for a bulk caller
- Add no_head_fallback and dry_run flags to delete_branches() so a bulk
- caller (the upcoming --prune-merged) can ask strictly about
- merged-into-upstream without a silent fallback to HEAD, and rehearse
- deletions with the same "Would delete branch ..." wording as the live
- run. Existing callers pass 0 for both and keep current behavior.
+ Add no_head_fallback and dry_run flags to delete_branches() so a
+ bulk caller (the upcoming --prune-merged) can ask strictly about
+ merged-into-upstream without a silent fallback to HEAD, and
+ rehearse deletions with the same "Would delete branch ..." wording
+ as the live run. Existing callers pass 0 for both and keep current
+ behavior.
When no_head_fallback is set, head_rev stays NULL through to
branch_merged(), whose "merged to X but not yet merged to HEAD"
- reminder otherwise compares against HEAD. That reminder is only
- meaningful when the caller actually cares about HEAD; for the
- bulk caller every candidate is known to have an upstream and HEAD
- is irrelevant to the decision. Guard the block on head_rev so the
- NULL case skips it instead of treating "NULL != reference_rev" as
- "diverges from HEAD" and emitting a spurious warning.
+ reminder otherwise compares against HEAD. For the bulk caller
+ every candidate is known to have an upstream, so HEAD is
+ irrelevant. Guard the block on head_rev so the NULL case skips
+ it instead of treating "NULL != reference_rev" as "diverges from
+ HEAD" and emitting a spurious warning.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
@@ builtin/branch.c: int cmd_branch(int argc,
- quiet, 0);
+ quiet, 0, 0, 0);
goto out;
- } else if (forked) {
- ret = list_forked_branches(argc, argv);
+ } else if (show_current) {
+ print_current_branch_name();
4: e7e03c1338 ! 4: cccfdb831c branch: add --prune-merged <branch>
@@ Commit message
upstream: the work has already landed on the upstream they track,
so the local copy is no longer needed.
- Reachability is read from the local refs only -- nothing is
- fetched. Users who want fresh upstream refs run "git fetch" first;
- the deletion path stays a separate, idempotent step that also
- works offline.
+ Reachability is read from local refs; nothing is fetched. Users
+ who want fresh upstream refs run "git fetch" first.
Three classes of branches are spared:
@@ Commit message
* any branch whose push destination equals its upstream
(<branch>@{push} == <branch>@{upstream}). Such a branch
cannot be distinguished from a freshly pulled trunk that
- just looks "fully merged" -- e.g. local "main" tracking and
+ just looks "fully merged", e.g. local "main" tracking and
pushing to "origin/main" right after a pull. Only branches
that push somewhere other than their upstream (typically
topics in a fork-based workflow) are treated as candidates.
@@ Commit message
mode and with the HEAD-fallback disabled: a branch that is not
yet fully merged to its upstream is reported as a one-line warning
and skipped, so a single un-mergeable topic does not abort the
- whole sweep, and there is no fallback to "merged into the
- currently checked out branch" -- we only act on upstream-merged
- status.
+ whole sweep. We only act on upstream-merged status.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
## Documentation/git-branch.adoc ##
-@@ Documentation/git-branch.adoc: git branch (-c|-C) [<old-branch>] <new-branch>
+@@ Documentation/git-branch.adoc: git branch (-m|-M) [<old-branch>] <new-branch>
+ git branch (-c|-C) [<old-branch>] <new-branch>
git branch (-d|-D) [-r] <branch-name>...
git branch --edit-description [<branch-name>]
- git branch --forked <branch>...
-+git branch --prune-merged <branch>...
++git branch (--prune-merged <branch>)...
DESCRIPTION
-----------
-@@ Documentation/git-branch.adoc: wildmatch pattern like `'origin/*'`. A bare configured-remote name
- to match the way `git checkout -b topic origin` picks a starting
- point. Multiple _<branch>_ arguments are unioned.
+@@ Documentation/git-branch.adoc: This option is only applicable in non-verbose mode.
+ `master`) or a shell-style glob (e.g. `'origin/*'`). The
+ option can be repeated to widen the filter.
-+`--prune-merged`::
++`--prune-merged <branch>`::
+ Delete the local branches that `--forked` would list for the
-+ same _<branch>_ arguments, but only those whose tip is
-+ reachable from their configured upstream. In other words,
-+ the work on the branch has already landed on the upstream it
-+ tracks, so the local copy is no longer needed.
++ same _<branch>_, but only those whose tip is reachable from
++ their configured upstream. In other words, the work on the
++ branch has already landed on the upstream it tracks, so the
++ local copy is no longer needed. May be given more than once to
++ union the matches; positional arguments are not accepted.
++
+Reachability is checked against whatever the upstream refs say
-+locally; nothing is fetched. Run `git fetch` first if you want
++locally; nothing is fetched. Run `git fetch` first if you want
+the upstream refs refreshed.
++
+A branch is left alone if any of the following holds:
@@ Documentation/git-branch.adoc: wildmatch pattern like `'origin/*'`. A bare conf
`--verbose`::
## builtin/branch.c ##
-@@ builtin/branch.c: static int collect_forked_branch(const struct reference *ref, void *cb_data)
+@@ builtin/branch.c: 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>] (--prune-merged <branch>)..."),
+ NULL
+ };
+
+@@ builtin/branch.c: static int upstream_matches(const char *short_upstream,
return 0;
}
--static int list_forked_branches(int argc, const char **argv)
-+static void collect_forked_set(int argc, const char **argv,
-+ struct string_list *out)
+-static int branch_upstream_matches(const char *full_refname,
++static int branch_upstream_matches(const char *short_branch_name,
+ const struct upstream_pattern *patterns,
+ size_t nr_patterns)
{
- struct upstream_pattern *patterns = NULL;
- size_t nr_patterns = 0;
-- struct string_list out = STRING_LIST_INIT_DUP;
-- struct string_list_item *item;
- struct forked_cb cb;
+- const char *short_name;
+- struct branch *branch;
++ struct branch *branch = branch_get(short_branch_name);
+ const char *upstream;
-- if (!argc)
-- die(_("--forked requires at least one <branch>"));
--
- parse_forked_args(argc, argv, &patterns, &nr_patterns);
- cb.patterns = patterns;
- cb.nr_patterns = nr_patterns;
-- cb.out = &out;
-+ cb.out = out;
+- if (!skip_prefix(full_refname, "refs/heads/", &short_name))
+- return 0;
+- branch = branch_get(short_name);
+ if (!branch)
+ return 0;
+ upstream = branch_get_upstream(branch, NULL);
+@@ builtin/branch.c: static void filter_array_by_forked(struct ref_array *array,
- refs_for_each_branch_ref(get_main_ref_store(the_repository),
- collect_forked_branch, &cb);
+ for (i = 0; i < array->nr; i++) {
+ struct ref_array_item *item = array->items[i];
+- if (branch_upstream_matches(item->refname,
+- patterns, nr_patterns))
++ const char *short_name;
++ if (skip_prefix(item->refname, "refs/heads/", &short_name) &&
++ branch_upstream_matches(short_name, patterns, nr_patterns))
+ array->items[kept++] = item;
+ else
+ free_ref_array_item(item);
+@@ builtin/branch.c: static void filter_array_by_forked(struct ref_array *array,
+ upstream_pattern_list_clear(patterns, nr_patterns);
+ }
-- string_list_sort(&out);
-+ string_list_sort(out);
++struct forked_cb {
++ const struct upstream_pattern *patterns;
++ size_t nr_patterns;
++ struct string_list *out;
++};
+
-+ upstream_pattern_list_clear(patterns, nr_patterns);
++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_upstream_matches(ref->name, cb->patterns, cb->nr_patterns))
++ string_list_append(cb->out, ref->name);
++ return 0;
+}
+
-+static int list_forked_branches(int argc, const char **argv)
++static void collect_forked_set(const struct string_list *upstreams,
++ struct string_list *out)
+{
-+ struct string_list out = STRING_LIST_INIT_DUP;
-+ struct string_list_item *item;
++ struct upstream_pattern *patterns = NULL;
++ size_t nr_patterns = 0;
++ struct forked_cb cb;
+
-+ if (!argc)
-+ die(_("--forked requires at least one <branch>"));
++ parse_forked_args(upstreams, &patterns, &nr_patterns);
++ cb.patterns = patterns;
++ cb.nr_patterns = nr_patterns;
++ cb.out = out;
+
-+ collect_forked_set(argc, argv, &out);
- for_each_string_list_item(item, &out)
- puts(item->string);
-
-- upstream_pattern_list_clear(patterns, nr_patterns);
- string_list_clear(&out, 0);
- return 0;
- }
-
-+static int prune_merged_branches(int argc, const char **argv, int quiet)
++ refs_for_each_branch_ref(get_main_ref_store(the_repository),
++ collect_forked_branch, &cb);
++
++ string_list_sort(out);
++
++ upstream_pattern_list_clear(patterns, nr_patterns);
++}
++
++static int prune_merged_branches(const struct string_list *upstreams,
++ int quiet)
+{
+ struct ref_store *refs = get_main_ref_store(the_repository);
+ struct string_list candidates = STRING_LIST_INIT_DUP;
@@ builtin/branch.c: static int collect_forked_branch(const struct reference *ref,
+ struct string_list_item *item;
+ int ret = 0;
+
-+ if (!argc)
++ if (!upstreams->nr)
+ die(_("--prune-merged requires at least one <branch>"));
+
-+ collect_forked_set(argc, argv, &candidates);
++ collect_forked_set(upstreams, &candidates);
+
+ for_each_string_list_item(item, &candidates) {
+ const char *short_name = item->string;
@@ builtin/branch.c: static int collect_forked_branch(const struct reference *ref,
@@ builtin/branch.c: 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;
+ struct string_list forked_upstreams = STRING_LIST_INIT_DUP;
++ struct string_list prune_merged_upstreams = STRING_LIST_INIT_DUP;
const char *new_upstream = NULL;
int noncreate_actions = 0;
/* possible options */
@@ builtin/branch.c: int cmd_branch(int argc,
N_("edit the description for the branch")),
- OPT_BOOL(0, "forked", &forked,
- N_("list local branches whose upstream matches the given <branch>...")),
-+ OPT_BOOL(0, "prune-merged", &prune_merged,
-+ N_("delete local branches whose upstream matches the given <branch>... and is merged")),
+ OPT_STRING_LIST(0, "forked", &forked_upstreams, N_("branch"),
+ N_("list local branches whose upstream matches <branch> (repeatable)")),
++ OPT_STRING_LIST(0, "prune-merged", &prune_merged_upstreams, N_("branch"),
++ N_("delete local branches whose upstream matches <branch> and is merged (repeatable)")),
OPT__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")),
@@ builtin/branch.c: 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 &&
+- !show_current && !unset_upstream && argc == 0)
++ !show_current && !unset_upstream && !prune_merged_upstreams.nr &&
+ argc == 0)
list = 1;
@@ builtin/branch.c: 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;
+- !!unset_upstream;
++ !!unset_upstream + !!prune_merged_upstreams.nr;
if (noncreate_actions > 1)
usage_with_options(builtin_branch_usage, options);
@@ builtin/branch.c: int cmd_branch(int argc,
- } else if (forked) {
- ret = list_forked_branches(argc, argv);
+ ret = delete_branches(argc, argv, delete > 1, filter.kind,
+ quiet, 0, 0, 0);
goto out;
-+ } else if (prune_merged) {
-+ ret = prune_merged_branches(argc, argv, quiet);
++ } else if (prune_merged_upstreams.nr) {
++ if (argc)
++ die(_("--prune-merged does not take positional arguments; "
++ "repeat --prune-merged for each <branch>"));
++ ret = prune_merged_branches(&prune_merged_upstreams, quiet);
+ goto out;
} else if (show_current) {
print_current_branch_name();
ret = 0;
+@@ builtin/branch.c: int cmd_branch(int argc,
+ out:
+ string_list_clear(&sorting_options, 0);
+ string_list_clear(&forked_upstreams, 0);
++ string_list_clear(&prune_merged_upstreams, 0);
+ return ret;
+ }
## t/t3200-branch.sh ##
-@@ t/t3200-branch.sh: test_expect_success '--forked requires at least one <branch>' '
- test_grep "at least one <branch>" err
+@@ t/t3200-branch.sh: test_expect_success '--forked requires a value' '
+ test_grep "requires a value" err
'
+test_expect_success '--prune-merged: setup' '
@@ t/t3200-branch.sh: test_expect_success '--forked requires at least one <branch>'
+ git -C pm-union branch --set-upstream-to=origin/main two &&
+ git -C pm-union checkout --detach &&
+
-+ git -C pm-union branch --prune-merged origin/next origin/main &&
++ git -C pm-union branch --prune-merged origin/next --prune-merged origin/main &&
+
+ test_must_fail git -C pm-union rev-parse --verify refs/heads/one &&
+ test_must_fail git -C pm-union rev-parse --verify refs/heads/two
@@ t/t3200-branch.sh: test_expect_success '--forked requires at least one <branch>'
+ test_must_fail git -C pm-push-diff rev-parse --verify refs/heads/topic
+'
+
-+test_expect_success '--prune-merged requires at least one <branch>' '
++test_expect_success '--prune-merged requires a value' '
+ test_must_fail git -C forked branch --prune-merged 2>err &&
-+ test_grep "at least one <branch>" err
++ test_grep "requires a value" err
++'
++
++test_expect_success '--prune-merged rejects positional arguments' '
++ test_must_fail git -C forked branch --prune-merged origin/one other/foreign 2>err &&
++ test_grep "does not take positional arguments" err
+'
+
test_done
5: 75b6d2366a ! 5: 5f793f8d0d branch: add branch.<name>.pruneMerged opt-out
@@ Documentation/git-branch.adoc: the upstream refs refreshed.
warnings and skipped; pass them to `git branch -D` explicitly if
## builtin/branch.c ##
-@@ builtin/branch.c: static int prune_merged_branches(int argc, const char **argv, int quiet)
+@@ builtin/branch.c: static int prune_merged_branches(const struct string_list *upstreams,
struct branch *branch = branch_get(short_name);
const char *upstream, *push;
struct strbuf full = STRBUF_INIT;
@@ builtin/branch.c: static int prune_merged_branches(int argc, const char **argv,
strbuf_addf(&full, "refs/heads/%s", short_name);
skip = !!branch_checked_out(full.buf);
-@@ builtin/branch.c: static int prune_merged_branches(int argc, const char **argv, int quiet)
+@@ builtin/branch.c: static int prune_merged_branches(const struct string_list *upstreams,
if (!push || !strcmp(push, upstream))
continue;
@@ builtin/branch.c: static int prune_merged_branches(int argc, const char **argv,
## t/t3200-branch.sh ##
-@@ t/t3200-branch.sh: test_expect_success '--prune-merged requires at least one <branch>' '
- test_grep "at least one <branch>" err
+@@ t/t3200-branch.sh: test_expect_success '--prune-merged rejects positional arguments' '
+ test_grep "does not take positional arguments" err
'
+test_expect_success '--prune-merged honours branch.<name>.pruneMerged=false' '
6: a1a42a6b19 ! 6: 1a0d5eab15 branch: add --dry-run for --prune-merged
@@ Commit message
branch: add --dry-run for --prune-merged
With --dry-run, --prune-merged prints the local branches it would
- delete -- one "Would delete branch <name>" line per candidate --
- and exits without touching any ref.
+ delete, one "Would delete branch <name>" line per candidate, and
+ exits without touching any ref.
- This is the natural sanity check before letting a broad pattern
- like 'origin/*' run for real: the @{push}-vs-@{upstream} and
- unmerged filtering still applies, so the dry-run output is
- exactly the set that the live run would delete.
+ The @{push}-vs-@{upstream} and unmerged filtering still applies,
+ so the dry-run output is exactly the set that the live run would
+ delete.
--dry-run is only meaningful in combination with --prune-merged
and is rejected otherwise.
@@ Commit message
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
## Documentation/git-branch.adoc ##
-@@ Documentation/git-branch.adoc: git branch (-c|-C) [<old-branch>] <new-branch>
+@@ Documentation/git-branch.adoc: git branch (-m|-M) [<old-branch>] <new-branch>
+ git branch (-c|-C) [<old-branch>] <new-branch>
git branch (-d|-D) [-r] <branch-name>...
git branch --edit-description [<branch-name>]
- git branch --forked <branch>...
--git branch --prune-merged <branch>...
-+git branch --prune-merged [--dry-run] <branch>...
+-git branch (--prune-merged <branch>)...
++git branch [--dry-run] (--prune-merged <branch>)...
DESCRIPTION
-----------
@@ Documentation/git-branch.adoc: Branches refused by the "fully merged" safety che
`--verbose`::
## builtin/branch.c ##
-@@ builtin/branch.c: static int list_forked_branches(int argc, const char **argv)
- return 0;
+@@ builtin/branch.c: static void collect_forked_set(const struct string_list *upstreams,
}
--static int prune_merged_branches(int argc, const char **argv, int quiet)
-+static int prune_merged_branches(int argc, const char **argv, int quiet,
-+ int dry_run)
+ static int prune_merged_branches(const struct string_list *upstreams,
+- int quiet)
++ int quiet, int dry_run)
{
struct ref_store *refs = get_main_ref_store(the_repository);
struct string_list candidates = STRING_LIST_INIT_DUP;
-@@ builtin/branch.c: static int prune_merged_branches(int argc, const char **argv, int quiet)
+@@ builtin/branch.c: static int prune_merged_branches(const struct string_list *upstreams,
quiet,
1, /* warn_only */
1, /* no_head_fallback */
@@ builtin/branch.c: static int prune_merged_branches(int argc, const char **argv,
string_list_clear(&candidates, 0);
@@ builtin/branch.c: int cmd_branch(int argc,
unset_upstream = 0, show_current = 0, edit_description = 0;
- int forked = 0;
- int prune_merged = 0;
+ struct string_list forked_upstreams = STRING_LIST_INIT_DUP;
+ struct string_list prune_merged_upstreams = STRING_LIST_INIT_DUP;
+ int dry_run = 0;
const char *new_upstream = NULL;
int noncreate_actions = 0;
/* possible options */
@@ builtin/branch.c: int cmd_branch(int argc,
- N_("list local branches whose upstream matches the given <branch>...")),
- OPT_BOOL(0, "prune-merged", &prune_merged,
- N_("delete local branches whose upstream matches the given <branch>... and is merged")),
+ N_("list local branches whose upstream matches <branch> (repeatable)")),
+ OPT_STRING_LIST(0, "prune-merged", &prune_merged_upstreams, N_("branch"),
+ N_("delete local branches whose upstream matches <branch> and is merged (repeatable)")),
+ OPT_BOOL(0, "dry-run", &dry_run,
+ N_("with --prune-merged, only print which branches would be deleted")),
OPT__FORCE(&force, N_("force creation, move/rename, deletion"), PARSE_OPT_NOCOMPLETE),
@@ builtin/branch.c: int cmd_branch(int argc,
if (noncreate_actions > 1)
usage_with_options(builtin_branch_usage, options);
-+ if (dry_run && !prune_merged)
++ if (dry_run && !prune_merged_upstreams.nr)
+ die(_("--dry-run requires --prune-merged"));
+
if (recurse_submodules_explicit) {
if (!submodule_propagate_branches)
die(_("branch with --recurse-submodules can only be used if submodule.propagateBranches is enabled"));
@@ builtin/branch.c: int cmd_branch(int argc,
- ret = list_forked_branches(argc, argv);
- goto out;
- } else if (prune_merged) {
-- ret = prune_merged_branches(argc, argv, quiet);
-+ ret = prune_merged_branches(argc, argv, quiet, dry_run);
+ if (argc)
+ die(_("--prune-merged does not take positional arguments; "
+ "repeat --prune-merged for each <branch>"));
+- ret = prune_merged_branches(&prune_merged_upstreams, quiet);
++ ret = prune_merged_branches(&prune_merged_upstreams, quiet, dry_run);
goto out;
} else if (show_current) {
print_current_branch_name();
@@ t/t3200-branch.sh: test_expect_success 'branch -d still deletes a pruneMerged=fa
+ git -C pm-dry branch two two-commit &&
+ git -C pm-dry branch --set-upstream-to=origin/next two &&
+
-+ git -C pm-dry branch --prune-merged --dry-run "origin/*" >actual &&
++ git -C pm-dry branch --dry-run --prune-merged "origin/*" >actual &&
+ test_grep "Would delete branch one " actual &&
+ test_grep "Would delete branch two " actual &&
+
@@ t/t3200-branch.sh: test_expect_success 'branch -d still deletes a pruneMerged=fa
+ git -C pm-dry-mixed branch merged one-commit &&
+ git -C pm-dry-mixed branch --set-upstream-to=origin/next merged &&
+
-+ git -C pm-dry-mixed branch --prune-merged --dry-run "origin/*" >out &&
++ git -C pm-dry-mixed branch --dry-run --prune-merged "origin/*" >out &&
+ test_grep "Would delete branch merged" out &&
+ test_grep ! "Would delete branch wip" out &&
+ git -C pm-dry-mixed rev-parse --verify refs/heads/wip &&
--
gitgitgadget
^ permalink raw reply
* Re: [PATCH 2/2] Documentation/MyFirstContribution: recommend the use of b4
From: Weijie Yuan @ 2026-06-03 8:00 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Junio C Hamano
In-Reply-To: <ah_dh3uozNdYcL0_@wyuan.org>
That said, my ideas may be too nitpicking. Overall, I do
think that introducing b4 support would be a good thing.
Thanks for the replies, and for bearing with my questions/comments.
Weijie Yuan
^ permalink raw reply
* Re: [PATCH 2/2] Documentation/MyFirstContribution: recommend the use of b4
From: Weijie Yuan @ 2026-06-03 7:53 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Junio C Hamano
In-Reply-To: <ah_PwOsbYfDCx0H2@pks.im>
On Wed, Jun 03, 2026 at 08:54:56AM +0200, Patrick Steinhardt wrote:
> Ah, that's what you're hinting at. So you mean to say that folks should
> first understand the basics before basically automating all of the parts
> for them?
>
> I guess I can see where you're coming from, but I'm not sure I agree
> with this a 100%. My main goal is to make it easier for new community
> members to contribute to Git, and that means that we should automate all
> the hard parts as far as possible. This saves those new contributors
> from frustration, and it means that reviewers on the mailing list won't
> have to teach every single new contributor about how they should thread
> the mails, generate range-diffs and the like.
>
> So in the end, it saves both their and our time, but the learning
> opportunity is of course a bit diminished. I'd gladly accept that
> tradeoff though.
Yeah, after I expressed my opinion, I also felt a bit conflicted though.
So I also agree with your intension.
Make an inappropriate metaphor: some usage of b4 and magit are "Porcelain"
to Git. Whether how you are good at using those porcelains like magit or
lazygit, in the end, you will eventually have to face git cli one day.
So the same for b4. If we list these three methods equally and
simultaneously, the logic might be not that correct.
Your proposal:
contribution workflow
|
-------------------------------------
| | |
v v v
GitGitGadget traditional email b4
```
But I would frame it more like this:
contribution workflow
|
-------------------------
| |
v v
GitGitGadget traditional email workflow
\
\
b4
For exmaple, If I am at this page the fisrt time:
https://git-scm.com/docs/MyFirstContribution
And, I see these 3 ways, okay, I choose b4.
After installing b4 and reading some manuals, I would wonder: what's
cover letter? what's Message-ID? So after a while, I would still have to
learn those stuff and how b4 indeed optimize those complicated process.
So, to put it another way.. b4 is developed for high-level maintainers,
who are apparently familiar with traditional ways. Therefore b4 saves
their time. But for some beginers like me, I still have to know those
concepts first.
But yeah, there are definitely some people would happily accept b4 and
contribute easily. Thus, I agree this tradeoff.
--
Sent before reading v2, hope there's no conflict :-)
^ permalink raw reply
* Re: [PATCH 1/2] b4: introduce configuration for the Git project
From: Weijie Yuan @ 2026-06-03 7:50 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Tuomas Ahola, git, Junio C Hamano
In-Reply-To: <ah_PyDwO1Sffr5yq@pks.im>
On Wed, Jun 03, 2026 at 08:55:04AM +0200, Patrick Steinhardt wrote:
> So this quote is definitely at odds with the configuration I have
> proposed. It's actually quite surprising to me that we recommend deep
> threading -- I personally find it extremely hard to navigate as the
> nesting eventually gets way too deep.
Sorry I'm a little confused. The example thread at git-scm.com:
https://git-scm.com/docs/MyFirstContribution#ready-to-share
Isn't this actually supporting shallow nesting?
> It's actually quite surprising to me that we recommend deep
> threading -- I personally find it extremely hard to navigate as the
> nesting eventually gets way too deep.
In my understanding, deep threading == --chain-reply-to, so can you
point out where do Git recommend deep threading? I always thought Git
supports shallow threading.
Thanks! And please forgive me if I am wrong :-)
^ permalink raw reply
* [PATCH v2 3/3] b4: introduce configuration for the Git project
From: Patrick Steinhardt @ 2026-06-03 6:59 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Tuomas Ahola, Weijie Yuan, Ramsay Jones
In-Reply-To: <20260603-pks-b4-v2-0-a8aea0aa2c23@pks.im>
We're about to extend our documentation to recommend b4 for sending
patch series to the mailing list. Prepare for this by introducing a b4
configuration so that the tool knows to honor our preferences. For now,
this configuration does two things:
- It configures "send-same-thread = shallow", which tells b4 to always
send subsequent versions of the same patch series as a reply to the
cover letter of the first version.
- It configures "prep-cover-template", which tells b4 to use a custom
template for the cover letter. The most important change compared to
the default template is that our custom template also includes a
range-diff.
There's potentially more things that we may want to configure going
forward, like for example auto-configuration of folks to Cc on certain
patches. But these two tweaks feel like a good place to start.
Note that these values only serve as defaults, and users may want to
tweak those defaults based on their own preference. Luckily, users can
do that without having to touch `.b4-config` at all, as b4 allows them
to override values via Git configuration:
```
$ git config set b4.prep-cover-template /does/not/exist
$ b4 send --dry-run
ERROR: prep-cover-template says to use x, but it does not exist
```
So this gives users an easy way to override our defaults without having
to touch ".b4-config", which would dirty the tree.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
.b4-config | 6 ++++++
.b4-cover-template | 11 +++++++++++
2 files changed, 17 insertions(+)
diff --git a/.b4-config b/.b4-config
new file mode 100644
index 0000000000..fd4fb56b6d
--- /dev/null
+++ b/.b4-config
@@ -0,0 +1,6 @@
+# Note that these are default values that you can tweak via the typical
+# git-config(1) machinery. You thus shouldn't ever have to change this file.
+# See also https://b4.docs.kernel.org/en/latest/config.html.
+[b4]
+send-same-thread = shallow
+prep-cover-template = ./.b4-cover-template
diff --git a/.b4-cover-template b/.b4-cover-template
new file mode 100644
index 0000000000..ab864933b5
--- /dev/null
+++ b/.b4-cover-template
@@ -0,0 +1,11 @@
+${cover}
+
+---
+${shortlog}
+
+${diffstat}
+
+${range_diff}
+---
+base-commit: ${base_commit}
+${prerequisites}
--
2.54.0.1064.gd145956f57.dirty
^ permalink raw reply related
* [PATCH v2 2/3] Documentation/MyFirstContribution: recommend the use of b4
From: Patrick Steinhardt @ 2026-06-03 6:59 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Tuomas Ahola, Weijie Yuan, Ramsay Jones
In-Reply-To: <20260603-pks-b4-v2-0-a8aea0aa2c23@pks.im>
The b4 tool originates from the Linux kernel community and is intended
to help mailing-list based workflows. It automates a lot of the annoying
bookkeeping tasks that contributors typically need to do: tracking the
list of recipients, Message-IDs, range-diffs and the like. In addition
to that, b4 also has many other subcommands that help the maintainer and
reviewers.
The Git project uses the same infrastructure as the kernel, so this tool
is also a very good fit for us. Adapt "MyFirstContribution" to
explicitly recommend its use.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Documentation/MyFirstContribution.adoc | 92 ++++++++++++++++++++++++++++++++--
Documentation/SubmittingPatches | 6 ++-
2 files changed, 93 insertions(+), 5 deletions(-)
diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
index 069020196c..fc0b06ae67 100644
--- a/Documentation/MyFirstContribution.adoc
+++ b/Documentation/MyFirstContribution.adoc
@@ -833,7 +833,7 @@ This patchset is part of the MyFirstContribution tutorial and should not
be merged.
----
-At this point the tutorial diverges, in order to demonstrate two
+At this point the tutorial diverges, in order to demonstrate three
different methods of formatting your patchset and getting it reviewed.
The first method to be covered is GitGitGadget, which is useful for those
@@ -845,9 +845,14 @@ more fine-grained control over the emails to be sent. This method requires some
setup which can change depending on your system and will not be covered in this
tutorial.
+The third method to be covered is `b4`, which builds on top of `git
+format-patch` and `git send-email`. This method is the recommended way to
+submit patches via mail as it automates a lot of the bookkeeping required by
+`git send-email`.
+
Regardless of which method you choose, your engagement with reviewers will be
-the same; the review process will be covered after the sections on GitGitGadget
-and `git send-email`.
+the same; the review process will be covered after the sections on GitGitGadget,
+`git send-email` and `b4`.
[[howto-ggg]]
== Sending Patches via GitGitGadget
@@ -1296,6 +1301,87 @@ index 88f126184c..38da593a60 100644
2.21.0.392.gf8f6787159e-goog
----
+[[howto-b4]]
+== Sending Patches with `b4`
+
+`b4` is a tool that builds on top of `git format-patch` and `git send-email`.
+It automates much of the bookkeeping involved in sending a patch series to a
+mailing-list-based project.
+
+Refer to the https://b4.docs.kernel.org/[b4 documentation] for a full reference.
+
+[[prep-b4]]
+=== Preparing a Patch Series
+
+`b4` tracks your patch series as a branch. To start tracking the `psuh` branch
+you have been working on, run:
+
+----
+$ b4 prep --enroll master
+----
+
+This enrolls the current branch, using `master` as the base of the topic. `b4`
+manages the cover letter as part of the branch, so you can edit it at any time
+with:
+
+----
+$ b4 prep --edit-cover
+----
+
+The cover letter not only tracks the content of the top-level mail, but also
+the set of recipients. You can add recipients by adding `To:` and `Cc:`
+trailer lines.
+
+[[send-b4]]
+=== Sending the Patches
+
+Before sending the series out for real, you can inspect what `b4` would send by
+passing `--dry-run`:
+
+----
+$ b4 send --dry-run
+----
+
+Once you are happy with the result, send the series with:
+
+----
+$ b4 send
+----
+
+[[v2-b4]]
+=== Sending v2
+
+When you are ready to send a new iteration of your series, refine your
+patches as usual using linkgit:git-rebase[1]. Note that you typically want to
+rebase on top of the cover letter. You can configure an alias to enable easy
+rebases going forward:
+
+---
+$ git config set alias.b4-rebase 'rebase "HEAD^{/--- b4-submit-tracking ---}"'
+$ git b4-rebase -i
+---
+
+Before sending out the new version you should also update the cover letter with
+`b4 prep --edit-cover` to note the relevant changes compared to the previous
+version. You can inspect the changes between the two versions with `b4 prep
+--compare-to=v1`.
+
+Same as with the first version, you can use `b4 send` to send out the second
+version. `b4` automatically bumps the version to `v2`, generates the range-diff
+against the previous iteration, and threads the new series as a reply to the
+cover letter of the first version.
+
+[[configure-b4]]
+=== Configure b4
+
+`b4` can be configured via linkgit:git-config[1]. In addition to that, projects
+can have their own set of defaults in `.b4-config` in the root tree, which also
+uses Git's config format. The user's configuration always takes precedence over
+the per-project defaults.
+
+Refer to the https://b4.docs.kernel.org/en/latest/config.html[b4 config documentation]
+for more information on the available options.
+
[[now-what]]
== My Patch Got Emailed - Now What?
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index d570184ec8..99427e1ee1 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -573,8 +573,10 @@ your existing e-mail client (often optimized for "multipart/*" MIME
type e-mails) might render your patches unusable.
NOTE: Here we outline the procedure using `format-patch` and
-`send-email`, but you can instead use GitGitGadget to send in your
-patches (see link:MyFirstContribution.html[MyFirstContribution]).
+`send-email`, but you can instead use GitGitGadget or `b4` to send in
+your patches (see link:MyFirstContribution.html[MyFirstContribution]).
+Contributors are encouraged to use `b4`, which automates much of the
+bookkeeping that is otherwise done by hand.
People on the Git mailing list need to be able to read and
comment on the changes you are submitting. It is important for
--
2.54.0.1064.gd145956f57.dirty
^ permalink raw reply related
* [PATCH v2 1/3] Documentation/MyFirstContribution: recommend shallow threading
From: Patrick Steinhardt @ 2026-06-03 6:58 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Tuomas Ahola, Weijie Yuan, Ramsay Jones
In-Reply-To: <20260603-pks-b4-v2-0-a8aea0aa2c23@pks.im>
The "MyFirstContribution" document recommends the use of deep threading:
every cover letter of subsequent iterations shall be linked to the cover
letter of the preceding version. The result of this is that eventually,
threads with many versions are getting nested so deep that it becomes
hard to follow.
Adapt the recommendation to instead propose shallow threading: instead
of linking the cover letter to the previous cover letter, the user is
supposed to always link it to the first cover letter. This still makes
it easy to follow the iterations, but has the benefit of nesting to a
much shallower level.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Documentation/MyFirstContribution.adoc | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
index b9fdefce02..069020196c 100644
--- a/Documentation/MyFirstContribution.adoc
+++ b/Documentation/MyFirstContribution.adoc
@@ -1227,8 +1227,8 @@ Message-ID: <foo.12345.author@example.com>
Your Message-ID is `<foo.12345.author@example.com>`. This example will be used
below as well; make sure to replace it with the correct Message-ID for your
-**previous cover letter** - that is, if you're sending v2, use the Message-ID
-from v1; if you're sending v3, use the Message-ID from v2.
+**first cover letter** - that is, for any subsequent version that you send,
+always use the Message-ID from v1.
While you're looking at the email, you should also note who is CC'd, as it's
common practice in the mailing list to keep all CCs on a thread. You can add
--
2.54.0.1064.gd145956f57.dirty
^ permalink raw reply related
* [PATCH v2 0/3] Documentation: recommend the use of b4
From: Patrick Steinhardt @ 2026-06-03 6:58 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Tuomas Ahola, Weijie Yuan, Ramsay Jones
In-Reply-To: <20260602-pks-b4-v1-0-a7ae5a49e9cf@pks.im>
Hi,
this small patch series wires up b4 in Git and recommends the use
thereof via "MyFirstContribution", as discussed in [1].
Changes in v2:
- Reorder commits so that the b4 docs are added first.
- Add a section that highlights how to configure b4, and that points
out that the per-project defaults can be overridden via Git
configuration.
- Add a patch to MyFirstContribution that recommends shallow
threading. I mostly intend this to be a discussion starter so that
the `.b4-config` file matches our preferred threading style.
- Fix a typo.
- Link to v1: https://patch.msgid.link/20260602-pks-b4-v1-0-a7ae5a49e9cf@pks.im
Thanks!
Patrick
[1]: <xmqqik81xpqx.fsf@gitster.g>
---
Patrick Steinhardt (3):
Documentation/MyFirstContribution: recommend shallow threading
Documentation/MyFirstContribution: recommend the use of b4
b4: introduce configuration for the Git project
.b4-config | 6 +++
.b4-cover-template | 11 ++++
Documentation/MyFirstContribution.adoc | 96 ++++++++++++++++++++++++++++++++--
Documentation/SubmittingPatches | 6 ++-
4 files changed, 112 insertions(+), 7 deletions(-)
Range-diff versus v1:
-: ---------- > 1: 359ce9ec24 Documentation/MyFirstContribution: recommend shallow threading
2: 55fffeb8f8 ! 2: ce9aa56846 Documentation/MyFirstContribution: recommend the use of b4
@@ Documentation/MyFirstContribution.adoc: index 88f126184c..38da593a60 100644
+version. `b4` automatically bumps the version to `v2`, generates the range-diff
+against the previous iteration, and threads the new series as a reply to the
+cover letter of the first version.
++
++[[configure-b4]]
++=== Configure b4
++
++`b4` can be configured via linkgit:git-config[1]. In addition to that, projects
++can have their own set of defaults in `.b4-config` in the root tree, which also
++uses Git's config format. The user's configuration always takes precedence over
++the per-project defaults.
++
++Refer to the https://b4.docs.kernel.org/en/latest/config.html[b4 config documentation]
++for more information on the available options.
+
[[now-what]]
== My Patch Got Emailed - Now What?
1: 0fe6cf8511 ! 3: e2bf7b6e46 b4: introduce configuration for the Git project
@@ Commit message
b4: introduce configuration for the Git project
We're about to extend our documentation to recommend b4 for sending
- patch series ot the mailing list. Prepare for this by introducing a b4
+ patch series to the mailing list. Prepare for this by introducing a b4
configuration so that the tool knows to honor our preferences. For now,
this configuration does two things:
@@ Commit message
forward, like for example auto-configuration of folks to Cc on certain
patches. But these two tweaks feel like a good place to start.
+ Note that these values only serve as defaults, and users may want to
+ tweak those defaults based on their own preference. Luckily, users can
+ do that without having to touch `.b4-config` at all, as b4 allows them
+ to override values via Git configuration:
+
+ ```
+ $ git config set b4.prep-cover-template /does/not/exist
+ $ b4 send --dry-run
+ ERROR: prep-cover-template says to use x, but it does not exist
+ ```
+
+ So this gives users an easy way to override our defaults without having
+ to touch ".b4-config", which would dirty the tree.
+
Signed-off-by: Patrick Steinhardt <ps@pks.im>
## .b4-config (new) ##
@@
++# Note that these are default values that you can tweak via the typical
++# git-config(1) machinery. You thus shouldn't ever have to change this file.
++# See also https://b4.docs.kernel.org/en/latest/config.html.
+[b4]
+send-same-thread = shallow
+prep-cover-template = ./.b4-cover-template
---
base-commit: 9ac3f193c05c2237e2b14ebaa1149e9fc8a1abe0
change-id: 20260602-pks-b4-31cc20d7f84b
^ permalink raw reply
* Re: [PATCH 1/2] b4: introduce configuration for the Git project
From: Patrick Steinhardt @ 2026-06-03 6:55 UTC (permalink / raw)
To: Weijie Yuan; +Cc: Tuomas Ahola, git, Junio C Hamano
In-Reply-To: <ah-Nhr2PboWUq6eU@wyuan.org>
On Wed, Jun 03, 2026 at 10:12:22AM +0800, Weijie Yuan wrote:
> On Tue, Jun 02, 2026 at 08:09:55PM +0300, Tuomas Ahola wrote:
> > Huh? Doesn't MyFirstContribution speak *against* shallow threading?
> >
> > [...] make sure to replace it with the correct Message-ID for your
> > **previous cover letter** - that is, if you're sending v2, use the Message-ID
> > from v1; if you're sending v3, use the Message-ID from v2.
>
> I don't get it. Doesn't shallow threading means every following patches
> are replying to the cover letter? Replying to the previous one is
> --chain-reply-to, if I'm not mistaken.
Shallow threading basically means that all patches are sent as a
response to the current cover letter, and the current cover letter is
always attached to the cover letter of the _first_ version.
So this quote is definitely at odds with the configuration I have
proposed. It's actually quite surprising to me that we recommend deep
threading -- I personally find it extremely hard to navigate as the
nesting eventually gets way too deep.
You know -- I'll include a patch that changes the wording there to also
use shallow nesting, mostly to kick off a discussion and arrive at a
decision there.
Thanks!
Patrick
^ permalink raw reply
* Re: [PATCH 2/2] Documentation/MyFirstContribution: recommend the use of b4
From: Patrick Steinhardt @ 2026-06-03 6:54 UTC (permalink / raw)
To: Weijie Yuan; +Cc: git, Junio C Hamano
In-Reply-To: <ah8ALHMDVA2Gzz10@wyuan.org>
On Wed, Jun 03, 2026 at 12:09:16AM +0800, Weijie Yuan wrote:
> > +Contributors are encouraged to use `b4`, which automates much of the
> > +bookkeeping that is otherwise done by hand.
>
> So for statement like this and with my personal experience, I would say
> b4 is a more suitable option for senior contributors, as they already
> know, for example, what Message-ID and range-diffs are. But apparently,
> whose who use forges may not know.
I think it's perfectly suitable for newcomers, too. It automates so many
of the concepts that a contributor has to learn way less about mailing
list specific concepts, which reduces the learning curve.
> Back to the patch, I think regarding b4 as a more advanced contribution
> way for those who had contributed via mailing lists for more than one
> time is a better expression or formulation. Here I mean "b4 prep", other
> usage like "b4 mbox" and "b4 am" are of course more basic, and be
> mentioned as tips when interacting with Git mailing list.
>
> A bit too wordy, in conclusion: Suggest that new contributors master
> classic git operations first. When they are familiar with those process,
> b4 might be a good option.
Ah, that's what you're hinting at. So you mean to say that folks should
first understand the basics before basically automating all of the parts
for them?
I guess I can see where you're coming from, but I'm not sure I agree
with this a 100%. My main goal is to make it easier for new community
members to contribute to Git, and that means that we should automate all
the hard parts as far as possible. This saves those new contributors
from frustration, and it means that reviewers on the mailing list won't
have to teach every single new contributor about how they should thread
the mails, generate range-diffs and the like.
So in the end, it saves both their and our time, but the learning
opportunity is of course a bit diminished. I'd gladly accept that
tradeoff though.
Thanks for your input!
Patrick
^ permalink raw reply
* Re: [PATCH 1/2] b4: introduce configuration for the Git project
From: Patrick Steinhardt @ 2026-06-03 6:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ramsay Jones, git
In-Reply-To: <871peopbvf.fsf@gitster.g>
On Wed, Jun 03, 2026 at 11:59:48AM +0900, Junio C Hamano wrote:
> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
>
> > On 02/06/2026 2:32 pm, Junio C Hamano wrote:
> >> Patrick Steinhardt <ps@pks.im> writes:
> >>
> >>> We're about to extend our documentation to recommend b4 for sending
> >>> patch series ot the mailing list. Prepare for this by introducing a b4
> >>> configuration so that the tool knows to honor our preferences. For now,
> >>> this configuration does two things:
> >>> ...
> >> (hence making the tree dirty).
> >
> > Hmm, for those of us not in the know, perhaps mention the b4 documentation
> > at 'b4.docs.kernel.org' (which includes how to install b4 ... ;) ).
>
> Thanks for raising an excellent point.
I already refer to the docs in the second commit. Let me maybe reorder
them so that we first show how it's used before tweaking it.
Patrick
^ permalink raw reply
* [PATCH v2 4/4] t: let prove fail when parsing invalid TAP output
From: Patrick Steinhardt @ 2026-06-03 5:39 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <20260603-pks-t7527-fix-tap-output-v2-0-cf3af5694e20@pks.im>
To make the result of our tests accessible we use the TAP protocol. This
protocol is parsed by either prove or by Meson. Unfortunately, these two
tools differ when it comes to their strictness when parsing the
protocol:
- Prove by default happily accepts lines not specified by the
protocol.
- Meson will also accept such lines, but prints a big and ugly warning
message.
We have fixed our test suite in the past to not print invalid TAP lines
anymore via b1dc2e796e (Merge branch 'ps/meson-tap-parse', 2025-06-17).
But as none of our tools perform a strict check it's still possible for
broken tests to sneak back in, like for example in 362f69547f (Merge
branch 'ps/t1006-tap-fix', 2025-07-16). This doesn't hurt at all when
using prove, but it's quite annoying when using Meson due to the
generated warnings.
Unfortunately, there doesn't seem to be a portable way to make all tools
complain about violations of the TAP format. The TAP 14 specification
has added pragmas to the protocol that would allow us to say `pragma
+strict`, and the effect of that would be to treat invalid TAP lines as
a test failure. But the release of TAP 14 is still rather recent, and
Test-Harness for example only gained support for it in version 3.48,
which was released in 2023.
In fact though, this pragma was already introduced as an inofficial
extension of the TAP protocol with Test-Harness 3.10, released in 2008.
So while not all tools understand the pragma, at least prove does for a
long time.
Unconditionally enable the pragma when using prove so that we'll detect
tests that emit broken TAP output right away. This would have detected
the issues fixed in preceding commits:
$ prove t7527-builtin-fsmonitor.sh
t7527-builtin-fsmonitor.sh .. All 69 subtests passed
(less 6 skipped subtests: 63 okay)
Test Summary Report
-------------------
t7527-builtin-fsmonitor.sh (Wstat: 0 Tests: 69 Failed: 0)
Parse errors: Unknown TAP token: "Initialized empty Git repository in /tmp/git/test_fsmonitor_smoke/.git/"
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
t/test-lib.sh | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index d1d24c4124..ceefb99bff 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1532,6 +1532,12 @@ then
BAIL_OUT 'You need to build test-tool; Run "make t/helper/test-tool" in the source (toplevel) directory'
fi
+if test -n "$HARNESS_ACTIVE"
+then
+ say "TAP version 13"
+ say "pragma +strict"
+fi
+
# Are we running this test at all?
remove_trash=
this_test=${0##*/}
--
2.54.0.1064.gd145956f57.dirty
^ permalink raw reply related
* [PATCH v2 3/4] t/lib-git-p4: silence output when killing p4d and its watchdog
From: Patrick Steinhardt @ 2026-06-03 5:39 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <20260603-pks-t7527-fix-tap-output-v2-0-cf3af5694e20@pks.im>
When stopping the p4d watchdog process via "kill -9", the shell may
print a job-control notification like:
./test-lib.sh: line 1269: 57960 Killed: 9 while true; do
if test $nr_tries_left -eq 0; then
kill -9 $p4d_pid; exit 1;
fi; sleep 1; nr_tries_left=$(($nr_tries_left - 1));
done 2> /dev/null 4>&2 (wd: ~)
This message is printed asynchronously by the shell when it reaps the
process. While harmless right now, this will cause breakage once we
enable strict parsing of the TAP protocol in a subsequent commit.
Fix this by using `wait` so that we can synchronously reap the watchdog
process and swallow the diagnostic.
While at it, deduplicate the logic we have in `stop_p4d_and_watchdog ()`
and `stop_and_cleanup_p4d ()`.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
t/lib-git-p4.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index d22e9c684a..9108868187 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -65,6 +65,7 @@ pidfile="$TRASH_DIRECTORY/p4d.pid"
stop_p4d_and_watchdog () {
kill -9 $p4d_pid $watchdog_pid
+ wait $p4d_pid $watchdog_pid 2>/dev/null
}
# git p4 submit generates a temp file, which will
@@ -174,8 +175,7 @@ retry_until_success () {
}
stop_and_cleanup_p4d () {
- kill -9 $p4d_pid $watchdog_pid
- wait $p4d_pid
+ stop_p4d_and_watchdog
rm -rf "$db" "$cli" "$pidfile"
}
--
2.54.0.1064.gd145956f57.dirty
^ permalink raw reply related
* [PATCH v2 2/4] t/test-lib: silence EBUSY errors on Windows during test cleanup
From: Patrick Steinhardt @ 2026-06-03 5:39 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <20260603-pks-t7527-fix-tap-output-v2-0-cf3af5694e20@pks.im>
When tests have finished we clean up the trash directory via `rm -rf`.
On Windows this can fail with EBUSY in cases where a process still holds
some of the files open, for example when we have spawned a daemonized
process that wasn't properly terminated. We thus retry several times,
but every failure will result in error messages being printed, and that
in turn breaks the TAP output format.
One such case where this is causing issues is in t921x, which contains
tests related to Scalar. Some tests spawn the fsmonitor daemon, and we
never properly terminate it.
The obvious fix would be to ensure that we never leak any processes, but
that gets ugly fast. Instead, let's work around the issue by silencing
error messages printed by the `rm -rf` calls. We already know to print
an error when the retry loop fails, so we don't loose much.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
t/test-lib.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 4a7357b547..d1d24c4124 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1299,10 +1299,10 @@ test_done () {
error "Tests passed but trash directory already removed before test cleanup; aborting"
cd "$TRASH_DIRECTORY/.." &&
- rm -fr "$TRASH_DIRECTORY" || {
+ rm -fr "$TRASH_DIRECTORY" 2>/dev/null || {
# try again in a bit
sleep 5;
- rm -fr "$TRASH_DIRECTORY"
+ rm -fr "$TRASH_DIRECTORY" 2>/dev/null
} ||
error "Tests passed but test cleanup failed; aborting"
fi
--
2.54.0.1064.gd145956f57.dirty
^ permalink raw reply related
* [PATCH v2 1/4] t7527: fix broken TAP output
From: Patrick Steinhardt @ 2026-06-03 5:39 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <20260603-pks-t7527-fix-tap-output-v2-0-cf3af5694e20@pks.im>
Before running the tests in t7527 we first verify whether the fsmonitor
even works, which seems to depend on the actual filesystem that is in
use. The verification executes outside of any prerequisite or test body,
so its stdout/stderr is not being redirected.
The consequence of this is that any command that prints to stdout/stderr
may break the TAP specification by printing invalid lines. And in fact
we already do that, as git-init(1) prints the path to the created Git
repository by default.
Fix this issue by moving the logic into a lazy prerequisite.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
t/t7527-builtin-fsmonitor.sh | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/t/t7527-builtin-fsmonitor.sh b/t/t7527-builtin-fsmonitor.sh
index b63c162f9b..d881e27466 100755
--- a/t/t7527-builtin-fsmonitor.sh
+++ b/t/t7527-builtin-fsmonitor.sh
@@ -25,7 +25,8 @@ maybe_timeout () {
"$@"
fi
}
-verify_fsmonitor_works () {
+
+test_lazy_prereq FSMONITOR_WORKS '
git init test_fsmonitor_smoke || return 1
GIT_TRACE_FSMONITOR="$PWD/smoke.trace" &&
@@ -50,9 +51,9 @@ verify_fsmonitor_works () {
ret=$?
rm -rf test_fsmonitor_smoke smoke.trace
return $ret
-}
+'
-if ! verify_fsmonitor_works
+if ! test_have_prereq FSMONITOR_WORKS
then
skip_all="filesystem does not deliver fsmonitor events (container/overlayfs?)"
test_done
--
2.54.0.1064.gd145956f57.dirty
^ permalink raw reply related
* [PATCH v2 0/4] t: fix broken TAP output
From: Patrick Steinhardt @ 2026-06-03 5:39 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <20260602-pks-t7527-fix-tap-output-v1-0-db3da2a1b137@pks.im>
Hi,
this small patch series fixes another instance of broken TAP output that
has landed via 4d11b9c218 (Merge branch 'pt/fsmonitor-linux', 2026-05-31).
As this has happened multiple times by now I decided to have a look at
whether we can fix this class of issues a bit more holistically. So this
series also contains a change that makes prove bail out when it sees
invalid TAP output, which uncovers a small set of preexisting issues in
our test suite.
Changes in v2:
- Fix waiting for p4d, and deduplicate the logic that does this.
- Link to v1: https://patch.msgid.link/20260602-pks-t7527-fix-tap-output-v1-0-db3da2a1b137@pks.im
Thanks!
Patrick
---
Patrick Steinhardt (4):
t7527: fix broken TAP output
t/test-lib: silence EBUSY errors on Windows during test cleanup
t/lib-git-p4: silence output when killing p4d and its watchdog
t: let prove fail when parsing invalid TAP output
t/lib-git-p4.sh | 4 ++--
t/t7527-builtin-fsmonitor.sh | 7 ++++---
t/test-lib.sh | 10 ++++++++--
3 files changed, 14 insertions(+), 7 deletions(-)
Range-diff versus v1:
1: 09977059d1 = 1: 18b4fc7b81 t7527: fix broken TAP output
2: 162d3d42d8 = 2: 8dd921534b t/test-lib: silence EBUSY errors on Windows during test cleanup
3: 4ecb8cb1ce < -: ---------- t/lib-git-p4: silence output when killing p4d and its watchdog
-: ---------- > 3: 8b343176fe t/lib-git-p4: silence output when killing p4d and its watchdog
4: 95fb0d07ae = 4: e69aa0ab79 t: let prove fail when parsing invalid TAP output
---
base-commit: 1666c1265231b0bc5f613fbbf3f0a9896cdef76e
change-id: 20260601-pks-t7527-fix-tap-output-105da1d73df0
^ permalink raw reply
* Re: [PATCH 1/2] b4: introduce configuration for the Git project
From: Junio C Hamano @ 2026-06-03 2:59 UTC (permalink / raw)
To: Ramsay Jones; +Cc: Patrick Steinhardt, git
In-Reply-To: <8dbdb553-9633-46bb-8a51-040d06d0d10e@ramsayjones.plus.com>
Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
> On 02/06/2026 2:32 pm, Junio C Hamano wrote:
>> Patrick Steinhardt <ps@pks.im> writes:
>>
>>> We're about to extend our documentation to recommend b4 for sending
>>> patch series ot the mailing list. Prepare for this by introducing a b4
>>> configuration so that the tool knows to honor our preferences. For now,
>>> this configuration does two things:
>>> ...
>> (hence making the tree dirty).
>
> Hmm, for those of us not in the know, perhaps mention the b4 documentation
> at 'b4.docs.kernel.org' (which includes how to install b4 ... ;) ).
Thanks for raising an excellent point.
^ permalink raw reply
* Re: [PATCH 1/2] b4: introduce configuration for the Git project
From: Weijie Yuan @ 2026-06-03 2:12 UTC (permalink / raw)
To: Tuomas Ahola; +Cc: Patrick Steinhardt, git, Junio C Hamano
In-Reply-To: <20260602170955.Z4b7y%taahol@utu.fi>
On Tue, Jun 02, 2026 at 08:09:55PM +0300, Tuomas Ahola wrote:
> Huh? Doesn't MyFirstContribution speak *against* shallow threading?
>
> [...] make sure to replace it with the correct Message-ID for your
> **previous cover letter** - that is, if you're sending v2, use the Message-ID
> from v1; if you're sending v3, use the Message-ID from v2.
I don't get it. Doesn't shallow threading means every following patches
are replying to the cover letter? Replying to the previous one is
--chain-reply-to, if I'm not mistaken.
Thanks.
^ permalink raw reply
* Re: [PATCH v2 0/3] contrib/subtree: reduce recursion during split
From: Colin Stagner @ 2026-06-03 1:37 UTC (permalink / raw)
To: Junio C Hamano
Cc: Ian Jackson, git, Christian Heusel, george, Christian Hesse,
Phillip Wood
In-Reply-To: <xmqqv7c13o5l.fsf@gitster.g>
On 6/1/26 17:13, Junio C Hamano wrote:
> I am tempted to mark the topic as stalled, to be discarded for
> inaction
No objection. I'd still like to see this reviewed, but we can revisit
this later if interest develops.
> While I do agree that avoiding bash-isms in the main part of Git and
> sticking to vanilla POSIX has merit, this particular one seems more
> like an artificial limit imposed by dash than sticking to the POSIX
> as the common denoninator, at least to me.
Correct, this topic is a workaround for an artificial limit. The limit
is Debian-specific and was introduced as a downstream patch in 2018 [1],
[2].
This git-subtree issue has been reported before in
<CAN7rbOve-EFOGPjr1wrD77r-3RQ+3+qso82_oV5Qud-skobL7w@mail.gmail.com>,
<26263.63341.878041.155047@chiark.greenend.org.uk>,
and probably other places. These are old reports, and I haven't found
anyone there still interested in a fix.
Colin
[1]: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=579815
[2]:
https://sources.debian.org/patches/dash/0.5.12-12/0009-dash-Fix-stack-overflow-from-infinite-recursion-in-s.patch/
^ permalink raw reply
* Re: [PATCH] http: preserve wwwauth_headers across redirects
From: Aaron Plattner @ 2026-06-03 0:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Rahul Rameshbabu
In-Reply-To: <xmqqpl28scll.fsf@gitster.g>
On 6/2/26 5:15 PM, Junio C Hamano wrote:
> Aaron Plattner <aplattner@nvidia.com> writes:
>
>> diff --git a/http.c b/http.c
>> index ea9b16861b..cac8c9bfc9 100644
>> --- a/http.c
>> +++ b/http.c
>> @@ -2425,7 +2425,21 @@ static int http_request_recoverable(const char *url,
>> if (options->effective_url && options->base_url) {
>> if (update_url_from_redirect(options->base_url,
>> url, options->effective_url)) {
>> + struct strvec wwwauth_headers = STRVEC_INIT;
>> +
>> + /*
>> + * Preserve wwwauth_headers across the call to
>> + * credential_from_url(): if the effective URL doesn't
>> + * specify its own credentials, a credential helper
>> + * might need the wwwauth[] array from the server's
>> + * redirect response in order to authenticate.
>> + */
>> + strvec_pushv(&wwwauth_headers,
>> + http_auth.wwwauth_headers.v);
>> credential_from_url(&http_auth, options->base_url->buf);
>> + strvec_pushv(&http_auth.wwwauth_headers,
>> + wwwauth_headers.v);
>> + strvec_clear(&wwwauth_headers);
>> url = options->effective_url->buf;
>> }
>> }
>
> As strvec_pushv() makes copies of the strings contained in .v[]
> array, the above will
>
> - make a deep copy of http_auth.wwwauth_headers.v[] and store it away
> in wwwauth_headers.v[];
>
> - let credential_from_url() get rid of
> http_auth.wwwauth_headers.v[] (the original is freed here, but we
> have a deep copy stashed away safely), and perhaps add some of
> its own there; then
>
> - add what we stashed away back to http_auth.wwwauth_headers.v[].
>
> So it does not leak and it does not have use-after-free, either,
> which is good, even though it may be a bit inefficient having to
> copy these strings so many times.
I agree, although in the grand scheme wwwauth_headers is a drop in the
bucket compared to, say, nearly any git object.
I tried to find an easy way to just not clear it in the first place, but
that doesn't really match what credential_clear() is intended to do.
> I briefly wondered if it is unconditionally adding back the original
> wwwauth_headers always the right thing to do, but I think this is
> good. In the context of http_request_recovorable(), the redirect
> has already happened, and the request to the redirect target has
> failed with a 401. The wwwauth_headers currently in http_auth were
> populated from this 401 response from the redirect target. Since we
> are updating http_auth's URL to match this redirect target (in order
> to query the helper for the correct host), the headers we currently
> have are the active challenges for this new URL. Thus, they must be
> preserved and passed to the helper.
This matches my understanding and I think it points to a more
fundamental design issue: wwwauth_headers aren't really credentials at
all, and maybe they shouldn't be in struct credential in the first
place. I wonder if it would make sense to encapsulate it in some other
http-related structure that lives alongside the credentials, presumably
along with the protocol, host, and path.
> A few design questions that came to my mind are:
>
> - Is wwwauth_headers the _only_ thing that needs to be preserved in
> the existing credential in http_auth? Will it stay to be the
> only thing, or will we need to rethink what this patch did in the
> future when we add such a new member to "struct credential"?
>
> - If we need to preserve some other members in "struct credential",
> or if we add such members to the struct in the future, what would
> be the recommended way to extend what this patch does to cover?
>
> If we add new members in the future to store other transient
> response-based authentication state (e.g. Authentication-Info
> headers, or proxy authentication states), they will be wiped by
> credential_from_url() and will need to be preserved the same way,
> no? This observation and thought experiment may hint that the
> manual save-and-restore approach is not robust against future
> extensions of struct credential.
>
> The current approach of manually saving and restoring
> wwwauth_headers in http.c creates a tight coupling between the HTTP
> layer and the internals of struct credential. If new transient
> fields are added in the future, developers must remember to update
> http.c to preserve them, which may be error-prone.
>
> I wonder if it would make the design more robust and future-proof to
> encapsulate this logic in credential.c instead. For example, we
> could introduce a helper function:
>
> void credential_update_url(struct credential *c, const char *url)
>
> that does what the new code added around credential_from_url() by
> this patch does, perhaps?
Yeah, maybe. I'll think about this design some more.
-- Aaron
^ permalink raw reply
* Re: [PATCH] http: preserve wwwauth_headers across redirects
From: Junio C Hamano @ 2026-06-03 0:15 UTC (permalink / raw)
To: Aaron Plattner; +Cc: git, Rahul Rameshbabu
In-Reply-To: <20260602161150.1527493-1-aplattner@nvidia.com>
Aaron Plattner <aplattner@nvidia.com> writes:
> diff --git a/http.c b/http.c
> index ea9b16861b..cac8c9bfc9 100644
> --- a/http.c
> +++ b/http.c
> @@ -2425,7 +2425,21 @@ static int http_request_recoverable(const char *url,
> if (options->effective_url && options->base_url) {
> if (update_url_from_redirect(options->base_url,
> url, options->effective_url)) {
> + struct strvec wwwauth_headers = STRVEC_INIT;
> +
> + /*
> + * Preserve wwwauth_headers across the call to
> + * credential_from_url(): if the effective URL doesn't
> + * specify its own credentials, a credential helper
> + * might need the wwwauth[] array from the server's
> + * redirect response in order to authenticate.
> + */
> + strvec_pushv(&wwwauth_headers,
> + http_auth.wwwauth_headers.v);
> credential_from_url(&http_auth, options->base_url->buf);
> + strvec_pushv(&http_auth.wwwauth_headers,
> + wwwauth_headers.v);
> + strvec_clear(&wwwauth_headers);
> url = options->effective_url->buf;
> }
> }
As strvec_pushv() makes copies of the strings contained in .v[]
array, the above will
- make a deep copy of http_auth.wwwauth_headers.v[] and store it away
in wwwauth_headers.v[];
- let credential_from_url() get rid of
http_auth.wwwauth_headers.v[] (the original is freed here, but we
have a deep copy stashed away safely), and perhaps add some of
its own there; then
- add what we stashed away back to http_auth.wwwauth_headers.v[].
So it does not leak and it does not have use-after-free, either,
which is good, even though it may be a bit inefficient having to
copy these strings so many times.
I briefly wondered if it is unconditionally adding back the original
wwwauth_headers always the right thing to do, but I think this is
good. In the context of http_request_recovorable(), the redirect
has already happened, and the request to the redirect target has
failed with a 401. The wwwauth_headers currently in http_auth were
populated from this 401 response from the redirect target. Since we
are updating http_auth's URL to match this redirect target (in order
to query the helper for the correct host), the headers we currently
have are the active challenges for this new URL. Thus, they must be
preserved and passed to the helper.
A few design questions that came to my mind are:
- Is wwwauth_headers the _only_ thing that needs to be preserved in
the existing credential in http_auth? Will it stay to be the
only thing, or will we need to rethink what this patch did in the
future when we add such a new member to "struct credential"?
- If we need to preserve some other members in "struct credential",
or if we add such members to the struct in the future, what would
be the recommended way to extend what this patch does to cover?
If we add new members in the future to store other transient
response-based authentication state (e.g. Authentication-Info
headers, or proxy authentication states), they will be wiped by
credential_from_url() and will need to be preserved the same way,
no? This observation and thought experiment may hint that the
manual save-and-restore approach is not robust against future
extensions of struct credential.
The current approach of manually saving and restoring
wwwauth_headers in http.c creates a tight coupling between the HTTP
layer and the internals of struct credential. If new transient
fields are added in the future, developers must remember to update
http.c to preserve them, which may be error-prone.
I wonder if it would make the design more robust and future-proof to
encapsulate this logic in credential.c instead. For example, we
could introduce a helper function:
void credential_update_url(struct credential *c, const char *url)
that does what the new code added around credential_from_url() by
this patch does, perhaps?
^ permalink raw reply
* [PATCH v3] index-pack: retain child bases in delta cache
From: Arijit Banerjee via GitGitGadget @ 2026-06-03 0:05 UTC (permalink / raw)
To: git
Cc: Ævar Arnfjörð Bjarmason, Junio C Hamano,
Derrick Stolee, Jeff King, Arijit Banerjee, Arijit Banerjee
In-Reply-To: <pull.2131.v2.git.1780330402264.gitgitgadget@gmail.com>
From: Arijit Banerjee <arijit@effectiveailabs.com>
When resolving a delta whose result has children of its own,
index-pack adds the result to work_head, accounts its data in
base_cache_used, and calls prune_base_data(). It then immediately frees
that same data.
This bypasses the existing delta base cache policy and can force later
descendants to reconstruct the queued base again. Let the existing
delta_base_cache_limit pruning policy decide whether to keep or evict
the data instead.
This does not add a new cache or increase the cache limit. The object
data is already accounted in base_cache_used before prune_base_data()
runs. Once all direct children of a base have been dispatched, and no
thread is actively retaining that base for patch_delta(), release the
cached bytes. The base_data struct itself remains alive until the
existing children_remaining bookkeeping says the whole subtree is done.
On a quiet Ubuntu 24.04 VM with 16 vCPUs, 32 GiB RAM, and local SSD,
standard p5302-pack-index.sh runs improved as follows:
libgit2: 3.17(11.49+0.60) -> 2.69(10.52+0.28), 15.1% faster
redis: 5.84(15.56+0.63) -> 4.95(14.05+0.32), 15.2% faster
git.git: 11.17(38.04+1.29) -> 9.67(35.29+0.60), 13.4% faster
cpython: 32.69(117.85+4.37) -> 28.60(109.25+1.91), 12.5% faster
linux: 279.22(797.69+40.86) -> 236.34(723.13+19.02), 15.4% faster
The linux p5302 number is from a single repeat; the others are from the
default three repeats.
End-to-end local full-clone spot checks also improved:
libgit2: 5.00s -> 4.54s, 9.2% faster
redis: 8.75s -> 7.92s, 9.5% faster
git.git: 25.04s -> 23.71s, 5.3% faster
cpython: 56.72s -> 55.94s, 1.4% faster
linux: 556.17s -> 523.83s, 5.8% faster
t/t5302-pack-index.sh passed, and GitGitGadget's linux-leaks CI also
exercised that test under SANITIZE=leak.
Signed-off-by: Arijit Banerjee <arijit@effectiveailabs.com>
---
index-pack: retain child bases in delta cache
Speed up the local index-pack phase used by clone/fetch for large
delta-compressed packs.
When index-pack reconstructs a child base and queues it for resolving
descendant deltas, it currently frees that data immediately. This can
force the same base to be reconstructed again. This patch keeps the data
in the existing delta base cache instead of immediately freeing it.
This does not add a new cache or increase the cache limit. The object
data is already accounted in base_cache_used, and prune_base_data() is
already called at this point.
To keep the retained data lifetime precise, the patch also releases
cached bytes once all direct children of a base have been dispatched and
no thread is actively retaining that base for patch_delta(). The
base_data struct itself still stays alive until the existing
children_remaining bookkeeping says the whole subtree is done.
Changes since v2:
* Addressed Jeff King's review question by releasing cached base data
after all direct children have been dispatched, while keeping the
existing subtree bookkeeping intact.
* Re-ran t/t5302-pack-index.sh, p5302-pack-index.sh, and end-to-end
full clone spot checks with the precise-release version.
Changes since v1:
* Added benchmark results and leak-safety context to the commit
message.
* Included standard p5302-pack-index.sh perf-suite results.
Correctness:
* t/t5302-pack-index.sh passed.
* GitGitGadget's linux-leaks CI exercises t5302-pack-index.sh under
SANITIZE=leak.
Benchmarks on a quiet Ubuntu 24.04 VM, 16 vCPU, 32 GiB RAM, local SSD:
Standard p5302-pack-index.sh perf-suite results using
GIT_PERF_LARGE_REPO=<repo> ./run HEAD~1 HEAD -- p5302-pack-index.sh:
repo HEAD~1 HEAD wall-time change libgit2 3.17(11.49+0.60)
2.69(10.52+0.28) 15.1% faster redis 5.84(15.56+0.63) 4.95(14.05+0.32)
15.2% faster git.git 11.17(38.04+1.29) 9.67(35.29+0.60) 13.4% faster
cpython 32.69(117.85+4.37) 28.60(109.25+1.91) 12.5% faster linux
279.22(797.69+40.86) 236.34(723.13+19.02) 15.4% faster
The linux p5302 row is from a single repeat; the others use the default
three repeats. These timings isolate the index-pack phase affected by
this patch.
End-to-end local full-clone spot checks also showed wins, though these
timings include both server-side pack-objects and client-side index-pack
running concurrently over a local file:// transport, so they are not
isolated index-pack timings.
These runs used git clone --bare --no-local, dropped page cache before
each clone, and used the matching build's bin-wrappers/git as the client
plus the matching bin-wrappers/git-upload-pack via --upload-pack.
repo baseline patched wall-time change libgit2 5.00s 4.54s 9.2% faster
redis 8.75s 7.92s 9.5% faster git.git 25.04s 23.71s 5.3% faster cpython
56.72s 55.94s 1.4% faster linux 556.17s 523.83s 5.8% faster
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2131%2Farijit91%2Findex-pack-retain-child-base-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2131/arijit91/index-pack-retain-child-base-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/2131
Range-diff vs v2:
1: 42eca38f51 ! 1: 51967f9edf index-pack: retain child bases in delta cache
@@ Commit message
This does not add a new cache or increase the cache limit. The object
data is already accounted in base_cache_used before prune_base_data()
- runs, and the existing pruning and base cleanup paths still release it.
+ runs. Once all direct children of a base have been dispatched, and no
+ thread is actively retaining that base for patch_delta(), release the
+ cached bytes. The base_data struct itself remains alive until the
+ existing children_remaining bookkeeping says the whole subtree is done.
On a quiet Ubuntu 24.04 VM with 16 vCPUs, 32 GiB RAM, and local SSD,
- direct index-pack timings on single-pack Linux fixtures improved as
- follows:
+ standard p5302-pack-index.sh runs improved as follows:
- linux blobless: 69.17s -> 57.98s (16.2% faster), RSS flat
- linux full: 280.72s -> 236.32s (15.8% faster), RSS +1.9%
+ libgit2: 3.17(11.49+0.60) -> 2.69(10.52+0.28), 15.1% faster
+ redis: 5.84(15.56+0.63) -> 4.95(14.05+0.32), 15.2% faster
+ git.git: 11.17(38.04+1.29) -> 9.67(35.29+0.60), 13.4% faster
+ cpython: 32.69(117.85+4.37) -> 28.60(109.25+1.91), 12.5% faster
+ linux: 279.22(797.69+40.86) -> 236.34(723.13+19.02), 15.4% faster
- Five-repeat medians on public repositories also improved:
+ The linux p5302 number is from a single repeat; the others are from the
+ default three repeats.
- git.git: 12.31s -> 10.70s (13.1% faster)
- libgit2: 3.35s -> 2.88s (14.0% faster)
- redis: 6.52s -> 5.64s (13.5% faster)
- cpython: 33.02s -> 31.44s (4.8% faster)
+ End-to-end local full-clone spot checks also improved:
- The standard p5302 perf test on a smaller git.git fixture was neutral:
-
- 5302.9 index-pack default threads:
- 11.21(38.07+1.33) -> 11.16(37.90+1.31), -0.4%
+ libgit2: 5.00s -> 4.54s, 9.2% faster
+ redis: 8.75s -> 7.92s, 9.5% faster
+ git.git: 25.04s -> 23.71s, 5.3% faster
+ cpython: 56.72s -> 55.94s, 1.4% faster
+ linux: 556.17s -> 523.83s, 5.8% faster
t/t5302-pack-index.sh passed, and GitGitGadget's linux-leaks CI also
exercised that test under SANITIZE=leak.
@@ Commit message
Signed-off-by: Arijit Banerjee <arijit@effectiveailabs.com>
## builtin/index-pack.c ##
+@@ builtin/index-pack.c: static void free_base_data(struct base_data *c)
+ }
+ }
+
++static int base_data_has_remaining_direct_children(struct base_data *c)
++{
++ return c->ref_first <= c->ref_last ||
++ c->ofs_first <= c->ofs_last;
++}
++
+ static void prune_base_data(struct base_data *retain)
+ {
+ struct list_head *pos;
+@@ builtin/index-pack.c: static void *threaded_second_pass(void *data)
+ }
+
+ work_lock();
+- if (parent)
++ if (parent) {
+ parent->retain_data--;
++ if (!parent->retain_data &&
++ !base_data_has_remaining_direct_children(parent))
++ free_base_data(parent);
++ }
+
+ if (child && child->data) {
+ /*
@@ builtin/index-pack.c: static void *threaded_second_pass(void *data)
list_add(&child->list, &work_head);
base_cache_used += child->size;
builtin/index-pack.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index cf0bd8280d..00b4dff419 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -433,6 +433,12 @@ static void free_base_data(struct base_data *c)
}
}
+static int base_data_has_remaining_direct_children(struct base_data *c)
+{
+ return c->ref_first <= c->ref_last ||
+ c->ofs_first <= c->ofs_last;
+}
+
static void prune_base_data(struct base_data *retain)
{
struct list_head *pos;
@@ -1201,8 +1207,12 @@ static void *threaded_second_pass(void *data)
}
work_lock();
- if (parent)
+ if (parent) {
parent->retain_data--;
+ if (!parent->retain_data &&
+ !base_data_has_remaining_direct_children(parent))
+ free_base_data(parent);
+ }
if (child && child->data) {
/*
@@ -1212,7 +1222,6 @@ static void *threaded_second_pass(void *data)
list_add(&child->list, &work_head);
base_cache_used += child->size;
prune_base_data(NULL);
- free_base_data(child);
} else if (child) {
/*
* This child does not have its own children. It may be
base-commit: c69baaf57ba26cf117c2b6793802877f19738b0d
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH] worktree: record creation time and free-form note
From: Kiesel, Norbert @ 2026-06-03 0:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqq1peots9i.fsf@gitster.g>
Yes, I could change my PR to use $GIT_COMMON_DIR/worktrees/$worktree/description
instead of the currently used $GIT_COMMON_DIR/worktrees/$worktree/note.
Give me a day, and I can create the updated diff.
Best,
Norbert
On Tue, Jun 2, 2026 at 4:52 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Kiesel, Norbert" <norbert.kiesel@creditkarma.com> writes:
>
> > From 130cd5e4a25e6672b2a97268e1100b6ef03fa552 Mon Sep 17 00:00:00 2001
> > From: Norbert Kiesel <norbert.kiesel@creditkarma.com>
> > Date: Mon, 1 Jun 2026 17:03:39 -0700
> > Subject: [PATCH] worktree: record creation time and free-form note
> >
> > Add per-worktree metadata so users can answer "what is this worktree
> > for, and when did I make it?" without resorting to external notes.
>
> Although I am not personally interested in this topic all that much,
> let me point out that we have $GIT_DIR/description file that may be
> useful for something like this. It has been the canonical place for
> the main repository to identify itself long before secondary worktrees
> were invented and $GIT_COMMON_DIR/worktrees/$worktree/description would
> be a natural extension of the concept, I'd presume.
--
Norbert Kiesel | Staff Software Engineer | Credit Karma
norbert.kiesel@creditkarma.com | www.creditkarma.com
This email may contain confidential and privileged information. Any
review, use, distribution, or disclosure by anyone other than the
intended recipient(s) is prohibited. If you are not the intended
recipient, please contact the sender by reply email and delete all
copies of this message.
^ permalink raw reply
* Re: [PATCH] worktree: record creation time and free-form note
From: Junio C Hamano @ 2026-06-02 23:57 UTC (permalink / raw)
To: Kiesel, Norbert; +Cc: git
In-Reply-To: <xmqq1peots9i.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> "Kiesel, Norbert" <norbert.kiesel@creditkarma.com> writes:
>
>> From 130cd5e4a25e6672b2a97268e1100b6ef03fa552 Mon Sep 17 00:00:00 2001
>> From: Norbert Kiesel <norbert.kiesel@creditkarma.com>
>> Date: Mon, 1 Jun 2026 17:03:39 -0700
>> Subject: [PATCH] worktree: record creation time and free-form note
Ah, I forgot to mention another thing. Please do not add these four
lines to your message body. The information belongs to the e-mail
header, and as long as your e-mail software is configured correctly
there shouldn't be a need to use From: or override the time when the
patch was made public with Date: in-body header.
Thanks.
^ permalink raw reply
* Re: [PATCH] worktree: record creation time and free-form note
From: Junio C Hamano @ 2026-06-02 23:52 UTC (permalink / raw)
To: Kiesel, Norbert; +Cc: git
In-Reply-To: <CAPGaHku+RAV+FA3C0md0xHiavfdB_anoqcMM06MAiU1VyMAdLA@mail.gmail.com>
"Kiesel, Norbert" <norbert.kiesel@creditkarma.com> writes:
> From 130cd5e4a25e6672b2a97268e1100b6ef03fa552 Mon Sep 17 00:00:00 2001
> From: Norbert Kiesel <norbert.kiesel@creditkarma.com>
> Date: Mon, 1 Jun 2026 17:03:39 -0700
> Subject: [PATCH] worktree: record creation time and free-form note
>
> Add per-worktree metadata so users can answer "what is this worktree
> for, and when did I make it?" without resorting to external notes.
Although I am not personally interested in this topic all that much,
let me point out that we have $GIT_DIR/description file that may be
useful for something like this. It has been the canonical place for
the main repository to identify itself long before secondary worktrees
were invented and $GIT_COMMON_DIR/worktrees/$worktree/description would
be a natural extension of the concept, I'd presume.
^ 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