git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "René Scharfe" <rene.scharfe@lsrfire.ath.cx>
To: Git mailing list <git@vger.kernel.org>
Cc: Junio C Hamano <gitster@pobox.com>,
	Johan Herland <johan@herland.net>, Miklos Vajna <vmiklos@suse.cz>
Subject: [PATCH 4/5] branch: skip commit checks when deleting symref branches
Date: Thu, 18 Oct 2012 14:07:11 +0200	[thread overview]
Message-ID: <507FF0EF.3070702@lsrfire.ath.cx> (raw)
In-Reply-To: <507FEF0B.1060309@lsrfire.ath.cx>

Before a branch is deleted, we check that it points to a valid
commit.  With -d we also check that the commit is a merged; this
check is not done with -D.

The reason for that is that commits pointed to by branches should
never go missing; if they do then something broke and it's better
to stop instead of adding to the mess.  And a non-merged commit
may contain changes that are worth preserving, so we require the
stronger option -D instead of -d to get rid of them.

If a branch consists of a symref, these concerns don't apply.
Deleting such a branch can't make a commit become unreferenced,
so we don't need to check if it is merged, or even if it is
actually a valid commit.  Skip them in that case.  This allows
us to delete dangling symref branches.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 builtin/branch.c  | 10 ++++++++--
 t/t3200-branch.sh |  9 +++++++++
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 5e1e5b4..d87035a 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -214,6 +214,9 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
 			die(_("Couldn't look up commit object for HEAD"));
 	}
 	for (i = 0; i < argc; i++, strbuf_release(&bname)) {
+		const char *target;
+		int flags = 0;
+
 		strbuf_branchname(&bname, argv[i]);
 		if (kinds == REF_LOCAL_BRANCH && !strcmp(head, bname.buf)) {
 			error(_("Cannot delete the branch '%s' "
@@ -225,7 +228,9 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
 		free(name);
 
 		name = mkpathdup(fmt, bname.buf);
-		if (read_ref(name, sha1)) {
+		target = resolve_ref_unsafe(name, sha1, 0, &flags);
+		if (!target ||
+		    (!(flags & REF_ISSYMREF) && is_null_sha1(sha1))) {
 			error(remote_branch
 			      ? _("remote branch '%s' not found.")
 			      : _("branch '%s' not found."), bname.buf);
@@ -233,7 +238,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
 			continue;
 		}
 
-		if (check_branch_commit(bname.buf, name, sha1, head_rev, kinds,
+		if (!(flags & REF_ISSYMREF) &&
+		    check_branch_commit(bname.buf, name, sha1, head_rev, kinds,
 					force)) {
 			ret = 1;
 			continue;
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index ec5f70e..1323f6f 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -273,6 +273,15 @@ test_expect_success 'deleting a symref' '
 	test_i18ncmp expect actual
 '
 
+test_expect_success 'deleting a dangling symref' '
+	git symbolic-ref refs/heads/dangling-symref nowhere &&
+	test_path_is_file .git/refs/heads/dangling-symref &&
+	echo "Deleted branch dangling-symref (was 0000000)." >expect &&
+	git branch -d dangling-symref >actual &&
+	test_path_is_missing .git/refs/heads/dangling-symref &&
+	test_i18ncmp expect actual
+'
+
 test_expect_success 'renaming a symref is not allowed' \
 '
 	git symbolic-ref refs/heads/master2 refs/heads/master &&
-- 
1.7.12

  parent reply	other threads:[~2012-10-18 12:07 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-15  8:50 BUG when trying to delete symbolic refs Johan Herland
2012-10-16 10:05 ` René Scharfe
2012-10-16 10:22   ` [PATCH] refs: lock symref that is to be deleted, not its target René Scharfe
2012-10-16 16:09   ` BUG when trying to delete symbolic refs Junio C Hamano
2012-10-18 11:59     ` René Scharfe
2012-10-18 12:02       ` [PATCH 1/5] branch: factor out check_branch_commit() René Scharfe
2012-10-18 12:04       ` [PATCH 2/5] branch: factor out delete_branch_config() René Scharfe
2012-10-18 12:05       ` [PATCH 3/5] branch: delete symref branch, not its target René Scharfe
2012-10-18 12:07       ` René Scharfe [this message]
2012-10-18 21:34         ` [PATCH 4/5] branch: skip commit checks when deleting symref branches Junio C Hamano
2012-10-21 10:40           ` [PATCH 0/2] Fix remaining issue when deleting symrefs Johan Herland
2012-10-21 10:40             ` [PATCH 1/2] t1400-update-ref: Add test verifying bug with symrefs in delete_ref() Johan Herland
2012-10-21 10:40             ` [PATCH 2/2] Fix failure to delete a packed ref through a symref Johan Herland
2012-10-21 17:46               ` René Scharfe
2012-10-21 19:09               ` Junio C Hamano
2012-10-18 12:08       ` [PATCH 5/5] branch: show targets of deleted symrefs, not sha1s René Scharfe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=507FF0EF.3070702@lsrfire.ath.cx \
    --to=rene.scharfe@lsrfire.ath.cx \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johan@herland.net \
    --cc=vmiklos@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).