Subject: [PATCH 7/8] refs: add 'update-symref' command to 'update-ref'

From: Karthik Nayak <[email-masked]>
Date: Sat, 30 Mar 2024 23:46:22 +0100

From: Karthik Nayak <[email-masked]>

The 'git-update-ref(1)' command allows transactional reference updates.
But currently only supports regular reference updates. Meaning, if one
wants to update HEAD (symbolic ref) in a transaction, there is no tool
to do so.

One option to obtain transactional updates for the HEAD ref is to
manually create the HEAD.lock file and commit. This is intrusive, where
the user needs to mimic internal git behavior. Also, this only works
when using the files backend.

To allow users to update the HEAD ref in a transaction, we introduce
'update-symref' command for 'git-update-ref(1)'. This command allows the
user to create symref in a transaction similar to the 'update' command
of 'git-update-ref(1)'. This command also works well with the existing
'no-deref' option.

The option can also be used to create new symrefs too. This means we
don't need a dedicated `create-symref` option. This is also because we
don't verify the old symref value when updating a symref. So in this
case update and create hold the same meaning.

The regular `delete` option can also be used to delete symrefs. So we
don't add a dedicated `delete-symref` option.

Signed-off-by: Karthik Nayak <[email-masked]>
---
 Documentation/git-update-ref.txt |  11 ++-
 builtin/update-ref.c             |  61 ++++++++++++---
 refs.c                           |  10 +++
 t/t0600-reffiles-backend.sh      |  30 ++++++++
 t/t1400-update-ref.sh            | 127 +++++++++++++++++++++++++++++++
 5 files changed, 229 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt
index 0561808cca..2ea8bc8167 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 <newvalue> LF
 	delete SP <ref> [SP <oldvalue>] LF
 	verify SP <ref> [SP <oldvalue>] LF
+	update-symref SP <ref> SP <newvalue> LF
 	option SP <opt> LF
 	start LF
 	prepare LF
@@ -86,6 +87,7 @@ quoting:
 	create SP <ref> NUL <newvalue> NUL
 	delete SP <ref> NUL [<oldvalue>] NUL
 	verify SP <ref> NUL [<oldvalue>] NUL
+	update-symref NUL <ref> NUL <newvalue> NUL
 	option SP <opt> NUL
 	start NUL
 	prepare NUL
@@ -111,12 +113,19 @@ create::
 
 delete::
 	Delete <ref> after verifying it exists with <oldvalue>, if
-	given.  If given, <oldvalue> may not be zero.
+	given.  If given, <oldvalue> may not be zero. Can also delete
+	symrefs.
 
 verify::
 	Verify <ref> against <oldvalue> but do not change it.  If
 	<oldvalue> is zero or missing, the ref must not exist.
 
+update-symref::
+	Update <ref> as a symbolic reference to point to the given
+	reference <newvalue>. By default, <ref> will be recursively
+	de-referenced, unless the `no-deref` option is used. Can also
+	be used to create new symrefs.
+
 option::
 	Modify the behavior of the next command naming a <ref>.
 	The only valid option is `no-deref` to avoid dereferencing
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 3807cf4106..357daf31b8 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -213,6 +213,48 @@ static void parse_cmd_update(struct ref_transaction *transaction,
 	strbuf_release(&err);
 }
 
+static void parse_cmd_update_symref(struct ref_transaction *transaction,
+				    const char *next, const char *end)
+{
+	struct strbuf err = STRBUF_INIT;
+	char *refname, *symref;
+
+	refname = parse_refname(&next);
+	if (!refname)
+		die("update-symref: missing <ref>");
+
+	if (line_termination) {
+		/* Without -z, consume SP and use next argument */
+		if (!*next || *next == line_termination)
+			die("update-symref %s: missing <newvalue>", refname);
+		if (*next != ' ')
+			die("update-symref %s: expected SP but got: %s",
+			    refname, next);
+	} else {
+		/* With -z, read the next NUL-terminated line */
+		if (*next)
+			die("update-symref %s: missing <newvalue>", refname);
+	}
+	next++;
+
+	symref = parse_refname(&next);
+	if (!symref)
+		die("update-symref %s: missing <newvalue>", refname);
+
+	if (*next != line_termination)
+		die("update-symref %s: extra input: %s", refname, next);
+
+	if (ref_transaction_update(transaction, refname, NULL, NULL,
+				   update_flags | create_reflog_flag | REF_UPDATE_SYMREF,
+				   msg, symref, &err))
+		die("%s", err.buf);
+
+	update_flags = default_flags;
+	free(symref);
+	free(refname);
+	strbuf_release(&err);
+}
+
 static void parse_cmd_create(struct ref_transaction *transaction,
 			     const char *next, const char *end)
 {
@@ -379,15 +421,16 @@ static const struct parse_cmd {
 	unsigned args;
 	enum update_refs_state state;
 } command[] = {
-	{ "update",  parse_cmd_update,  3, UPDATE_REFS_OPEN },
-	{ "create",  parse_cmd_create,  2, UPDATE_REFS_OPEN },
-	{ "delete",  parse_cmd_delete,  2, UPDATE_REFS_OPEN },
-	{ "verify",  parse_cmd_verify,  2, UPDATE_REFS_OPEN },
-	{ "option",  parse_cmd_option,  1, UPDATE_REFS_OPEN },
-	{ "start",   parse_cmd_start,   0, UPDATE_REFS_STARTED },
-	{ "prepare", parse_cmd_prepare, 0, UPDATE_REFS_PREPARED },
-	{ "abort",   parse_cmd_abort,   0, UPDATE_REFS_CLOSED },
-	{ "commit",  parse_cmd_commit,  0, UPDATE_REFS_CLOSED },
+	{ "update",        parse_cmd_update,        3, UPDATE_REFS_OPEN },
+	{ "update-symref", parse_cmd_update_symref, 2, UPDATE_REFS_OPEN },
+	{ "create",        parse_cmd_create,        2, UPDATE_REFS_OPEN },
+	{ "delete",        parse_cmd_delete,        2, UPDATE_REFS_OPEN },
+	{ "verify",        parse_cmd_verify,        2, UPDATE_REFS_OPEN },
+	{ "option",        parse_cmd_option,        1, UPDATE_REFS_OPEN },
+	{ "start",         parse_cmd_start,         0, UPDATE_REFS_STARTED },
+	{ "prepare",       parse_cmd_prepare,       0, UPDATE_REFS_PREPARED },
+	{ "abort",         parse_cmd_abort,         0, UPDATE_REFS_CLOSED },
+	{ "commit",        parse_cmd_commit,        0, UPDATE_REFS_CLOSED },
 };
 
 static void update_refs_stdin(void)
diff --git a/refs.c b/refs.c
index 69b89a1aa2..706dcd6deb 100644
--- a/refs.c
+++ b/refs.c
@@ -1216,6 +1216,7 @@ void ref_transaction_free(struct ref_transaction *transaction)
 	}
 
 	for (i = 0; i < transaction->nr; i++) {
+		free(transaction->updates[i]->symref_target);
 		free(transaction->updates[i]->msg);
 		free(transaction->updates[i]);
 	}
@@ -1235,6 +1236,9 @@ struct ref_update *ref_transaction_add_update(
 	if (transaction->state != REF_TRANSACTION_OPEN)
 		BUG("update called for transaction that is not open");
 
+	if ((flags & (REF_HAVE_NEW | REF_UPDATE_SYMREF)) == (REF_HAVE_NEW | REF_UPDATE_SYMREF))
+		BUG("cannot create regular ref and symref at once");
+
 	FLEX_ALLOC_STR(update, refname, refname);
 	ALLOC_GROW(transaction->updates, transaction->nr + 1, transaction->alloc);
 	transaction->updates[transaction->nr++] = update;
@@ -1245,6 +1249,8 @@ struct ref_update *ref_transaction_add_update(
 		oidcpy(&update->new_oid, new_oid);
 	if (flags & REF_HAVE_OLD)
 		oidcpy(&update->old_oid, old_oid);
+	if (flags & REF_UPDATE_SYMREF)
+		update->symref_target = xstrdup(symref);
 	update->msg = normalize_reflog_message(msg);
 	return update;
 }
@@ -2337,6 +2343,10 @@ static int run_transaction_hook(struct ref_transaction *transaction,
 	for (i = 0; i < transaction->nr; i++) {
 		struct ref_update *update = transaction->updates[i];
 
+		// Reference transaction does not support symbolic updates.
+		if (update->flags & REF_UPDATE_SYMREF)
+			continue;
+
 		strbuf_reset(&buf);
 		strbuf_addf(&buf, "%s %s %s\n",
 			    oid_to_hex(&update->old_oid),
diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
index 64214340e7..6d334cb477 100755
--- a/t/t0600-reffiles-backend.sh
+++ b/t/t0600-reffiles-backend.sh
@@ -472,4 +472,34 @@ test_expect_success POSIXPERM 'git reflog expire honors core.sharedRepository' '
 	esac
 '
 
+test_expect_success SYMLINKS 'symref transaction supports symlinks' '
+	git update-ref refs/heads/new @ &&
+	test_config core.prefersymlinkrefs true &&
+	cat >stdin <<-EOF &&
+	start
+	update-symref TESTSYMREFONE refs/heads/new
+	prepare
+	commit
+	EOF
+	git update-ref --no-deref --stdin <stdin &&
+	test_path_is_symlink .git/TESTSYMREFONE &&
+	test "$(test_readlink .git/TESTSYMREFONE)" = refs/heads/new
+'
+
+test_expect_success 'symref transaction supports false symlink config' '
+	git update-ref refs/heads/new @ &&
+	test_config core.prefersymlinkrefs false &&
+	cat >stdin <<-EOF &&
+	start
+	update-symref TESTSYMREFONE refs/heads/new
+	prepare
+	commit
+	EOF
+	git update-ref --no-deref --stdin <stdin &&
+	test_path_is_file .git/TESTSYMREFONE &&
+	git symbolic-ref TESTSYMREFONE >actual &&
+	echo refs/heads/new >expect &&
+	test_cmp expect actual
+'
+
 test_done
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 6ebc3ef945..2a6036471b 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -868,6 +868,105 @@ test_expect_success 'stdin delete symref works flag --no-deref' '
 	test_cmp expect actual
 '
 
+test_expect_success 'stdin update-symref creates symref with --no-deref' '
+	# ensure that the symref does not already exist 
+	test_must_fail git symbolic-ref --no-recurse refs/heads/symref &&
+	cat >stdin <<-EOF &&
+	update-symref refs/heads/symref $b
+	EOF
+	git update-ref --no-deref --stdin <stdin &&
+	echo $b >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 $b)" actual
+'
+
+test_expect_success 'stdin update-symref updates symref with --no-deref' '
+	# ensure that the symref already exists
+	git symbolic-ref --no-recurse refs/heads/symref &&
+	cat >stdin <<-EOF &&
+	update-symref refs/heads/symref $a
+	EOF
+	git update-ref --no-deref --stdin <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 "$(git rev-parse $b) $(git rev-parse $a)" actual
+'
+
+test_expect_success 'stdin update-symref noop update with --no-deref' '
+	git symbolic-ref --no-recurse refs/heads/symref >actual &&
+	echo $a >expect &&
+	test_cmp expect actual &&
+	cat >stdin <<-EOF &&
+	update-symref refs/heads/symref $a
+	EOF
+	git update-ref --no-deref --stdin <stdin &&
+	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-symref regular ref with --no-deref' '
+	git update-ref refs/heads/regularref $a &&
+	cat >stdin <<-EOF &&
+	update-symref refs/heads/regularref $a
+	EOF
+	git update-ref --no-deref --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 update-symref creates symref' '
+	# delete the ref since it already exists from previous tests
+	git update-ref --no-deref -d refs/heads/symref &&
+	cat >stdin <<-EOF &&
+	update-symref refs/heads/symref $b
+	EOF
+	git update-ref --stdin <stdin &&
+	echo $b >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 $b)" actual
+'
+
+test_expect_success 'stdin update-symref updates symref' '
+	git update-ref refs/heads/symref2 $b &&
+	git symbolic-ref --no-recurse refs/heads/symref refs/heads/symref2 &&
+	cat >stdin <<-EOF &&
+	update-symref refs/heads/symref $a
+	EOF
+	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 $b) $(git rev-parse $b)" actual
+'
+
+test_expect_success 'stdin update-symref regular ref' '
+	git update-ref --no-deref refs/heads/regularref $a &&
+	cat >stdin <<-EOF &&
+	update-symref refs/heads/regularref $a
+	EOF
+	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 delete ref works with right old value' '
 	echo "delete $b $m~1" >stdin &&
 	git update-ref --stdin <stdin &&
@@ -1641,4 +1740,32 @@ test_expect_success PIPE 'transaction flushes status updates' '
 	test_cmp expected actual
 '
 
+test_expect_success 'transaction can commit symref update' '
+	git symbolic-ref TESTSYMREFONE $a &&
+	cat >stdin <<-EOF &&
+	start
+	update-symref TESTSYMREFONE refs/heads/branch
+	prepare
+	commit
+	EOF
+	git update-ref --no-deref --stdin <stdin &&
+	echo refs/heads/branch >expect &&
+	git symbolic-ref TESTSYMREFONE >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'transaction can abort symref update' '
+	git symbolic-ref TESTSYMREFONE $a &&
+	cat >stdin <<-EOF &&
+	start
+	update-symref TESTSYMREFONE refs/heads/branch
+	prepare
+	abort
+	EOF
+	git update-ref --no-deref --stdin <stdin &&
+	echo $a >expect &&
+	git symbolic-ref TESTSYMREFONE >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.43.GIT