git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Johan Herland <johan@herland.net>
To: gitster@pobox.com
Cc: git@vger.kernel.org, rene.scharfe@lsrfire.ath.cx,
	vmiklos@suse.cz, Johan Herland <johan@herland.net>
Subject: [PATCH 2/5] delete_ref(): Fix deletion of refs through symbolic refs
Date: Tue, 16 Oct 2012 15:44:51 +0200	[thread overview]
Message-ID: <1350395094-11404-3-git-send-email-johan@herland.net> (raw)
In-Reply-To: <1350395094-11404-1-git-send-email-johan@herland.net>

When deleting a ref through a symref (e.g. using 'git update-ref -d HEAD'
to delete refs/heads/master), the packed version of that ref would not
be deleted, because delete_ref() would pass the symref name (as opposed
to the dereferenced ref name) to repack_without_ref().

This patch revamps the logic in delete_ref() to make it easier to read,
and to clarify how it operates when given a symref.

Signed-off-by: Johan Herland <johan@herland.net>
---
 refs.c                | 33 +++++++++++++++++----------------
 t/t1400-update-ref.sh |  2 +-
 2 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/refs.c b/refs.c
index da74a2b..df4fe20 100644
--- a/refs.c
+++ b/refs.c
@@ -1751,34 +1751,35 @@ static int repack_without_ref(const char *refname)
 int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
 {
 	struct ref_lock *lock;
-	int err, i = 0, ret = 0, flag = 0;
+	int err, delete_symref, ret = 0, flag = 0;
 
 	lock = lock_ref_sha1_basic(refname, sha1, 0, &flag);
 	if (!lock)
 		return 1;
-	if (!(flag & REF_ISPACKED) || flag & REF_ISSYMREF) {
-		/* loose */
-		const char *path;
 
-		if (!(delopt & REF_NODEREF)) {
-			i = strlen(lock->lk->filename) - 5; /* .lock */
-			lock->lk->filename[i] = 0;
-			path = lock->lk->filename;
-		} else {
-			path = git_path("%s", refname);
-		}
-		err = unlink_or_warn(path);
+	/* The following variables are at play here:
+	 *  - refname may be a symref (in this case lock->orig_ref_name holds
+	 *    the symref name, and lock->ref_name holds the dereferenced name)
+	 *  - The dereferenced ref name (lock->ref_name) may be a loose ref, a
+	 *    packed ref, or both.
+	 *  - If REF_NODEREF is enabled - and refname is a symref, we should
+	 *    delete the symref; otherwise delete the dereferenced ref.
+	 */
+	delete_symref = (flag & REF_ISSYMREF && delopt & REF_NODEREF);
+	refname = delete_symref ? lock->orig_ref_name : lock->ref_name;
+
+	if (!(flag & REF_ISPACKED) || delete_symref) {
+		/* loose */
+		err = unlink_or_warn(git_path("%s", refname));
 		if (err && errno != ENOENT)
 			ret = 1;
-
-		if (!(delopt & REF_NODEREF))
-			lock->lk->filename[i] = '.';
 	}
 	/* removing the loose one could have resurrected an earlier
 	 * packed one.  Also, if it was not loose we need to repack
 	 * without it.
 	 */
-	ret |= repack_without_ref(refname);
+	if (!delete_symref)
+		ret |= repack_without_ref(refname);
 
 	unlink_or_warn(git_path("logs/%s", lock->ref_name));
 	invalidate_ref_cache(NULL);
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index f7ec203..e415ee0 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -85,7 +85,7 @@ test_expect_success \
 	"move $m (by HEAD)" \
 	"git update-ref HEAD $B $A &&
 	 test $B"' = $(cat .git/'"$m"')'
-test_expect_failure "delete $m (by HEAD) should remove both packed and loose $m" '
+test_expect_success "delete $m (by HEAD) should remove both packed and loose $m" '
 	git update-ref -d HEAD $B &&
 	! grep "$m" .git/packed-refs &&
 	! test -f .git/$m
-- 
1.7.12.1.609.g5cd6968

  parent reply	other threads:[~2012-10-16 13:45 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CAPc5daUws-MfzC9imkytTrLaHyyywE4_OX1jAUVPCTK2WyUF=w@mail.gmail.com>
2012-10-16 13:44 ` [PATCH 0/5] Fixing problem with deleting symrefs Johan Herland
2012-10-16 13:44   ` [PATCH 1/5] t1400-update-ref: Add test verifying bug with symrefs in delete_ref() Johan Herland
2012-10-16 13:44   ` Johan Herland [this message]
2012-10-16 13:44   ` [PATCH 3/5] delete_ref(): Remove the correct reflog when deleting symrefs Johan Herland
2012-10-16 13:44   ` [PATCH 4/5] Add tests for using symbolic refs as branch name aliases Johan Herland
2012-10-16 13:44   ` [PATCH 5/5] branch -d: Fix failure to remove branch aliases (symrefs) Johan Herland
2012-10-16 16:54   ` [PATCH 0/5] Fixing problem with deleting symrefs Junio C Hamano

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=1350395094-11404-3-git-send-email-johan@herland.net \
    --to=johan@herland.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=rene.scharfe@lsrfire.ath.cx \
    --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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).