refs: excessive hook execution with packed refs

Hi,

this is the fourth 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 v3:

- Fixed a memory leak in files_delete_refs().

- Refactored the packed_downcast() invocation such that we don't have to mark its unused returned variable as used.

- Removed a spurious whitespace change.

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             | 26 ++++++++++++-----
 refs/packed-backend.c            | 28 +++++++++++++-----
 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(+), 19 deletions(-)

Range-diff against v3:
1: abbc28822b ! 1: 14775046e1 refs: extract packed_refs_delete_refs() to allow control of transaction
@@ refs/files-backend.c: static int files_delete_refs(struct ref_store *ref_store,

packed_refs_unlock(refs->packed_ref_store);

+@@ refs/files-backend.c: static int files_delete_refs(struct ref_store *ref_store, const char *msg,
+ result |= error(_("could not remove reference %s"), refname);
+ }
+
++ ref_transaction_free(transaction);
+ strbuf_release(&err);
+ return result;
+
@@ refs/files-backend.c: static int files_delete_refs(struct ref_store *ref_store, const char *msg,
else
error(_("could not delete references: %s"), err.buf);
@@ refs/packed-backend.c: static int packed_delete_refs(struct ref_store *ref_store
+ struct string_list *refnames,
+ unsigned int flags)
+{
-+ struct packed_ref_store *refs =
-+ packed_downcast(ref_store, REF_STORE_WRITE, "delete_refs");
+ struct strbuf err = STRBUF_INIT;
+ struct string_list_item *item;
+ int ret;
+
-+ (void)(refs); /* We need the check above, but don't use the variable */
++ /* Assert that the ref store refers to a packed backend. */
++ packed_downcast(ref_store, REF_STORE_WRITE, "delete_refs");
+
for_each_string_list_item(item, refnames) {
if (ref_transaction_delete(transaction, item->string, NULL,
flags, msg, &err)) {
-@@ refs/packed-backend.c: static int packed_delete_refs(struct ref_store *ref_store, const char *msg,
- }
-
- ret = ref_transaction_commit(transaction, &err);
--
- if (ret) {
- if (refnames->nr == 1)
- error(_("could not delete reference %s: %s"),
@@ refs/packed-backend.c: static int packed_delete_refs(struct ref_store *ref_store, const char *msg,
error(_("could not delete references: %s"), err.buf);
}

2:  9dd172a757 = 2:  d4ac24c8b8 refs: allow passing flags when beginning transactions
3:  be826bae3b = 3:  f4a07fe9a8 refs: allow skipping the reference-transaction hook
4:  662a6e6244 = 4:  a8981baef7 refs: demonstrate excessive execution of the reference-transaction hook
5:  d83f309b9c = 5:  23c344854e refs: do not execute reference-transaction hook on packing refs
6:  279eadc41c = 6:  d6c7d765af refs: skip hooks when deleting uncovered packed refs

2.34.1