branch -d: base the "already-merged" safety on the branch it merges with

When a branch is marked to merge with another ref (e.g. local 'next' that
merges from and pushes back to origin's 'next', with 'branch.next.merge'
set to 'refs/heads/next'), it makes little sense to base the "branch -d"
safety, whose purpose is not to lose commits that are not merged to other
branches, on the current branch.  It is much more sensible to check if it
is merged with the other branch it merges with.

Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin-branch.c b/builtin-branch.c
index c87e63b02dd3bb0e12e5bf14aec22a0f219867b0..d2a35fe00d97bd95dc7a18740599e1165ecf46f1 100644 (file)
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -93,9 +93,60 @@ static const char *branch_get_color(enum color_branch ix)
        return "";
 }
 
+static int branch_merged(int kind, const char *name,
+                        struct commit *rev, struct commit *head_rev)
+{
+       /*
+        * 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;
+       int merged;
+
+       if (kind == REF_LOCAL_BRANCH) {
+               struct branch *branch = branch_get(name);
+               unsigned char sha1[20];
+
+               if (branch &&
+                   branch->merge &&
+                   branch->merge[0] &&
+                   branch->merge[0]->dst &&
+                   (reference_name =
+                    resolve_ref(branch->merge[0]->dst, sha1, 1, NULL)) != NULL)
+                       reference_rev = lookup_commit_reference(sha1);
+       }
+       if (!reference_rev)
+               reference_rev = head_rev;
+
+       merged = in_merge_bases(rev, &reference_rev, 1);
+
+       /*
+        * After the safety valve is fully redefined to "check with
+        * upstream, if any, otherwise with HEAD", we should just
+        * return the result of the in_merge_bases() above without
+        * any of the following code, but during the transition period,
+        * a gentle reminder is in order.
+        */
+       if ((head_rev != reference_rev) &&
+           in_merge_bases(rev, &head_rev, 1) != merged) {
+               if (merged)
+                       warning("deleting branch '%s' that has been merged to\n"
+                               "         '%s', but it is not yet merged to HEAD.",
+                               name, reference_name);
+               else
+                       warning("not deleting branch '%s' that is not yet merged to\n"
+                               "         '%s', even though it is merged to HEAD.",
+                               name, reference_name);
+       }
+       return merged;
+}
+
 static int delete_branches(int argc, const char **argv, int force, int kinds)
 {
-       struct commit *rev, *head_rev = head_rev;
+       struct commit *rev, *head_rev = NULL;
        unsigned char sha1[20];
        char *name = NULL;
        const char *fmt, *remote;
@@ -148,15 +199,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds)
                        continue;
                }
 
-               /* This checks whether the merge bases of branch and
-                * HEAD contains branch -- which means that the HEAD
-                * contains everything in both.
-                */
-
-               if (!force &&
-                   !in_merge_bases(rev, &head_rev, 1)) {
-                       error("The branch '%s' is not an ancestor of "
-                             "your current HEAD.\n"
+               if (!force && !branch_merged(kinds, bname.buf, rev, head_rev)) {
+                       error("The branch '%s' is not fully merged.\n"
                              "If you are sure you want to delete it, "
                              "run 'git branch -D %s'.", bname.buf, bname.buf);
                        ret = 1;

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index d59a9b4aef5fc53e42ec95a3b96a18fb67aa82cb..e0b760513cfc065126cecd6e273180826c8f6bc9 100755 (executable)
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -468,4 +468,30 @@ test_expect_success 'detect misconfigured autosetuprebase (no value)' '
        git config --unset branch.autosetuprebase
 '
 
+test_expect_success 'attempt to delete a branch without base and unmerged to HEAD' '
+       git checkout my9 &&
+       git config --unset branch.my8.merge &&
+       test_must_fail git branch -d my8
+'
+
+test_expect_success 'attempt to delete a branch merged to its base' '
+       # we are on my9 which is the initial commit; traditionally
+       # we would not have allowed deleting my8 that is not merged
+       # to my9, but it is set to track master that already has my8
+       git config branch.my8.merge refs/heads/master &&
+       git branch -d my8
+'
+
+test_expect_success 'attempt to delete a branch merged to its base' '
+       git checkout master &&
+       echo Third >>A &&
+       git commit -m "Third commit" A &&
+       git branch -t my10 my9 &&
+       git branch -f my10 HEAD^ &&
+       # we are on master which is at the third commit, and my10
+       # is behind us, so traditionally we would have allowed deleting
+       # it; but my10 is set to track my9 that is further behind.
+       test_must_fail git branch -d my10
+'
+
 test_done