Subject: Re: [PATCH 4/4] refs: do not clobber dangling symrefs

From: Toon Claes <[email-masked]>
Date: Mon, 22 Sep 2025 14:23:32 +0200

Hi Peff,

At $DAYJOB we hit into an edge-case where this patch breaks our expectancies.

We use `update FOO_HEAD 000...000 000..000` to delete a symref, if that symref
is dangling (otherwise the old oid would have resolved to something). I've
attached a patch that would allow this (on top of your patches). Do you think it
makes sense to allow this scenario?

--
Cheers,
Toon

--- >8 ---
Subject: [PATCH] refs: allow deleting dangling symrefs by updating to zero oid

In 450fc2bace (refs: do not clobber dangling symrefs, 2025-08-19) we
changed how dangling symrefs are dealt with. This guards us from
creating a symref, while it already exists.

But this breaks behavior when you want to delete such dangling symref.
When you're aware your symref is dangling, you know the old oid resolves
to the null oid, and thus you can pass that together with the null oid
the new oid to delete the symref. Thus when the new oid is the null oid,
continue the ref update as before the change mentioned earlier.

Signed-off-by: Toon Claes <[email-masked]>
---
 refs/files-backend.c    | 2 +-
 refs/reftable-backend.c | 2 +-
 t/t1400-update-ref.sh   | 9 +++++++++
 3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 1b3bf26add..5e46d3a110 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2537,7 +2537,7 @@ static enum ref_transaction_error check_old_oid(struct ref_update *update,
 		 * that case to preserve the dangling symref.
 		 */
 		if ((update->flags & REF_NO_DEREF) && referent->len &&
-		    is_null_oid(oid)) {
+		    is_null_oid(oid) && !is_null_oid(&update->new_oid)) {
 			strbuf_addf(err, "cannot lock ref '%s': "
 				    "dangling symref already exists",
 				    ref_update_original_update_refname(update));
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index 9e889da2ff..ed505f6054 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -1294,7 +1294,7 @@ static enum ref_transaction_error prepare_single_update(struct reftable_ref_stor
 			 */
 			if ((u->flags & REF_NO_DEREF) &&
 			    referent->len &&
-			    is_null_oid(&u->old_oid)) {
+			    is_null_oid(&u->old_oid) && !is_null_oid(&u->new_oid)) {
 				strbuf_addf(err, _("cannot lock ref '%s': "
 					    "dangling symref already exists"),
 					    ref_update_original_update_refname(u));
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index b7415ec9d5..85cd9da0af 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -2389,4 +2389,13 @@ test_expect_success 'dangling symref overwritten without old oid' '
 	test_must_fail git rev-parse --verify refs/heads/does-not-exist
 '

+test_expect_success 'dangling symref delete with old oid zero' '
+	test_when_finished "git update-ref -d refs/heads/dangling" &&
+	git symbolic-ref refs/heads/dangling refs/heads/does-not-exist &&
+	echo "update refs/heads/dangling $Z $Z" >stdin &&
+	git update-ref --no-deref --stdin <stdin &&
+	test_must_fail git rev-parse --verify refs/heads/dangling &&
+	test_must_fail git rev-parse --verify refs/heads/does-not-exist
+'
+
 test_done
--
2.51.0