[PATCH v3 5/8] update-ref: support symrefs in the delete command

[Karthik Nayak <karthik.188@gmail.com>]

From: Karthik Nayak <karthik.188@gmail.com>

The 'delete' command in 'git-update-ref' allows users to delete `<ref>`
after verifying it exists with `<old-oid>`, if given. Extend this command
to alternatively take in `ref:<old-target>` which is used to verify if
the symbolic ref targets the provided `<old-target>` before deletion.
This will only work when used with the 'no-deref' mode as it doesn't
make sense to deref a symref during deletion.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 Documentation/git-update-ref.txt | 10 +++++---
 builtin/fetch.c                  |  2 +-
 builtin/receive-pack.c           |  3 ++-
 builtin/update-ref.c             | 18 +++++++++----
 refs.c                           | 12 ++++++---
 refs.h                           |  4 ++-
 refs/files-backend.c             |  2 +-
 refs/reftable-backend.c          |  2 +-
 t/t1400-update-ref.sh            | 44 ++++++++++++++++++++++++++++++++
 9 files changed, 79 insertions(+), 18 deletions(-)

diff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt
index 9f8c059944..f28b026cd7 100644
--- a/Documentation/git-update-ref.txt
+++ b/Documentation/git-update-ref.txt
@@ -63,7 +63,7 @@ performs all modifications together.  Specify commands of the form:
 
 	update SP <ref> SP <new-oid> [SP <old-oid>] LF
 	create SP <ref> SP <new-oid> LF
-	delete SP <ref> [SP <old-oid>] LF
+	delete SP <ref> [SP (<old-oid> | ref:<old-target>)] LF
 	verify SP <ref> [SP (<old-oid> | ref:<old-target>)] LF
 	option SP <opt> LF
 	start LF
@@ -84,7 +84,7 @@ quoting:
 
 	update SP <ref> NUL <new-oid> NUL [<old-oid>] NUL
 	create SP <ref> NUL <new-oid> NUL
-	delete SP <ref> NUL [<old-oid>] NUL
+	delete SP <ref> NUL [(<old-oid> | ref:<old-target>)] NUL
 	verify SP <ref> NUL [(<old-oid> | ref:<old-target>)] NUL
 	option SP <opt> NUL
 	start NUL
@@ -116,8 +116,10 @@ create::
 	exist.  The given <new-oid> may not be zero.
 
 delete::
-	Delete <ref> after verifying it exists with <old-oid>, if
-	given.  If given, <old-oid> may not be zero.
+	Delete <ref> after verifying it exists with <old-oid>, if given.
+	If given, <old-oid> may not be zero.  If instead, ref:<old-target>
+	is provided, verify that the symbolic ref <ref> targets
+	<old-target> before deleting it.
 
 verify::
 	Verify <ref> against <old-oid> but do not change it.  If
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 66840b7c5b..d02592efca 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1383,7 +1383,7 @@ static int prune_refs(struct display_state *display_state,
 		if (transaction) {
 			for (ref = stale_refs; ref; ref = ref->next) {
 				result = ref_transaction_delete(transaction, ref->name, NULL, 0,
-								"fetch: prune", &err);
+								NULL, "fetch: prune", &err);
 				if (result)
 					goto cleanup;
 			}
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index b150ef39a8..9a4667d57d 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1576,7 +1576,8 @@ static const char *update(struct command *cmd, struct shallow_info *si)
 		if (ref_transaction_delete(transaction,
 					   namespaced_name,
 					   old_oid,
-					   0, "push", &err)) {
+					   0, NULL,
+					   "push", &err)) {
 			rp_error("%s", err.buf);
 			ret = "failed to delete";
 		} else {
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 246167e835..cee7a5ebc0 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -274,6 +274,7 @@ static void parse_cmd_delete(struct ref_transaction *transaction,
 			     const char *next, const char *end)
 {
 	struct strbuf err = STRBUF_INIT;
+	struct strbuf old_target = STRBUF_INIT;
 	char *refname;
 	struct object_id old_oid;
 	int have_old;
@@ -282,26 +283,33 @@ static void parse_cmd_delete(struct ref_transaction *transaction,
 	if (!refname)
 		die("delete: missing <ref>");
 
-	if (parse_next_arg(&next, end, &old_oid, NULL,
-			   "delete", refname, PARSE_SHA1_OLD)) {
+	if (parse_next_arg(&next, end, &old_oid, &old_target,
+			   "delete", refname, PARSE_SHA1_OLD |
+			   PARSE_REFNAME_TARGETS)) {
 		have_old = 0;
 	} else {
-		if (is_null_oid(&old_oid))
+		if (!old_target.len && is_null_oid(&old_oid))
 			die("delete %s: zero <old-oid>", refname);
-		have_old = 1;
+		have_old = 1 && !old_target.len;
 	}
 
+	if (old_target.len && !(update_flags & REF_NO_DEREF))
+		die("delete %s: cannot operate on symrefs in deref mode", refname);
+
 	if (*next != line_termination)
 		die("delete %s: extra input: %s", refname, next);
 
 	if (ref_transaction_delete(transaction, refname,
 				   have_old ? &old_oid : NULL,
-				   update_flags, msg, &err))
+				   update_flags,
+				   old_target.len ? old_target.buf : NULL,
+				   msg, &err))
 		die("%s", err.buf);
 
 	update_flags = default_flags;
 	free(refname);
 	strbuf_release(&err);
+	strbuf_release(&old_target);
 }
 
 static void parse_cmd_verify(struct ref_transaction *transaction,
diff --git a/refs.c b/refs.c
index 0e1013b5ab..6b7c46bfd8 100644
--- a/refs.c
+++ b/refs.c
@@ -979,7 +979,7 @@ int refs_delete_ref(struct ref_store *refs, const char *msg,
 	transaction = ref_store_transaction_begin(refs, &err);
 	if (!transaction ||
 	    ref_transaction_delete(transaction, refname, old_oid,
-				   flags, msg, &err) ||
+				   flags, NULL, msg, &err) ||
 	    ref_transaction_commit(transaction, &err)) {
 		error("%s", err.buf);
 		ref_transaction_free(transaction);
@@ -1318,14 +1318,18 @@ int ref_transaction_create(struct ref_transaction *transaction,
 int ref_transaction_delete(struct ref_transaction *transaction,
 			   const char *refname,
 			   const struct object_id *old_oid,
-			   unsigned int flags, const char *msg,
+			   unsigned int flags,
+			   const char *old_target,
+			   const char *msg,
 			   struct strbuf *err)
 {
 	if (old_oid && is_null_oid(old_oid))
 		BUG("delete called with old_oid set to zeros");
+	if (old_target && !(flags & REF_NO_DEREF))
+		BUG("delete cannot operate on symrefs with deref mode");
 	return ref_transaction_update(transaction, refname,
 				      null_oid(), old_oid,
-				      NULL, NULL, flags,
+				      NULL, old_target, flags,
 				      msg, err);
 }
 
@@ -2752,7 +2756,7 @@ int refs_delete_refs(struct ref_store *refs, const char *logmsg,
 
 	for_each_string_list_item(item, refnames) {
 		ret = ref_transaction_delete(transaction, item->string,
-					     NULL, flags, msg, &err);
+					     NULL, flags, NULL, msg, &err);
 		if (ret) {
 			warning(_("could not delete reference %s: %s"),
 				item->string, err.buf);
diff --git a/refs.h b/refs.h
index 27b9aeaf54..4be4930f04 100644
--- a/refs.h
+++ b/refs.h
@@ -766,7 +766,9 @@ int ref_transaction_create(struct ref_transaction *transaction,
 int ref_transaction_delete(struct ref_transaction *transaction,
 			   const char *refname,
 			   const struct object_id *old_oid,
-			   unsigned int flags, const char *msg,
+			   unsigned int flags,
+			   const char *old_target,
+			   const char *msg,
 			   struct strbuf *err);
 
 /*
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 53197fa3af..fc5037fe5a 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2516,7 +2516,7 @@ static int lock_ref_for_update(struct files_ref_store *refs,
 
 	files_assert_main_repository(refs, "lock_ref_for_update");
 
-	if ((update->flags & REF_HAVE_NEW) && is_null_oid(&update->new_oid))
+	if ((update->flags & REF_HAVE_NEW) && ref_update_is_null_new_value(update))
 		update->flags |= REF_DELETING;
 
 	if (head_ref) {
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index a2474245aa..2b2cbca8c0 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -1120,7 +1120,7 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data
 		if (u->flags & REF_LOG_ONLY)
 			continue;
 
-		if (u->flags & REF_HAVE_NEW && is_null_oid(&u->new_oid)) {
+		if (u->flags & REF_HAVE_NEW && ref_update_is_null_new_value(u)) {
 			struct reftable_ref_record ref = {
 				.refname = (char *)u->refname,
 				.update_index = ts,
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 1f2b63755a..cd1ad0d2ec 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -1714,6 +1714,50 @@ do
 		test_cmp expect actual
 	'
 
+	test_expect_success "stdin ${type} delete symref fails without --no-deref" '
+		git symbolic-ref refs/heads/symref $a &&
+		create_stdin_buf ${type} "delete refs/heads/symref" "ref:$a" &&
+		test_must_fail git update-ref --stdin ${type} <stdin 2>err &&
+		grep "fatal: delete refs/heads/symref: cannot operate on symrefs in deref mode" err
+	'
+
+	test_expect_success "stdin ${type} delete symref fails with no ref" '
+		create_stdin_buf ${type} "delete " &&
+		test_must_fail git update-ref --stdin ${type} --no-deref <stdin 2>err &&
+		grep "fatal: delete: missing <ref>" err
+	'
+
+	test_expect_success "stdin ${type} delete symref fails with too many arguments" '
+		create_stdin_buf ${type} "delete refs/heads/symref" "ref:$a" "ref:$a" &&
+		test_must_fail git update-ref --stdin ${type} --no-deref <stdin 2>err &&
+		if test "$type" = "-z"
+		then
+			grep "fatal: unknown command: ref:$a" err
+		else
+			grep "fatal: delete refs/heads/symref: extra input:  ref:$a" err
+		fi
+	'
+
+	test_expect_success "stdin ${type} delete symref fails with wrong old value" '
+		create_stdin_buf ${type} "delete refs/heads/symref" "ref:$m" &&
+		test_must_fail git update-ref --stdin ${type} --no-deref <stdin 2>err &&
+		if test_have_prereq REFTABLE
+		then
+			grep "fatal: verifying symref target: ${SQ}refs/heads/symref${SQ}: is at $a but expected refs/heads/main" err
+		else
+			grep "fatal: cannot lock ref ${SQ}refs/heads/symref${SQ}" err
+		fi &&
+		git symbolic-ref refs/heads/symref >expect &&
+		echo $a >actual &&
+		test_cmp expect actual
+	'
+
+	test_expect_success "stdin ${type} delete symref works with right old value" '
+		create_stdin_buf ${type} "delete refs/heads/symref" "ref:$a" &&
+		git update-ref --stdin ${type} --no-deref <stdin &&
+		test_must_fail git rev-parse --verify -q $b
+	'
+
 done
 
 test_done
-- 
2.43.GIT