[PATCH 2/2] branch: protect local upstreams from -d

["Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com>]

From: Harald Nordgren <haraldnordgren@gmail.com>

A local branch may be fully merged into its own upstream while still
serving as the base of a surviving stacked branch. Deleting it with
"git branch -d" then leaves the surviving branch with a missing
upstream.

Use the existing stacked-branch protection after checking every
requested deletion. This makes multi-branch deletion independent of
argument order: a branch that fails its safety check remains available
to protect its upstream. Allow -D to override the protection, and allow
a complete stack to be deleted together.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 Documentation/git-branch.adoc |  4 +++-
 builtin/branch.c              | 38 +++++++++++++++++++++++++++++--
 t/t1507-rev-parse-upstream.sh |  4 ++--
 t/t3200-branch.sh             | 43 +++++++++++++++++++++++++++++++++++
 t/t6040-tracking-info.sh      |  2 +-
 5 files changed, 85 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-branch.adoc b/Documentation/git-branch.adoc
index bfdf459329..5c2a3339b2 100644
--- a/Documentation/git-branch.adoc
+++ b/Documentation/git-branch.adoc
@@ -102,7 +102,9 @@ OPTIONS
 `--delete`::
 	Delete a branch. The branch must be fully merged in its
 	upstream branch, or in `HEAD` if no upstream was set with
-	`--track` or `--set-upstream-to`.
+	`--track` or `--set-upstream-to`, and must not be an upstream,
+	directly or indirectly, of another local branch that will remain
+	after the operation.
 
 `-D`::
 	Shortcut for `--delete --force`.
diff --git a/builtin/branch.c b/builtin/branch.c
index 87f0aa4051..7f76789027 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -294,12 +294,13 @@ static int delete_branches(int argc, const char **argv, int kinds,
 	struct object_id oid;
 	char *name = NULL;
 	const char *fmt;
-	int i;
 	int ret = 0;
 	int remote_branch = 0;
 	struct strbuf bname = STRBUF_INIT;
 	enum interpret_branch_kind allowed_interpret;
 	struct string_list refs_to_delete = STRING_LIST_INIT_DUP;
+	struct strset deletable_branch_names = STRSET_INIT;
+	struct strset protected_branch_names = STRSET_INIT;
 	struct string_list_item *item;
 	int branch_name_pos;
 	const char *fmt_remotes = "refs/remotes/%s";
@@ -326,7 +327,7 @@ static int delete_branches(int argc, const char **argv, int kinds,
 	    !(flags & DELETE_BRANCH_NO_HEAD_FALLBACK))
 		head_rev = lookup_commit_reference(the_repository, &head_oid);
 
-	for (i = 0; i < argc; i++, strbuf_reset(&bname)) {
+	for (int i = 0; i < argc; i++, strbuf_reset(&bname)) {
 		char *target = NULL;
 		int ref_flags = 0;
 
@@ -397,11 +398,42 @@ static int delete_branches(int argc, const char **argv, int kinds,
 		item->util = xstrdup((ref_flags & REF_ISBROKEN) ? "broken"
 				    : (ref_flags & REF_ISSYMREF) ? target
 				    : repo_find_unique_abbrev(the_repository, &oid, DEFAULT_ABBREV));
+		if (!remote_branch && !(flags & (DELETE_BRANCH_FORCE |
+						 DELETE_BRANCH_SKIP_UNMERGED)))
+			strset_add(&deletable_branch_names, bname.buf);
 
 	next:
 		free(target);
 	}
 
+	if (!remote_branch &&
+	    !(flags & (DELETE_BRANCH_FORCE | DELETE_BRANCH_SKIP_UNMERGED)) &&
+	    refs_to_delete.nr) {
+		protect_stacked_branch_bases(get_main_ref_store(the_repository),
+					     &deletable_branch_names, &protected_branch_names);
+		for (size_t i = refs_to_delete.nr; i; i--) {
+			const char *branch_name;
+
+			item = &refs_to_delete.items[i - 1];
+			if (!skip_prefix(item->string, "refs/heads/",
+					 &branch_name))
+				BUG("expected local branch ref, got '%s'",
+				    item->string);
+			if (strset_contains(&deletable_branch_names, branch_name))
+				continue;
+
+			error(_("the branch '%s' is an upstream of another branch"),
+			      branch_name);
+			advise_if_enabled(ADVICE_FORCE_DELETE_BRANCH,
+					  _("If you are sure you want to delete it, "
+					    "run 'git branch -D %s'"),
+					  branch_name);
+			ret = 1;
+			unsorted_string_list_delete_item(&refs_to_delete, i - 1,
+							 1);
+		}
+	}
+
 	if (!(flags & DELETE_BRANCH_DRY_RUN) &&
 	    refs_delete_refs(get_main_ref_store(the_repository), NULL, &refs_to_delete, REF_NO_DEREF))
 		ret = 1;
@@ -428,6 +460,8 @@ static int delete_branches(int argc, const char **argv, int kinds,
 		free(describe_ref);
 	}
 	string_list_clear(&refs_to_delete, 0);
+	strset_clear(&deletable_branch_names);
+	strset_clear(&protected_branch_names);
 
 	free(name);
 	strbuf_release(&bname);
diff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh
index cb9ef7e329..04abfb6f94 100755
--- a/t/t1507-rev-parse-upstream.sh
+++ b/t/t1507-rev-parse-upstream.sh
@@ -146,9 +146,9 @@ test_expect_success 'merge my-side@{u} records the correct name' '
 )
 '
 
-test_expect_success 'branch -d other@{u}' '
+test_expect_success 'branch -D other@{u}' '
 	git checkout -t -b other main &&
-	git branch -d @{u} &&
+	git branch -D @{u} &&
 	git for-each-ref refs/heads/main >actual &&
 	test_must_be_empty actual
 '
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index cdb6c6a634..a3d492ffcd 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -2173,6 +2173,49 @@ test_expect_success "branch -d still deletes a deleteMerged=false branch" '
 	)
 '
 
+test_expect_success 'branch -d keeps the upstream of a surviving branch' '
+	setup_repo_for_delete_merged &&
+	(
+		cd repo &&
+		git branch foundation origin/next --track &&
+		git checkout -b topic foundation --track &&
+		git commit --allow-empty -m "topic work" &&
+		git checkout --detach &&
+
+		test_must_fail git branch -d foundation 2>err &&
+		test_grep "branch .foundation. is an upstream of another branch" err &&
+		test_ref_exists refs/heads/foundation &&
+		test_ref_exists refs/heads/topic &&
+
+		git branch -D foundation &&
+		test_ref_missing refs/heads/foundation &&
+		test_ref_exists refs/heads/topic
+	)
+'
+
+test_expect_success 'branch -d protects a base when another deletion fails' '
+	setup_repo_for_delete_merged &&
+	(
+		cd repo &&
+		git branch foundation origin/next --track &&
+		git checkout -b topic foundation --track &&
+		git commit --allow-empty -m "topic work" &&
+		git checkout --detach &&
+
+		test_must_fail git branch -d foundation topic 2>err &&
+		test_grep "branch .foundation. is an upstream of another branch" err &&
+		test_grep "branch .topic. is not fully merged" err &&
+		test_ref_exists refs/heads/foundation &&
+		test_ref_exists refs/heads/topic &&
+
+		test_must_fail git branch -d topic foundation 2>err &&
+		test_grep "branch .foundation. is an upstream of another branch" err &&
+		test_grep "branch .topic. is not fully merged" err &&
+		test_ref_exists refs/heads/foundation &&
+		test_ref_exists refs/heads/topic
+	)
+'
+
 test_expect_success '--dry-run without --delete-merged is rejected' '
 	test_must_fail git -C forked branch --dry-run 2>err &&
 	test_grep "requires --delete-merged" err
diff --git a/t/t6040-tracking-info.sh b/t/t6040-tracking-info.sh
index e95d420972..01145f6681 100755
--- a/t/t6040-tracking-info.sh
+++ b/t/t6040-tracking-info.sh
@@ -34,7 +34,7 @@ test_expect_success setup '
 		git checkout -b brokenbase origin &&
 		git checkout -b b5 --track brokenbase &&
 		advance g &&
-		git branch -d brokenbase &&
+		git branch -D brokenbase &&
 		git checkout -b b6 origin
 	) &&
 	git checkout -b follower --track main &&
-- 
gitgitgadget