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

@@ -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

@@ -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);

@@ -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.

@@ -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