[PATCH v2 6/7] update-ref: add support for symref-update

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

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

Add 'symref-update' to allow updates of symbolic refs in a transaction
via the 'git-update-ref' command. The 'symref-update' command takes in a
<new-ref>, which the <ref> will be updated to. If the <ref> doesn't
exist it will be created.

It also optionally takes either an <old-ref> or <old-oid>. If the
<old-ref> is provided, it checks to see if the <ref> ponints to the
<old-ref> before the update. If <old-oid> is provided it checks <ref> to
ensure that it is a regular ref and <old-oid> is the OID before the
update. This by extension also means that this when a zero <old-oid> is
provided, it ensures that the ref didn't exist before.

This command will also support deref mode, to ensure that we can update
dereferenced regular refs to symrefs.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 Documentation/git-update-ref.txt |   6 ++
 builtin/update-ref.c             |  49 +++++++++++
 refs.c                           |  24 ++----
 refs/files-backend.c             |  15 ++--
 refs/reftable-backend.c          |   7 +-
 t/t1400-update-ref.sh            | 143 +++++++++++++++++++++++++++++++
 6 files changed, 220 insertions(+), 24 deletions(-)

diff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt
index a5b1f42728..9710c9bc78 100644
--- a/Documentation/git-update-ref.txt
+++ b/Documentation/git-update-ref.txt
@@ -65,6 +65,7 @@ performs all modifications together.  Specify commands of the form:
 	create SP <ref> SP <new-oid> LF
 	delete SP <ref> [SP <old-oid>] LF
 	verify SP <ref> [SP <old-oid>] LF
+	symref-update SP <ref> SP <new-ref> [SP (<old-ref> | <old-oid>)] LF
 	symref-create SP <ref> SP <new-ref> LF
 	symref-delete SP <ref> [SP <old-ref>] LF
 	symref-verify SP <ref> [SP <old-ref>] LF
@@ -89,6 +90,7 @@ quoting:
 	create SP <ref> NUL <new-oid> NUL
 	delete SP <ref> NUL [<old-oid>] NUL
 	verify SP <ref> NUL [<old-oid>] NUL
+	symref-update SP <ref> NUL <new-ref> [NUL (<old-ref> | <old-oid>)] NUL
 	symref-create SP <ref> NUL <new-ref> NUL
 	symref-delete SP <ref> [NUL <old-ref>] NUL
 	symref-verify SP <ref> [NUL <old-ref>] NUL
@@ -123,6 +125,10 @@ verify::
 	Verify <ref> against <old-oid> but do not change it.  If
 	<old-oid> is zero or missing, the ref must not exist.
 
+symref-update::
+	Set <ref> to <new-ref> after verifying <old-ref> or <old-oid>,
+	if given. Can be used to delete or create symrefs too.
+
 symref-create::
 	Create symbolic ref <ref> with <new-ref> after verifying
 	it does not exist.  Can only be used in `no-deref` mode.
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 24556a28a8..809c1c7a76 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -238,6 +238,54 @@ static void parse_cmd_update(struct ref_transaction *transaction,
 	strbuf_release(&err);
 }
 
+static void parse_cmd_symref_update(struct ref_transaction *transaction,
+				    const char *next, const char *end)
+{
+	struct strbuf err = STRBUF_INIT;
+	char *refname, *new_ref, *old_ref;
+	struct object_id old_oid;
+	int have_old = 0;
+
+	refname = parse_refname(&next);
+	if (!refname)
+		die("symref-update: missing <ref>");
+
+	new_ref = parse_next_refname(&next);
+	if (!new_ref)
+		die("symref-update %s: missing <new-ref>", refname);
+	if (read_ref(new_ref, NULL))
+		die("symref-update %s: invalid <new-ref>", refname);
+
+	old_ref = parse_next_refname(&next);
+	/*
+	 * Since the user can also send in an old-oid, we try to parse
+	 * it as such too.
+	 */
+	if (old_ref && read_ref(old_ref, NULL)) {
+		if (!repo_get_oid(the_repository, old_ref, &old_oid)) {
+			old_ref = NULL;
+			have_old = 1;
+		} else
+			die("symref-update %s: invalid <old-ref> or <old-oid>", refname);
+	}
+
+	if (*next != line_termination)
+		die("symref-update %s: extra input: %s", refname, next);
+
+	update_flags |= create_reflog_flag | REF_SYMREF_UPDATE;
+	if (ref_transaction_update(transaction, refname, NULL,
+				   have_old ? &old_oid : NULL,
+				   new_ref, old_ref, update_flags,
+				   msg, &err))
+		die("%s", err.buf);
+
+	update_flags = default_flags;
+	free(refname);
+	free(old_ref);
+	free(new_ref);
+	strbuf_release(&err);
+}
+
 static void parse_cmd_create(struct ref_transaction *transaction,
 			     const char *next, const char *end)
 {
@@ -509,6 +557,7 @@ static const struct parse_cmd {
 	{ "create",        parse_cmd_create,        2, UPDATE_REFS_OPEN },
 	{ "delete",        parse_cmd_delete,        2, UPDATE_REFS_OPEN },
 	{ "verify",        parse_cmd_verify,        2, UPDATE_REFS_OPEN },
+	{ "symref-update", parse_cmd_symref_update, 3, UPDATE_REFS_OPEN },
 	{ "symref-create", parse_cmd_symref_create, 2, UPDATE_REFS_OPEN },
 	{ "symref-delete", parse_cmd_symref_delete, 2, UPDATE_REFS_OPEN },
 	{ "symref-verify", parse_cmd_symref_verify, 2, UPDATE_REFS_OPEN },
diff --git a/refs.c b/refs.c
index e62c0f4aca..31c09c3317 100644
--- a/refs.c
+++ b/refs.c
@@ -1246,21 +1246,15 @@ struct ref_update *ref_transaction_add_update(
 
 	update->flags = flags;
 
-	/*
-	 * The ref values are to be considered over the oid values when we're
-	 * doing symref operations.
-	 */
-	if (update->flags & REF_SYMREF_UPDATE) {
-		if (old_ref)
-			update->old_ref = xstrdup(old_ref);
-		if (new_ref)
-			update->new_ref = xstrdup(new_ref);
-	} else {
-		if (flags & REF_HAVE_NEW)
-			oidcpy(&update->new_oid, new_oid);
-		if (flags & REF_HAVE_OLD)
-			oidcpy(&update->old_oid, old_oid);
-	}
+	if (old_ref)
+		update->old_ref = xstrdup(old_ref);
+	if (new_ref)
+		update->new_ref = xstrdup(new_ref);
+	if (new_oid && flags & REF_HAVE_NEW)
+		oidcpy(&update->new_oid, new_oid);
+	if (old_oid && flags & REF_HAVE_OLD)
+		oidcpy(&update->old_oid, old_oid);
+
 	update->msg = normalize_reflog_message(msg);
 	return update;
 }
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 59d438878a..fb9886484c 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2386,7 +2386,7 @@ static int split_symref_update(struct ref_update *update,
 	new_update = ref_transaction_add_update(
 			transaction, referent, new_flags,
 			&update->new_oid, &update->old_oid,
-			NULL, NULL, update->msg);
+			update->new_ref, update->old_ref, update->msg);
 
 	new_update->parent_update = update;
 
@@ -2609,7 +2609,9 @@ static int lock_ref_for_update(struct files_ref_store *refs,
 		}
 	}
 
-	if (update->flags & REF_SYMREF_UPDATE && update->new_ref) {
+	if (update->flags & REF_SYMREF_UPDATE &&
+	    !(update->flags & REF_LOG_ONLY) &&
+	    update->new_ref) {
 		if (create_symref_lock(refs, lock, update->refname, update->new_ref)) {
 			ret = TRANSACTION_GENERIC_ERROR;
 			goto out;
@@ -2627,12 +2629,9 @@ static int lock_ref_for_update(struct files_ref_store *refs,
 		 * phase of the transaction only needs to commit the lock.
 		 */
 		update->flags |= REF_NEEDS_COMMIT;
-	}
-
-
-	if ((update->flags & REF_HAVE_NEW) &&
-	    !(update->flags & REF_DELETING) &&
-	    !(update->flags & REF_LOG_ONLY)) {
+	} else if ((update->flags & REF_HAVE_NEW) &&
+		   !(update->flags & REF_DELETING) &&
+		   !(update->flags & REF_LOG_ONLY)) {
 		if (!(update->type & REF_ISSYMREF) &&
 		    oideq(&lock->old_oid, &update->new_oid)) {
 			/*
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index 6d42838e15..4bc6a369c7 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -908,7 +908,7 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,
 				 */
 				new_update = ref_transaction_add_update(
 						transaction, referent.buf, new_flags,
-						&u->new_oid, &u->old_oid, NULL, NULL, u->msg);
+						&u->new_oid, &u->old_oid, u->new_ref, u->old_ref, u->msg);
 				new_update->parent_update = u;
 
 				/*
@@ -1106,6 +1106,11 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data
 			    should_write_log(&arg->refs->base, u->refname))) {
 			struct reftable_log_record *log;
 
+			if (u->flags & REF_SYMREF_UPDATE && u->new_ref)
+				if (!refs_resolve_ref_unsafe(&arg->refs->base, u->new_ref,
+				     RESOLVE_REF_READING, &u->new_oid, NULL))
+					goto done;
+
 			ALLOC_GROW(logs, logs_nr + 1, logs_alloc);
 			log = &logs[logs_nr++];
 			memset(log, 0, sizeof(*log));
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index f4e63fae6e..eeb3ee7952 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -1360,6 +1360,7 @@ test_expect_success 'fails with duplicate HEAD update' '
 '
 
 test_expect_success 'fails with duplicate ref update via symref' '
+	test_when_finished "git symbolic-ref -d refs/heads/symref2" &&
 	git branch target2 $A &&
 	git symbolic-ref refs/heads/symref2 refs/heads/target2 &&
 	cat >stdin <<-EOF &&
@@ -1812,6 +1813,148 @@ test_expect_success "stdin ${type} symref-create reflogs with --create-reflog" '
 	git reflog exists refs/heads/symref
 '
 
+test_expect_success "stdin ${type} fails symref-update with no ref" '
+	create_stdin_buf ${type} "symref-update " >stdin &&
+	test_must_fail git update-ref --stdin ${type} --no-deref <stdin 2>err &&
+	grep "fatal: symref-update: missing <ref>" err
+'
+
+test_expect_success "stdin ${type} fails symref-update with no new value" '
+	create_stdin_buf ${type} "symref-update refs/heads/symref" >stdin &&
+	test_must_fail git update-ref --stdin ${type} --no-deref <stdin 2>err &&
+	grep "fatal: symref-update refs/heads/symref: missing <new-ref>" err
+'
+
+test_expect_success "stdin ${type} fails symref-update with too many arguments" '
+	create_stdin_buf ${type} "symref-update refs/heads/symref" "$a" "$a" "$a" >stdin &&
+	test_must_fail git update-ref --stdin ${type} --no-deref <stdin 2>err &&
+	if test "$type" = "-z"
+	then
+		grep "fatal: unknown command: $a" err
+	else
+		grep "fatal: symref-update refs/heads/symref: extra input:  $a" err
+	fi
+'
+
+test_expect_success "stdin ${type} symref-update ref creates with zero old value" '
+	test_when_finished "git symbolic-ref -d refs/heads/symref" &&
+	create_stdin_buf ${type} "symref-update refs/heads/symref" "$a" "$Z" >stdin &&
+	git update-ref --stdin ${type} --no-deref <stdin &&
+	echo $a >expect &&
+	git symbolic-ref refs/heads/symref >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success "stdin ${type} symref-update ref creates with empty old value" '
+	test_when_finished "git symbolic-ref -d refs/heads/symref" &&
+	create_stdin_buf ${type} "symref-update refs/heads/symref" "$a" >stdin &&
+	git update-ref --stdin ${type} --no-deref <stdin &&
+	echo $a >expect &&
+	git symbolic-ref refs/heads/symref >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success "stdin ${type} symref-update ref fails with wrong old value" '
+	test_when_finished "git symbolic-ref -d refs/heads/symref" &&
+	git symbolic-ref refs/heads/symref $a &&
+	create_stdin_buf ${type} "symref-update refs/heads/symref" "$m" "$b" >stdin &&
+	test_must_fail git update-ref --stdin ${type} --no-deref <stdin 2>err &&
+	grep "fatal: symref-update refs/heads/symref: invalid <old-ref> or <old-oid>" err &&
+	test_must_fail git rev-parse --verify -q $c
+'
+
+test_expect_success "stdin ${type} symref-update ref works with right old value" '
+	test_when_finished "git symbolic-ref -d refs/heads/symref" &&
+	git symbolic-ref refs/heads/symref $a &&
+	create_stdin_buf ${type} "symref-update refs/heads/symref" "$m" "$a" >stdin &&
+	git update-ref --stdin ${type} --no-deref <stdin &&
+	echo $m >expect &&
+	git symbolic-ref refs/heads/symref >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success "stdin ${type} symref-update creates symref (with deref)" '
+	test_when_finished "git symbolic-ref -d refs/heads/symref" &&
+	create_stdin_buf ${type} "symref-update refs/heads/symref" "$a" >stdin &&
+	git update-ref --stdin ${type} <stdin &&
+	echo $a >expect &&
+	git symbolic-ref --no-recurse refs/heads/symref >actual &&
+	test_cmp expect actual &&
+	test-tool ref-store main for-each-reflog-ent refs/heads/symref >actual &&
+	grep "$Z $(git rev-parse $a)" actual
+'
+
+test_expect_success "stdin ${type} symref-update updates symref (with deref)" '
+	test_when_finished "git symbolic-ref -d refs/heads/symref" &&
+	test_when_finished "git update-ref -d --no-deref refs/heads/symref2" &&
+	git update-ref refs/heads/symref2 $a &&
+	git symbolic-ref --no-recurse refs/heads/symref refs/heads/symref2 &&
+	create_stdin_buf ${type} "symref-update refs/heads/symref" "$a" >stdin &&
+	git update-ref --stdin ${type} <stdin &&
+	echo $a >expect &&
+	git symbolic-ref --no-recurse refs/heads/symref2 >actual &&
+	test_cmp expect actual &&
+	echo refs/heads/symref2 >expect &&
+	git symbolic-ref --no-recurse refs/heads/symref >actual &&
+	test_cmp expect actual &&
+	test-tool ref-store main for-each-reflog-ent refs/heads/symref >actual &&
+	grep "$(git rev-parse $a) $(git rev-parse $a)" actual
+'
+
+test_expect_success "stdin ${type} symref-update regular ref" '
+	test_when_finished "git symbolic-ref -d --no-recurse refs/heads/regularref" &&
+	git update-ref --no-deref refs/heads/regularref $a &&
+	create_stdin_buf ${type} "symref-update refs/heads/regularref" "$a" >stdin &&
+	git update-ref --stdin ${type} <stdin &&
+	echo $a >expect &&
+	git symbolic-ref --no-recurse refs/heads/regularref >actual &&
+	test_cmp expect actual &&
+	test-tool ref-store main for-each-reflog-ent refs/heads/regularref >actual &&
+	grep "$(git rev-parse $a) $(git rev-parse $a)" actual
+'
+
+test_expect_success "stdin ${type} symref-update regular ref with correct old-oid" '
+	test_when_finished "git symbolic-ref -d --no-recurse refs/heads/regularref" &&
+	git update-ref --no-deref refs/heads/regularref $a &&
+	create_stdin_buf ${type} "symref-update refs/heads/regularref" "$a" "$(git rev-parse $a)" >stdin &&
+	git update-ref --stdin ${type} <stdin &&
+	echo $a >expect &&
+	git symbolic-ref --no-recurse refs/heads/regularref >actual &&
+	test_cmp expect actual &&
+	test-tool ref-store main for-each-reflog-ent refs/heads/regularref >actual &&
+	grep "$(git rev-parse $a) $(git rev-parse $a)" actual
+'
+
+test_expect_success "stdin ${type} symref-update regular ref fails with wrong old-oid" '
+	test_when_finished "git update-ref -d refs/heads/regularref" &&
+	git update-ref --no-deref refs/heads/regularref $a &&
+	create_stdin_buf ${type} "symref-update refs/heads/regularref" "$a" "$(git rev-parse refs/heads/target2)" >stdin &&
+	test_must_fail git update-ref --stdin ${type} <stdin 2>err &&
+	echo $(git rev-parse $a) >expect &&
+	git rev-parse refs/heads/regularref >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success "stdin ${type} symref-update with zero old-oid" '
+	test_when_finished "git symbolic-ref -d --no-recurse refs/heads/symref" &&
+	create_stdin_buf ${type} "symref-update refs/heads/symref" "$a" "$Z" >stdin &&
+	git update-ref --stdin ${type} <stdin 2>err &&
+	echo $a >expect &&
+	git symbolic-ref refs/heads/symref >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success "stdin ${type} symref-update ref with zero old-oid" '
+	test_when_finished "git symbolic-ref -d --no-recurse refs/heads/symref" &&
+	git symbolic-ref refs/heads/symref refs/heads/target2 &&
+	create_stdin_buf ${type} "symref-update refs/heads/symref" "$a" "$Z" >stdin &&
+	test_must_fail git update-ref --stdin ${type} <stdin 2>err &&
+	grep "fatal: cannot lock ref '"'"'refs/heads/symref'"'"': reference already exists" err &&
+	echo refs/heads/target2 >expect &&
+	git symbolic-ref refs/heads/symref >actual &&
+	test_cmp expect actual
+'
+
 done
 
 test_done
-- 
2.43.GIT