{"thread":{"id":"66303","subject":"[PATCH] refs/files: avoid packed-refs lock for root ref deletion","startedAt":"2026-09-10T06:45:57Z","lastAt":"2026-09-12T02:12:34Z","messageCount":5,"participants":["Ariel Keselman","Patrick Steinhardt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"552412","messageId":"CAMuXvLD_ZsT8Jnfs_x6yO_aW6hrxQyjnuES_b21cq8a7nD=sKg@mail.gmail.com","threadId":"66303","inReplyTo":null,"subject":"[PATCH] refs/files: avoid packed-refs lock for root ref deletion","fromName":"Ariel Keselman","fromEmail":"skariel@gmail.com","sentAt":"2026-09-10T06:45:46Z","receivedAt":"2026-09-10T06:45:57Z","isPatch":true,"body":"Hi,\n\nDeleting root refs in the files backend unnecessarily locks\npacked-refs, even though root refs cannot be packed. This can cause\npost-commit cleanup to report an error after a successful commit in a\nlinked worktree with read-only shared metadata.\n\nThe attached patch skips that lock for root-ref deletion and adds\nregression tests. All seven new tests fail without the fix and pass\nwith it; broader ref, worktree, and sequencer tests also pass.\n\nAI assistance was used to generate the patch, tests, and commit message.\n\nThanks,\nAriel\n\n\nFrom c5d12e97a78123965590553fddc0dd78af3e006e Mon Sep 17 00:00:00 2001\nFrom: Ariel Keselman <skariel@gmail.com>\nDate: Wed, 9 Sep 2026 22:16:50 -0700\nTo: git@vger.kernel.org\nSubject: [PATCH] refs/files: avoid packed-refs lock for root ref deletion\n\nDeleting a root ref queues a packed-ref transaction in the files\nbackend, even though root refs cannot be packed. For example, holding\n.git/packed-refs.lock makes \"git update-ref --no-deref -d AUTO_MERGE\"\nfail, whether or not AUTO_MERGE exists.\n\nThis also affects post-commit cleanup, which deletes AUTO_MERGE after\nupdating HEAD. In a linked worktree with read-only shared metadata,\ncommit succeeds but cleanup reports a packed-refs.lock error. Deleting\nCHERRY_PICK_HEAD and REVERT_HEAD is affected as well.\n\nSkip the packed transaction for root-ref deletions. Keep loose-ref\nlocking and packed-ref deletion for other refs unchanged.\n\nTest existing and absent root refs with packed-refs.lock held. Also\ncheck that root-ref deletion leaves packed refs intact, and that a\npacked branch still requires the lock and can be deleted once it is\nreleased.\n\nSigned-off-by: Ariel Keselman <skariel@gmail.com>\n---\nAI assistance was used to generate the patch, tests, and commit message.\n\nBased on maint at e9019fcafe (Git 2.55).\n\nValidation:\n- All seven new tests fail with packed-refs.lock errors without the fix.\n- With the fix, t0600 and t0601 pass (one platform skip in t0600).\n- Broader ref, worktree, rebase, cherry-pick, commit and merge tests pass:\n  123 scripts, 4083 tests on the maint-based tree.\n- After merging with master at b8242b093d, 128 scripts / 4178 tests and\n  260 unit tests pass.\n\n refs/files-backend.c        |  9 +++++---\n t/t0600-reffiles-backend.sh | 46 +++++++++++++++++++++++++++++++++++++\n 2 files changed, 52 insertions(+), 3 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex a4c7858787..41887f180f 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -2981,10 +2981,13 @@ static int files_transaction_prepare(struct ref_store *ref_store,\n \n \t\tif (update->flags & REF_DELETING &&\n \t\t    !(update->flags & REF_LOG_ONLY) &&\n-\t\t    !(update->flags & REF_IS_PRUNING)) {\n+\t\t    !(update->flags & REF_IS_PRUNING) &&\n+\t\t    !is_root_ref(update->refname)) {\n \t\t\t/*\n-\t\t\t * This reference has to be deleted from\n-\t\t\t * packed-refs if it exists there.\n+\t\t\t * Root refs cannot be packed. Do not acquire the shared\n+\t\t\t * packed-refs lock when deleting a per-worktree root ref.\n+\t\t\t * Other references have to be deleted from\n+\t\t\t * packed-refs if they exist there.\n \t\t\t */\n \t\t\tif (!packed_transaction) {\n \t\t\t\tpacked_transaction = ref_store_transaction_begin(\ndiff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh\nindex 74bfa2e9ba..b7f3287841 100755\n--- a/t/t0600-reffiles-backend.sh\n+++ b/t/t0600-reffiles-backend.sh\n@@ -519,4 +519,50 @@ test_expect_success 'symref transaction supports false symlink config' '\n \ttest_cmp expect actual\n '\n \n+for ref in AUTO_MERGE CHERRY_PICK_HEAD REVERT_HEAD\n+do\n+\tfor state in existing missing\n+\tdo\n+\t\ttest_expect_success \"deleting $state $ref does not lock packed-refs\" '\n+\t\t\ttest_when_finished \"rm -rf root-ref\" &&\n+\t\t\tgit init root-ref &&\n+\t\t\t(\n+\t\t\t\tcd root-ref &&\n+\t\t\t\ttest_commit initial &&\n+\t\t\t\tif test \"$state\" = existing\n+\t\t\t\tthen\n+\t\t\t\t\tgit update-ref \"$ref\" HEAD\n+\t\t\t\tfi &&\n+\t\t\t\t: >.git/packed-refs.lock &&\n+\t\t\t\tgit -c core.packedRefsTimeout=0 update-ref --no-deref -d \"$ref\" &&\n+\t\t\t\ttest_path_is_missing \".git/$ref\" &&\n+\t\t\t\ttest_path_is_file .git/packed-refs.lock\n+\t\t\t)\n+\t\t'\n+\tdone\n+done\n+\n+test_expect_success 'root ref deletion preserves packed refs and their locking' '\n+\ttest_when_finished \"rm -rf root-ref\" &&\n+\tgit init root-ref &&\n+\t(\n+\t\tcd root-ref &&\n+\t\ttest_commit initial &&\n+\t\tgit update-ref refs/heads/packed-branch HEAD &&\n+\t\tgit pack-refs --all &&\n+\t\ttest_path_is_missing .git/refs/heads/packed-branch &&\n+\t\tcp .git/packed-refs expect &&\n+\t\tgit update-ref AUTO_MERGE HEAD &&\n+\t\t: >.git/packed-refs.lock &&\n+\t\tgit -c core.packedRefsTimeout=0 update-ref --no-deref -d AUTO_MERGE &&\n+\t\ttest_cmp expect .git/packed-refs &&\n+\t\ttest_must_fail git -c core.packedRefsTimeout=0 update-ref -d refs/heads/packed-branch 2>err &&\n+\t\ttest_grep \"Unable to create .*packed-refs.lock\" err &&\n+\t\ttest_cmp expect .git/packed-refs &&\n+\t\trm .git/packed-refs.lock &&\n+\t\tgit update-ref -d refs/heads/packed-branch &&\n+\t\ttest_must_fail git rev-parse --verify refs/heads/packed-branch\n+\t)\n+'\n+\n test_done\n-- \n2.55.0\n\n"},{"id":"552421","messageId":"aqJr0ZB8qpthTEGT@pks.im","threadId":"66303","inReplyTo":"CAMuXvLD_ZsT8Jnfs_x6yO_aW6hrxQyjnuES_b21cq8a7nD=sKg@mail.gmail.com","subject":"Re: [PATCH] refs/files: avoid packed-refs lock for root ref deletion","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-10T08:35:29Z","receivedAt":"2026-09-10T08:35:34Z","isPatch":true,"body":"Hi,\n\nOn Wed, Sep 09, 2026 at 11:45:46PM -0700, Ariel Keselman wrote:\n> Hi,\n> \n> Deleting root refs in the files backend unnecessarily locks\n> packed-refs, even though root refs cannot be packed. This can cause\n> post-commit cleanup to report an error after a successful commit in a\n> linked worktree with read-only shared metadata.\n> \n> The attached patch skips that lock for root-ref deletion and adds\n> regression tests. All seven new tests fail without the fix and pass\n> with it; broader ref, worktree, and sequencer tests also pass.\n> \n> AI assistance was used to generate the patch, tests, and commit message.\n\nPlease consult Documentation/SubmittingPatches. The expectation is that\npatches will not be sent as attachments. I'd recommend using a tool like\nb4 to send your patches, which handles a lot of the nuisances for you.\n\n> From c5d12e97a78123965590553fddc0dd78af3e006e Mon Sep 17 00:00:00 2001\n> From: Ariel Keselman <skariel@gmail.com>\n> Date: Wed, 9 Sep 2026 22:16:50 -0700\n> To: git@vger.kernel.org\n> Subject: [PATCH] refs/files: avoid packed-refs lock for root ref deletion\n> \n> Deleting a root ref queues a packed-ref transaction in the files\n> backend, even though root refs cannot be packed. For example, holding\n> .git/packed-refs.lock makes \"git update-ref --no-deref -d AUTO_MERGE\"\n> fail, whether or not AUTO_MERGE exists.\n> \n> This also affects post-commit cleanup, which deletes AUTO_MERGE after\n> updating HEAD. In a linked worktree with read-only shared metadata,\n> commit succeeds but cleanup reports a packed-refs.lock error. Deleting\n> CHERRY_PICK_HEAD and REVERT_HEAD is affected as well.\n> \n> Skip the packed transaction for root-ref deletions. Keep loose-ref\n> locking and packed-ref deletion for other refs unchanged.\n\nMakes sense indeed. Root refs are never packed, and consequently it does\nnot make any sense for us to try to evict them from packed-refs, either.\n\n> diff --git a/refs/files-backend.c b/refs/files-backend.c\n> index a4c7858787..41887f180f 100644\n> --- a/refs/files-backend.c\n> +++ b/refs/files-backend.c\n> @@ -2981,10 +2981,13 @@ static int files_transaction_prepare(struct ref_store *ref_store,\n>  \n>  \t\tif (update->flags & REF_DELETING &&\n>  \t\t    !(update->flags & REF_LOG_ONLY) &&\n> -\t\t    !(update->flags & REF_IS_PRUNING)) {\n> +\t\t    !(update->flags & REF_IS_PRUNING) &&\n> +\t\t    !is_root_ref(update->refname)) {\n>  \t\t\t/*\n> -\t\t\t * This reference has to be deleted from\n> -\t\t\t * packed-refs if it exists there.\n> +\t\t\t * Root refs cannot be packed. Do not acquire the shared\n> +\t\t\t * packed-refs lock when deleting a per-worktree root ref.\n> +\t\t\t * Other references have to be deleted from\n> +\t\t\t * packed-refs if they exist there.\n>  \t\t\t */\n\nNit: I feel like this comment is a bit too focussed on the root refs\nnow. A small, incremental change could've been:\n\n\t/*\n     * This reference has to be deleted from packed-refs if it exists\n     * there. Note that root refs are never packed, so we don't have to\n     * deltee those from packed-refs.\n\t */\n\n>  \t\t\tif (!packed_transaction) {\n>  \t\t\t\tpacked_transaction = ref_store_transaction_begin(\n> diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh\n> index 74bfa2e9ba..b7f3287841 100755\n> --- a/t/t0600-reffiles-backend.sh\n> +++ b/t/t0600-reffiles-backend.sh\n> @@ -519,4 +519,50 @@ test_expect_success 'symref transaction supports false symlink config' '\n>  \ttest_cmp expect actual\n>  '\n>  \n> +for ref in AUTO_MERGE CHERRY_PICK_HEAD REVERT_HEAD\n\nIsn't it a bit excessive to test for all these different root refs? I\ndon't see much value in doing that.\n\n> +do\n> +\tfor state in existing missing\n\nLikewise, I'm not quite sure what we prove here. Should be fine to just\ntest with an existing root ref.\n\n> +\tdo\n> +\t\ttest_expect_success \"deleting $state $ref does not lock packed-refs\" '\n> +\t\t\ttest_when_finished \"rm -rf root-ref\" &&\n> +\t\t\tgit init root-ref &&\n> +\t\t\t(\n> +\t\t\t\tcd root-ref &&\n> +\t\t\t\ttest_commit initial &&\n> +\t\t\t\tif test \"$state\" = existing\n> +\t\t\t\tthen\n> +\t\t\t\t\tgit update-ref \"$ref\" HEAD\n> +\t\t\t\tfi &&\n> +\t\t\t\t: >.git/packed-refs.lock &&\n> +\t\t\t\tgit -c core.packedRefsTimeout=0 update-ref --no-deref -d \"$ref\" &&\n\nSetting the timeout shouldn't really have any impact on the test result,\nshould it?\n\n> +\t\t\t\ttest_path_is_missing \".git/$ref\" &&\n> +\t\t\t\ttest_path_is_file .git/packed-refs.lock\n> +\t\t\t)\n> +\t\t'\n> +\tdone\n> +done\n> +\n> +test_expect_success 'root ref deletion preserves packed refs and their locking' '\n> +\ttest_when_finished \"rm -rf root-ref\" &&\n> +\tgit init root-ref &&\n> +\t(\n> +\t\tcd root-ref &&\n> +\t\ttest_commit initial &&\n> +\t\tgit update-ref refs/heads/packed-branch HEAD &&\n> +\t\tgit pack-refs --all &&\n> +\t\ttest_path_is_missing .git/refs/heads/packed-branch &&\n> +\t\tcp .git/packed-refs expect &&\n> +\t\tgit update-ref AUTO_MERGE HEAD &&\n> +\t\t: >.git/packed-refs.lock &&\n> +\t\tgit -c core.packedRefsTimeout=0 update-ref --no-deref -d AUTO_MERGE &&\n> +\t\ttest_cmp expect .git/packed-refs &&\n> +\t\ttest_must_fail git -c core.packedRefsTimeout=0 update-ref -d refs/heads/packed-branch 2>err &&\n> +\t\ttest_grep \"Unable to create .*packed-refs.lock\" err &&\n> +\t\ttest_cmp expect .git/packed-refs &&\n> +\t\trm .git/packed-refs.lock &&\n> +\t\tgit update-ref -d refs/heads/packed-branch &&\n> +\t\ttest_must_fail git rev-parse --verify refs/heads/packed-branch\n> +\t)\n> +'\n\nAnd this test feels like it's testing almost exactly what the other\ntest does. The only difference is that we have an actual packed-refs\nfile, but that can easily be squashed into the other test, too.\n\nThat being said, what we're missing is a test that creates a single\ntransaction that updates both a root ref and a non-root-ref with a\npreexisting lockfile. Such a transaction should fail even though we skip\nthe packed transaction for the roof ref itself.\n\nThanks!\n\nPatrick\n"},{"id":"552446","messageId":"20260910145528.309340-1-skariel@gmail.com","threadId":"66303","inReplyTo":"aqJr0ZB8qpthTEGT@pks.im","subject":"[PATCH v2] refs/files: avoid packed-refs lock for root ref deletion","fromName":"Ariel Keselman","fromEmail":"skariel@gmail.com","sentAt":"2026-09-10T14:55:28Z","receivedAt":"2026-09-10T14:55:33Z","isPatch":true,"body":"Deleting a root ref queues a packed-ref transaction in the files\nbackend, even though root refs cannot be packed. For example, holding\n.git/packed-refs.lock makes \"git update-ref --no-deref -d AUTO_MERGE\"\nfail, whether or not AUTO_MERGE exists.\n\nThis also affects post-commit cleanup, which deletes AUTO_MERGE after\nupdating HEAD. In a linked worktree with read-only shared metadata,\ncommit succeeds but cleanup reports a packed-refs.lock error. Deleting\nCHERRY_PICK_HEAD and REVERT_HEAD is affected as well.\n\nSkip the packed transaction for root-ref deletions. Keep loose-ref\nlocking and packed-ref deletion for other refs unchanged.\n\nTest deleting a root ref with packed-refs.lock held, and check that a\ntransaction deleting both a root ref and a packed branch still fails\nwithout changing either ref.\n\nSigned-off-by: Ariel Keselman <skariel@gmail.com>\n---\nThanks for the review, Patrick.\n\nChanges since v1:\n- Keep the packed-ref deletion comment focused on its original purpose.\n- Use one existing root ref and include a packed-refs file in the test.\n- Drop the timeout overrides and redundant individual-ref cases.\n- Add a transaction deleting a root ref and a packed branch together;\n  check that a held packed-ref lock causes failure and preserves both refs.\n\nAI assistance was used to generate the patch, tests, and commit message,\nincluding this revision.\n\nThe root-ref deletion regression fails without the fix. With the fix,\n123 test scripts / 4078 tests pass, along with 249 unit tests and t0600\nwith SHA-256 (one platform skip in t0600).\n\n refs/files-backend.c        |  8 ++++---\n t/t0600-reffiles-backend.sh | 43 +++++++++++++++++++++++++++++++++++++\n 2 files changed, 48 insertions(+), 3 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex a4c7858787..0333d5d355 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -2981,10 +2981,12 @@ static int files_transaction_prepare(struct ref_store *ref_store,\n \n \t\tif (update->flags & REF_DELETING &&\n \t\t    !(update->flags & REF_LOG_ONLY) &&\n-\t\t    !(update->flags & REF_IS_PRUNING)) {\n+\t\t    !(update->flags & REF_IS_PRUNING) &&\n+\t\t    !is_root_ref(update->refname)) {\n \t\t\t/*\n-\t\t\t * This reference has to be deleted from\n-\t\t\t * packed-refs if it exists there.\n+\t\t\t * This reference has to be deleted from packed-refs if it\n+\t\t\t * exists there. Root refs are never packed, so we do not\n+\t\t\t * have to delete them from packed-refs.\n \t\t\t */\n \t\t\tif (!packed_transaction) {\n \t\t\t\tpacked_transaction = ref_store_transaction_begin(\ndiff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh\nindex 74bfa2e9ba..65ca19e84b 100755\n--- a/t/t0600-reffiles-backend.sh\n+++ b/t/t0600-reffiles-backend.sh\n@@ -519,4 +519,47 @@ test_expect_success 'symref transaction supports false symlink config' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'deleting a root ref does not lock packed-refs' '\n+\ttest_when_finished \"rm -rf root-ref\" &&\n+\tgit init root-ref &&\n+\t(\n+\t\tcd root-ref &&\n+\t\ttest_commit initial &&\n+\t\tgit pack-refs --all &&\n+\t\tcp .git/packed-refs expect &&\n+\t\tgit update-ref AUTO_MERGE HEAD &&\n+\t\t: >.git/packed-refs.lock &&\n+\t\tgit update-ref --no-deref -d AUTO_MERGE &&\n+\t\ttest_path_is_missing .git/AUTO_MERGE &&\n+\t\ttest_path_is_file .git/packed-refs.lock &&\n+\t\ttest_cmp expect .git/packed-refs\n+\t)\n+'\n+\n+test_expect_success 'deleting root and packed refs in one transaction requires packed-refs lock' '\n+\ttest_when_finished \"rm -rf root-ref\" &&\n+\tgit init root-ref &&\n+\t(\n+\t\tcd root-ref &&\n+\t\ttest_commit initial &&\n+\t\tgit update-ref refs/heads/packed-branch HEAD &&\n+\t\tgit pack-refs --all &&\n+\t\ttest_path_is_missing .git/refs/heads/packed-branch &&\n+\t\tgit update-ref AUTO_MERGE HEAD &&\n+\t\tgit rev-parse AUTO_MERGE refs/heads/packed-branch >expect &&\n+\t\tcat >stdin <<-EOF &&\n+\t\tstart\n+\t\tdelete AUTO_MERGE\n+\t\tdelete refs/heads/packed-branch\n+\t\tprepare\n+\t\tcommit\n+\t\tEOF\n+\t\t: >.git/packed-refs.lock &&\n+\t\ttest_must_fail git update-ref --no-deref --stdin <stdin 2>err &&\n+\t\ttest_grep \"Unable to create .*packed-refs.lock\" err &&\n+\t\tgit rev-parse AUTO_MERGE refs/heads/packed-branch >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n test_done\n-- \n2.55.0\n\n"},{"id":"552525","messageId":"aqO4OukzSa4SWcGG@pks.im","threadId":"66303","inReplyTo":"20260910145528.309340-1-skariel@gmail.com","subject":"Re: [PATCH v2] refs/files: avoid packed-refs lock for root ref deletion","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-11T08:13:46Z","receivedAt":"2026-09-11T08:13:51Z","isPatch":true,"body":"On Thu, Sep 10, 2026 at 07:55:28AM -0700, Ariel Keselman wrote:\n> Deleting a root ref queues a packed-ref transaction in the files\n> backend, even though root refs cannot be packed. For example, holding\n> .git/packed-refs.lock makes \"git update-ref --no-deref -d AUTO_MERGE\"\n> fail, whether or not AUTO_MERGE exists.\n> \n> This also affects post-commit cleanup, which deletes AUTO_MERGE after\n> updating HEAD. In a linked worktree with read-only shared metadata,\n> commit succeeds but cleanup reports a packed-refs.lock error. Deleting\n> CHERRY_PICK_HEAD and REVERT_HEAD is affected as well.\n> \n> Skip the packed transaction for root-ref deletions. Keep loose-ref\n> locking and packed-ref deletion for other refs unchanged.\n> \n> Test deleting a root ref with packed-refs.lock held, and check that a\n> transaction deleting both a root ref and a packed branch still fails\n> without changing either ref.\n\nNit: this last paragraph doesn't really add any value, as it's trivially\nvisible from the patch that we add tests.\n\n> Signed-off-by: Ariel Keselman <skariel@gmail.com>\n> ---\n> Thanks for the review, Patrick.\n> \n> Changes since v1:\n> - Keep the packed-ref deletion comment focused on its original purpose.\n> - Use one existing root ref and include a packed-refs file in the test.\n> - Drop the timeout overrides and redundant individual-ref cases.\n> - Add a transaction deleting a root ref and a packed branch together;\n>   check that a held packed-ref lock causes failure and preserves both refs.\n> \n> AI assistance was used to generate the patch, tests, and commit message,\n> including this revision.\n> \n> The root-ref deletion regression fails without the fix. With the fix,\n> 123 test scripts / 4078 tests pass, along with 249 unit tests and t0600\n> with SHA-256 (one platform skip in t0600).\n\nHuh? I hope that _all_ tests pass with this, not only 4078, and I would\nassume that you verified that this is the case at least on your machine.\n\n> diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh\n> index 74bfa2e9ba..65ca19e84b 100755\n> --- a/t/t0600-reffiles-backend.sh\n> +++ b/t/t0600-reffiles-backend.sh\n> @@ -519,4 +519,47 @@ test_expect_success 'symref transaction supports false symlink config' '\n>  \ttest_cmp expect actual\n>  '\n>  \n> +test_expect_success 'deleting a root ref does not lock packed-refs' '\n> +\ttest_when_finished \"rm -rf root-ref\" &&\n> +\tgit init root-ref &&\n> +\t(\n> +\t\tcd root-ref &&\n> +\t\ttest_commit initial &&\n> +\t\tgit pack-refs --all &&\n> +\t\tcp .git/packed-refs expect &&\n> +\t\tgit update-ref AUTO_MERGE HEAD &&\n\nFor added benefit we could even execute git-pack-refs(1) after having\ncreated AUTO_MERGE and then execute `test_path_is_file` for it just to\nprove that it really doesn't get packed. But other than that the tests\nlook good to me.\n\n> +\t\t: >.git/packed-refs.lock &&\n> +\t\tgit update-ref --no-deref -d AUTO_MERGE &&\n> +\t\ttest_path_is_missing .git/AUTO_MERGE &&\n> +\t\ttest_path_is_file .git/packed-refs.lock &&\n> +\t\ttest_cmp expect .git/packed-refs\n> +\t)\n> +'\n> +\n> +test_expect_success 'deleting root and packed refs in one transaction requires packed-refs lock' '\n> +\ttest_when_finished \"rm -rf root-ref\" &&\n> +\tgit init root-ref &&\n> +\t(\n> +\t\tcd root-ref &&\n> +\t\ttest_commit initial &&\n> +\t\tgit update-ref refs/heads/packed-branch HEAD &&\n> +\t\tgit pack-refs --all &&\n> +\t\ttest_path_is_missing .git/refs/heads/packed-branch &&\n> +\t\tgit update-ref AUTO_MERGE HEAD &&\n> +\t\tgit rev-parse AUTO_MERGE refs/heads/packed-branch >expect &&\n\nWe could strengthen this a bit by listing the state of all refs:\n\n    git refs list --include-root-refs >expect\n\n> +\t\tcat >stdin <<-EOF &&\n> +\t\tstart\n> +\t\tdelete AUTO_MERGE\n> +\t\tdelete refs/heads/packed-branch\n> +\t\tprepare\n> +\t\tcommit\n\nWe can drop start/prepare/commit here, those are optional. We can also\ndrop the extra file and just write the data into git-update-ref(1)\ndirectly via the heredoc.\n\n> +\t\tEOF\n> +\t\t: >.git/packed-refs.lock &&\n> +\t\ttest_must_fail git update-ref --no-deref --stdin <stdin 2>err &&\n> +\t\ttest_grep \"Unable to create .*packed-refs.lock\" err &&\n> +\t\tgit rev-parse AUTO_MERGE refs/heads/packed-branch >actual &&\n> +\t\ttest_cmp expect actual\n> +\t)\n> +'\n\nThanks!\n\nPatrick\n"},{"id":"552618","messageId":"CAMuXvLCN9V7SHm-wWpZK6eFF6DzNr57Dj5=KpiFWK51a83duOg@mail.gmail.com","threadId":"66303","inReplyTo":"aqO4OukzSa4SWcGG@pks.im","subject":"Re: [PATCH v2] refs/files: avoid packed-refs lock for root ref deletion","fromName":"Ariel Keselman","fromEmail":"skariel@gmail.com","sentAt":"2026-09-12T02:12:22Z","receivedAt":"2026-09-12T02:12:34Z","isPatch":true,"body":"Sorry, I accidentally sent v3 as a new thread. It is available here:\n\nhttps://lore.kernel.org/git/20260912014609.535922-1-skariel@gmail.com/\n\n\nOn Fri, Sep 11, 2026 at 1:13 AM Patrick Steinhardt <ps@pks.im> wrote:\n>\n> On Thu, Sep 10, 2026 at 07:55:28AM -0700, Ariel Keselman wrote:\n> > Deleting a root ref queues a packed-ref transaction in the files\n> > backend, even though root refs cannot be packed. For example, holding\n> > .git/packed-refs.lock makes \"git update-ref --no-deref -d AUTO_MERGE\"\n> > fail, whether or not AUTO_MERGE exists.\n> >\n> > This also affects post-commit cleanup, which deletes AUTO_MERGE after\n> > updating HEAD. In a linked worktree with read-only shared metadata,\n> > commit succeeds but cleanup reports a packed-refs.lock error. Deleting\n> > CHERRY_PICK_HEAD and REVERT_HEAD is affected as well.\n> >\n> > Skip the packed transaction for root-ref deletions. Keep loose-ref\n> > locking and packed-ref deletion for other refs unchanged.\n> >\n> > Test deleting a root ref with packed-refs.lock held, and check that a\n> > transaction deleting both a root ref and a packed branch still fails\n> > without changing either ref.\n>\n> Nit: this last paragraph doesn't really add any value, as it's trivially\n> visible from the patch that we add tests.\n>\n> > Signed-off-by: Ariel Keselman <skariel@gmail.com>\n> > ---\n> > Thanks for the review, Patrick.\n> >\n> > Changes since v1:\n> > - Keep the packed-ref deletion comment focused on its original purpose.\n> > - Use one existing root ref and include a packed-refs file in the test.\n> > - Drop the timeout overrides and redundant individual-ref cases.\n> > - Add a transaction deleting a root ref and a packed branch together;\n> >   check that a held packed-ref lock causes failure and preserves both refs.\n> >\n> > AI assistance was used to generate the patch, tests, and commit message,\n> > including this revision.\n> >\n> > The root-ref deletion regression fails without the fix. With the fix,\n> > 123 test scripts / 4078 tests pass, along with 249 unit tests and t0600\n> > with SHA-256 (one platform skip in t0600).\n>\n> Huh? I hope that _all_ tests pass with this, not only 4078, and I would\n> assume that you verified that this is the case at least on your machine.\n>\n> > diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh\n> > index 74bfa2e9ba..65ca19e84b 100755\n> > --- a/t/t0600-reffiles-backend.sh\n> > +++ b/t/t0600-reffiles-backend.sh\n> > @@ -519,4 +519,47 @@ test_expect_success 'symref transaction supports false symlink config' '\n> >       test_cmp expect actual\n> >  '\n> >\n> > +test_expect_success 'deleting a root ref does not lock packed-refs' '\n> > +     test_when_finished \"rm -rf root-ref\" &&\n> > +     git init root-ref &&\n> > +     (\n> > +             cd root-ref &&\n> > +             test_commit initial &&\n> > +             git pack-refs --all &&\n> > +             cp .git/packed-refs expect &&\n> > +             git update-ref AUTO_MERGE HEAD &&\n>\n> For added benefit we could even execute git-pack-refs(1) after having\n> created AUTO_MERGE and then execute `test_path_is_file` for it just to\n> prove that it really doesn't get packed. But other than that the tests\n> look good to me.\n>\n> > +             : >.git/packed-refs.lock &&\n> > +             git update-ref --no-deref -d AUTO_MERGE &&\n> > +             test_path_is_missing .git/AUTO_MERGE &&\n> > +             test_path_is_file .git/packed-refs.lock &&\n> > +             test_cmp expect .git/packed-refs\n> > +     )\n> > +'\n> > +\n> > +test_expect_success 'deleting root and packed refs in one transaction requires packed-refs lock' '\n> > +     test_when_finished \"rm -rf root-ref\" &&\n> > +     git init root-ref &&\n> > +     (\n> > +             cd root-ref &&\n> > +             test_commit initial &&\n> > +             git update-ref refs/heads/packed-branch HEAD &&\n> > +             git pack-refs --all &&\n> > +             test_path_is_missing .git/refs/heads/packed-branch &&\n> > +             git update-ref AUTO_MERGE HEAD &&\n> > +             git rev-parse AUTO_MERGE refs/heads/packed-branch >expect &&\n>\n> We could strengthen this a bit by listing the state of all refs:\n>\n>     git refs list --include-root-refs >expect\n>\n> > +             cat >stdin <<-EOF &&\n> > +             start\n> > +             delete AUTO_MERGE\n> > +             delete refs/heads/packed-branch\n> > +             prepare\n> > +             commit\n>\n> We can drop start/prepare/commit here, those are optional. We can also\n> drop the extra file and just write the data into git-update-ref(1)\n> directly via the heredoc.\n>\n> > +             EOF\n> > +             : >.git/packed-refs.lock &&\n> > +             test_must_fail git update-ref --no-deref --stdin <stdin 2>err &&\n> > +             test_grep \"Unable to create .*packed-refs.lock\" err &&\n> > +             git rev-parse AUTO_MERGE refs/heads/packed-branch >actual &&\n> > +             test_cmp expect actual\n> > +     )\n> > +'\n>\n> Thanks!\n>\n> Patrick\n"}]}