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

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

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

The 'update' command in 'git-update-ref' allows users to set `<ref>` to
`<new-oid>` after verifying `<old-oid>`, if given. Extend this command
to alternatively take in `ref:<new-target>` which is used to update to/a
symref with `<new-target>` as its target. And take in `ref:<old-target>`
which is used to verify that the symref points to `<old-target>` before
the update.

With this the 'update' command can also now be used to:
- create symrefs
- change a regular ref to a symref
- change a symref to a regular ref

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 |  10 +-
 builtin/update-ref.c             |  21 ++--
 refs/files-backend.c             |  14 ++-
 refs/reftable-backend.c          |   3 +-
 t/t1400-update-ref.sh            | 168 +++++++++++++++++++++++++++++++
 5 files changed, 198 insertions(+), 18 deletions(-)

diff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt
index 1202769178..79e29fead6 100644
--- a/Documentation/git-update-ref.txt
+++ b/Documentation/git-update-ref.txt
@@ -61,7 +61,7 @@ still contains <old-oid>.
 With `--stdin`, update-ref reads instructions from standard input and
 performs all modifications together.  Specify commands of the form:
 
-	update SP <ref> SP <new-oid> [SP <old-oid>] LF
+	update SP <ref> SP (<new-oid> | ref:<new-target>) [SP (<old-oid> | ref:<old-target>)] LF
 	create SP <ref> SP (<new-oid> | ref:<new-target>) LF
 	delete SP <ref> [SP (<old-oid> | ref:<old-target>)] LF
 	verify SP <ref> [SP (<old-oid> | ref:<old-target>)] LF
@@ -82,7 +82,7 @@ specify a missing value, omit the value and its preceding SP entirely.
 Alternatively, use `-z` to specify in NUL-terminated format, without
 quoting:
 
-	update SP <ref> NUL <new-oid> NUL [<old-oid>] NUL
+	update SP <ref> NUL (<new-oid> | ref:<new-target>) NUL [(<old-oid> | ref:<old-target>)] NUL
 	create SP <ref> NUL (<new-oid> | ref:<new-target>) NUL
 	delete SP <ref> NUL [(<old-oid> | ref:<old-target>)] NUL
 	verify SP <ref> NUL [(<old-oid> | ref:<old-target>)] NUL
@@ -109,7 +109,11 @@ update::
 	Set <ref> to <new-oid> after verifying <old-oid>, if given.
 	Specify a zero <new-oid> to ensure the ref does not exist
 	after the update and/or a zero <old-oid> to make sure the
-	ref does not exist before the update.
+	ref does not exist before the update.  If ref:<old-target>
+	is provided, we verify that the <ref> is an existing symbolic
+	ref which targets <old-target>.  If ref:<new-target> is given,
+	the update ensures <ref> is a symbolic ref which targets
+	<new-target>.
 
 create::
 	Create <ref> with <new-oid> after verifying it does not
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index afab706cd7..175579148f 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -210,6 +210,8 @@ static void parse_cmd_update(struct ref_transaction *transaction,
 			     const char *next, const char *end)
 {
 	struct strbuf err = STRBUF_INIT;
+	struct strbuf new_target = STRBUF_INIT;
+	struct strbuf old_target = STRBUF_INIT;
 	char *refname;
 	struct object_id new_oid, old_oid;
 	int have_old;
@@ -218,19 +220,24 @@ static void parse_cmd_update(struct ref_transaction *transaction,
 	if (!refname)
 		die("update: missing <ref>");
 
-	if (parse_next_arg(&next, end, &new_oid, NULL,
-			   "update", refname, PARSE_SHA1_ALLOW_EMPTY))
+	if (parse_next_arg(&next, end, &new_oid,
+			   &new_target, "update", refname,
+			   PARSE_SHA1_ALLOW_EMPTY | PARSE_REFNAME_TARGETS))
 		die("update %s: missing <new-oid>", refname);
 
-	have_old = !parse_next_arg(&next, end, &old_oid, NULL,
-				   "update", refname, PARSE_SHA1_OLD);
+	have_old = !parse_next_arg(&next, end, &old_oid,
+				   &old_target, "update", refname,
+				   PARSE_SHA1_OLD | PARSE_REFNAME_TARGETS);
+	have_old = have_old & !old_target.len;
 
 	if (*next != line_termination)
 		die("update %s: extra input: %s", refname, next);
 
 	if (ref_transaction_update(transaction, refname,
-				   &new_oid, have_old ? &old_oid : NULL,
-				   NULL, NULL,
+				   new_target.len ? NULL : &new_oid,
+				   have_old ? &old_oid : NULL,
+				   new_target.len ? new_target.buf : NULL,
+				   old_target.len ? old_target.buf : NULL,
 				   update_flags | create_reflog_flag,
 				   msg, &err))
 		die("%s", err.buf);
@@ -238,6 +245,8 @@ static void parse_cmd_update(struct ref_transaction *transaction,
 	update_flags = default_flags;
 	free(refname);
 	strbuf_release(&err);
+	strbuf_release(&old_target);
+	strbuf_release(&new_target);
 }
 
 static void parse_cmd_create(struct ref_transaction *transaction,
diff --git a/refs/files-backend.c b/refs/files-backend.c
index f5e271a442..59d1ab3eeb 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2386,7 +2386,8 @@ 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_target, update->old_target,
+			update->msg);
 
 	new_update->parent_update = update;
 
@@ -2610,7 +2611,7 @@ static int lock_ref_for_update(struct files_ref_store *refs,
 		}
 	}
 
-	if (update->new_target) {
+	if (update->new_target && !(update->flags & REF_LOG_ONLY)) {
 		if (create_symref_lock(refs, lock, update->refname, update->new_target)) {
 			ret = TRANSACTION_GENERIC_ERROR;
 			goto out;
@@ -2628,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 e203c697f2..a00f55802a 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -908,7 +908,8 @@ 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_target,
+						u->old_target, u->msg);
 				new_update->parent_update = u;
 
 				/*
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index e85d08ce5c..5b2d23da37 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 &&
@@ -1813,6 +1814,173 @@ do
 		git reflog exists refs/heads/symref
 	'
 
+	test_expect_success "stdin ${type} update symref fails with too many arguments" '
+		create_stdin_buf ${type} "update refs/heads/symref" "ref:$a" "ref:$a" "ref:$a" >stdin &&
+		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: update refs/heads/symref: extra input:  ref:$a" err
+		fi
+	'
+
+	test_expect_success "stdin ${type} update creates symref with zero old value" '
+		test_when_finished "git symbolic-ref -d refs/heads/symref" &&
+		create_stdin_buf ${type} "update refs/heads/symref" "ref:$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} update creates symref with empty old value" '
+		test_when_finished "git symbolic-ref -d refs/heads/symref" &&
+		create_stdin_buf ${type} "update refs/heads/symref" "ref:$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} update symref 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} "update refs/heads/symref" "ref:$m" "ref:$b" >stdin &&
+		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 $b" err
+		else
+			grep "fatal: cannot lock ref ${SQ}refs/heads/symref${SQ}: is at $a but expected $b" err
+		fi &&
+		test_must_fail git rev-parse --verify -q $c
+	'
+
+	test_expect_success "stdin ${type} update symref 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} "update refs/heads/symref" "ref:$m" "ref:$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} update creates symref (with deref)" '
+		test_when_finished "git symbolic-ref -d refs/heads/symref" &&
+		create_stdin_buf ${type} "update refs/heads/symref" "ref:$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} update regular ref to symref 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} "update refs/heads/regularref" "ref:$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} update regular ref to symref 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} "update refs/heads/regularref" "ref:$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} update existing symref 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} "update refs/heads/symref" "ref:$a" "$Z" >stdin &&
+		test_must_fail git update-ref --stdin ${type} <stdin 2>err &&
+		grep "fatal: cannot lock ref ${SQ}refs/heads/symref${SQ}: reference already exists" err &&
+		echo refs/heads/target2 >expect &&
+		git symbolic-ref refs/heads/symref >actual &&
+		test_cmp expect actual
+	'
+
+	test_expect_success "stdin ${type} update existing symref to regular ref" '
+		test_when_finished "git update-ref -d refs/heads/symref" &&
+		git symbolic-ref refs/heads/symref refs/heads/target2 &&
+		create_stdin_buf ${type} "update refs/heads/symref" "$(git rev-parse $a)" "ref:refs/heads/target2" >stdin &&
+		git update-ref --stdin ${type} --no-deref <stdin &&
+		echo $(git rev-parse $a) >expect &&
+		git rev-parse 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 refs/heads/target2) $(git rev-parse $a)" actual
+	'
+
 done
 
+test_expect_success "stdin update 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 &&
+	echo "update refs/heads/symref" "ref:$a" >stdin &&
+	git update-ref --stdin <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 -z update 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 &&
+	printf "$F" "update refs/heads/symref" "ref:$a" "" >stdin &&
+	git update-ref --stdin -z <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 update regular ref to symref" '
+	test_when_finished "git symbolic-ref -d --no-recurse refs/heads/regularref" &&
+	git update-ref --no-deref refs/heads/regularref $a &&
+	echo "update refs/heads/regularref" "ref:$a" >stdin &&
+	git update-ref --stdin <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 -z update regular ref to symref" '
+	test_when_finished "git symbolic-ref -d --no-recurse refs/heads/regularref" &&
+	git update-ref --no-deref refs/heads/regularref $a &&
+	printf "$F" "update refs/heads/regularref" "ref:$a" "" >stdin &&
+	git update-ref --stdin -z <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_done
-- 
2.43.GIT