diff mbox series

[v2,7/7] refs: support symrefs in 'reference-transaction' hook

Message ID 20240412095908.1134387-8-knayak@gitlab.com (mailing list archive)
State New
Headers show
Series update-ref: add symref oriented commands | expand

Commit Message

Karthik Nayak April 12, 2024, 9:59 a.m. UTC 
From: Karthik Nayak <karthik.188@gmail.com>

The 'reference-transaction' hook runs whenever a reference update is
made to the system. In the previous commits, we added support for
various symref commands in `git-update-ref`. While it allowed us to now
manipulate symbolic refs via `git-update-ref`, it didn't activate the
'reference-transaction' hook.

Let's activate the hook for symbolic reference updates too. There is no
new format described for this and we stick to the existing format of:
    <old-value> SP <new-value> SP <ref-name> LF
but now, <old-value> and <new-value> could also denote references
instead of objects.

While this seems to be backward incompatible, it is okay, since the only
way the `reference-transaction` hook has refs in its output is when
`git-update-ref` is used with `update-symref` command. Also the
documentation for reference-transaction hook always stated that support
for symbolic references may be added in the future.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---

We did initially discuss the possibility of having {symref-<command>} as
the prefix [1]. But this information is not propagated through the system
and we don't know which command is actually registering at the hook. We
could use flags for propagating this, but this simply complicates things.

I then thought of adding 'symref' prefix to the symref updates. But I
realized that that is no different from not having the symref prefix, because
even 'symref' prefix entries received by the hook as input could contain
either object IDs or refnames, like updating a regular ref to a symref. So
the hook needs to dynamically read inputs anyways. So might as well keep the
current syntax.

[1]: https://lore.kernel.org/git/CAOLa=ZTLv39b4Q=AAUA39tXKgOSuu54xk3-r9OUenzxR-6qcag@mail.gmail.com/

 Documentation/githooks.txt       | 13 ++++++----
 refs.c                           | 17 ++++++++-----
 t/t1416-ref-transaction-hooks.sh | 41 ++++++++++++++++++++++++++++++++
 3 files changed, 60 insertions(+), 11 deletions(-)
diff mbox series

Patch

diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index ee9b92c90d..1db5ab02d6 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -486,7 +486,7 @@  reference-transaction
 This hook is invoked by any Git command that performs reference
 updates. It executes whenever a reference transaction is prepared,
 committed or aborted and may thus get called multiple times. The hook
-does not cover symbolic references (but that may change in the future).
+also cover symbolic references.
 
 The hook takes exactly one argument, which is the current state the
 given reference transaction is in:
@@ -503,16 +503,19 @@  given reference transaction is in:
 For each reference update that was added to the transaction, the hook
 receives on standard input a line of the format:
 
-  <old-oid> SP <new-oid> SP <ref-name> LF
+  <old-value> SP <new-value> SP <ref-name> LF
 
-where `<old-oid>` is the old object name passed into the reference
-transaction, `<new-oid>` is the new object name to be stored in the
+where `<old-value>` is the old object name passed into the reference
+transaction, `<new-value>` is the new object name to be stored in the
 ref and `<ref-name>` is the full name of the ref. When force updating
 the reference regardless of its current value or when the reference is
-to be created anew, `<old-oid>` is the all-zeroes object name. To
+to be created anew, `<old-value>` is the all-zeroes object name. To
 distinguish these cases, you can inspect the current value of
 `<ref-name>` via `git rev-parse`.
 
+For symbolic reference updates the `<old_value>` and `<new-value>`
+fields could denote references instead of objects.
+
 The exit status of the hook is ignored for any state except for the
 "prepared" state. In the "prepared" state, a non-zero exit status will
 cause the transaction to be aborted. The hook will not be called with
diff --git a/refs.c b/refs.c
index 31c09c3317..010f426def 100644
--- a/refs.c
+++ b/refs.c
@@ -2362,15 +2362,20 @@  static int run_transaction_hook(struct ref_transaction *transaction,
 
 	for (i = 0; i < transaction->nr; i++) {
 		struct ref_update *update = transaction->updates[i];
+		const char *new_value, *old_value;
 
-		if (update->flags & REF_SYMREF_UPDATE)
-			continue;
+		new_value = oid_to_hex(&update->new_oid);
+		old_value = oid_to_hex(&update->old_oid);
+
+		if (update->flags & REF_SYMREF_UPDATE) {
+			if (update->flags & REF_HAVE_NEW && !null_new_value(update))
+				new_value = update->new_ref;
+			if (update->flags & REF_HAVE_OLD && update->old_ref)
+				old_value = update->old_ref;
+		}
 
 		strbuf_reset(&buf);
-		strbuf_addf(&buf, "%s %s %s\n",
-			    oid_to_hex(&update->old_oid),
-			    oid_to_hex(&update->new_oid),
-			    update->refname);
+		strbuf_addf(&buf, "%s %s %s\n", old_value, new_value, update->refname);
 
 		if (write_in_full(proc.in, buf.buf, buf.len) < 0) {
 			if (errno != EPIPE) {
diff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh
index 2092488090..104e2c5da4 100755
--- a/t/t1416-ref-transaction-hooks.sh
+++ b/t/t1416-ref-transaction-hooks.sh
@@ -108,6 +108,10 @@  test_expect_success 'hook gets all queued updates in aborted state' '
 	test_cmp expect actual
 '
 
+# This test doesn't add a check for 'symref-delete' since there is a
+# variation between the ref backends WRT 'delete'. In the files backend,
+# 'delete' also triggers an additional transaction update on the
+# packed-refs backend, which constitutes additional reflog entries.
 test_expect_success 'interleaving hook calls succeed' '
 	test_when_finished "rm -r target-repo.git" &&
 
@@ -134,4 +138,41 @@  test_expect_success 'interleaving hook calls succeed' '
 	test_cmp expect target-repo.git/actual
 '
 
+test_expect_success 'hook gets all queued symref updates' '
+	test_when_finished "rm actual" &&
+
+	git update-ref refs/heads/branch $POST_OID &&
+	git symbolic-ref refs/heads/symref refs/heads/main &&
+	git symbolic-ref refs/heads/symrefu refs/heads/main &&
+
+	test_hook reference-transaction <<-\EOF &&
+		echo "$*" >>actual
+		while read -r line
+		do
+			printf "%s\n" "$line"
+		done >>actual
+	EOF
+
+	cat >expect <<-EOF &&
+		prepared
+		refs/heads/main $ZERO_OID refs/heads/symref
+		$ZERO_OID refs/heads/main refs/heads/symrefc
+		refs/heads/main refs/heads/branch refs/heads/symrefu
+		committed
+		refs/heads/main $ZERO_OID refs/heads/symref
+		$ZERO_OID refs/heads/main refs/heads/symrefc
+		refs/heads/main refs/heads/branch refs/heads/symrefu
+	EOF
+
+	git update-ref --no-deref --stdin <<-EOF &&
+		start
+		symref-verify refs/heads/symref refs/heads/main
+		symref-create refs/heads/symrefc refs/heads/main
+		symref-update refs/heads/symrefu refs/heads/branch refs/heads/main
+		prepare
+		commit
+	EOF
+	test_cmp expect actual
+'
+
 test_done