Re: [PATCH v17 5/7] branch: add --delete-merged <branch>

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Harald

On 22/06/2026 08:29, Harald Nordgren via GitGitGadget wrote:
> From: Harald Nordgren <haraldnordgren@gmail.com>
> 
> +static int collect_upstream(const struct reference *ref, void *cb_data)
> +{
> +	struct string_list *upstreams = cb_data;
> +	struct branch *branch = branch_get(ref->name);
> +	const char *upstream = branch_get_upstream(branch, NULL);
> +
> +	string_list_append(upstreams, ref->name)->util =
> +		xstrdup_or_null(upstream);
> +	return 0;
> +}
> +
> +/*
> + * Keep any branch that another, surviving branch tracks as its
> + * upstream, so we never delete a branch out from under one stacked on
> + * top of it.  Sparing a branch makes it a survivor whose own upstream
> + * then needs the same protection, so repeat until nothing changes.
> + */
> +static void spare_stacked_bases(struct ref_store *refs, struct strset *deletable)
> +{
> +	struct string_list upstreams = STRING_LIST_INIT_DUP;
> +	struct string_list_item *item;
> +	bool spared;
> +
> +	refs_for_each_branch_ref(refs, collect_upstream, &upstreams);
> +	do {
> +		spared = false;
> +		for_each_string_list_item(item, &upstreams) {
> +			const char *up = item->util, *up_short;
> +
> +			if (!up || strset_contains(deletable, item->string))
> +				continue;
> +			if (!skip_prefix(up, "refs/heads/", &up_short) ||
> +			    !strset_contains(deletable, up_short))
> +				continue;
> +
> +			strset_remove(deletable, up_short);
> +			spared = true;
> +		}
> +	} while (spared);
> +
> +	string_list_clear(&upstreams, 1);
> +}

This keeps the whole chain of branches, which is the safest thing to
do but potentially keeps unneeded branches around. It is only really
the upstream branches of unmerged branches which are useful so we
could just keep those, and if their upstream branch is going to be
deleted clear the upstream config for that branch.

For example, if we have branch feature-3 with upstream feature-2 which
has upstream feature-1, then if feature-1 and feature-2 are merged we'd
delete feature-1 but keep feature-2 and clear its upstream config. If we
also had feature-4 that was not merged and had upstream feature-1 we'd
keep feature-1 and leave the upstream config for feature-2 unchanged.

Here is a possible implementation for that. It compiles but I've not
written any new tests. It does pass the existing tests which means
either it is buggy or we don't have coverage for keeping a chain of
branches.

static int collect_upstreams(const struct reference *ref, void *cb_data)
{
	struct collect_upstream_data *data = cb_data;
	struct strset *deletable = data->deletable;
	struct strset *upstreams = data->upstreams;
	struct branch *branch;
	const char *upstream_ref, *upstream_name;

	/*
	 * We're only interested in the upstreams of branches that
	 * are not being deleted.
	 */
	if (strset_contains(deletable, ref->name))
		return 0;
	branch = branch_get(ref->name);
	if (!branch)
		return 0;
	upstream_ref = branch_get_upstream(branch, NULL);
	/*
	 * We're only interested in the upstream if it is going to
	 * be deleted.
	 */
	if (!upstream_ref ||
	    !skip_prefix(upstream_ref, "refs/heads/", &upstream_name) ||
	    !strset_contains(deletable, upstream_name))
		return 0;
	/*
	 * Do not delete this branch because it is the upstream of
	 * an unmerged branch. Also remember it so we can check if
	 * its upstream is marked for deletion once we've visited all
	 * branches
	 */
	strset_remove(deletable, upstream_name);
	strset_add(upstreams, upstream_name);

	return 0;
}

/*
  * Keep any branch that another, surviving branch tracks as its
  * upstream, so we never delete a branch out from under one stacked on
  * top of it.  If the upstream branch has an upstream set that is marked
  * for deletion clear its upstream config.
  */
static void spare_stacked_bases(struct ref_store *refs, struct strset *deletable)
{
	struct strset upstreams = STRSET_INIT;
	struct collect_upstream_data data = {
		.deletable = deletable,
		.upstreams = &upstreams,
	};
	struct strbuf buf = STRBUF_INIT;
	struct hashmap_iter iter;
	struct strmap_entry *entry;

	refs_for_each_branch_ref(refs, collect_upstreams, &data);
	strset_for_each_entry(&upstreams, &iter, entry) {
		const char *upstream_upstream;
		struct branch *upstream_branch;

		/* We know upstream_ref is a branch, skip "refs/heads/" */
		upstream_branch = branch_get(entry->key);
		upstream_upstream = branch_get_upstream(upstream_branch, NULL);
		if (upstream_upstream &&
		    strset_contains(deletable, upstream_upstream)) {
			/*
			 * This branch has been merged and is the upstream of
			 * an unmerged branch.  Its upstream is marked for
			 * deletion because it is not the upstream of any
			 * unmerged branch so clear its upstream config.
			 */
			strbuf_reset(&buf);
			strbuf_addf(&buf, "branch.%s.merge", upstream_branch->name);
			repo_config_set_gently(the_repository, buf.buf, NULL);
			strbuf_setlen(&buf, buf.len - 5);
			strbuf_addstr(&buf, "remote");
			repo_config_set_gently(the_repository, buf.buf, NULL);
		}
	}
	strbuf_release(&buf);
	strset_clear(&upstreams);

}


> +	for (i = 0; i < candidates.nr; i++) {
> +		const char *short_name;
> +
> +		if (skip_prefix(candidates.items[i]->refname, "refs/heads/",
> +				&short_name) &&
> +		    strset_contains(&deletable, short_name))
> +			strvec_push(&to_delete, short_name);
> +	}

It would be nicer to use strset_for_each_entry() here. First declare

	struct hashmap_iter iter;
	struct strmap_entry *entry;

at the top of the function and then replace the loop with

	strset_for_each_entry(&deletable, &iter, entry)
		strvec_push(&to_delete, entry->key);

Thanks

Phillip


> +	if (to_delete.nr)
> +		ret = delete_branches(to_delete.nr, to_delete.v,
> +				      FILTER_REFS_BRANCHES,
> +				      DELETE_BRANCH_SKIP_UNMERGED |
> +				      DELETE_BRANCH_NO_HEAD_FALLBACK |
> +				      flags);
> +
> +	strvec_clear(&to_delete);
> +	strset_clear(&deletable);
> +	ref_array_clear(&candidates);
> +	ref_filter_clear(&filter);
> +	return ret;
> +}
> +
>   static GIT_PATH_FUNC(edit_description, "EDIT_DESCRIPTION")
>   
>   static int edit_branch_description(const char *branch_name)
> @@ -746,6 +862,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 delete_merged = 0;
>   	const char *new_upstream = NULL;
>   	int noncreate_actions = 0;
>   	/* possible options */
> @@ -799,6 +916,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, "delete-merged", &delete_merged,
> +			N_("delete local branches whose upstream matches <branch> and are merged")),
>   		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")),
> @@ -846,7 +965,8 @@ int cmd_branch(int argc,
>   			     0);
>   
>   	if (!delete && !rename && !copy && !edit_description && !new_upstream &&
> -	    !show_current && !unset_upstream && argc == 0)
> +	    !show_current && !unset_upstream && !delete_merged &&
> +	    argc == 0)
>   		list = 1;
>   
>   	if (filter.with_commit || filter.no_commit ||
> @@ -856,7 +976,7 @@ int cmd_branch(int argc,
>   
>   	noncreate_actions = !!delete + !!rename + !!copy + !!new_upstream +
>   			    !!show_current + !!list + !!edit_description +
> -			    !!unset_upstream;
> +			    !!unset_upstream + !!delete_merged;
>   	if (noncreate_actions > 1)
>   		usage_with_options(builtin_branch_usage, options);
>   
> @@ -898,6 +1018,10 @@ int cmd_branch(int argc,
>   				      (delete > 1 ? DELETE_BRANCH_FORCE : 0) |
>   				      (quiet ? DELETE_BRANCH_QUIET : 0));
>   		goto out;
> +	} else if (delete_merged) {
> +		ret = delete_merged_branches(argc, argv,
> +					     quiet ? DELETE_BRANCH_QUIET : 0);
> +		goto out;
>   	} else if (show_current) {
>   		print_current_branch_name();
>   		ret = 0;
> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> index 3104c555f6..1d372f95e8 100755
> --- a/t/t3200-branch.sh
> +++ b/t/t3200-branch.sh
> @@ -1839,4 +1839,155 @@ test_expect_success '--forked narrows a <pattern> argument' '
>   	test_cmp expect actual
>   '
>   
> +test_expect_success '--delete-merged: setup' '
> +	git init -b main upstream &&
> +	(
> +		cd upstream &&
> +		test_commit base &&
> +		git checkout -b next &&
> +		test_commit next-work &&
> +		git checkout main
> +	) &&
> +	git init -b main other &&
> +	test_commit -C other other-base &&
> +	git init -b main fork
> +'
> +
> +setup_repo_for_delete_merged () {
> +	rm -rf repo &&
> +	git clone upstream repo &&
> +	(
> +		cd repo &&
> +		git remote add fork ../fork &&
> +		git remote add other ../other &&
> +		git config remote.pushDefault fork &&
> +		git config push.default current &&
> +		git fetch other
> +	)
> +}
> +
> +merged_branch () {
> +	(
> +		cd repo &&
> +		git checkout -b "$1" "$2" &&
> +		git commit --allow-empty -m "$1 work" &&
> +		git push origin "$1:next" &&
> +		git fetch origin &&
> +		git branch --set-upstream-to="$2" "$1"
> +	)
> +}
> +
> +test_expect_success '--delete-merged deletes merged branches and spares the rest' '
> +	test_when_finished "rm -rf repo" &&
> +	setup_repo_for_delete_merged &&
> +	merged_branch merged origin/next &&
> +	(
> +		cd repo &&
> +		git checkout -b unmerged origin/next &&
> +		git commit --allow-empty -m "unmerged work" &&
> +		git branch --set-upstream-to=origin/next unmerged &&
> +		git checkout -b tracks-other other/main &&
> +		git branch --set-upstream-to=other/main tracks-other &&
> +		git checkout --detach
> +	) &&
> +	sha=$(git -C repo rev-parse --short merged) &&
> +
> +	git -C repo branch --delete-merged origin/next >actual 2>&1 &&
> +
> +	echo "Deleted branch merged (was $sha)." >expect &&
> +	test_cmp expect actual &&
> +	git -C repo for-each-ref --format="%(refname:short)" refs/heads/ >actual &&
> +	cat >expect <<-\EOF &&
> +	main
> +	tracks-other
> +	unmerged
> +	EOF
> +	test_cmp expect actual
> +'
> +
> +test_expect_success '--delete-merged deletes merged branches and spares protected ones' '
> +	test_when_finished "rm -rf repo" &&
> +	setup_repo_for_delete_merged &&
> +	merged_branch on-next origin/next &&
> +	merged_branch checked-out origin/next &&
> +	merged_branch upstream-gone origin/next &&
> +	(
> +		cd repo &&
> +		git checkout -b mainline main &&
> +		git checkout -b on-local mainline &&
> +		git branch --set-upstream-to=mainline on-local &&
> +		git update-ref refs/remotes/origin/topic refs/remotes/origin/next &&
> +		git branch --set-upstream-to=origin/topic upstream-gone &&
> +		git update-ref -d refs/remotes/origin/topic &&
> +		git branch --set-upstream-to=origin/main main &&
> +		git config branch.main.pushRemote origin &&
> +		git checkout -b tracks-other other/main &&
> +		git branch --set-upstream-to=other/main tracks-other &&
> +		git checkout checked-out
> +	) &&
> +
> +	git -C repo branch --delete-merged origin/next mainline &&
> +
> +	git -C repo for-each-ref --format="%(refname:short)" refs/heads/ >actual &&
> +	cat >expect <<-\EOF &&
> +	checked-out
> +	main
> +	mainline
> +	tracks-other
> +	upstream-gone
> +	EOF
> +	test_cmp expect actual
> +'
> +
> +test_expect_success '--delete-merged requires at least one <branch>' '
> +	test_must_fail git -C forked branch --delete-merged 2>err &&
> +	test_grep "requires at least one <branch>" err
> +'
> +
> +test_expect_success '--delete-merged keeps a branch that is an upstream' '
> +	test_when_finished "rm -rf repo" &&
> +	setup_repo_for_delete_merged &&
> +	merged_branch feature origin/next &&
> +	(
> +		cd repo &&
> +		git checkout -b topic feature &&
> +		git commit --allow-empty -m "topic work" &&
> +		git branch --set-upstream-to=feature topic &&
> +		git checkout --detach
> +	) &&
> +
> +	git -C repo branch --delete-merged origin/next 2>err &&
> +
> +	test_must_be_empty err &&
> +	git -C repo rev-parse --verify refs/heads/feature &&
> +	git -C repo rev-parse --verify refs/heads/topic
> +'
> +
> +test_expect_success '--delete-merged keeps a chain of upstreams of a kept branch' '
> +	test_when_finished "rm -rf repo" &&
> +	setup_repo_for_delete_merged &&
> +	(
> +		cd repo &&
> +		git branch b3 origin/next &&
> +		git branch --set-upstream-to=origin/next b3 &&
> +		git branch b2 origin/next &&
> +		git branch --set-upstream-to=b3 b2 &&
> +		git checkout -b b1 b2 &&
> +		git commit --allow-empty -m "b1 work" &&
> +		git branch --set-upstream-to=b2 b1 &&
> +		git checkout --detach
> +	) &&
> +
> +	git -C repo branch --delete-merged origin/next &&
> +
> +	git -C repo for-each-ref --format="%(refname:short)" refs/heads/ >actual &&
> +	cat >expect <<-\EOF &&
> +	b1
> +	b2
> +	b3
> +	main
> +	EOF
> +	test_cmp expect actual
> +'
> +
>   test_done