On Mon, May 4, 2026, at 20:27, Harald Nordgren via GitGitGadget wrote:
> From: Harald Nordgren <haraldnordgren@gmail.com>
>
> List local branches whose configured upstream falls within any of
> the given <remote> arguments. <remote> may be either a configured
> remote name (matching all of its remote-tracking refs) or a single
> remote-tracking ref. Multiple <remote> arguments are unioned.
>
> This is the building block for --prune-merged, which deletes the
> listed branches.
>
> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>

s/remote-tracking refs/remote-tracking branches/g

Here and below and on the other patches.

> ---
>  Documentation/git-branch.adoc |  12 ++++
>  builtin/branch.c              | 110 +++++++++++++++++++++++++++++++++-
>  t/t3200-branch.sh             |  54 +++++++++++++++++
>  3 files changed, 174 insertions(+), 2 deletions(-)
>[snip]

"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> +`--prune-merged`::
> +	Delete the local branches that `--forked` would list for
> +	the same _<branch>_ arguments, but only those whose tip is
> +	reachable from their configured upstream.
> ++
> +For arguments that refer to remote-tracking branches, run
> +`git fetch` first ...

Please don't.  

"git branch -d <derived>" checks if <derived> has already been
merged to its <upstream> as your repository currently sees it, and
this makes "--prune-merged" inconsistent.  Before deciding to use
"--prune-merged", the user may have sanity checked if it is safe to
remove by running "git branch -no-merged", whose answer hence user's
sanity checking will be invalidated by your auto-update from the
upstream.

It also means that you cannot prune already merged ones while you
are not online.

"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> diff --git a/builtin/branch.c b/builtin/branch.c
> index 1572a4f9ef..1e24c95a69 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -28,6 +28,7 @@
>  #include "help.h"
>  #include "advice.h"
>  #include "commit-reach.h"
> +#include "wildmatch.h"
>  
>  static const char * const builtin_branch_usage[] = {
>  	N_("git branch [<options>] [-r | -a] [--merged] [--no-merged]"),
> @@ -38,6 +39,7 @@ static const char * const builtin_branch_usage[] = {
>  	N_("git branch [<options>] (-c | -C) [<old-branch>] <new-branch>"),
>  	N_("git branch [<options>] [-r | -a] [--points-at]"),
>  	N_("git branch [<options>] [-r | -a] [--format]"),
> +	N_("git branch [<options>] --forked <branch>..."),
>  	NULL
>  };
>  
> @@ -191,7 +193,8 @@ static int branch_merged(int kind, const char *name,
>  
>  static int check_branch_commit(const char *branchname, const char *refname,
>  			       const struct object_id *oid, struct commit *head_rev,
> -			       int kinds, int force)
> +			       int kinds, int force, int warn_only,
> +			       int *n_not_merged)
>  {
>  	struct commit *rev = lookup_commit_reference(the_repository, oid);
>  	if (!force && !rev) {
> @@ -199,10 +202,18 @@ static int check_branch_commit(const char *branchname, const char *refname,
>  		return -1;
>  	}
>  	if (!force && !branch_merged(kinds, branchname, rev, head_rev)) {
> -		error(_("the branch '%s' is not fully merged"), branchname);
> -		advise_if_enabled(ADVICE_FORCE_DELETE_BRANCH,
> -				  _("If you are sure you want to delete it, "
> -				  "run 'git branch -D %s'"), branchname);
> +		if (warn_only) {
> +			warning(_("the branch '%s' is not fully merged"),
> +				branchname);
> +		} else {
> +			error(_("the branch '%s' is not fully merged"),
> +			      branchname);
> +			advise_if_enabled(ADVICE_FORCE_DELETE_BRANCH,
> +					  _("If you are sure you want to delete it, "
> +					  "run 'git branch -D %s'"), branchname);
> +		}
> +		if (n_not_merged)
> +			(*n_not_merged)++;
>  		return -1;
>  	}
>  	return 0;

This function originally was to see if "git branch -d <derived>" is
allowed to remove it by calling branch_merged(), which uses the
upstream association of <derived>, falling back to head_rev.  For
our purpose of "--forked" check, we are not deleting, so existing
calls to error() and the phrasing used in its messages that talks
about deletion are not appropriate.  Hence we introduce the
warn_only mode to just state the fact the branch is not fully merged
to its upstream.

It is inherited from the original code, and it is outside the scope
of this series, but after the dust settles, we may want to consider
saying what other branch this branch is expected to be merged to
(i.e. "not fully merged to %s").

It is unclear how n_not_merged would be useful.  Wouldn't the caller
be capable to count the failure returns from this function?

> @@ -218,7 +229,7 @@ static void delete_branch_config(const char *branchname)
>  }
>  
>  static int delete_branches(int argc, const char **argv, int force, int kinds,
> -			   int quiet)
> +			   int quiet, int warn_only, int *n_not_merged)

It is strange to see any need to touch delete_branches() when the
only thing you are adding is the "--forked" option.  For that
matter, the change to check_branch_commit() is also suspect.

In short, this does a lot more than necessary to add "--forked".
Why such a split in the series?

> +static void parse_forked_args(int argc, const char **argv,
> +			      struct string_list *upstream_patterns)
> +{
> +	int i;
> +
> +	for (i = 0; i < argc; i++) {
> +		const char *arg = argv[i];
> +		struct object_id oid;
> +		char *full_ref = NULL;
> +		const char *short_ref;
> +
> +		if (has_glob_specials(arg)) {
> +			string_list_insert(upstream_patterns, arg);
> +			continue;
> +		}
> +
> +		if (repo_dwim_ref(the_repository, arg, strlen(arg), &oid,
> +				  &full_ref, 0) == 1 &&
> +		    (skip_prefix(full_ref, "refs/heads/", &short_ref) ||
> +		     skip_prefix(full_ref, "refs/remotes/", &short_ref))) {
> +			string_list_insert(upstream_patterns, short_ref);
> +			free(full_ref);
> +			continue;
> +		}
> +		free(full_ref);
> +
> +		die(_("'%s' is not a valid branch or pattern"), arg);
> +	}
> +}

This one does look like very much relevant to "--forked".

> +struct forked_cb {
> +	const struct string_list *upstream_patterns;
> +	struct string_list *out;
> +};

So does this.

> +static int collect_forked_branch(const struct reference *ref, void *cb_data)
> +{
> +	struct forked_cb *cb = cb_data;
> +	struct branch *branch;
> +	const char *upstream, *short_upstream;
> +	const struct string_list_item *item;
> +
> +	if (ref->flags & REF_ISSYMREF)
> +		return 0;
> +	branch = branch_get(ref->name);
> +	if (!branch)
> +		return 0;
> +	upstream = branch_get_upstream(branch, NULL);
> +	if (!upstream)
> +		return 0;
> +	short_upstream = upstream;
> +	(void)(skip_prefix(short_upstream, "refs/heads/", &short_upstream) ||
> +	       skip_prefix(short_upstream, "refs/remotes/", &short_upstream));
> +
> +	for_each_string_list_item(item, cb->upstream_patterns)

Sholdn't each element of upstream_patterns remember if it is
suitable for wildmatch or not when it gets parsed in the earlier
function we saw, so that this loop can refrain from calling
wildmatch() for non-wildcard elements?

> +		if (!wildmatch(item->string, short_upstream, WM_PATHNAME)) {
> +			string_list_append(cb->out, ref->name)->util =
> +				xstrdup(upstream);
> +			return 0;
> +		}
> +	return 0;
> +}
> +
> +static void collect_forked_set(int argc, const char **argv,
> +			       struct string_list *out)
> +{
> +	struct string_list upstream_patterns = STRING_LIST_INIT_DUP;
> +	struct forked_cb cb = {
> +		.upstream_patterns = &upstream_patterns,
> +		.out = out,
> +	};
> +
> +	parse_forked_args(argc, argv, &upstream_patterns);
> +
> +	refs_for_each_branch_ref(get_main_ref_store(the_repository),
> +				 collect_forked_branch, &cb);
> +
> +	string_list_clear(&upstream_patterns, 0);
> +}
> +
> +static int list_forked_branches(int argc, const char **argv)
> +{
> +	struct string_list out = STRING_LIST_INIT_DUP;
> +	struct string_list_item *item;
> +
> +	if (!argc)
> +		die(_("--forked requires at least one <branch>"));
> +
> +	collect_forked_set(argc, argv, &out);
> +	for_each_string_list_item(item, &out)
> +		puts(item->string);
> +
> +	string_list_clear(&out, 1);
> +	return 0;
> +}

Up to point the changes are very much appropriate for adding the
"--forked" option.

> @@ -714,6 +822,7 @@ int cmd_branch(int argc,
>  	/* possible actions */
>  	int delete = 0, rename = 0, copy = 0, list = 0,
>  	    unset_upstream = 0, show_current = 0, edit_description = 0;
> +	int forked = 0;
>  	const char *new_upstream = NULL;
>  	int noncreate_actions = 0;
>  	/* possible options */
> @@ -767,6 +876,8 @@ int cmd_branch(int argc,
>  		OPT_BOOL(0, "create-reflog", &reflog, N_("create the branch's reflog")),
>  		OPT_BOOL(0, "edit-description", &edit_description,
>  			 N_("edit the description for the branch")),
> +		OPT_BOOL(0, "forked", &forked,
> +			N_("list local branches whose upstream matches the given <branch>...")),
>  		OPT__FORCE(&force, N_("force creation, move/rename, deletion"), PARSE_OPT_NOCOMPLETE),
>  		OPT_MERGED(&filter, N_("print only branches that are merged")),
>  		OPT_NO_MERGED(&filter, N_("print only branches that are not merged")),
> @@ -811,7 +922,7 @@ int cmd_branch(int argc,
>  			     0);
>  
>  	if (!delete && !rename && !copy && !edit_description && !new_upstream &&
> -	    !show_current && !unset_upstream && argc == 0)
> +	    !show_current && !unset_upstream && !forked && argc == 0)
>  		list = 1;
>  
>  	if (filter.with_commit || filter.no_commit ||
> @@ -820,7 +931,7 @@ int cmd_branch(int argc,
>  
>  	noncreate_actions = !!delete + !!rename + !!copy + !!new_upstream +
>  			    !!show_current + !!list + !!edit_description +
> -			    !!unset_upstream;
> +			    !!unset_upstream + !!forked;
>  	if (noncreate_actions > 1)
>  		usage_with_options(builtin_branch_usage, options);
>  
> @@ -858,7 +969,11 @@ int cmd_branch(int argc,
>  	if (delete) {
>  		if (!argc)
>  			die(_("branch name required"));
> -		ret = delete_branches(argc, argv, delete > 1, filter.kind, quiet);
> +		ret = delete_branches(argc, argv, delete > 1, filter.kind,
> +				      quiet, 0, NULL);
> +		goto out;

This does not belong to "--forked" as far as I can tell.

> +	} else if (forked) {
> +		ret = list_forked_branches(argc, argv);
>  		goto out;
>  	} else if (show_current) {
>  		print_current_branch_name();

"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:



> diff --git a/builtin/branch.c b/builtin/branch.c
> index 1e24c95a69..29d38e9060 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c

Due to the way the patch is split between 1/4 and 2/4, it is
impossible to comment on the change to delete_branches etc. that are
needed for this step.  I'll use "git diff master... builtin/" instead.

> diff --git a/builtin/branch.c b/builtin/branch.c
> index 1572a4f9ef..b89fd56112 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -1,43 +1,47 @@
> ...
> @@ -132,78 +136,87 @@ static const char *branch_get_color(enum color_branch ix)
>  static int branch_merged(int kind, const char *name,
>  			 struct commit *rev, struct commit *head_rev)
>  {

This one is called from check_branch_commit().  In "--prune-merged"
code paths, as we will see below, kind==FILTER_REFS_BRANCHES is
passed.

But note that there is no reason to expect "--prune-merged" will be
the only one to pass FILTER_REFS_BRANCHES to this function.  Most
notably, "git branch -d" without "-r" would set FILTER_REFS_BRANCHES
to filter.kind and passes the value here.

>  	/*
>  	 * This checks whether the merge bases of branch and HEAD (or
>  	 * the other branch this branch builds upon) contains the
>  	 * branch, which means that the branch has already been merged
>  	 * safely to HEAD (or the other branch).
>  	 */
>  	struct commit *reference_rev = NULL;
>  	const char *reference_name = NULL;
>  	void *reference_name_to_free = NULL;
>  	int merged;
>  
>  	if (kind == FILTER_REFS_BRANCHES) {
>  		struct branch *branch = branch_get(name);
>  		const char *upstream = branch_get_upstream(branch, NULL);
>  		struct object_id oid;
>  
>  		if (upstream &&
>  		    (reference_name = reference_name_to_free =
>  		     refs_resolve_refdup(get_main_ref_store(the_repository), upstream, RESOLVE_REF_READING,
>  					 &oid, NULL)) != NULL)
>  			reference_rev = lookup_commit_reference(the_repository,
>  								&oid);
>  	}

We find its upstream in reference_rev; if this is non-NULL, the
branch MUST be merged to that revision for it to be safely removed.

>  	if (!reference_rev)
>  		reference_rev = head_rev;

But when ehad_rev is given, and if there is no upstream, we check if
the branch is merged to whatever happens to be checked out instead.
For the purpose of "--prune-merged", therefore, we MUST pass NULL in
head_rev when we call this function.  We'll see what is done in the
caller later.

"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> diff --git a/builtin/branch.c b/builtin/branch.c
> index 1e24c95a69..29d38e9060 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -172,8 +174,8 @@ static int branch_merged(int kind, const char *name,
>  	 * any of the following code, but during the transition period,
>  	 * a gentle reminder is in order.
>  	 */
> -	if (head_rev != reference_rev) {
> -		int expect = head_rev ? repo_in_merge_bases(the_repository, rev, head_rev) : 0;
> +	if (head_rev && head_rev != reference_rev) {
> +		int expect = repo_in_merge_bases(the_repository, rev, head_rev);

Any caller who passed head_rev==NULL still used to come into this
block, set expect to 0, and have it compared with merged.  If merged
is 0 (which is what happens when reference_rev is NULL and head_rev
is NULL), the control left this block silently due to /* okay */
below (which is in the post-context that is not shown).

The updated code refuses control to come into this block when
head_rev is NULL.  I am not sure why the change in this hunk is
needed.

> @@ -748,6 +750,25 @@ static int collect_forked_branch(const struct reference *ref, void *cb_data)
>  	return 0;
>  }
>  
> +static int collect_default_branch_name(struct remote *remote, void *cb_data)
> +{
> +	struct string_list *protected = cb_data;
> +	struct ref_store *refs = get_main_ref_store(the_repository);
> +	struct strbuf head = STRBUF_INIT;
> +	const char *target;
> +
> +	strbuf_addf(&head, "refs/remotes/%s/HEAD", remote->name);
> +	target = refs_resolve_ref_unsafe(refs, head.buf,
> +					 RESOLVE_REF_NO_RECURSE, NULL, NULL);
> +	if (target) {
> +		const char *leaf = strrchr(target, '/');
> +		if (leaf)
> +			string_list_insert(protected, leaf + 1);
> +	}
> +	strbuf_release(&head);
> +	return 0;
> +}

It is strange to assume that whatever upstream repository happens to
use as its primary branch name has anything to do with how the local
repository names its primary branch.  Shouldn't this instead use
init.defaultBranch configuration or something?

> @@ -781,6 +802,63 @@ static int list_forked_branches(int argc, const char **argv)
>  	return 0;
>  }
>  
> +static int prune_merged_branches(int argc, const char **argv, int quiet)
> +{
> +	struct ref_store *refs = get_main_ref_store(the_repository);
> +	struct string_list candidates = STRING_LIST_INIT_DUP;
> +	struct string_list protected_default_names = STRING_LIST_INIT_DUP;
> +	struct strvec deletable = STRVEC_INIT;
> +	struct strbuf buf = STRBUF_INIT;
> +	struct string_list_item *item;
> +	int n_not_merged = 0;
> +	int ret = 0;
> +
> +	if (!argc)
> +		die(_("--prune-merged requires at least one <branch>"));
> +
> +	collect_forked_set(argc, argv, &candidates);

Due to poor separation of changes, all the necessary information to
assess how sane the above code is is hidden in [1/4] and not here.

> +	for_each_remote(collect_default_branch_name, &protected_default_names);

This looks inefficient, and worse, incorrect.

If we have multiple branches in argv[], they may have come from
different upstream, and because you pay attention to what the
refs/remotes/<upstream>/HEAD symrefs point at, you have multiple
such "protected" default names.  You'd compare each and every
candidates against these names using linear search in the
string_list to see if they are protected (inefficient), and you
reject removal of argv[1] even when it happens to be the same name
as the default in the repository from which the upstream of argv[3]
came from, i.e., has no relationship with argv[1] (incorrect).

> +	for_each_string_list_item(item, &candidates) {
> +		const char *short_name = item->string;
> +		const char *upstream = item->util;
> +
> +		strbuf_reset(&buf);
> +		strbuf_addf(&buf, "refs/heads/%s", short_name);
> +		if (branch_checked_out(buf.buf))
> +			continue;
> +
> +		if (string_list_has_string(&protected_default_names,
> +					   short_name))
> +			continue;
> +
> +		if (!refs_ref_exists(refs, upstream))
> +			continue;
> +
> +		strvec_push(&deletable, short_name);
> +	}
> +	strbuf_release(&buf);
> +
> +	if (deletable.nr)
> +		ret = delete_branches(deletable.nr, deletable.v,
> +				      0, FILTER_REFS_BRANCHES, quiet,
> +				      1, &n_not_merged);

Add comments on parameters, perhaps, to make it readable?

		ret = delete_branches(deletable.nr, deletable.v,
				      0, /* no force */
				      FILTER_REFS_BRANCHES,
				      quiet,
				      1, /* warn only */
				      &n_not_merged);

or something?

Unfortunately the body of delete_branches() updated by this series
is not visible in this patch, making a sensible review impossible,
but if I recall correctly from what I saw in [1/4], with no-force
set, it does not leave head_rev NULL, and instead reads the HEAD
into it, and that is eventually passed to check_branch_commit()
call.

Now, check_branch_commit() passes head_rev to branch_merged(), which
falls back to "HEAD" when head_rev is given.  That sounds incorrect
for at least two reasons.  We are only dealing with branches that do
have upstream, so fallback should not trigger and we shouldn't be
allowing head_rev to be passed.  It _might_ be debatable that it is
prudent to still expect branch_merged() not to find the upstream to
detect errors in the logic, but falling back to head_rev in such a
case does not make any sense.

> +	if (n_not_merged && !quiet)
> +		fprintf(stderr,
> +			Q_("Skipped %d branch that is not fully merged; "
> +			   "delete it with 'git branch -D' if you are sure.\n",
> +			   "Skipped %d branches that are not fully merged; "
> +			   "delete them with 'git branch -D' if you are sure.\n",
> +			   n_not_merged),
> +			n_not_merged);

I do not think we unconditionally want to see this, and "--quiet"
shouldn't be the onlyl way to squelch this message.

When !quiet, the warn_only call to check_branch_commit() would
already have reported which branches are not fully merged, and
after seeing this message a few times, even the most novice user
would know how to use "git branch -D" to remove unneeded branches.

Use of advice_if_enabled() may make it more palatable.

> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> index 45455cb8ce..c8589cd3a6 100755
> --- a/t/t3200-branch.sh
> +++ b/t/t3200-branch.sh
> @@ -1798,4 +1798,143 @@ test_expect_success '--forked requires at least one <branch>' '
>  	test_grep "at least one <branch>" err
>  '
>  
> +test_expect_success '--prune-merged: setup' '
> +	test_create_repo pm-upstream &&
> +	test_commit -C pm-upstream base &&
> +	git -C pm-upstream checkout -b next &&
> +	test_commit -C pm-upstream one-commit &&
> +	test_commit -C pm-upstream two-commit &&
> +	git -C pm-upstream branch one HEAD~ &&
> +	git -C pm-upstream branch two HEAD &&
> +	git -C pm-upstream branch wip main &&
> +	git -C pm-upstream checkout main
> +'
> +
> +test_expect_success '--prune-merged deletes branches integrated into upstream' '
> +	test_when_finished "rm -rf pm-merged" &&
> +	git clone pm-upstream pm-merged &&
> +	git -C pm-merged branch one one-commit &&
> +	git -C pm-merged branch --set-upstream-to=origin/next one &&
> +	git -C pm-merged branch two two-commit &&
> +	git -C pm-merged branch --set-upstream-to=origin/next two &&
> +
> +	git -C pm-merged branch --prune-merged "origin/*" &&
> +
> +	test_must_fail git -C pm-merged rev-parse --verify refs/heads/one &&
> +	test_must_fail git -C pm-merged rev-parse --verify refs/heads/two
> +'
> +
> +test_expect_success '--prune-merged with a literal upstream argument' '
> +	test_when_finished "rm -rf pm-literal" &&
> +	git clone pm-upstream pm-literal &&
> +	git -C pm-literal branch one one-commit &&
> +	git -C pm-literal branch --set-upstream-to=origin/next one &&
> +	git -C pm-literal branch keepme one-commit &&
> +	git -C pm-literal branch --set-upstream-to=origin/main keepme &&
> +
> +	git -C pm-literal branch --prune-merged origin/next &&
> +
> +	test_must_fail git -C pm-literal rev-parse --verify refs/heads/one &&
> +	git -C pm-literal rev-parse --verify refs/heads/keepme
> +'
> +
> +test_expect_success '--prune-merged unions multiple <branch> arguments' '
> +	test_when_finished "rm -rf pm-union" &&
> +	git clone pm-upstream pm-union &&
> +	git -C pm-union branch one one-commit &&
> +	git -C pm-union branch --set-upstream-to=origin/next one &&
> +	git -C pm-union branch two base &&
> +	git -C pm-union branch --set-upstream-to=origin/main two &&
> +
> +	git -C pm-union branch --prune-merged origin/next 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
> +'
> +
> +test_expect_success '--prune-merged with a local-branch argument' '
> +	test_create_repo pm-local &&
> +	test_when_finished "rm -rf pm-local" &&
> +	test_commit -C pm-local base &&
> +	git -C pm-local branch topic base &&
> +	git -C pm-local config branch.topic.remote . &&
> +	git -C pm-local config branch.topic.merge refs/heads/main &&
> +	git -C pm-local checkout --detach &&
> +
> +	git -C pm-local branch --prune-merged main &&
> +
> +	test_must_fail git -C pm-local rev-parse --verify refs/heads/topic &&
> +	git -C pm-local rev-parse --verify refs/heads/main
> +'
> +
> +test_expect_success '--prune-merged spares branches with un-integrated commits' '
> +	test_when_finished "rm -rf pm-unmerged" &&
> +	git clone pm-upstream pm-unmerged &&
> +	git -C pm-unmerged checkout -b wip origin/wip &&
> +	git -C pm-unmerged branch --set-upstream-to=origin/next wip &&
> +	test_commit -C pm-unmerged local-only &&
> +	git -C pm-unmerged checkout - &&
> +
> +	git -C pm-unmerged branch --prune-merged "origin/*" 2>err &&
> +	test_grep "not fully merged" err &&
> +	test_grep "Skipped 1 branch" err &&
> +	test_grep "git branch -D" err &&
> +	test_grep ! "If you are sure you want to delete it" err &&
> +	git -C pm-unmerged rev-parse --verify refs/heads/wip
> +'
> +
> +test_expect_success '--prune-merged skips branches whose upstream is gone' '
> +	test_when_finished "rm -rf pm-upstream-gone" &&
> +	git clone pm-upstream pm-upstream-gone &&
> +	git -C pm-upstream-gone branch one one-commit &&
> +	git -C pm-upstream-gone branch --set-upstream-to=origin/next one &&
> +
> +	git -C pm-upstream-gone update-ref -d refs/remotes/origin/next &&
> +	git -C pm-upstream-gone branch --prune-merged "origin/*" &&
> +
> +	git -C pm-upstream-gone rev-parse --verify refs/heads/one
> +'
> +
> +test_expect_success '--prune-merged never deletes the checked-out branch' '
> +	test_when_finished "rm -rf pm-head" &&
> +	git clone pm-upstream pm-head &&
> +	git -C pm-head checkout -b one one-commit &&
> +	git -C pm-head branch --set-upstream-to=origin/next one &&
> +
> +	git -C pm-head branch --prune-merged "origin/*" &&
> +
> +	git -C pm-head rev-parse --verify refs/heads/one
> +'
> +
> +test_expect_success '--prune-merged spares the local default branch' '
> +	test_when_finished "rm -rf pm-default" &&
> +	git clone pm-upstream pm-default &&
> +	git -C pm-default checkout --detach &&
> +	git -C pm-default branch --prune-merged "origin/*" &&
> +	git -C pm-default rev-parse --verify refs/heads/main
> +'
> +
> +test_expect_success '--prune-merged protects the default branch by name only' '
> +	test_when_finished "rm -rf pm-default-alias" &&
> +	git clone pm-upstream pm-default-alias &&
> +	git -C pm-default-alias branch --track trunk origin/main &&
> +	git -C pm-default-alias checkout --detach &&
> +	git -C pm-default-alias branch --prune-merged "origin/*" &&
> +	git -C pm-default-alias rev-parse --verify refs/heads/main &&
> +	test_must_fail git -C pm-default-alias rev-parse --verify refs/heads/trunk
> +'
> +
> +test_expect_success '--prune-merged with literal arg also protects default-name' '
> +	test_when_finished "rm -rf pm-literal-default" &&
> +	git clone pm-upstream pm-literal-default &&
> +	git -C pm-literal-default checkout --detach &&
> +	git -C pm-literal-default branch --prune-merged origin/main &&
> +	git -C pm-literal-default rev-parse --verify refs/heads/main
> +'
> +
> +test_expect_success '--prune-merged requires at least one <branch>' '
> +	test_must_fail git -C pm-upstream branch --prune-merged 2>err &&
> +	test_grep "at least one <branch>" err
> +'
> +
>  test_done

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

> "Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>
>
>> diff --git a/builtin/branch.c b/builtin/branch.c
>> index 1e24c95a69..29d38e9060 100644
>> --- a/builtin/branch.c
>> +++ b/builtin/branch.c
>
> Due to the way the patch is split between 1/4 and 2/4, it is
> impossible to comment on the change to delete_branches etc. that are
> needed for this step.  I'll use "git diff master... builtin/" instead.
>

Please discard this version.  I had unnecessary draft comments that
I used as reference in it.

> Please discard this version.  I had unnecessary draft comments that
> I used as reference in it.

I'm taking this to mean starting over from v9 and implementing the
'origin/*' idea again from there. Correct?


Harald

Harald Nordgren <haraldnordgren@gmail.com> writes:

>> Please discard this version.  I had unnecessary draft comments that
>> I used as reference in it.
>
> I'm taking this to mean starting over from v9 and implementing the
> 'origin/*' idea again from there. Correct?

No, what I meant was "please discard the review message I was
responding to".

"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> diff --git a/builtin/branch.c b/builtin/branch.c
> index 1e24c95a69..29d38e9060 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -172,8 +174,8 @@ static int branch_merged(int kind, const char *name,
>  	 * any of the following code, but during the transition period,
>  	 * a gentle reminder is in order.
>  	 */
> -	if (head_rev != reference_rev) {
> -		int expect = head_rev ? repo_in_merge_bases(the_repository, rev, head_rev) : 0;
> +	if (head_rev && head_rev != reference_rev) {
> +		int expect = repo_in_merge_bases(the_repository, rev, head_rev);

Any caller who passed head_rev==NULL still used to come into this
block, set expect to 0, and have it compared with merged.  If merged
is 0 (which is what happens when reference_rev is NULL and head_rev
is NULL), the control left this block silently due to /* okay */
below (which is in the post-context that is not shown).

The updated code refuses control to come into this block when
head_rev is NULL.  I am not sure why the change in this hunk is
needed.

> @@ -748,6 +750,25 @@ static int collect_forked_branch(const struct reference *ref, void *cb_data)
>  	return 0;
>  }
>  
> +static int collect_default_branch_name(struct remote *remote, void *cb_data)
> +{
> +	struct string_list *protected = cb_data;
> +	struct ref_store *refs = get_main_ref_store(the_repository);
> +	struct strbuf head = STRBUF_INIT;
> +	const char *target;
> +
> +	strbuf_addf(&head, "refs/remotes/%s/HEAD", remote->name);
> +	target = refs_resolve_ref_unsafe(refs, head.buf,
> +					 RESOLVE_REF_NO_RECURSE, NULL, NULL);
> +	if (target) {
> +		const char *leaf = strrchr(target, '/');
> +		if (leaf)
> +			string_list_insert(protected, leaf + 1);
> +	}
> +	strbuf_release(&head);
> +	return 0;
> +}

It is strange to assume that whatever upstream repository happens to
use as its primary branch name has anything to do with how the local
repository names its primary branch.  Shouldn't this instead use
init.defaultBranch configuration or something?

> @@ -781,6 +802,63 @@ static int list_forked_branches(int argc, const char **argv)
>  	return 0;
>  }
>  
> +static int prune_merged_branches(int argc, const char **argv, int quiet)
> +{
> +	struct ref_store *refs = get_main_ref_store(the_repository);
> +	struct string_list candidates = STRING_LIST_INIT_DUP;
> +	struct string_list protected_default_names = STRING_LIST_INIT_DUP;
> +	struct strvec deletable = STRVEC_INIT;
> +	struct strbuf buf = STRBUF_INIT;
> +	struct string_list_item *item;
> +	int n_not_merged = 0;
> +	int ret = 0;
> +
> +	if (!argc)
> +		die(_("--prune-merged requires at least one <branch>"));
> +
> +	collect_forked_set(argc, argv, &candidates);

Due to poor separation of changes, all the necessary information to
assess how sane the above code is is hidden in [1/4] and not here.

> +	for_each_remote(collect_default_branch_name, &protected_default_names);

This looks inefficient, and worse, incorrect.

If we have multiple branches in argv[], they may have come from
different upstream, and because you pay attention to what the
refs/remotes/<upstream>/HEAD symrefs point at, you have multiple
such "protected" default names.  You'd compare each and every
candidates against these names using linear search in the
string_list to see if they are protected (inefficient), and you
reject removal of argv[1] even when it happens to be the same name
as the default in the repository from which the upstream of argv[3]
came from, i.e., has no relationship with argv[1] (incorrect).

> +	for_each_string_list_item(item, &candidates) {
> +		const char *short_name = item->string;
> +		const char *upstream = item->util;
> +
> +		strbuf_reset(&buf);
> +		strbuf_addf(&buf, "refs/heads/%s", short_name);
> +		if (branch_checked_out(buf.buf))
> +			continue;
> +
> +		if (string_list_has_string(&protected_default_names,
> +					   short_name))
> +			continue;
> +
> +		if (!refs_ref_exists(refs, upstream))
> +			continue;
> +
> +		strvec_push(&deletable, short_name);
> +	}
> +	strbuf_release(&buf);
> +
> +	if (deletable.nr)
> +		ret = delete_branches(deletable.nr, deletable.v,
> +				      0, FILTER_REFS_BRANCHES, quiet,
> +				      1, &n_not_merged);

Add comments on parameters, perhaps, to make it readable?

		ret = delete_branches(deletable.nr, deletable.v,
				      0, /* no force */
				      FILTER_REFS_BRANCHES,
				      quiet,
				      1, /* warn only */
				      &n_not_merged);

or something?

Unfortunately the body of delete_branches() updated by this series
is not visible in this patch, making a sensible review impossible,
but if I recall correctly from what I saw in [1/4], with no-force
set, it does not leave head_rev NULL, and instead reads the HEAD
into it, and that is eventually passed to check_branch_commit()
call.

Now, check_branch_commit() passes head_rev to branch_merged(), which
falls back to "HEAD" when head_rev is given.  That sounds incorrect
for at least two reasons.  We are only dealing with branches that do
have upstream, so fallback should not trigger and we shouldn't be
allowing head_rev to be passed.  It _might_ be debatable that it is
prudent to still expect branch_merged() not to find the upstream to
detect errors in the logic, but falling back to head_rev in such a
case does not make any sense.

> +	if (n_not_merged && !quiet)
> +		fprintf(stderr,
> +			Q_("Skipped %d branch that is not fully merged; "
> +			   "delete it with 'git branch -D' if you are sure.\n",
> +			   "Skipped %d branches that are not fully merged; "
> +			   "delete them with 'git branch -D' if you are sure.\n",
> +			   n_not_merged),
> +			n_not_merged);

I do not think we unconditionally want to see this, and "--quiet"
shouldn't be the onlyl way to squelch this message.

When !quiet, the warn_only call to check_branch_commit() would
already have reported which branches are not fully merged, and
after seeing this message a few times, even the most novice user
would know how to use "git branch -D" to remove unneeded branches.

Use of advice_if_enabled() may make it more palatable.

Am 22.05.26 um 00:40 schrieb Harald Nordgren via GitGitGadget:
> diff --git a/Documentation/git-branch.adoc b/Documentation/git-branch.adoc
> index c0afddc424..3a421f6663 100644
> --- a/Documentation/git-branch.adoc
> +++ b/Documentation/git-branch.adoc
> @@ -24,6 +24,7 @@ git branch (-m|-M) [<old-branch>] <new-branch>
>  git branch (-c|-C) [<old-branch>] <new-branch>
>  git branch (-d|-D) [-r] <branch-name>...
>  git branch --edit-description [<branch-name>]
> +git branch --forked <branch>...

I would have preferred that this option is another filter of --list
mode, not its own mode of operation. Consequently, each --forked option
would take only a single argument (which can contain globs), and can be
given multiple times.

>  
>  DESCRIPTION
>  -----------
> @@ -199,6 +200,12 @@ 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 matches any
> +	of the given _<branch>_ arguments. Each argument is either
> +	a ref (e.g. `origin/master`, `master`) or a shell-style
> +	glob (e.g. `'origin/*'`). Multiple arguments are unioned.

So this could perhaps read:

`--forked`::
	List only branches whose configured upstream matches
	_<branch>_. The argument can contain a shell-style glob
	 (e.g. `'origin/*'`). The option can be repeated to
	widen the filter.

Note that there is no reason to say "local branches". ("... are unioned"
sounds strange, so this is may attempt to express the same in a
different way.)

The icing on the cake would now be that

    git branch --merged origin/main --forked origin/*

provides the list of branches forked from origin that have already been
integrated.

-- Hannes

Johannes Sixt <j6t@kdbg.org> writes:

> The icing on the cake would now be that
>
>     git branch --merged origin/main --forked origin/*
>
> provides the list of branches forked from origin that have already been
> integrated.

Yup, that is very nice.  Also with "--merged" replaced with
"--not-merged", i.e., "our work building on top of origin's, and
still need to be finished", would give us a good list to work on.

> Johannes Sixt <j6t@kdbg.org> writes:
>
> > The icing on the cake would now be that
> >
> >     git branch --merged origin/main --forked origin/*
> >
> > provides the list of branches forked from origin that have already been
> > integrated.
>
> Yup, that is very nice.  Also with "--merged" replaced with
> "--not-merged", i.e., "our work building on top of origin's, and
> still need to be finished", would give us a good list to work on.

This is nice, but I think this would require an overhaul of other
infra as well, maybe better to do as a follow-up?


Harald

Am 22.05.26 um 12:49 schrieb Harald Nordgren:
>> Johannes Sixt <j6t@kdbg.org> writes:
>>
>>> The icing on the cake would now be that
>>>
>>>     git branch --merged origin/main --forked origin/*
>>>
>>> provides the list of branches forked from origin that have already been
>>> integrated.
>>
>> Yup, that is very nice.  Also with "--merged" replaced with
>> "--not-merged", i.e., "our work building on top of origin's, and
>> still need to be finished", would give us a good list to work on.
> 
> This is nice, but I think this would require an overhaul of other
> infra as well, maybe better to do as a follow-up?
This can certainly be done as an extension in a follow-up patch. But the
UI must still be planned accordingly, i.e., --forked can only take a
single argument. For example, in

    git branch --forked foo bar

'bar' is the pattern of branches to show. The "list" is filtered
according to '--forked foo'. That is, if 'bar' was not forked from
'foo', the output is empty.

You would have to require

    git branch --forked foo --forked bar

to list all branches forked from 'foo' or 'bar'.

In the first implementation, you can restrict uses of other options with
--forked or even with a branch pattern. But you cannot be loose by
accepting multiple branch patterns with one --forked option, because
that would later clash with --list mode.

-- Hannes