diff mbox series

[v5,5/7] refs: use transaction in `refs_create_symref()`

Message ID 20240501202229.2695774-6-knayak@gitlab.com (mailing list archive)
State Superseded
Headers show
Series refs: add support for transactional symref updates | expand

Commit Message

Karthik Nayak May 1, 2024, 8:22 p.m. UTC 
From: Karthik Nayak <karthik.188@gmail.com>

The `refs_create_symref()` function updates a symref to a given new
target. To do this, it uses a ref-backend specific function
`create_symref()`.

In this previous commit, we introduce symref support in transactions.
This means we can now use transactions to perform symref updates and not
have to resort to `create_symref()`. Doing this allows us to remove and
cleanup `create_symref()`, which we will do in the following commit.

Modify the expected error message for a test in
't/t0610-reftable-basics.sh', since the error is now thrown from the
'refs.c'. This is because in transactional updates, F/D conflicts are
caught before we're in the reference backend.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 refs.c                           | 24 +++++++++++++++++-------
 t/t0610-reftable-basics.sh       |  2 +-
 t/t1416-ref-transaction-hooks.sh | 23 +++++++++++++++++++++++
 3 files changed, 41 insertions(+), 8 deletions(-)

Comments

Patrick Steinhardt May 2, 2024, 7:47 a.m. UTC | #1 
On Wed, May 01, 2024 at 10:22:27PM +0200, Karthik Nayak wrote:
> From: Karthik Nayak <karthik.188@gmail.com>
> 
> The `refs_create_symref()` function updates a symref to a given new
> target. To do this, it uses a ref-backend specific function
> `create_symref()`.
> 
> In this previous commit, we introduce symref support in transactions.
> This means we can now use transactions to perform symref updates and not
> have to resort to `create_symref()`. Doing this allows us to remove and

Nit: "not have to" -> "don't have to"

[snip]
> diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh
> index 178791e086..9e8d22bcbd 100755
> --- a/t/t0610-reftable-basics.sh
> +++ b/t/t0610-reftable-basics.sh
> @@ -286,7 +286,7 @@ test_expect_success 'ref transaction: creating symbolic ref fails with F/D confl
>  	git init repo &&
>  	test_commit -C repo A &&
>  	cat >expect <<-EOF &&
> -	error: unable to write symref for refs/heads: file/directory conflict
> +	error: ${SQ}refs/heads/main${SQ} exists; cannot create ${SQ}refs/heads${SQ}
>  	EOF
>  	test_must_fail git -C repo symbolic-ref refs/heads refs/heads/foo 2>err &&
>  	test_cmp expect err

Nice. Not only do we have less code to worry about, but the error
message is better, too.

> diff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh
> index 2092488090..4433ac2177 100755
> --- a/t/t1416-ref-transaction-hooks.sh
> +++ b/t/t1416-ref-transaction-hooks.sh
> @@ -134,4 +134,27 @@ test_expect_success 'interleaving hook calls succeed' '
>  	test_cmp expect target-repo.git/actual
>  '
>  
> +test_expect_success 'hook captures git-symbolic-ref updates' '
> +	test_when_finished "rm actual" &&
> +
> +	test_hook reference-transaction <<-\EOF &&
> +		echo "$*" >>actual
> +		while read -r line
> +		do
> +			printf "%s\n" "$line"
> +		done >>actual
> +	EOF
> +
> +	git symbolic-ref refs/heads/symref refs/heads/main &&
> +
> +	cat >expect <<-EOF &&
> +		prepared
> +		$ZERO_OID ref:refs/heads/main refs/heads/symref
> +		committed
> +		$ZERO_OID ref:refs/heads/main refs/heads/symref
> +	EOF

Nit: the contents of the heredoc should be indented one level less.

Patrick
diff mbox series

Patch

diff --git a/refs.c b/refs.c
index a4dca08244..1b67c87f47 100644
--- a/refs.c
+++ b/refs.c
@@ -2291,14 +2291,24 @@  int refs_create_symref(struct ref_store *refs,
 		       const char *refs_heads_master,
 		       const char *logmsg)
 {
-	char *msg;
-	int retval;
+	struct ref_transaction *transaction;
+	struct strbuf err = STRBUF_INIT;
+	int ret = 0;
 
-	msg = normalize_reflog_message(logmsg);
-	retval = refs->be->create_symref(refs, ref_target, refs_heads_master,
-					 msg);
-	free(msg);
-	return retval;
+	transaction = ref_store_transaction_begin(refs, &err);
+	if (!transaction ||
+	    ref_transaction_update(transaction, ref_target, NULL, NULL,
+				   refs_heads_master, NULL, REF_NO_DEREF,
+				   logmsg, &err) ||
+	    ref_transaction_commit(transaction, &err)) {
+		ret = error("%s", err.buf);
+	}
+
+	strbuf_release(&err);
+	if (transaction)
+		ref_transaction_free(transaction);
+
+	return ret;
 }
 
 int create_symref(const char *ref_target, const char *refs_heads_master,
diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh
index 178791e086..9e8d22bcbd 100755
--- a/t/t0610-reftable-basics.sh
+++ b/t/t0610-reftable-basics.sh
@@ -286,7 +286,7 @@  test_expect_success 'ref transaction: creating symbolic ref fails with F/D confl
 	git init repo &&
 	test_commit -C repo A &&
 	cat >expect <<-EOF &&
-	error: unable to write symref for refs/heads: file/directory conflict
+	error: ${SQ}refs/heads/main${SQ} exists; cannot create ${SQ}refs/heads${SQ}
 	EOF
 	test_must_fail git -C repo symbolic-ref refs/heads refs/heads/foo 2>err &&
 	test_cmp expect err
diff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh
index 2092488090..4433ac2177 100755
--- a/t/t1416-ref-transaction-hooks.sh
+++ b/t/t1416-ref-transaction-hooks.sh
@@ -134,4 +134,27 @@  test_expect_success 'interleaving hook calls succeed' '
 	test_cmp expect target-repo.git/actual
 '
 
+test_expect_success 'hook captures git-symbolic-ref updates' '
+	test_when_finished "rm actual" &&
+
+	test_hook reference-transaction <<-\EOF &&
+		echo "$*" >>actual
+		while read -r line
+		do
+			printf "%s\n" "$line"
+		done >>actual
+	EOF
+
+	git symbolic-ref refs/heads/symref refs/heads/main &&
+
+	cat >expect <<-EOF &&
+		prepared
+		$ZERO_OID ref:refs/heads/main refs/heads/symref
+		committed
+		$ZERO_OID ref:refs/heads/main refs/heads/symref
+	EOF
+
+	test_cmp expect actual
+'
+
 test_done