Hi, this is the third version of this patch series, which addresses an issue where the reference-transaction hook is being invoked twice when deleting refs both in the packed-refs and loose-refs file. The following things changed in comparison to v2: - Fixed some typos in commit messages. - Improved the subject of the first patch to more clearly highlight the purpose of it, not only say what the patch does. - Fixed a missing declaration for `struct string_list`. - Clarified why the last patch does the right thing even in case the ref only exists in the packed-refs backend, and in case it doesn't exist in either of the backends. Thanks for your feedback! Patrick Patrick Steinhardt (6): refs: extract packed_refs_delete_refs() to allow control of transaction refs: allow passing flags when beginning transactions refs: allow skipping the reference-transaction hook refs: demonstrate excessive execution of the reference-transaction hook refs: do not execute reference-transaction hook on packing refs refs: skip hooks when deleting uncovered packed refs refs.c | 11 +++++-- refs.h | 8 ++++- refs/files-backend.c | 25 +++++++++++----- refs/packed-backend.c | 30 ++++++++++++++----- refs/packed-backend.h | 7 +++++ refs/refs-internal.h | 1 + sequencer.c | 2 +- t/t1416-ref-transaction-hooks.sh | 50 ++++++++++++++++++++++++++++++++ 8 files changed, 114 insertions(+), 20 deletions(-) Range-diff against v2: 1: 0739f085b2 ! 1: abbc28822b refs: open-code deletion of packed refs @@ Metadata Author: Patrick Steinhardt ## Commit message ## - refs: open-code deletion of packed refs + refs: extract packed_refs_delete_refs() to allow control of transaction When deleting loose refs, then we also have to delete the refs in the packed backend. This is done by calling `refs_delete_refs()`, which @@ Commit message Extract a new function `packed_refs_delete_refs()`, which hosts most of the logic to delete refs except for creating the transaction itself. - Like this, we can easily create the transactionion in the files backend + Like this, we can easily create the transaction in the files backend and thus exert more control over it. Signed-off-by: Patrick Steinhardt @@ refs/packed-backend.c: static int packed_delete_refs(struct ref_store *ref_store } ## refs/packed-backend.h ## +@@ + + struct repository; + struct ref_transaction; ++struct string_list; + + /* + * Support for storing references in a `packed-refs` file. @@ refs/packed-backend.h: int packed_refs_lock(struct ref_store *ref_store, int flags, struct strbuf *err) void packed_refs_unlock(struct ref_store *ref_store); int packed_refs_is_locked(struct ref_store *ref_store); 2: 629be01d50 = 2: 9dd172a757 refs: allow passing flags when beginning transactions 3: 550d89a323 = 3: be826bae3b refs: allow skipping the reference-transaction hook 4: b52e59cdac ! 4: 662a6e6244 refs: demonstrate excessive execution of the reference-transaction hook @@ Metadata ## Commit message ## refs: demonstrate excessive execution of the reference-transaction hook - Add tests which demonstate which demonstrates that we're executing the + Add tests which demonstate that we're executing the reference-transaction hook too often in some cases, which thus leaks implementation details about the reference store's implementation itself. Behaviour will be fixed in follow-up commits. 5: 1539e9711f = 5: d83f309b9c refs: do not execute reference-transaction hook on packing refs 6: 0fbf68dbf4 ! 6: 279eadc41c refs: skip hooks when deleting uncovered packed refs @@ Commit message Fix this behaviour and don't execute the reference-transaction hook at all when refs in the packed-refs backend if it's driven by the files - backend. + backend. This works as expected even in case the refs to be deleted only + exist in the packed-refs backend because the loose-backend always queues + refs in its own transaction even if they don't exist such that they can + be locked for concurrent creation. And it also does the right thing in + case neither of the backends has the ref because that would cause the + transaction to fail completely. Signed-off-by: Patrick Steinhardt -- 2.34.1