git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 5/6] refs: do not execute reference-transaction hook on packing refs

From
Patrick Steinhardt <ps@pks.im>
Date
Dec 7, 2021, 10:56 UTC
Message-ID
<e2120adf57f0c8b00a9ee66faad01325300a5254.1638874287.git.ps@pks.im>
In-Reply-To
<cover.1638874287.git.ps@pks.im>

The reference-transaction hook is supposed to track logical changes to references, but it currently also gets executed when packing refs in a repository. This is unexpected and ultimately not all that useful: packing refs is not supposed to result in any user-visible change to the refs' state, and it ultimately is an implementation detail of how refs stores work.

Fix this excessive execution of the hook when packing refs.
Reported-by: Waleed Khan <me@waleedkhan.name>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 refs/files-backend.c             |  6 ++++--
 t/t1416-ref-transaction-hooks.sh | 11 +----------
 2 files changed, 5 insertions(+), 12 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 95583dcd00..f2bc72f81b 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1122,7 +1122,8 @@ static void prune_ref(struct files_ref_store *refs, struct ref_to_prune *r)
 	if (check_refname_format(r->name, 0))
 		return;
 
-	transaction = ref_store_transaction_begin(&refs->base, 0, &err);
+	transaction = ref_store_transaction_begin(&refs->base,
+						  REF_TRANSACTION_SKIP_HOOK, &err);
 	if (!transaction)
 		goto cleanup;
 	ref_transaction_add_update(
@@ -1193,7 +1194,8 @@ static int files_pack_refs(struct ref_store *ref_store, unsigned int flags)
 	struct strbuf err = STRBUF_INIT;
 	struct ref_transaction *transaction;
 
-	transaction = ref_store_transaction_begin(refs->packed_ref_store, 0, &err);
+	transaction = ref_store_transaction_begin(refs->packed_ref_store,
+						  REF_TRANSACTION_SKIP_HOOK, &err);
 	if (!transaction)
 		return -1;
 
diff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh
index 0567fbdf0b..f9d3d5213f 100755
--- a/t/t1416-ref-transaction-hooks.sh
+++ b/t/t1416-ref-transaction-hooks.sh
@@ -150,21 +150,12 @@ test_expect_success 'hook does not get called on packing refs' '
 	git pack-refs --all &&
 
 	# We only expect a single hook invocation, which is the call to
-	# git-update-ref(1). But currently, packing refs will also trigger the
-	# hook.
+	# git-update-ref(1).
 	cat >expect <<-EOF &&
 		prepared
 		$ZERO_OID $POST_OID refs/heads/unpacked-ref
 		committed
 		$ZERO_OID $POST_OID refs/heads/unpacked-ref
-		prepared
-		$ZERO_OID $POST_OID refs/heads/unpacked-ref
-		committed
-		$ZERO_OID $POST_OID refs/heads/unpacked-ref
-		prepared
-		$POST_OID $ZERO_OID refs/heads/unpacked-ref
-		committed
-		$POST_OID $ZERO_OID refs/heads/unpacked-ref
 	EOF
 
 	test_cmp expect actual
-- 
2.34.1
Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 6 of 51 in “refs: excessive hook execution with packed refs”
  1. 0/6 refs: excessive hook execution with packed refsPatrick Steinhardt, Dec 7, 2021
  2. 1/6 refs: open-code deletion of packed refsPatrick Steinhardt, Dec 7, 2021
  3. 2/6 refs: allow passing flags when beginning transactionsPatrick Steinhardt, Dec 7, 2021
  4. 3/6 refs: allow skipping the reference-transaction hookPatrick Steinhardt, Dec 7, 2021
  5. 4/6 refs: demonstrate excessive execution of the reference-transaction hookPatrick Steinhardt, Dec 7, 2021
  6. 5/6 refs: do not execute reference-transaction hook on packing refsPatrick Steinhardt, Dec 7, 2021
  7. 6/6 refs: skip hooks when deleting uncovered packed refsPatrick Steinhardt, Dec 7, 2021
  8. 0/6 refs: excessive hook execution with packed refsPatrick Steinhardt, Jan 7, 2022
  9. 1/6 refs: open-code deletion of packed refsPatrick Steinhardt, Jan 7, 2022
  10. Junio C HamanoJan 8, 2022
  11. Junio C HamanoJan 8, 2022
  12. Junio C HamanoJan 13, 2022
  13. 2/6 refs: allow passing flags when beginning transactionsPatrick Steinhardt, Jan 7, 2022
  14. Junio C HamanoJan 8, 2022
  15. 3/6 refs: allow skipping the reference-transaction hookPatrick Steinhardt, Jan 7, 2022
  16. 4/6 refs: demonstrate excessive execution of the reference-transaction hookPatrick Steinhardt, Jan 7, 2022
  17. Junio C HamanoJan 8, 2022
  18. Patrick SteinhardtJan 10, 2022
  19. Eric SunshineJan 8, 2022
  20. 6/6 refs: skip hooks when deleting uncovered packed refsPatrick Steinhardt, Jan 7, 2022
  21. Junio C HamanoJan 8, 2022
  22. Patrick SteinhardtJan 10, 2022
  23. Junio C HamanoJan 10, 2022
  24. 5/6 refs: do not execute reference-transaction hook on packing refsPatrick Steinhardt, Jan 7, 2022
  25. Junio C HamanoJan 7, 2022
  26. Han-Wen NienhuysJan 13, 2022
  27. Patrick SteinhardtJan 17, 2022
  28. Han-Wen NienhuysJan 17, 2022
  29. Patrick SteinhardtJan 24, 2022
  30. 0/6 refs: excessive hook execution with packed refsPatrick Steinhardt, Jan 13, 2022
  31. 1/6 refs: extract packed_refs_delete_refs() to allow control of transactionPatrick Steinhardt, Jan 13, 2022
  32. Ævar Arnfjörð BjarmasonJan 13, 2022
  33. Patrick SteinhardtJan 17, 2022
  34. 2/6 refs: allow passing flags when beginning transactionsPatrick Steinhardt, Jan 13, 2022
  35. 3/6 refs: allow skipping the reference-transaction hookPatrick Steinhardt, Jan 13, 2022
  36. Ævar Arnfjörð BjarmasonJan 13, 2022
  37. Patrick SteinhardtJan 17, 2022
  38. 4/6 refs: demonstrate excessive execution of the reference-transaction hookPatrick Steinhardt, Jan 13, 2022
  39. 5/6 refs: do not execute reference-transaction hook on packing refsPatrick Steinhardt, Jan 13, 2022
  40. Ævar Arnfjörð BjarmasonJan 13, 2022
  41. Patrick SteinhardtJan 17, 2022
  42. 6/6 refs: skip hooks when deleting uncovered packed refsPatrick Steinhardt, Jan 13, 2022
  43. Ævar Arnfjörð BjarmasonJan 13, 2022
  44. Patrick SteinhardtJan 17, 2022
  45. 0/6 refs: excessive hook execution with packed refsPatrick Steinhardt, Jan 17, 2022
  46. 1/6 refs: extract packed_refs_delete_refs() to allow control of transactionPatrick Steinhardt, Jan 17, 2022
  47. 2/6 refs: allow passing flags when beginning transactionsPatrick Steinhardt, Jan 17, 2022
  48. 3/6 refs: allow skipping the reference-transaction hookPatrick Steinhardt, Jan 17, 2022
  49. 4/6 refs: demonstrate excessive execution of the reference-transaction hookPatrick Steinhardt, Jan 17, 2022
  50. 5/6 refs: do not execute reference-transaction hook on packing refsPatrick Steinhardt, Jan 17, 2022
  51. 6/6 refs: skip hooks when deleting uncovered packed refsPatrick Steinhardt, Jan 17, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.