git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Karthik Nayak <karthik.188@gmail.com>
To: git@vger.kernel.org
Cc: jltobler@gmail.com, ps@pks.im, gitster@pobox.com,
	 sunshine@sunshineco.com,
	Christian Couder <chriscool@tuxfamily.org>,
	 Karthik Nayak <karthik.188@gmail.com>
Subject: [PATCH v5 1/2] refs/files: skip updates with errors in batched updates
Date: Fri, 20 Jun 2025 09:15:44 +0200	[thread overview]
Message-ID: <20250620-6769-address-test-failures-in-the-next-branch-caused-by-batched-reference-updates-v5-1-f35ee6b59a82@gmail.com> (raw)
In-Reply-To: <20250620-6769-address-test-failures-in-the-next-branch-caused-by-batched-reference-updates-v5-0-f35ee6b59a82@gmail.com>

The commit 23fc8e4f61 (refs: implement batch reference update support,
2025-04-08) introduced support for batched reference updates. This
allows users to batch updates together, while allowing some of the
updates to fail.

Under the hood, batched updates use the reference transaction mechanism.
Each update which fails is marked as such. Any failed updates must be
skipped over in the rest of the code, as they wouldn't apply any more.
In two of the loops within 'files_transaction_finish()' of the files
backend, the failed updates aren't skipped over. This can cause a
SEGFAULT otherwise. Add the missing skips and a test to validate the
same.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 refs/files-backend.c  |  7 +++++++
 t/t1400-update-ref.sh | 45 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 4d1f65a57a..c4a0f29072 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3208,6 +3208,10 @@ static int files_transaction_finish(struct ref_store *ref_store,
 	 */
 	for (i = 0; i < transaction->nr; i++) {
 		struct ref_update *update = transaction->updates[i];
+
+		if (update->rejection_err)
+			continue;
+
 		if (update->flags & REF_DELETING &&
 		    !(update->flags & REF_LOG_ONLY) &&
 		    !(update->flags & REF_IS_PRUNING)) {
@@ -3239,6 +3243,9 @@ static int files_transaction_finish(struct ref_store *ref_store,
 		struct ref_update *update = transaction->updates[i];
 		struct ref_lock *lock = update->backend_data;
 
+		if (update->rejection_err)
+			continue;
+
 		if (update->flags & REF_DELETING &&
 		    !(update->flags & REF_LOG_ONLY)) {
 			update->flags |= REF_DELETED_RMDIR;
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index d29d23cb89..ca7eee7de2 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -2299,6 +2299,51 @@ do
 			test_grep -q "refname conflict" stdout
 		)
 	'
+
+	test_expect_success "stdin $type batch-updates delete incorrect symbolic ref" '
+		git init repo &&
+		test_when_finished "rm -fr repo" &&
+		(
+			cd repo &&
+			test_commit c1 &&
+			head=$(git rev-parse HEAD) &&
+			git symbolic-ref refs/heads/symbolic refs/heads/non-existent &&
+
+			format_command $type "delete refs/heads/symbolic" "$head" >stdin &&
+			git update-ref $type --stdin --batch-updates <stdin >stdout &&
+			test_grep "reference does not exist" stdout
+		)
+	'
+
+	test_expect_success "stdin $type batch-updates delete with incorrect old_oid" '
+		git init repo &&
+		test_when_finished "rm -fr repo" &&
+		(
+			cd repo &&
+			test_commit c1 &&
+			git branch new-branch &&
+			test_commit c2 &&
+			head=$(git rev-parse HEAD) &&
+
+			format_command $type "delete refs/heads/new-branch" "$head" >stdin &&
+			git update-ref $type --stdin --batch-updates <stdin >stdout &&
+			test_grep "incorrect old value provided" stdout
+		)
+	'
+
+	test_expect_success "stdin $type batch-updates delete non-existent ref" '
+		git init repo &&
+		test_when_finished "rm -fr repo" &&
+		(
+			cd repo &&
+			test_commit commit &&
+			head=$(git rev-parse HEAD) &&
+
+			format_command $type "delete refs/heads/non-existent" "$head" >stdin &&
+			git update-ref $type --stdin --batch-updates <stdin >stdout &&
+			test_grep "reference does not exist" stdout
+		)
+	'
 done
 
 test_expect_success 'update-ref should also create reflog for HEAD' '

-- 
2.49.0


  reply	other threads:[~2025-06-20  7:15 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-02  9:57 [PATCH 0/3] refs: fix some bugs with batched-updates Karthik Nayak
2025-06-02  9:57 ` [PATCH 1/3] refs/files: skip updates with errors in batched updates Karthik Nayak
2025-06-02 12:00   ` Patrick Steinhardt
2025-06-02 12:46     ` Karthik Nayak
2025-06-02 15:06       ` Junio C Hamano
2025-06-02 17:13         ` Karthik Nayak
2025-06-02  9:57 ` [PATCH 2/3] t5516: use double quotes for tests with variables Karthik Nayak
2025-06-02 16:59   ` Eric Sunshine
2025-06-02 17:13     ` Karthik Nayak
2025-06-02  9:57 ` [PATCH 3/3] receive-pack: handle reference deletions separately Karthik Nayak
2025-06-02 11:59   ` Patrick Steinhardt
2025-06-02 12:54     ` Karthik Nayak
2025-06-02 15:20     ` Junio C Hamano
2025-06-02 15:56       ` Patrick Steinhardt
2025-06-04 11:08         ` Karthik Nayak
2025-06-05  8:19 ` [PATCH v2 0/2] refs: fix some bugs with batched-updates Karthik Nayak
2025-06-05  8:19   ` [PATCH v2 1/2] refs/files: skip updates with errors in batched updates Karthik Nayak
2025-06-05  8:19   ` [PATCH v2 2/2] receive-pack: handle reference deletions separately Karthik Nayak
2025-06-05  8:47     ` Patrick Steinhardt
2025-06-05  9:08       ` Karthik Nayak
2025-06-06  8:41 ` [PATCH v3 0/2] refs: fix some bugs with batched-updates Karthik Nayak
2025-06-06  8:41   ` [PATCH v3 1/2] refs/files: skip updates with errors in batched updates Karthik Nayak
2025-06-06  8:41   ` [PATCH v3 2/2] receive-pack: handle reference deletions separately Karthik Nayak
2025-06-12 17:03     ` Christian Couder
2025-06-12 20:40       ` Junio C Hamano
2025-06-13  7:23       ` Karthik Nayak
2025-06-13  8:10 ` [PATCH v4 0/2] refs: fix some bugs with batched-updates Karthik Nayak
2025-06-13  8:10   ` [PATCH v4 1/2] refs/files: skip updates with errors in batched updates Karthik Nayak
2025-06-13  8:10   ` [PATCH v4 2/2] receive-pack: handle reference deletions separately Karthik Nayak
2025-06-13 15:46     ` Junio C Hamano
2025-06-19  9:39       ` Karthik Nayak
2025-06-13 12:43   ` [PATCH v4 0/2] refs: fix some bugs with batched-updates Christian Couder
2025-06-13 18:57     ` Junio C Hamano
2025-06-20  7:15 ` [PATCH v5 " Karthik Nayak
2025-06-20  7:15   ` Karthik Nayak [this message]
2025-06-20  7:15   ` [PATCH v5 2/2] receive-pack: handle reference deletions separately Karthik Nayak
2025-06-20 16:21   ` [PATCH v5 0/2] refs: fix some bugs with batched-updates Junio C Hamano
2025-06-21 11:08     ` Karthik Nayak
2025-06-22  4:23       ` Junio C Hamano
2025-06-22 14:20         ` Karthik Nayak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250620-6769-address-test-failures-in-the-next-branch-caused-by-batched-reference-updates-v5-1-f35ee6b59a82@gmail.com \
    --to=karthik.188@gmail.com \
    --cc=chriscool@tuxfamily.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jltobler@gmail.com \
    --cc=ps@pks.im \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).