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

[PATCH v6 1/1] refs: report old values to transaction hooks

From
Maciej Ciemborowicz <maciej.ciemborowicz@gmail.com>
Date
Sep 24, 2026, 22:33 UTC
Message-ID
<2af3eeadd18806c5298d53072428656885cff89d.1790269745.git.maciej.ciemborowicz@gmail.com>
In-Reply-To
<cover.1790269745.git.maciej.ciemborowicz@gmail.com>

The reference-transaction hook reports an all-zero old object ID whenever the caller does not supply an expected old value. Consequently, batched branch, tag, and remote-ref deletions report zero as both the old and new object IDs because refs_delete_refs() intentionally queues unconditional deletions.

Changing those callers to provide expected old values would make the deletions conditional and alter existing command behavior. Instead, record the current raw ref value separately for the hook. Read it before the "preparing" hook, then refresh it after the backend has locked the refs so that the "prepared" and later phases report the value protected by the transaction's locks. Keep this value separate from old_oid and old_target so it does not set REF_HAVE_OLD or otherwise constrain the update.

Only resolve these values when a reference-transaction hook exists. Preserve symbolic refs as targets, consistent with the hook's existing symref format. Document that an unlocked "preparing" value may differ from later phases if the ref changes before it is locked.

Add coverage for batched branch deletion, tag deletion, and remote pruning. Also exercise a concurrent update from the "preparing" hook to verify that the deletion remains unconditional while later hook phases report the value actually removed.

Signed-off-by: Maciej Ciemborowicz <maciej.ciemborowicz@gmail.com>
---
 Documentation/githooks.adoc      | 17 +++++----
 refs.c                           | 54 ++++++++++++++++++++++++---
 refs/refs-internal.h             |  8 ++++
 t/t1416-ref-transaction-hooks.sh | 64 +++++++++++++++++++++++++++++++-
 4 files changed, 128 insertions(+), 15 deletions(-)
diff --git a/Documentation/githooks.adoc b/Documentation/githooks.adoc
index ed045940d1..f60dd1d582 100644
--- a/Documentation/githooks.adoc
+++ b/Documentation/githooks.adoc
@@ -509,14 +509,15 @@ receives on standard input a line of the format:
   <old-value> SP <new-value> SP <ref-name> LF
 
 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-value>` is the all-zeroes object name. To
-distinguish these cases, you can inspect the current value of
-`<ref-name>` via `git rev-parse`. During the "preparing" state, symbolic
-references are not resolved: `<ref-name>` will reflect the symbolic reference
-itself rather than the object it points to.
+transaction, or the value observed while preparing the transaction if no
+old object name was passed. `<new-value>` is the new object name to be
+stored in the ref and `<ref-name>` is the full name of the ref. When the
+reference does not exist, `<old-value>` is the all-zeroes object name.
+Because references are not yet locked in the "preparing" state, its observed
+old value may differ from the value reported in subsequent states if the
+reference changes before it is locked. During the "preparing" state,
+symbolic references are not resolved: `<ref-name>` will reflect the symbolic
+reference itself rather than the object it points to.
 
 For symbolic reference updates the `<old_value>` and `<new-value>`
 fields could denote references instead of objects. A reference will be
diff --git a/refs.c b/refs.c
index 92d5df5b71..d2d25402c3 100644
--- a/refs.c
+++ b/refs.c
@@ -1260,6 +1260,7 @@ void ref_transaction_free(struct ref_transaction *transaction)
 		free(transaction->updates[i]->committer_info);
 		free((char *)transaction->updates[i]->new_target);
 		free((char *)transaction->updates[i]->old_target);
+		free(transaction->updates[i]->hook_old_target);
 		free((char *)transaction->updates[i]->rejection_details);
 		free(transaction->updates[i]);
 	}
@@ -2606,6 +2607,8 @@ static int transaction_hook_feed_stdin(int hook_stdin_fd, void *pp_cb, void *pp_
 	struct transaction_feed_cb_data *feed_cb_data = pp_task_cb;
 	struct strbuf *buf = &feed_cb_data->buf;
 	struct ref_update *update;
+	const struct object_id *old_oid;
+	const char *old_target;
 	size_t i = feed_cb_data->index++;
 	int ret;
 
@@ -2619,12 +2622,18 @@ static int transaction_hook_feed_stdin(int hook_stdin_fd, void *pp_cb, void *pp_
 
 	strbuf_reset(buf);
 
-	if (!(update->flags & REF_HAVE_OLD))
-		strbuf_addf(buf, "%s ", oid_to_hex(null_oid(transaction->ref_store->repo->hash_algo)));
-	else if (update->old_target)
-		strbuf_addf(buf, "ref:%s ", update->old_target);
+	if (update->flags & REF_HAVE_OLD) {
+		old_oid = &update->old_oid;
+		old_target = update->old_target;
+	} else {
+		old_oid = &update->hook_old_oid;
+		old_target = update->hook_old_target;
+	}
+
+	if (old_target)
+		strbuf_addf(buf, "ref:%s ", old_target);
 	else
-		strbuf_addf(buf, "%s ", oid_to_hex(&update->old_oid));
+		strbuf_addf(buf, "%s ", oid_to_hex(old_oid));
 
 	if (!(update->flags & REF_HAVE_NEW))
 		strbuf_addf(buf, "%s ", oid_to_hex(null_oid(transaction->ref_store->repo->hash_algo)));
@@ -2660,6 +2669,36 @@ static void transaction_feed_cb_data_free(void *data)
 	free(d);
 }
 
+static void resolve_transaction_hook_old_values(struct ref_transaction *transaction)
+{
+	struct ref_store *refs = transaction->ref_store;
+	struct strbuf referent = STRBUF_INIT;
+
+	if (!hook_exists(refs->repo, "reference-transaction"))
+		return;
+
+	for (size_t i = 0; i < transaction->nr; i++) {
+		struct ref_update *update = transaction->updates[i];
+		unsigned int type = 0;
+		int failure_errno;
+
+		if (update->flags & (REF_HAVE_OLD | REF_LOG_ONLY))
+			continue;
+
+		oidclr(&update->hook_old_oid, refs->repo->hash_algo);
+		FREE_AND_NULL(update->hook_old_target);
+		strbuf_reset(&referent);
+
+		if (!refs_read_raw_ref(refs, update->refname,
+				       &update->hook_old_oid, &referent,
+				       &type, &failure_errno) &&
+		    (type & REF_ISSYMREF))
+			update->hook_old_target = xstrdup(referent.buf);
+	}
+
+	strbuf_release(&referent);
+}
+
 static int run_transaction_hook(struct ref_transaction *transaction,
 				const char *state)
 {
@@ -2709,6 +2748,8 @@ int ref_transaction_prepare(struct ref_transaction *transaction,
 	if (ref_update_reject_duplicates(&transaction->refnames, err))
 		return REF_TRANSACTION_ERROR_GENERIC;
 
+	resolve_transaction_hook_old_values(transaction);
+
 	/* Preparing checks before locking references */
 	ret = run_transaction_hook(transaction, "preparing");
 	if (ret) {
@@ -2720,6 +2761,9 @@ int ref_transaction_prepare(struct ref_transaction *transaction,
 	if (ret)
 		return ret;
 
+	/* Refresh old values now that the references are locked. */
+	resolve_transaction_hook_old_values(transaction);
+
 	ret = run_transaction_hook(transaction, "prepared");
 	if (ret) {
 		ref_transaction_abort(transaction, err);
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index c3ac7b556f..a7471b2481 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -99,6 +99,14 @@ struct ref_update {
 	 */
 	struct object_id old_oid;
 
+	/*
+	 * The old value observed for the reference-transaction hook when the
+	 * caller did not provide an expected old value. Unlike old_oid and
+	 * old_target, these fields do not constrain the update.
+	 */
+	struct object_id hook_old_oid;
+	char *hook_old_target;
+
 	/*
 	 * If the new_oid points to a tag object, set this to the peeled
 	 * object ID for optimized retrieval without needed to hit the odb.
diff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh
index 4fe9d9b234..fcc7404943 100755
--- a/t/t1416-ref-transaction-hooks.sh
+++ b/t/t1416-ref-transaction-hooks.sh
@@ -14,6 +14,66 @@ test_expect_success setup '
 	POST_OID=$(git rev-parse POST)
 '
 
+test_expect_success 'hook gets old values for batched unconditional deletion' '
+	test_when_finished "rm -f actual" &&
+	test_when_finished "git remote remove origin && rm -rf empty.git" &&
+	git init --bare empty.git &&
+	git remote add origin ./empty.git &&
+	git branch delete-a PRE &&
+	git branch delete-b POST &&
+	git tag delete-tag POST &&
+	git update-ref refs/remotes/origin/to-prune $PRE_OID &&
+	test_hook reference-transaction <<-\EOF &&
+		if test "$1" = committed
+		then
+			cat >>actual
+		fi
+	EOF
+	git branch -D delete-a delete-b &&
+	git tag -d delete-tag &&
+	git remote prune origin &&
+	cat >expect <<-EOF &&
+		$PRE_OID $ZERO_OID refs/heads/delete-a
+		$POST_OID $ZERO_OID refs/heads/delete-b
+		$POST_OID $ZERO_OID refs/tags/delete-tag
+		$PRE_OID $ZERO_OID refs/remotes/origin/to-prune
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success 'unconditional deletion remains unconditional' '
+	test_when_finished "rm -f actual" &&
+	test_when_finished "rm -f \"$(git rev-parse --git-path delete-race-once)\"" &&
+	git branch delete-race PRE &&
+	test_hook reference-transaction <<-\EOF &&
+		state=$1
+		while read -r old new ref
+		do
+			if test "$state" != aborted
+			then
+				case "$new" in
+				*[!0]*) ;;
+				*) echo "$state $old $new $ref" >>actual ;;
+				esac
+			fi
+		done
+		marker=$(git rev-parse --git-path delete-race-once)
+		if test "$state" = preparing && test ! -e "$marker"
+		then
+			>"$marker"
+			git update-ref refs/heads/delete-race POST
+		fi
+	EOF
+	git branch -D delete-race &&
+	cat >expect <<-EOF &&
+		preparing $PRE_OID $ZERO_OID refs/heads/delete-race
+		prepared $POST_OID $ZERO_OID refs/heads/delete-race
+		committed $POST_OID $ZERO_OID refs/heads/delete-race
+	EOF
+	test_cmp expect actual &&
+	test_must_fail git show-ref --verify refs/heads/delete-race
+'
+
 test_expect_success 'hook allows updating ref if successful' '
 	git reset --hard PRE &&
 	test_hook reference-transaction <<-\EOF &&
@@ -65,7 +125,7 @@ test_expect_success 'hook gets all queued updates in prepared state' '
 		fi
 	EOF
 	cat >expect <<-EOF &&
-		$ZERO_OID $POST_OID refs/heads/main
+		$PRE_OID $POST_OID refs/heads/main
 	EOF
 	git update-ref HEAD POST <<-EOF &&
 		update HEAD $ZERO_OID $POST_OID
@@ -87,7 +147,7 @@ test_expect_success 'hook gets all queued updates in committed state' '
 		fi
 	EOF
 	cat >expect <<-EOF &&
-		$ZERO_OID $POST_OID refs/heads/main
+		$PRE_OID $POST_OID refs/heads/main
 	EOF
 	git update-ref HEAD POST &&
 	test_cmp expect actual

-- 
2.39.3 (Apple Git-146)
Previous: Maciej CiemborowiczNext: Maciej Ciemborowicz
Message 48 of 52 in “[BUG] reference-transaction reports zero OIDs for branch and tag deletion”
  1. Maciej CiemborowiczSep 19, 2026
  2. D. Ben KnobleSep 19, 2026
  3. Maciej CiemborowiczSep 19, 2026
  4. 0/3 refs: report old OIDs for batched deletionsMaciej Ciemborowicz, Sep 19, 2026
  5. 1/3 refs: allow callers to supply old OIDs for batch deletionMaciej Ciemborowicz, Sep 19, 2026
  6. Karthik NayakSep 19, 2026
  7. Maciej CiemborowiczSep 20, 2026
  8. 0/3 refs: report old OIDs for batched deletionsMaciej Ciemborowicz, Sep 20, 2026
  9. 1/3 refs: allow callers to supply old OIDs for batch deletionMaciej Ciemborowicz, Sep 20, 2026
  10. Karthik NayakSep 21, 2026
  11. Junio C HamanoSep 21, 2026
  12. 2/3 branch, tag: retain old OIDs in batched deletionsMaciej Ciemborowicz, Sep 20, 2026
  13. Karthik NayakSep 21, 2026
  14. 3/3 fetch, remote: retain old OIDs when pruning refsMaciej Ciemborowicz, Sep 20, 2026
  15. Karthik NayakSep 21, 2026
  16. Karthik NayakSep 21, 2026
  17. Maciej CiemborowiczSep 21, 2026
  18. 0/3 refs: report old OIDs for batched deletionsMaciej Ciemborowicz, Sep 22, 2026
  19. 1/3 refs: allow callers to supply old OIDs for batch deletionMaciej Ciemborowicz, Sep 22, 2026
  20. Junio C HamanoSep 22, 2026
  21. Maciej CiemborowiczSep 22, 2026
  22. Junio C HamanoSep 22, 2026
  23. 2/3 branch, tag: retain old OIDs in batched deletionsMaciej Ciemborowicz, Sep 22, 2026
  24. 3/3 fetch, remote: retain old OIDs when pruning refsMaciej Ciemborowicz, Sep 22, 2026
  25. Junio C HamanoSep 22, 2026
  26. 0/3 refs: report old OIDs for batched deletionsMaciej Ciemborowicz, Sep 22, 2026
  27. 1/3 refs: allow callers to supply old OIDs for batch deletionMaciej Ciemborowicz, Sep 22, 2026
  28. 2/3 branch, tag: retain old OIDs in batched deletionsMaciej Ciemborowicz, Sep 22, 2026
  29. 3/3 fetch, remote: retain old OIDs when pruning refsMaciej Ciemborowicz, Sep 22, 2026
  30. Junio C HamanoSep 23, 2026
  31. Maciej CiemborowiczSep 23, 2026
  32. 0/3 refs: report old OIDs for batched deletionsMaciej Ciemborowicz, Sep 23, 2026
  33. 1/3 refs: allow callers to supply old OIDs for batch deletionMaciej Ciemborowicz, Sep 23, 2026
  34. Karthik NayakSep 24, 2026
  35. Junio C HamanoSep 24, 2026
  36. Maciej CiemborowiczSep 24, 2026
  37. Patrick SteinhardtSep 24, 2026
  38. Junio C HamanoSep 24, 2026
  39. Maciej CiemborowiczSep 24, 2026
  40. Patrick SteinhardtSep 28, 2026
  41. Patrick SteinhardtSep 28, 2026
  42. Maciej CiemborowiczSep 24, 2026
  43. 2/3 branch, tag: retain old OIDs in batched deletionsMaciej Ciemborowicz, Sep 23, 2026
  44. Patrick SteinhardtSep 24, 2026
  45. 3/3 fetch, remote: retain old OIDs when pruning refsMaciej Ciemborowicz, Sep 23, 2026
  46. Junio C HamanoSep 23, 2026
  47. 0/1 refs: report old values to transaction hooksMaciej Ciemborowicz, Sep 24, 2026
  48. 1/1 refs: report old values to transaction hooksMaciej Ciemborowicz, Sep 24, 2026
  49. Maciej CiemborowiczSep 30, 2026
  50. Maciej CiemborowiczOct 1, 2026
  51. 2/3 branch, tag: retain old OIDs in batched deletionsMaciej Ciemborowicz, Sep 19, 2026
  52. 3/3 fetch, remote: retain old OIDs when pruning refsMaciej Ciemborowicz, Sep 19, 2026

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.