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

[PATCH 4/6] refs: demonstrate excessive execution of the reference-transaction hook

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

Add tests which demonstate which demonstrates 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.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t1416-ref-transaction-hooks.sh | 64 ++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)
diff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh
index 6c941027a8..0567fbdf0b 100755
--- a/t/t1416-ref-transaction-hooks.sh
+++ b/t/t1416-ref-transaction-hooks.sh
@@ -136,4 +136,68 @@ test_expect_success 'interleaving hook calls succeed' '
 	test_cmp expect target-repo.git/actual
 '
 
+test_expect_success 'hook does not get called on packing refs' '
+	# Pack references first such that we are in a known state.
+	git pack-refs --all &&
+
+	write_script .git/hooks/reference-transaction <<-\EOF &&
+		echo "$@" >>actual
+		cat >>actual
+	EOF
+	rm -f actual &&
+
+	git update-ref refs/heads/unpacked-ref $POST_OID &&
+	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.
+	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
+'
+
+test_expect_success 'deleting packed ref calls hook once' '
+	# Create a reference and pack it.
+	git update-ref refs/heads/to-be-deleted $POST_OID &&
+	git pack-refs --all &&
+
+	write_script .git/hooks/reference-transaction <<-\EOF &&
+		echo "$@" >>actual
+		cat >>actual
+	EOF
+	rm -f actual &&
+
+	git update-ref -d refs/heads/to-be-deleted $POST_OID &&
+
+	# We only expect a single hook invocation, which is the logical
+	# deletion. But currently, we see two interleaving transactions, once
+	# for deleting the loose refs and once for deleting the packed ref.
+	cat >expect <<-EOF &&
+		prepared
+		$ZERO_OID $ZERO_OID refs/heads/to-be-deleted
+		prepared
+		$POST_OID $ZERO_OID refs/heads/to-be-deleted
+		committed
+		$ZERO_OID $ZERO_OID refs/heads/to-be-deleted
+		committed
+		$POST_OID $ZERO_OID refs/heads/to-be-deleted
+	EOF
+
+	test_cmp expect actual
+'
+
 test_done
-- 
2.34.1
Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 5 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.