Re: [PATCH 4/4] refs: do not clobber dangling symrefs
- From
Toon Claes <toon@iotcl.com>
- Date
- Sep 22, 2025, 12:23 UTC
- Message-ID
- <20250922122332.584428-1-toon@iotcl.com>
- In-Reply-To
- <20250819192934.GD1059295@coredump.intra.peff.net>
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 <toon@iotcl.com> --- 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