[PATCH v3] refs/files: avoid packed-refs lock for root ref deletion

[PATCH v3] refs/files: avoid packed-refs lock for root ref deletion

[Ariel Keselman]

Deleting a root ref queues a packed-ref transaction in the files
backend, even though root refs cannot be packed. For example, holding
.git/packed-refs.lock makes "git update-ref --no-deref -d AUTO_MERGE"
fail, whether or not AUTO_MERGE exists.

This also affects post-commit cleanup, which deletes AUTO_MERGE after
updating HEAD. In a linked worktree with read-only shared metadata,
commit succeeds but cleanup reports a packed-refs.lock error. Deleting
CHERRY_PICK_HEAD and REVERT_HEAD is affected as well.

Skip the packed transaction for root-ref deletions. Keep loose-ref
locking and packed-ref deletion for other refs unchanged.

Signed-off-by: Ariel Keselman <skariel@gmail.com>
---
Thanks for the review, Patrick.

Changes since v2:
- Run pack-refs after creating AUTO_MERGE and verify that it remains loose.
- Compare the state of all refs around the failed mixed transaction.
- Feed the deletes directly to update-ref and omit the optional transaction
  commands.
- Drop the redundant test summary from the commit message.

AI assistance was used to generate the patch, tests, and commit message,
including this revision.

The full test suite and all 249 unit tests pass. t0600 also passes with
SHA-256 (one platform-dependent skip).

 refs/files-backend.c        |  8 +++++---
 t/t0600-reffiles-backend.sh | 40 +++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index a4c7858787..0333d5d355 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2981,10 +2981,12 @@ static int files_transaction_prepare(struct ref_store *ref_store,
 
 		if (update->flags & REF_DELETING &&
 		    !(update->flags & REF_LOG_ONLY) &&
-		    !(update->flags & REF_IS_PRUNING)) {
+		    !(update->flags & REF_IS_PRUNING) &&
+		    !is_root_ref(update->refname)) {
 			/*
-			 * This reference has to be deleted from
-			 * packed-refs if it exists there.
+			 * This reference has to be deleted from packed-refs if it
+			 * exists there. Root refs are never packed, so we do not
+			 * have to delete them from packed-refs.
 			 */
 			if (!packed_transaction) {
 				packed_transaction = ref_store_transaction_begin(
diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
index 74bfa2e9ba..560caf56b0 100755
--- a/t/t0600-reffiles-backend.sh
+++ b/t/t0600-reffiles-backend.sh
@@ -519,4 +519,44 @@ test_expect_success 'symref transaction supports false symlink config' '
 	test_cmp expect actual
 '
 
+test_expect_success 'deleting a root ref does not lock packed-refs' '
+	test_when_finished "rm -rf root-ref" &&
+	git init root-ref &&
+	(
+		cd root-ref &&
+		test_commit initial &&
+		git update-ref AUTO_MERGE HEAD &&
+		git pack-refs --all &&
+		test_path_is_file .git/AUTO_MERGE &&
+		cp .git/packed-refs expect &&
+		: >.git/packed-refs.lock &&
+		git update-ref --no-deref -d AUTO_MERGE &&
+		test_path_is_missing .git/AUTO_MERGE &&
+		test_path_is_file .git/packed-refs.lock &&
+		test_cmp expect .git/packed-refs
+	)
+'
+
+test_expect_success 'deleting root and packed refs in one transaction requires packed-refs lock' '
+	test_when_finished "rm -rf root-ref" &&
+	git init root-ref &&
+	(
+		cd root-ref &&
+		test_commit initial &&
+		git update-ref refs/heads/packed-branch HEAD &&
+		git pack-refs --all &&
+		test_path_is_missing .git/refs/heads/packed-branch &&
+		git update-ref AUTO_MERGE HEAD &&
+		git refs list --include-root-refs >expect &&
+		: >.git/packed-refs.lock &&
+		test_must_fail git update-ref --no-deref --stdin 2>err <<-EOF &&
+		delete AUTO_MERGE
+		delete refs/heads/packed-branch
+		EOF
+		test_grep "Unable to create .*packed-refs.lock" err &&
+		git refs list --include-root-refs >actual &&
+		test_cmp expect actual
+	)
+'
+
 test_done

Range-diff against v2:
1:  370bb6441b ! 1:  e035ce46b4 refs/files: avoid packed-refs lock for root ref deletion
    @@ Commit message
         Skip the packed transaction for root-ref deletions. Keep loose-ref
         locking and packed-ref deletion for other refs unchanged.
     
    -    Test deleting a root ref with packed-refs.lock held, and check that a
    -    transaction deleting both a root ref and a packed branch still fails
    -    without changing either ref.
    -
         Signed-off-by: Ariel Keselman <skariel@gmail.com>
     
      ## refs/files-backend.c ##
    @@ t/t0600-reffiles-backend.sh: test_expect_success 'symref transaction supports fa
     +	(
     +		cd root-ref &&
     +		test_commit initial &&
    ++		git update-ref AUTO_MERGE HEAD &&
     +		git pack-refs --all &&
    ++		test_path_is_file .git/AUTO_MERGE &&
     +		cp .git/packed-refs expect &&
    -+		git update-ref AUTO_MERGE HEAD &&
     +		: >.git/packed-refs.lock &&
     +		git update-ref --no-deref -d AUTO_MERGE &&
     +		test_path_is_missing .git/AUTO_MERGE &&
    @@ t/t0600-reffiles-backend.sh: test_expect_success 'symref transaction supports fa
     +		git pack-refs --all &&
     +		test_path_is_missing .git/refs/heads/packed-branch &&
     +		git update-ref AUTO_MERGE HEAD &&
    -+		git rev-parse AUTO_MERGE refs/heads/packed-branch >expect &&
    -+		cat >stdin <<-EOF &&
    -+		start
    ++		git refs list --include-root-refs >expect &&
    ++		: >.git/packed-refs.lock &&
    ++		test_must_fail git update-ref --no-deref --stdin 2>err <<-EOF &&
     +		delete AUTO_MERGE
     +		delete refs/heads/packed-branch
    -+		prepare
    -+		commit
     +		EOF
    -+		: >.git/packed-refs.lock &&
    -+		test_must_fail git update-ref --no-deref --stdin <stdin 2>err &&
     +		test_grep "Unable to create .*packed-refs.lock" err &&
    -+		git rev-parse AUTO_MERGE refs/heads/packed-branch >actual &&
    ++		git refs list --include-root-refs >actual &&
     +		test_cmp expect actual
     +	)
     +'
-- 
2.55.0