Subject: [PATCH v4 2/7] refs: specify error for regular refs with `old_target`

From: Karthik Nayak <[email-masked]>
Date: Wed, 5 Jun 2024 12:29:53 +0200

From: Karthik Nayak <[email-masked]>

When a regular reference update contains `old_target` set, we call the
`ref_update_check_old_target` function to check the referent value. But
for regular refs we know that the referent value is not set and this
simply raises a generic error which says nothing about this being a
regular ref. Instead let's raise a more specific error when a regular
ref update contains `old_target`.

Signed-off-by: Karthik Nayak <[email-masked]>
---
 refs/files-backend.c    | 13 +++++++------
 refs/reftable-backend.c |  9 +++++++++
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 194e74eb4d..f516d4d82f 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2491,14 +2491,15 @@ static int lock_ref_for_update(struct files_ref_store *refs,
 
 		/*
 		 * Even if the ref is a regular ref, if `old_target` is set, we
-		 * check the referent value. Ideally `old_target` should only
-		 * be set for symrefs, but we're strict about its usage.
+		 * fail with an error.
 		 */
 		if (update->old_target) {
-			if (ref_update_check_old_target(referent.buf, update, err)) {
-				ret = TRANSACTION_GENERIC_ERROR;
-				goto out;
-			}
+			strbuf_addf(err, _("cannot update regular ref: '%s': "
+					   "symref target '%s' set"),
+				    ref_update_original_update_refname(update),
+				    update->old_target);
+			ret = TRANSACTION_GENERIC_ERROR;
+			goto out;
 		} else if  (check_old_oid(update, &lock->old_oid, err)) {
 			ret = TRANSACTION_GENERIC_ERROR;
 			goto out;
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index b838cf8f00..bd89ce8d76 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -928,6 +928,15 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,
 		 * backend returns, which keeps our tests happy.
 		 */
 		if (u->old_target) {
+			if (!(u->type & REF_ISSYMREF)) {
+				strbuf_addf(err, _("cannot update regular ref: '%s': "
+					    "symref target '%s' set"),
+					    ref_update_original_update_refname(u),
+					    u->old_target);
+				ret = -1;
+				goto done;
+			}
+
 			if (ref_update_check_old_target(referent.buf, u, err)) {
 				ret = -1;
 				goto done;
-- 
2.43.GIT