{"thread":{"id":"66350","subject":"[BUG] reference-transaction hook misses destination of git branch -m","startedAt":"2026-09-19T13:33:57Z","lastAt":"2026-10-05T06:03:18Z","messageCount":12,"participants":["Maciej Ciemborowicz","Karthik Nayak","Junio C Hamano","Patrick Steinhardt"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"552876","messageId":"CACQ=SRHCOCcmVCgHqd+sjMsZ9LCdSHuXdCo0gkwxXwYgF7iwig@mail.gmail.com","threadId":"66350","inReplyTo":null,"subject":"[BUG] reference-transaction hook misses destination of git branch -m","fromName":"Maciej Ciemborowicz","fromEmail":"maciej.ciemborowicz@gmail.com","sentAt":"2026-09-19T13:33:44Z","receivedAt":"2026-09-19T13:33:57Z","isPatch":false,"body":"Hello,\n\n`git branch -m old new` changes refs/heads/old into refs/heads/new,\nbut the reference-transaction hook does not report both ref changes.\n\nObserved with Git 2.55:\n\n* files backend: the hook reports deletion of refs/heads/old, but\n does not report creation of refs/heads/new;\n* reftable backend: the hook reports no usable rename payload.\n\nThe equivalent atomic operation performed with `git update-ref --stdin`\nreports both updates correctly.\n\nI expected the transaction to include:\n\n refs/heads/old\n refs/heads/new\n\nMinimal reproducer:\n\n #!/bin/sh\n set -eu\n\n format=${1:-files}\n root=$(mktemp -d)\n trap 'rm -rf \"$root\"' EXIT\n\n repo=$root/repo\n hooks=$root/hooks\n log=$root/transactions\n\n git init -q --ref-format=\"$format\" \"$repo\"\n git -C \"$repo\" config user.name Reproducer\n git -C \"$repo\" config user.email repro@example.com\n git -C \"$repo\" commit --allow-empty -qm initial\n git -C \"$repo\" branch old\n\n mkdir \"$hooks\"\n cat >\"$hooks/reference-transaction\" <<'HOOK'\n #!/bin/sh\n printf '%s\\n' \"--- $1\" >>\"$HOOK_LOG\"\n cat >>\"$HOOK_LOG\"\n HOOK\n chmod +x \"$hooks/reference-transaction\"\n\n git -C \"$repo\" config core.hooksPath \"$hooks\"\n export HOOK_LOG=$log\n : >\"$log\"\n\n git -C \"$repo\" branch -m old new\n cat \"$log\"\n\nThe behavior was also tested across Git 2.28–2.55:\nhttps://github.com/ciembor/git-hooks-ext/blob/v0.2.0/tests/compat/README.md\n\nThe reference-transaction documentation says that the hook is invoked by\nGit commands performing reference updates. A branch rename changes two refs,\nbut the destination update is not visible to the hook.\n\nThanks,\nMaciej Ciemborowicz\n"},{"id":"552899","messageId":"CAOLa=ZTN1TU2A1sgEhiw=ymMYr6Ge11cMEubSaeKqr4WNU=2EQ@mail.gmail.com","threadId":"66350","inReplyTo":"CACQ=SRHCOCcmVCgHqd+sjMsZ9LCdSHuXdCo0gkwxXwYgF7iwig@mail.gmail.com","subject":"Re: [BUG] reference-transaction hook misses destination of git branch -m","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-09-19T20:52:22Z","receivedAt":"2026-09-19T20:52:24Z","isPatch":false,"body":"Maciej Ciemborowicz writes:\n\n> Hello,\n>\n> `git branch -m old new` changes refs/heads/old into refs/heads/new,\n> but the reference-transaction hook does not report both ref changes.\n>\n\nMy understanding without digging too deep is that both 'copy' and\n'rename' do not use a transaction, as such they don't trigger the\n'reference-transaction' hook. This is definitely absurd, since we would\nexpect that all reference operations should use the transaction and\ntrigger the hook.\n\n> Observed with Git 2.55:\n>\n> * files backend: the hook reports deletion of refs/heads/old, but\n> does not report creation of refs/heads/new;\n\nYeah, because the files backend calls `refs_delete_ref()` on the old\nreference which is done within a transaction.\n\n> * reftable backend: the hook reports no usable rename payload.\n\nReftable doesn't call `refs_delete_ref()` and simply writes a TOMBSTONE\nentry to delete the old reference.\n\n> The equivalent atomic operation performed with `git update-ref --stdin`\n> reports both updates correctly.\n>\n\nBy equivalent atomic operation do you a 'delete' and 'create' command?\n\n> I expected the transaction to include:\n>\n> refs/heads/old\n> refs/heads/new\n>\n> Minimal reproducer:\n>\n> #!/bin/sh\n> set -eu\n>\n> format=${1:-files}\n> root=$(mktemp -d)\n> trap 'rm -rf \"$root\"' EXIT\n>\n> repo=$root/repo\n> hooks=$root/hooks\n> log=$root/transactions\n>\n> git init -q --ref-format=\"$format\" \"$repo\"\n> git -C \"$repo\" config user.name Reproducer\n> git -C \"$repo\" config user.email repro@example.com\n> git -C \"$repo\" commit --allow-empty -qm initial\n> git -C \"$repo\" branch old\n>\n> mkdir \"$hooks\"\n> cat >\"$hooks/reference-transaction\" <<'HOOK'\n> #!/bin/sh\n> printf '%s\\n' \"--- $1\" >>\"$HOOK_LOG\"\n> cat >>\"$HOOK_LOG\"\n> HOOK\n> chmod +x \"$hooks/reference-transaction\"\n>\n> git -C \"$repo\" config core.hooksPath \"$hooks\"\n> export HOOK_LOG=$log\n> : >\"$log\"\n>\n> git -C \"$repo\" branch -m old new\n> cat \"$log\"\n>\n> The behavior was also tested across Git 2.28–2.55:\n> https://github.com/ciembor/git-hooks-ext/blob/v0.2.0/tests/compat/README.md\n>\n> The reference-transaction documentation says that the hook is invoked by\n> Git commands performing reference updates. A branch rename changes two refs,\n> but the destination update is not visible to the hook.\n>\n\nEither ways, I think we should fix this, I could have a look if you\naren't submitting a patch yourself :)\n\n> Thanks,\n> Maciej Ciemborowicz\n"},{"id":"552913","messageId":"20260920165037.88524-1-maciej.ciemborowicz@gmail.com","threadId":"66350","inReplyTo":"CAOLa=ZTN1TU2A1sgEhiw=ymMYr6Ge11cMEubSaeKqr4WNU=2EQ@mail.gmail.com","subject":"[PATCH] refs: run copy and rename through transactions","fromName":"Maciej Ciemborowicz","fromEmail":"maciej.ciemborowicz@gmail.com","sentAt":"2026-09-20T16:50:37Z","receivedAt":"2026-09-20T16:50:41Z","isPatch":true,"body":"Reference copy and rename operations currently bypass the transaction API.\nConsequently, the reference-transaction hook sees only the source deletion\nwith the files backend and no useful update with the reftable backend.\n\nRepresent both operations as reference transactions containing their\nlogical updates. A rename is a deletion of the old reference and creation\nof the new reference in the same transaction. Retain backend-specific\nreflog handling: the files backend stages its existing rename procedure\nacross prepare, finish and abort, while reftable stages an addition while\nholding the stack lock. Suppress hooks for the files backend's nested\ndeletion transactions so that callers observe one logical transaction.\n\nRecord and verify the source and destination values after taking backend\nlocks. This rejects concurrent changes instead of applying a rename or copy\nthat differs from the payload shown to the preparing hook. Preserve D/F\nrenames and restore overwritten references and reflogs when a prepared hook\nrejects the operation.\n\nAdd coverage for rename, copy, forced updates, both directions of D/F\nconflicts, concurrent updates and prepared-hook rollback.\n\nHelped-by: Karthik Nayak \nSigned-off-by: Maciej Ciemborowicz \n---\n refs.c | 137 ++++++++++++---\n refs.h | 3 +\n refs/debug.c | 25 ---\n refs/files-backend.c | 276 +++++++++++++++++++++++++++----\n refs/packed-backend.c | 2 -\n refs/refs-internal.h | 38 +++--\n refs/reftable-backend.c | 194 ++++++++++++++++------\n t/t1416-ref-transaction-hooks.sh | 142 ++++++++++++++++\n 8 files changed, 679 insertions(+), 138 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 92d5df5b7..22c000f7f 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1004,15 +1004,17 @@ long get_files_ref_lock_timeout_ms(struct repository *repo)\n \treturn timeout_ms;\n }\n \n-int refs_delete_ref(struct ref_store *refs, const char *msg,\n-\t\t const char *refname,\n-\t\t const struct object_id *old_oid,\n-\t\t unsigned int flags)\n+int refs_delete_ref_with_transaction_flags(struct ref_store *refs,\n+\t\t\t\t\t const char *msg,\n+\t\t\t\t\t const char *refname,\n+\t\t\t\t\t const struct object_id *old_oid,\n+\t\t\t\t\t unsigned int flags,\n+\t\t\t\t\t unsigned int transaction_flags)\n {\n \tstruct ref_transaction *transaction;\n \tstruct strbuf err = STRBUF_INIT;\n \n-\ttransaction = ref_store_transaction_begin(refs, 0, &err);\n+\ttransaction = ref_store_transaction_begin(refs, transaction_flags, &err);\n \tif (!transaction ||\n \t ref_transaction_delete(transaction, refname, old_oid,\n \t\t\t\t NULL, flags, msg, &err) ||\n@@ -1027,6 +1029,15 @@ int refs_delete_ref(struct ref_store *refs, const char *msg,\n \treturn 0;\n }\n \n+int refs_delete_ref(struct ref_store *refs, const char *msg,\n+\t\t const char *refname,\n+\t\t const struct object_id *old_oid,\n+\t\t unsigned int flags)\n+{\n+\treturn refs_delete_ref_with_transaction_flags(refs, msg, refname,\n+\t\t\t\t\t\t old_oid, flags, 0);\n+}\n+\n static void copy_reflog_msg(struct strbuf *sb, const char *msg)\n {\n \tchar c;\n@@ -1270,6 +1281,10 @@ void ref_transaction_free(struct ref_transaction *transaction)\n \n \tstring_list_clear(&transaction->refnames, 0);\n \tfree(transaction->updates);\n+\tfree(transaction->old_refname);\n+\tfree(transaction->new_refname);\n+\tfree(transaction->logmsg);\n+\tfree(transaction->destination_target);\n \tfree(transaction);\n }\n \n@@ -2710,7 +2725,8 @@ int ref_transaction_prepare(struct ref_transaction *transaction,\n \t\treturn REF_TRANSACTION_ERROR_GENERIC;\n \n \t/* Preparing checks before locking references */\n-\tret = run_transaction_hook(transaction, \"preparing\");\n+\tret = transaction->flags & REF_TRANSACTION_FLAG_SKIP_HOOK ? 0 :\n+\t\trun_transaction_hook(transaction, \"preparing\");\n \tif (ret) {\n \t\tref_transaction_abort(transaction, err);\n \t\tdie(_(abort_by_ref_transaction_hook), \"preparing\");\n@@ -2720,7 +2736,8 @@ int ref_transaction_prepare(struct ref_transaction *transaction,\n \tif (ret)\n \t\treturn ret;\n \n-\tret = run_transaction_hook(transaction, \"prepared\");\n+\tret = transaction->flags & REF_TRANSACTION_FLAG_SKIP_HOOK ? 0 :\n+\t\trun_transaction_hook(transaction, \"prepared\");\n \tif (ret) {\n \t\tref_transaction_abort(transaction, err);\n \t\tdie(_(abort_by_ref_transaction_hook), \"prepared\");\n@@ -2750,7 +2767,8 @@ int ref_transaction_abort(struct ref_transaction *transaction,\n \t\tbreak;\n \t}\n \n-\trun_transaction_hook(transaction, \"aborted\");\n+\tif (!(transaction->flags & REF_TRANSACTION_FLAG_SKIP_HOOK))\n+\t\trun_transaction_hook(transaction, \"aborted\");\n \n \tref_transaction_free(transaction);\n \treturn ret;\n@@ -2781,7 +2799,8 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \t}\n \n \tret = refs->be->transaction_finish(refs, transaction, err);\n-\tif (!ret && !(transaction->flags & REF_TRANSACTION_FLAG_INITIAL))\n+\tif (!ret && !(transaction->flags & (REF_TRANSACTION_FLAG_INITIAL |\n+\t\t\t\t\t REF_TRANSACTION_FLAG_SKIP_HOOK)))\n \t\trun_transaction_hook(transaction, \"committed\");\n \treturn ret;\n }\n@@ -3123,28 +3142,100 @@ int refs_delete_refs(struct ref_store *refs, const char *logmsg,\n \treturn ret;\n }\n \n-int refs_rename_ref(struct ref_store *refs, const char *oldref,\n-\t\t const char *newref, const char *logmsg)\n+static int refs_copy_or_rename_ref(struct ref_store *refs, const char *oldref,\n+\t\t\t\t const char *newref, const char *logmsg,\n+\t\t\t\t int copy)\n {\n-\tchar *msg;\n-\tint retval;\n+\tstruct ref_transaction *transaction = NULL;\n+\tstruct object_id old_oid, new_oid;\n+\tstruct strbuf new_target = STRBUF_INIT;\n+\tstruct strbuf err = STRBUF_INIT;\n+\tchar *msg = normalize_reflog_message(logmsg);\n+\tint old_flags, new_flags = 0, new_exists = 0, ret = 1;\n \n-\tmsg = normalize_reflog_message(logmsg);\n-\tretval = refs->be->rename_ref(refs, oldref, newref, msg);\n+\tif (!strcmp(oldref, newref)) {\n+\t\tret = 0;\n+\t\tgoto out;\n+\t}\n+\n+\tif (!refs_resolve_ref_unsafe(refs, oldref,\n+\t\t\t\t RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n+\t\t\t\t &old_oid, &old_flags)) {\n+\t\terror(\"refname %s not found\", oldref);\n+\t\tgoto out;\n+\t}\n+\tif (old_flags & REF_ISSYMREF) {\n+\t\terror(\"refname %s is a symbolic ref, %s it is not supported\",\n+\t\t oldref, copy ? \"copying\" : \"renaming\");\n+\t\tgoto out;\n+\t}\n+\n+\ttransaction = ref_store_transaction_begin(refs, 0, &err);\n+\tif (!transaction)\n+\t\tgoto error;\n+\ttransaction->type = copy ? REF_TRANSACTION_TYPE_COPY :\n+\t\tREF_TRANSACTION_TYPE_RENAME;\n+\ttransaction->old_refname = xstrdup(oldref);\n+\ttransaction->new_refname = xstrdup(newref);\n+\ttransaction->logmsg = xstrdup(msg);\n+\toidcpy(&transaction->source_oid, &old_oid);\n+\n+\tif (!copy && ref_transaction_delete(transaction, oldref, &old_oid, NULL,\n+\t\t\t\t\t REF_NO_DEREF, msg, &err))\n+\t\tgoto error;\n+\n+\tif (refs_resolve_ref_unsafe(refs, newref,\n+\t\t\t\t RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n+\t\t\t\t &new_oid, &new_flags)) {\n+\t\tnew_exists = 1;\n+\t\tif ((new_flags & REF_ISSYMREF) &&\n+\t\t refs_read_symbolic_ref(refs, newref, &new_target) < 0) {\n+\t\t\tstrbuf_addf(&err, \"unable to read symbolic ref %s\", newref);\n+\t\t\tgoto error;\n+\t\t}\n+\t} else {\n+\t\toidclr(&new_oid, refs->repo->hash_algo);\n+\t}\n+\ttransaction->destination_exists = new_exists;\n+\tif (new_flags & REF_ISSYMREF)\n+\t\ttransaction->destination_target = xstrdup(new_target.buf);\n+\telse if (transaction->destination_exists)\n+\t\toidcpy(&transaction->destination_oid, &new_oid);\n+\n+\tif (ref_transaction_update(transaction, newref, &old_oid,\n+\t\t\t\t (new_flags & REF_ISSYMREF) ? NULL : &new_oid,\n+\t\t\t\t NULL,\n+\t\t\t\t (new_flags & REF_ISSYMREF) ? new_target.buf : NULL,\n+\t\t\t\t REF_NO_DEREF | REF_SKIP_CREATE_REFLOG,\n+\t\t\t\t NULL, &err))\n+\t\tgoto error;\n+\n+\tif (ref_transaction_commit(transaction, &err))\n+\t\tgoto error;\n+\n+\tret = 0;\n+\tgoto out;\n+\n+error:\n+\terror(\"%s\", err.buf);\n+out:\n+\tref_transaction_free(transaction);\n+\tstrbuf_release(&new_target);\n+\tstrbuf_release(&err);\n \tfree(msg);\n-\treturn retval;\n+\treturn ret;\n }\n \n-int refs_copy_existing_ref(struct ref_store *refs, const char *oldref,\n+int refs_rename_ref(struct ref_store *refs, const char *oldref,\n \t\t const char *newref, const char *logmsg)\n {\n-\tchar *msg;\n-\tint retval;\n+\treturn refs_copy_or_rename_ref(refs, oldref, newref, logmsg, 0);\n+}\n \n-\tmsg = normalize_reflog_message(logmsg);\n-\tretval = refs->be->copy_ref(refs, oldref, newref, msg);\n-\tfree(msg);\n-\treturn retval;\n+int refs_copy_existing_ref(struct ref_store *refs, const char *oldref,\n+\t\t const char *newref, const char *logmsg)\n+{\n+\treturn refs_copy_or_rename_ref(refs, oldref, newref, logmsg, 1);\n }\n \n const char *ref_update_original_update_refname(struct ref_update *update)\ndiff --git a/refs.h b/refs.h\nindex 9979446d1..25bb8720f 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -784,6 +784,9 @@ enum ref_transaction_flag {\n \t * while rejecting updates which do not match the expected state.\n \t */\n \tREF_TRANSACTION_ALLOW_FAILURE = (1 << 1),\n+\n+\t/* Suppress hooks for an update nested in another transaction. */\n+\tREF_TRANSACTION_FLAG_SKIP_HOOK = (1 << 2),\n };\n \n /*\ndiff --git a/refs/debug.c b/refs/debug.c\nindex 639db0f26..87b84e767 100644\n--- a/refs/debug.c\n+++ b/refs/debug.c\n@@ -143,28 +143,6 @@ static int debug_optimize_required(struct ref_store *ref_store,\n \treturn res;\n }\n \n-static int debug_rename_ref(struct ref_store *ref_store, const char *oldref,\n-\t\t\t const char *newref, const char *logmsg)\n-{\n-\tstruct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;\n-\tint res = drefs->refs->be->rename_ref(drefs->refs, oldref, newref,\n-\t\t\t\t\t logmsg);\n-\ttrace_printf_key(&trace_refs, \"rename_ref: %s -> %s \\\"%s\\\": %d\\n\", oldref, newref,\n-\t\tlogmsg, res);\n-\treturn res;\n-}\n-\n-static int debug_copy_ref(struct ref_store *ref_store, const char *oldref,\n-\t\t\t const char *newref, const char *logmsg)\n-{\n-\tstruct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;\n-\tint res =\n-\t\tdrefs->refs->be->copy_ref(drefs->refs, oldref, newref, logmsg);\n-\ttrace_printf_key(&trace_refs, \"copy_ref: %s -> %s \\\"%s\\\": %d\\n\", oldref, newref,\n-\t\tlogmsg, res);\n-\treturn res;\n-}\n-\n struct debug_ref_iterator {\n \tstruct ref_iterator base;\n \tstruct ref_iterator *iter;\n@@ -453,9 +431,6 @@ struct ref_storage_be refs_be_debug = {\n \t.optimize = debug_optimize,\n \t.optimize_required = debug_optimize_required,\n \n-\t.rename_ref = debug_rename_ref,\n-\t.copy_ref = debug_copy_ref,\n-\n \t.iterator_begin = debug_ref_iterator_begin,\n \t.read_raw_ref = debug_read_raw_ref,\n \t.read_symbolic_ref = debug_read_symbolic_ref,\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 71628550f..92601b5e9 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -1594,6 +1594,7 @@ static int files_optimize_required(struct ref_store *ref_store,\n * live into logs/refs.\n */\n #define TMP_RENAMED_LOG \"refs/.tmp-renamed-log\"\n+#define TMP_RENAMED_LOG_DESTINATION \"refs/.tmp-renamed-log-destination\"\n \n struct rename_cb {\n \tconst char *tmp_renamed_log;\n@@ -1685,12 +1686,24 @@ static int refs_rename_ref_available(struct ref_store *refs,\n \treturn ok;\n }\n \n+struct files_copy_or_rename_transaction_data {\n+\tstruct ref_lock *lock;\n+\tstruct object_id orig_oid;\n+\tstruct object_id destination_oid;\n+\tchar *destination_target;\n+\tint logmoved;\n+\tint destination_exists;\n+\tint destination_log_backed_up;\n+};\n+\n static int files_copy_or_rename_ref(struct ref_store *ref_store,\n \t\t\t const char *oldrefname, const char *newrefname,\n-\t\t\t const char *logmsg, int copy)\n+\t\t\t const char *logmsg, int copy,\n+\t\t\t struct ref_transaction *transaction)\n {\n \tstruct files_ref_store *refs =\n-\t\tfiles_downcast(ref_store, REF_STORE_WRITE, \"rename_ref\");\n+\t\tfiles_downcast(ref_store, REF_STORE_WRITE,\n+\t\t\t \"ref_transaction_prepare\");\n \tstruct object_id orig_oid;\n \tint flag = 0, logmoved = 0;\n \tstruct ref_lock *lock;\n@@ -1698,12 +1711,19 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,\n \tstruct strbuf sb_oldref = STRBUF_INIT;\n \tstruct strbuf sb_newref = STRBUF_INIT;\n \tstruct strbuf tmp_renamed_log = STRBUF_INIT;\n+\tstruct strbuf tmp_destination_log = STRBUF_INIT;\n+\tstruct strbuf destination_target = STRBUF_INIT;\n \tint log, ret;\n+\tint destination_exists = 0, destination_flags = 0;\n+\tint destination_log_backed_up = 0;\n+\tstruct object_id destination_oid;\n+\tstruct files_copy_or_rename_transaction_data *data;\n \tstruct strbuf err = STRBUF_INIT;\n \n \tfiles_reflog_path(refs, &sb_oldref, oldrefname);\n \tfiles_reflog_path(refs, &sb_newref, newrefname);\n \tfiles_reflog_path(refs, &tmp_renamed_log, TMP_RENAMED_LOG);\n+\tfiles_reflog_path(refs, &tmp_destination_log, TMP_RENAMED_LOG_DESTINATION);\n \n \tlog = !lstat(sb_oldref.buf, &loginfo);\n \tif (log && S_ISLNK(loginfo.st_mode)) {\n@@ -1727,11 +1747,67 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,\n \t\t\t\t oldrefname);\n \t\tgoto out;\n \t}\n+\tif (!oideq(&orig_oid, &transaction->source_oid)) {\n+\t\tret = error(\"refname %s is at %s but expected %s\",\n+\t\t\t oldrefname, oid_to_hex(&orig_oid),\n+\t\t\t oid_to_hex(&transaction->source_oid));\n+\t\tgoto out;\n+\t}\n \tif (!refs_rename_ref_available(&refs->base, oldrefname, newrefname)) {\n \t\tret = 1;\n \t\tgoto out;\n \t}\n \n+\tif (refs_resolve_ref_unsafe(&refs->base, newrefname,\n+\t\t\t\t RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n+\t\t\t\t &destination_oid, &destination_flags)) {\n+\t\tdestination_exists = 1;\n+\t\tif ((destination_flags & REF_ISSYMREF) &&\n+\t\t refs_read_symbolic_ref(&refs->base, newrefname,\n+\t\t\t\t\t &destination_target) < 0) {\n+\t\t\tret = error(\"unable to read symbolic ref %s\", newrefname);\n+\t\t\tgoto out;\n+\t\t}\n+\t}\n+\tif (destination_exists != transaction->destination_exists) {\n+\t\tret = error(\"refname %s changed while renaming\", newrefname);\n+\t\tgoto out;\n+\t}\n+\tif (destination_exists) {\n+\t\tif (destination_flags & REF_ISSYMREF) {\n+\t\t\tif (!transaction->destination_target ||\n+\t\t\t strcmp(destination_target.buf,\n+\t\t\t\t transaction->destination_target)) {\n+\t\t\t\tret = error(\"refname %s changed while renaming\",\n+\t\t\t\t\t newrefname);\n+\t\t\t\tgoto out;\n+\t\t\t}\n+\t\t} else if (transaction->destination_target ||\n+\t\t\t !oideq(&destination_oid,\n+\t\t\t\t &transaction->destination_oid)) {\n+\t\t\tret = error(\"refname %s changed while renaming\", newrefname);\n+\t\t\tgoto out;\n+\t\t}\n+\t}\n+\n+\tif (!lstat(sb_newref.buf, &loginfo)) {\n+\t\tif (S_ISLNK(loginfo.st_mode)) {\n+\t\t\tret = error(\"reflog for %s is a symlink\", newrefname);\n+\t\t\tgoto out;\n+\t\t}\n+\t\tif (S_ISREG(loginfo.st_mode)) {\n+\t\t\tif (copy_file(refs->base.repo, tmp_destination_log.buf,\n+\t\t\t\t sb_newref.buf, 0644)) {\n+\t\t\t\tif (errno != EEXIST)\n+\t\t\t\t\tunlink(tmp_destination_log.buf);\n+\t\t\t\tret = error(\"unable to back up logfile logs/%s: %s\",\n+\t\t\t\t\t newrefname, strerror(errno));\n+\t\t\t\tgoto out;\n+\t\t\t}\n+\t\t\tdestination_log_backed_up = 1;\n+\t\t}\n+\t}\n+\n \tif (!copy && log && rename(sb_oldref.buf, tmp_renamed_log.buf)) {\n \t\tret = error(\"unable to move logfile logs/%s to logs/\"TMP_RENAMED_LOG\": %s\",\n \t\t\t oldrefname, strerror(errno));\n@@ -1744,8 +1820,10 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,\n \t\tgoto out;\n \t}\n \n-\tif (!copy && refs_delete_ref(&refs->base, logmsg, oldrefname,\n-\t\t\t &orig_oid, REF_NO_DEREF)) {\n+\tif (!copy && refs_delete_ref_with_transaction_flags(&refs->base, logmsg,\n+\t\t\t\t\t\t\t oldrefname, &orig_oid,\n+\t\t\t\t\t\t\t REF_NO_DEREF,\n+\t\t\t\t\t\t\t REF_TRANSACTION_FLAG_SKIP_HOOK)) {\n \t\terror(\"unable to delete old %s\", oldrefname);\n \t\tgoto rollback;\n \t}\n@@ -1760,8 +1838,9 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,\n \tif (!copy && refs_resolve_ref_unsafe(&refs->base, newrefname,\n \t\t\t\t\t RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n \t\t\t\t\t NULL, NULL) &&\n-\t refs_delete_ref(&refs->base, NULL, newrefname,\n-\t\t\t NULL, REF_NO_DEREF)) {\n+\t refs_delete_ref_with_transaction_flags(&refs->base, NULL, newrefname,\n+\t\t\t\t\t\t NULL, REF_NO_DEREF,\n+\t\t\t\t\t\t REF_TRANSACTION_FLAG_SKIP_HOOK)) {\n \t\tif (errno == EISDIR) {\n \t\t\tstruct strbuf path = STRBUF_INIT;\n \t\t\tint result;\n@@ -1796,13 +1875,25 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,\n \t}\n \toidcpy(&lock->old_oid, &orig_oid);\n \n-\tif (write_ref_to_lockfile(refs, lock, &orig_oid, &err) ||\n-\t commit_ref_update(refs, lock, &orig_oid, logmsg, 0, &err)) {\n+\tif (write_ref_to_lockfile(refs, lock, &orig_oid, &err)) {\n \t\terror(\"unable to write current sha1 into %s: %s\", newrefname, err.buf);\n \t\tstrbuf_release(&err);\n \t\tgoto rollback;\n \t}\n \n+\tCALLOC_ARRAY(data, 1);\n+\tdata->lock = lock;\n+\toidcpy(&data->orig_oid, &orig_oid);\n+\tdata->logmoved = logmoved;\n+\tdata->destination_exists = destination_exists;\n+\tdata->destination_log_backed_up = destination_log_backed_up;\n+\tif (destination_exists && !(destination_flags & REF_ISSYMREF))\n+\t\toidcpy(&data->destination_oid, &destination_oid);\n+\tif (destination_flags & REF_ISSYMREF)\n+\t\tdata->destination_target = strbuf_detach(&destination_target, NULL);\n+\ttransaction->backend_data = data;\n+\ttransaction->state = REF_TRANSACTION_PREPARED;\n+\n \tret = 0;\n \tgoto out;\n \n@@ -1821,38 +1912,40 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,\n \t}\n \n rollbacklog:\n-\tif (logmoved && rename(sb_newref.buf, sb_oldref.buf))\n-\t\terror(\"unable to restore logfile %s from %s: %s\",\n-\t\t\toldrefname, newrefname, strerror(errno));\n+\tif (logmoved) {\n+\t\tif (rename(sb_newref.buf, tmp_renamed_log.buf)) {\n+\t\t\terror(\"unable to restore logfile %s from %s: %s\",\n+\t\t\t oldrefname, newrefname, strerror(errno));\n+\t\t} else {\n+\t\t\ttry_remove_empty_parents(refs, newrefname,\n+\t\t\t\t\t\t REMOVE_EMPTY_PARENTS_REFLOG);\n+\t\t\tif (rename_tmp_log(refs, oldrefname))\n+\t\t\t\terror(\"unable to restore logfile %s from logs/\"\n+\t\t\t\t TMP_RENAMED_LOG \": %s\",\n+\t\t\t\t oldrefname, strerror(errno));\n+\t\t}\n+\t}\n \tif (!logmoved && log &&\n \t rename(tmp_renamed_log.buf, sb_oldref.buf))\n \t\terror(\"unable to restore logfile %s from logs/\"TMP_RENAMED_LOG\": %s\",\n \t\t\toldrefname, strerror(errno));\n+\tif (destination_log_backed_up &&\n+\t rename(tmp_destination_log.buf, sb_newref.buf))\n+\t\terror(\"unable to restore logfile %s: %s\",\n+\t\t newrefname, strerror(errno));\n \tret = 1;\n out:\n+\tif (ret && destination_log_backed_up)\n+\t\tunlink(tmp_destination_log.buf);\n \tstrbuf_release(&sb_newref);\n \tstrbuf_release(&sb_oldref);\n \tstrbuf_release(&tmp_renamed_log);\n+\tstrbuf_release(&tmp_destination_log);\n+\tstrbuf_release(&destination_target);\n \n \treturn ret;\n }\n \n-static int files_rename_ref(struct ref_store *ref_store,\n-\t\t\t const char *oldrefname, const char *newrefname,\n-\t\t\t const char *logmsg)\n-{\n-\treturn files_copy_or_rename_ref(ref_store, oldrefname,\n-\t\t\t\t newrefname, logmsg, 0);\n-}\n-\n-static int files_copy_ref(struct ref_store *ref_store,\n-\t\t\t const char *oldrefname, const char *newrefname,\n-\t\t\t const char *logmsg)\n-{\n-\treturn files_copy_or_rename_ref(ref_store, oldrefname,\n-\t\t\t\t newrefname, logmsg, 1);\n-}\n-\n static int close_ref_gently(struct ref_lock *lock)\n {\n \tif (close_lock_file_gently(&lock->lk))\n@@ -2962,6 +3055,13 @@ static int files_transaction_prepare(struct ref_store *ref_store,\n \tstruct ref_transaction *packed_transaction = NULL;\n \n \tassert(err);\n+\tif (transaction->type != REF_TRANSACTION_TYPE_NORMAL)\n+\t\treturn files_copy_or_rename_ref(ref_store,\n+\t\t\t\ttransaction->old_refname,\n+\t\t\t\ttransaction->new_refname,\n+\t\t\t\ttransaction->logmsg,\n+\t\t\t\ttransaction->type == REF_TRANSACTION_TYPE_COPY,\n+\t\t\t\ttransaction);\n \n \tif (transaction->flags & REF_TRANSACTION_FLAG_INITIAL)\n \t\tgoto cleanup;\n@@ -3318,6 +3418,10 @@ static int files_transaction_finish_initial(struct files_ref_store *refs,\n \treturn ret;\n }\n \n+static int files_transaction_abort(struct ref_store *ref_store,\n+\t\t\t\t struct ref_transaction *transaction,\n+\t\t\t\t struct strbuf *err);\n+\n static int files_transaction_finish(struct ref_store *ref_store,\n \t\t\t\t struct ref_transaction *transaction,\n \t\t\t\t struct strbuf *err)\n@@ -3333,6 +3437,33 @@ static int files_transaction_finish(struct ref_store *ref_store,\n \n \n \tassert(err);\n+\tif (transaction->type != REF_TRANSACTION_TYPE_NORMAL) {\n+\t\tstruct files_copy_or_rename_transaction_data *data =\n+\t\t\ttransaction->backend_data;\n+\t\tint special_ret;\n+\n+\t\tspecial_ret = commit_ref_update(refs, data->lock, &data->orig_oid,\n+\t\t\t\t\t\ttransaction->logmsg, 0, err);\n+\t\tif (special_ret) {\n+\t\t\terror(\"unable to write current sha1 into %s: %s\",\n+\t\t\t transaction->new_refname, err->buf);\n+\t\t\tdata->lock = NULL;\n+\t\t\tfiles_transaction_abort(ref_store, transaction, err);\n+\t\t\treturn special_ret;\n+\t\t} else if (data->destination_log_backed_up) {\n+\t\t\tstruct strbuf path = STRBUF_INIT;\n+\n+\t\t\tfiles_reflog_path(refs, &path, TMP_RENAMED_LOG_DESTINATION);\n+\t\t\tif (unlink(path.buf) < 0 && errno != ENOENT)\n+\t\t\t\twarning_errno(\"unable to remove '%s'\", path.buf);\n+\t\t\tstrbuf_release(&path);\n+\t\t}\n+\t\tfree(data->destination_target);\n+\t\tfree(data);\n+\t\ttransaction->backend_data = NULL;\n+\t\ttransaction->state = REF_TRANSACTION_CLOSED;\n+\t\treturn special_ret;\n+\t}\n \n \tif (transaction->flags & REF_TRANSACTION_FLAG_INITIAL)\n \t\treturn files_transaction_finish_initial(refs, transaction, err);\n@@ -3476,11 +3607,98 @@ static int files_transaction_finish(struct ref_store *ref_store,\n \n static int files_transaction_abort(struct ref_store *ref_store,\n \t\t\t\t struct ref_transaction *transaction,\n-\t\t\t\t struct strbuf *err UNUSED)\n+\t\t\t\t struct strbuf *err)\n {\n \tstruct files_ref_store *refs =\n \t\tfiles_downcast(ref_store, 0, \"ref_transaction_abort\");\n \n+\tif (transaction->type != REF_TRANSACTION_TYPE_NORMAL) {\n+\t\tstruct files_copy_or_rename_transaction_data *data =\n+\t\t\ttransaction->backend_data;\n+\t\tstruct strbuf new_log = STRBUF_INIT;\n+\t\tstruct strbuf destination_log = STRBUF_INIT;\n+\t\tstruct strbuf temporary_log = STRBUF_INIT;\n+\t\tstruct ref_transaction *restore_transaction = NULL;\n+\t\tstruct ref_lock *lock;\n+\t\tint ret = 0;\n+\n+\t\tif (data->lock)\n+\t\t\tunlock_ref(data->lock);\n+\t\tif (transaction->type == REF_TRANSACTION_TYPE_RENAME) {\n+\t\t\tlock = lock_ref_oid_basic(refs, transaction->old_refname, err);\n+\t\t\tif (!lock ||\n+\t\t\t write_ref_to_lockfile(refs, lock, &data->orig_oid, err) ||\n+\t\t\t commit_ref_update(refs, lock, &data->orig_oid, NULL,\n+\t\t\t\t\t REF_SKIP_CREATE_REFLOG, err))\n+\t\t\t\tret = -1;\n+\t\t}\n+\n+\t\tif (data->logmoved) {\n+\t\t\tfiles_reflog_path(refs, &new_log, transaction->new_refname);\n+\t\t\tif (transaction->type == REF_TRANSACTION_TYPE_RENAME) {\n+\t\t\t\tfiles_reflog_path(refs, &temporary_log, TMP_RENAMED_LOG);\n+\t\t\t\tif (rename(new_log.buf, temporary_log.buf) < 0) {\n+\t\t\t\t\tstrbuf_addf(err, \"unable to restore logfile %s: %s\",\n+\t\t\t\t\t\t transaction->old_refname, strerror(errno));\n+\t\t\t\t\tret = -1;\n+\t\t\t\t} else {\n+\t\t\t\t\ttry_remove_empty_parents(refs,\n+\t\t\t\t\t\t\t transaction->new_refname,\n+\t\t\t\t\t\t\t REMOVE_EMPTY_PARENTS_REFLOG);\n+\t\t\t\t\tif (rename_tmp_log(refs,\n+\t\t\t\t\t\t\t transaction->old_refname)) {\n+\t\t\t\t\t\tstrbuf_addf(err, \"unable to restore logfile %s: %s\",\n+\t\t\t\t\t\t\t transaction->old_refname,\n+\t\t\t\t\t\t\t strerror(errno));\n+\t\t\t\t\t\tret = -1;\n+\t\t\t\t\t}\n+\t\t\t\t}\n+\t\t\t} else if (unlink(new_log.buf) < 0 && errno != ENOENT) {\n+\t\t\t\tstrbuf_addf(err, \"unable to remove logfile %s: %s\",\n+\t\t\t\t\t transaction->new_refname, strerror(errno));\n+\t\t\t\tret = -1;\n+\t\t\t}\n+\t\t}\n+\t\tif (data->destination_log_backed_up) {\n+\t\t\tfiles_reflog_path(refs, &destination_log,\n+\t\t\t\t\t TMP_RENAMED_LOG_DESTINATION);\n+\t\t\tif (rename(destination_log.buf, new_log.buf) < 0) {\n+\t\t\t\tstrbuf_addf(err, \"unable to restore logfile %s: %s\",\n+\t\t\t\t\t transaction->new_refname, strerror(errno));\n+\t\t\t\tret = -1;\n+\t\t\t}\n+\t\t}\n+\n+\t\tif (transaction->type == REF_TRANSACTION_TYPE_RENAME &&\n+\t\t data->destination_exists) {\n+\t\t\trestore_transaction = ref_store_transaction_begin(\n+\t\t\t\t\t&refs->base, REF_TRANSACTION_FLAG_SKIP_HOOK, err);\n+\t\t\tif (!restore_transaction ||\n+\t\t\t ref_transaction_update(restore_transaction,\n+\t\t\t\t\t\t transaction->new_refname,\n+\t\t\t\t\t\t data->destination_target ? NULL :\n+\t\t\t\t\t\t\t&data->destination_oid,\n+\t\t\t\t\t\t NULL,\n+\t\t\t\t\t\t data->destination_target,\n+\t\t\t\t\t\t NULL,\n+\t\t\t\t\t\t REF_NO_DEREF |\n+\t\t\t\t\t\t\tREF_SKIP_CREATE_REFLOG,\n+\t\t\t\t\t\t NULL, err) ||\n+\t\t\t ref_transaction_commit(restore_transaction, err))\n+\t\t\t\tret = -1;\n+\t\t\tref_transaction_free(restore_transaction);\n+\t\t}\n+\n+\t\tstrbuf_release(&destination_log);\n+\t\tstrbuf_release(&temporary_log);\n+\t\tstrbuf_release(&new_log);\n+\t\tfree(data->destination_target);\n+\t\tfree(data);\n+\t\ttransaction->backend_data = NULL;\n+\t\ttransaction->state = REF_TRANSACTION_CLOSED;\n+\t\treturn ret;\n+\t}\n+\n \tfiles_transaction_cleanup(refs, transaction);\n \treturn 0;\n }\n@@ -4095,8 +4313,6 @@ struct ref_storage_be refs_be_files = {\n \n \t.optimize = files_optimize,\n \t.optimize_required = files_optimize_required,\n-\t.rename_ref = files_rename_ref,\n-\t.copy_ref = files_copy_ref,\n \n \t.iterator_begin = files_ref_iterator_begin,\n \t.read_raw_ref = files_read_raw_ref,\ndiff --git a/refs/packed-backend.c b/refs/packed-backend.c\nindex a73fc6aca..364a91291 100644\n--- a/refs/packed-backend.c\n+++ b/refs/packed-backend.c\n@@ -2164,8 +2164,6 @@ struct ref_storage_be refs_be_packed = {\n \t.optimize = packed_optimize,\n \t.optimize_required = packed_optimize_required,\n \n-\t.rename_ref = NULL,\n-\t.copy_ref = NULL,\n \n \t.iterator_begin = packed_ref_iterator_begin,\n \t.read_raw_ref = packed_read_raw_ref,\ndiff --git a/refs/refs-internal.h b/refs/refs-internal.h\nindex c3ac7b556..406b54b65 100644\n--- a/refs/refs-internal.h\n+++ b/refs/refs-internal.h\n@@ -187,6 +187,13 @@ struct ref_update *ref_transaction_add_update(\n \t\tconst char *committer_info,\n \t\tconst char *msg);\n \n+int refs_delete_ref_with_transaction_flags(struct ref_store *refs,\n+\t\t\t\t\t const char *msg,\n+\t\t\t\t\t const char *refname,\n+\t\t\t\t\t const struct object_id *old_oid,\n+\t\t\t\t\t unsigned int flags,\n+\t\t\t\t\t unsigned int transaction_flags);\n+\n /*\n * Transaction states.\n *\n@@ -212,6 +219,12 @@ enum ref_transaction_state {\n \tREF_TRANSACTION_CLOSED = 2\n };\n \n+enum ref_transaction_type {\n+\tREF_TRANSACTION_TYPE_NORMAL = 0,\n+\tREF_TRANSACTION_TYPE_RENAME,\n+\tREF_TRANSACTION_TYPE_COPY,\n+};\n+\n /*\n * Data structure to hold indices of updates which were rejected, for batched\n * reference updates. While the updates themselves hold the rejection error,\n@@ -240,6 +253,21 @@ struct ref_transaction {\n \tvoid *backend_data;\n \tunsigned int flags;\n \tuint64_t max_index;\n+\n+\t/*\n+\t * Rename and copy operations need backend-specific reflog handling.\n+\t * Their logical updates still live in `updates`, so hooks see the\n+\t * operation like any other reference transaction. The fields below\n+\t * retain the state that backends verify after taking their locks.\n+\t */\n+\tenum ref_transaction_type type;\n+\tchar *old_refname;\n+\tchar *new_refname;\n+\tchar *logmsg;\n+\tstruct object_id source_oid;\n+\tstruct object_id destination_oid;\n+\tchar *destination_target;\n+\tunsigned int destination_exists:1;\n };\n \n /*\n@@ -451,13 +479,6 @@ typedef int optimize_required_fn(struct ref_store *ref_store,\n \t\t\t\t struct refs_optimize_opts *opts,\n \t\t\t\t bool *required);\n \n-typedef int rename_ref_fn(struct ref_store *ref_store,\n-\t\t\t const char *oldref, const char *newref,\n-\t\t\t const char *logmsg);\n-typedef int copy_ref_fn(struct ref_store *ref_store,\n-\t\t\t const char *oldref, const char *newref,\n-\t\t\t const char *logmsg);\n-\n /*\n * Iterate over the references in `ref_store` whose names start with\n * `prefix`. `prefix` is matched as a literal string, without regard\n@@ -577,9 +598,6 @@ struct ref_storage_be {\n \n \toptimize_fn *optimize;\n \toptimize_required_fn *optimize_required;\n-\trename_ref_fn *rename_ref;\n-\tcopy_ref_fn *copy_ref;\n-\n \tref_iterator_begin_fn *iterator_begin;\n \tread_raw_ref_fn *read_raw_ref;\n \ndiff --git a/refs/reftable-backend.c b/refs/reftable-backend.c\nindex 10db03991..cf8a16af5 100644\n--- a/refs/reftable-backend.c\n+++ b/refs/reftable-backend.c\n@@ -953,6 +953,14 @@ struct reftable_transaction_data {\n \tsize_t args_nr, args_alloc;\n };\n \n+struct reftable_copy_or_rename_transaction_data {\n+\tstruct reftable_addition *addition;\n+};\n+\n+static int reftable_be_copy_or_rename_prepare(struct ref_store *ref_store,\n+\t\t\t\t\t struct ref_transaction *transaction,\n+\t\t\t\t\t struct strbuf *err);\n+\n static void free_transaction_data(struct reftable_transaction_data *tx_data)\n {\n \tif (!tx_data)\n@@ -1326,6 +1334,10 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,\n \tsize_t i;\n \tint ret;\n \n+\tif (transaction->type != REF_TRANSACTION_TYPE_NORMAL)\n+\t\treturn reftable_be_copy_or_rename_prepare(ref_store, transaction,\n+\t\t\t\t\t\t\t err);\n+\n \tret = refs->err;\n \tif (ret < 0)\n \t\tgoto done;\n@@ -1419,7 +1431,20 @@ static int reftable_be_transaction_abort(struct ref_store *ref_store UNUSED,\n \t\t\t\t\t struct ref_transaction *transaction,\n \t\t\t\t\t struct strbuf *err UNUSED)\n {\n-\tstruct reftable_transaction_data *tx_data = transaction->backend_data;\n+\tstruct reftable_transaction_data *tx_data;\n+\n+\tif (transaction->type != REF_TRANSACTION_TYPE_NORMAL) {\n+\t\tstruct reftable_copy_or_rename_transaction_data *data =\n+\t\t\ttransaction->backend_data;\n+\n+\t\treftable_addition_destroy(data->addition);\n+\t\tfree(data);\n+\t\ttransaction->backend_data = NULL;\n+\t\ttransaction->state = REF_TRANSACTION_CLOSED;\n+\t\treturn 0;\n+\t}\n+\n+\ttx_data = transaction->backend_data;\n \tfree_transaction_data(tx_data);\n \ttransaction->state = REF_TRANSACTION_CLOSED;\n \treturn 0;\n@@ -1667,9 +1692,28 @@ static int reftable_be_transaction_finish(struct ref_store *ref_store UNUSED,\n \t\t\t\t\t struct ref_transaction *transaction,\n \t\t\t\t\t struct strbuf *err)\n {\n-\tstruct reftable_transaction_data *tx_data = transaction->backend_data;\n+\tstruct reftable_transaction_data *tx_data;\n \tint ret = 0;\n \n+\tif (transaction->type != REF_TRANSACTION_TYPE_NORMAL) {\n+\t\tstruct reftable_copy_or_rename_transaction_data *data =\n+\t\t\ttransaction->backend_data;\n+\t\tint special_ret = reftable_addition_commit(data->addition);\n+\n+\t\treftable_addition_destroy(data->addition);\n+\t\tfree(data);\n+\t\ttransaction->backend_data = NULL;\n+\t\ttransaction->state = REF_TRANSACTION_CLOSED;\n+\t\tif (special_ret < 0) {\n+\t\t\tstrbuf_addf(err, _(\"reftable: transaction failure: %s\"),\n+\t\t\t\t reftable_error_str(special_ret));\n+\t\t\treturn -1;\n+\t\t}\n+\t\treturn 0;\n+\t}\n+\n+\ttx_data = transaction->backend_data;\n+\n \tfor (size_t i = 0; i < tx_data->args_nr; i++) {\n \t\ttx_data->args[i].max_index = transaction->max_index;\n \n@@ -1764,17 +1808,20 @@ struct write_create_symref_arg {\n struct write_copy_arg {\n \tstruct reftable_ref_store *refs;\n \tstruct reftable_backend *be;\n+\tstruct strbuf *err;\n \tconst char *oldname;\n \tconst char *newname;\n \tconst char *logmsg;\n \tint delete_old;\n+\tstruct ref_transaction *transaction;\n };\n \n static int write_copy_table(struct reftable_writer *writer, void *cb_data)\n {\n \tstruct write_copy_arg *arg = cb_data;\n \tuint64_t deletion_ts, creation_ts;\n-\tstruct reftable_ref_record old_ref = {0}, refs[2] = {0};\n+\tstruct reftable_ref_record old_ref = {0}, destination_ref = {0};\n+\tstruct reftable_ref_record refs[2] = {0};\n \tstruct reftable_log_record old_log = {0}, *logs = NULL;\n \tstruct reftable_iterator it = {0};\n \tstruct string_list skip = STRING_LIST_INIT_NODUP;\n@@ -1789,14 +1836,75 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data)\n \t\tBUG(\"failed splitting committer info\");\n \n \tif (reftable_stack_read_ref(arg->be->stack, arg->oldname, &old_ref)) {\n-\t\tret = error(_(\"refname %s not found\"), arg->oldname);\n+\t\tstrbuf_addf(arg->err, _(\"refname %s not found\"), arg->oldname);\n+\t\tret = -1;\n \t\tgoto done;\n \t}\n \tif (old_ref.value_type == REFTABLE_REF_SYMREF) {\n-\t\tret = error(_(\"refname %s is a symbolic ref, copying it is not supported\"),\n+\t\tstrbuf_addf(arg->err,\n+\t\t\t _(\"refname %s is a symbolic ref, copying it is not supported\"),\n \t\t\t arg->oldname);\n+\t\tret = -1;\n \t\tgoto done;\n \t}\n+\tif (arg->transaction) {\n+\t\tstruct object_id oid;\n+\n+\t\tif (old_ref.value_type == REFTABLE_REF_VAL2)\n+\t\t\toidread(&oid, old_ref.value.val2.value,\n+\t\t\t\targ->refs->base.repo->hash_algo);\n+\t\telse\n+\t\t\toidread(&oid, old_ref.value.val1,\n+\t\t\t\targ->refs->base.repo->hash_algo);\n+\t\tif (!oideq(&oid, &arg->transaction->source_oid)) {\n+\t\t\tstrbuf_addf(arg->err,\n+\t\t\t\t _(\"refname %s is at %s but expected %s\"),\n+\t\t\t\t arg->oldname, oid_to_hex(&oid),\n+\t\t\t\t oid_to_hex(&arg->transaction->source_oid));\n+\t\t\tret = -1;\n+\t\t\tgoto done;\n+\t\t}\n+\n+\t\tret = reftable_stack_read_ref(arg->be->stack, arg->newname,\n+\t\t\t\t\t &destination_ref);\n+\t\tif (ret < 0)\n+\t\t\tgoto done;\n+\t\tif (arg->transaction->destination_exists != !ret) {\n+\t\t\tstrbuf_addf(arg->err,\n+\t\t\t\t _(\"refname %s changed while renaming\"),\n+\t\t\t\t arg->newname);\n+\t\t\tret = -1;\n+\t\t\tgoto done;\n+\t\t}\n+\t\tif (!ret) {\n+\t\t\tif (destination_ref.value_type == REFTABLE_REF_SYMREF) {\n+\t\t\t\tif (!arg->transaction->destination_target ||\n+\t\t\t\t strcmp(destination_ref.value.symref,\n+\t\t\t\t\t arg->transaction->destination_target)) {\n+\t\t\t\t\tstrbuf_addf(arg->err,\n+\t\t\t\t\t\t _(\"refname %s changed while renaming\"),\n+\t\t\t\t\t\t arg->newname);\n+\t\t\t\t\tret = -1;\n+\t\t\t\t\tgoto done;\n+\t\t\t\t}\n+\t\t\t} else {\n+\t\t\t\tif (destination_ref.value_type == REFTABLE_REF_VAL2)\n+\t\t\t\t\toidread(&oid, destination_ref.value.val2.value,\n+\t\t\t\t\t\targ->refs->base.repo->hash_algo);\n+\t\t\t\telse\n+\t\t\t\t\toidread(&oid, destination_ref.value.val1,\n+\t\t\t\t\t\targ->refs->base.repo->hash_algo);\n+\t\t\t\tif (arg->transaction->destination_target ||\n+\t\t\t\t !oideq(&oid, &arg->transaction->destination_oid)) {\n+\t\t\t\t\tstrbuf_addf(arg->err,\n+\t\t\t\t\t\t _(\"refname %s changed while renaming\"),\n+\t\t\t\t\t\t arg->newname);\n+\t\t\t\t\tret = -1;\n+\t\t\t\t\tgoto done;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t}\n+\t}\n \n \t/*\n \t * There's nothing to do in case the old and new name are the same, so\n@@ -1815,7 +1923,7 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data)\n \tret = refs_verify_refname_available(&arg->refs->base, arg->newname,\n \t\t\t\t\t NULL, &skip, 0, &errbuf);\n \tif (ret < 0) {\n-\t\terror(\"%s\", errbuf.buf);\n+\t\tstrbuf_addbuf(arg->err, &errbuf);\n \t\tgoto done;\n \t}\n \n@@ -1980,68 +2088,60 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data)\n \tfor (i = 0; i < ARRAY_SIZE(refs); i++)\n \t\treftable_ref_record_release(&refs[i]);\n \treftable_ref_record_release(&old_ref);\n+\treftable_ref_record_release(&destination_ref);\n \treftable_log_record_release(&old_log);\n \treturn ret;\n }\n \n-static int reftable_be_rename_ref(struct ref_store *ref_store,\n-\t\t\t\t const char *oldrefname,\n-\t\t\t\t const char *newrefname,\n-\t\t\t\t const char *logmsg)\n+static int reftable_be_copy_or_rename_prepare(struct ref_store *ref_store,\n+\t\t\t\t\t struct ref_transaction *transaction,\n+\t\t\t\t\t struct strbuf *err)\n {\n \tstruct reftable_ref_store *refs =\n-\t\treftable_be_downcast(ref_store, REF_STORE_WRITE, \"rename_ref\");\n+\t\treftable_be_downcast(ref_store, REF_STORE_WRITE,\n+\t\t\t\t \"ref_transaction_prepare\");\n+\tstruct reftable_copy_or_rename_transaction_data *data = NULL;\n \tstruct write_copy_arg arg = {\n \t\t.refs = refs,\n-\t\t.oldname = oldrefname,\n-\t\t.newname = newrefname,\n-\t\t.logmsg = logmsg,\n-\t\t.delete_old = 1,\n+\t\t.err = err,\n+\t\t.oldname = transaction->old_refname,\n+\t\t.newname = transaction->new_refname,\n+\t\t.logmsg = transaction->logmsg,\n+\t\t.delete_old = transaction->type == REF_TRANSACTION_TYPE_RENAME,\n+\t\t.transaction = transaction,\n \t};\n \tint ret;\n \n+\tCALLOC_ARRAY(data, 1);\n \tret = refs->err;\n \tif (ret < 0)\n \t\tgoto done;\n-\n-\tret = backend_for(&arg.be, refs, newrefname, &newrefname, 1);\n+\tret = backend_for(&arg.be, refs, transaction->new_refname,\n+\t\t\t &arg.newname, 1);\n \tif (ret)\n \t\tgoto done;\n-\tret = reftable_stack_add(arg.be->stack, &write_copy_table, &arg,\n-\t\t\t\t &reftable_be_write_options(refs)->opts);\n-\n-done:\n-\tassert(ret != REFTABLE_API_ERROR);\n-\treturn ret;\n-}\n-\n-static int reftable_be_copy_ref(struct ref_store *ref_store,\n-\t\t\t\tconst char *oldrefname,\n-\t\t\t\tconst char *newrefname,\n-\t\t\t\tconst char *logmsg)\n-{\n-\tstruct reftable_ref_store *refs =\n-\t\treftable_be_downcast(ref_store, REF_STORE_WRITE, \"copy_ref\");\n-\tstruct write_copy_arg arg = {\n-\t\t.refs = refs,\n-\t\t.oldname = oldrefname,\n-\t\t.newname = newrefname,\n-\t\t.logmsg = logmsg,\n-\t};\n-\tint ret;\n-\n-\tret = refs->err;\n-\tif (ret < 0)\n+\tret = reftable_stack_addition_new(&data->addition, arg.be->stack,\n+\t\t\t\t\t &reftable_be_write_options(refs)->opts);\n+\tif (ret)\n \t\tgoto done;\n-\n-\tret = backend_for(&arg.be, refs, newrefname, &newrefname, 1);\n+\tret = reftable_addition_add(data->addition, &write_copy_table, &arg);\n \tif (ret)\n \t\tgoto done;\n-\tret = reftable_stack_add(arg.be->stack, &write_copy_table, &arg,\n-\t\t\t\t &reftable_be_write_options(refs)->opts);\n+\n+\ttransaction->backend_data = data;\n+\ttransaction->state = REF_TRANSACTION_PREPARED;\n+\treturn 0;\n \n done:\n \tassert(ret != REFTABLE_API_ERROR);\n+\tif (data) {\n+\t\treftable_addition_destroy(data->addition);\n+\t\tfree(data);\n+\t}\n+\ttransaction->state = REF_TRANSACTION_CLOSED;\n+\tif (ret && !err->len)\n+\t\tstrbuf_addf(err, _(\"reftable: transaction prepare: %s\"),\n+\t\t\t reftable_error_str(ret));\n \treturn ret;\n }\n \n@@ -2872,8 +2972,6 @@ struct ref_storage_be refs_be_reftable = {\n \t.optimize = reftable_be_optimize,\n \t.optimize_required = reftable_be_optimize_required,\n \n-\t.rename_ref = reftable_be_rename_ref,\n-\t.copy_ref = reftable_be_copy_ref,\n \n \t.iterator_begin = reftable_be_iterator_begin,\n \t.read_raw_ref = reftable_be_read_raw_ref,\ndiff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh\nindex 4fe9d9b23..116b2ff07 100755\n--- a/t/t1416-ref-transaction-hooks.sh\n+++ b/t/t1416-ref-transaction-hooks.sh\n@@ -93,6 +93,148 @@ test_expect_success 'hook gets all queued updates in committed state' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'hook gets both updates when renaming a branch' '\n+\ttest_when_finished \"rm -f actual\" &&\n+\tgit branch old PRE &&\n+\ttest_hook reference-transaction <<-\\EOF &&\n+\t\techo \"$1\" >>actual &&\n+\t\tcat >>actual\n+\tEOF\n+\tcat >expect <<-EOF &&\n+\tpreparing\n+\t$PRE_OID $ZERO_OID refs/heads/old\n+\t$ZERO_OID $PRE_OID refs/heads/new\n+\tprepared\n+\t$PRE_OID $ZERO_OID refs/heads/old\n+\t$ZERO_OID $PRE_OID refs/heads/new\n+\tcommitted\n+\t$PRE_OID $ZERO_OID refs/heads/old\n+\t$ZERO_OID $PRE_OID refs/heads/new\n+\tEOF\n+\tgit branch -m old new &&\n+\ttest_cmp expect actual &&\n+\ttest_must_fail git rev-parse --verify refs/heads/old &&\n+\ttest_cmp_rev PRE refs/heads/new\n+'\n+\n+test_expect_success 'hook gets destination update when copying a branch' '\n+\ttest_when_finished \"rm -f actual\" &&\n+\tgit branch copy-source PRE &&\n+\ttest_hook reference-transaction <<-\\EOF &&\n+\t\techo \"$1\" >>actual &&\n+\t\tcat >>actual\n+\tEOF\n+\tcat >expect <<-EOF &&\n+\tpreparing\n+\t$ZERO_OID $PRE_OID refs/heads/copy-destination\n+\tprepared\n+\t$ZERO_OID $PRE_OID refs/heads/copy-destination\n+\tcommitted\n+\t$ZERO_OID $PRE_OID refs/heads/copy-destination\n+\tEOF\n+\tgit branch -c copy-source copy-destination &&\n+\ttest_cmp expect actual &&\n+\ttest_cmp_rev PRE refs/heads/copy-source &&\n+\ttest_cmp_rev PRE refs/heads/copy-destination\n+'\n+\n+test_expect_success 'hook gets overwritten values for forced rename and copy' '\n+\tgit branch force-old PRE &&\n+\tgit branch force-new POST &&\n+\tgit branch force-copy-source PRE &&\n+\tgit branch force-copy-destination POST &&\n+\ttest_hook reference-transaction <<-\\EOF &&\n+\t\tif test \"$1\" = committed\n+\t\tthen\n+\t\t\tcat >>actual\n+\t\tfi\n+\tEOF\n+\tgit branch -M force-old force-new &&\n+\tgit branch -C force-copy-source force-copy-destination &&\n+\tcat >expect <<-EOF &&\n+\t$PRE_OID $ZERO_OID refs/heads/force-old\n+\t$POST_OID $PRE_OID refs/heads/force-new\n+\t$POST_OID $PRE_OID refs/heads/force-copy-destination\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'hook can abort a branch rename after preparation' '\n+\tgit branch abort-old PRE &&\n+\tgit branch abort-new POST &&\n+\tgit reflog show --format=%gs abort-old >old-log &&\n+\tgit reflog show --format=%gs abort-new >new-log &&\n+\ttest_hook reference-transaction <<-\\EOF &&\n+\t\ttest \"$1\" != prepared\n+\tEOF\n+\ttest_must_fail git branch -M abort-old abort-new &&\n+\ttest_cmp_rev PRE refs/heads/abort-old &&\n+\ttest_cmp_rev POST refs/heads/abort-new &&\n+\tgit reflog show --format=%gs abort-old >old-log-after &&\n+\tgit reflog show --format=%gs abort-new >new-log-after &&\n+\ttest_cmp old-log old-log-after &&\n+\ttest_cmp new-log new-log-after\n+'\n+\n+test_expect_success 'hook can abort a D/F branch rename after preparation' '\n+\tgit branch df-old PRE &&\n+\tgit reflog show --format=%gs df-old >df-log &&\n+\ttest_hook reference-transaction <<-\\EOF &&\n+\t\ttest \"$1\" != prepared\n+\tEOF\n+\ttest_must_fail git branch -m df-old df-old/child &&\n+\ttest_cmp_rev PRE refs/heads/df-old &&\n+\ttest_must_fail git rev-parse --verify refs/heads/df-old/child &&\n+\tgit reflog show --format=%gs df-old >df-log-after &&\n+\ttest_cmp df-log df-log-after\n+'\n+\n+test_expect_success 'hook can abort a reverse D/F rename after preparation' '\n+\tgit branch reverse/old PRE &&\n+\tgit reflog show --format=%gs reverse/old >reverse-log &&\n+\ttest_hook reference-transaction <<-\\EOF &&\n+\t\ttest \"$1\" != prepared\n+\tEOF\n+\ttest_must_fail git branch -m reverse/old reverse &&\n+\ttest_cmp_rev PRE refs/heads/reverse/old &&\n+\ttest_must_fail git rev-parse --verify refs/heads/reverse &&\n+\tgit reflog show --format=%gs reverse/old >reverse-log-after &&\n+\ttest_cmp reverse-log reverse-log-after\n+'\n+\n+test_expect_success 'hook can abort a forced branch copy after preparation' '\n+\tgit branch copy-abort-old PRE &&\n+\tgit branch copy-abort-new POST &&\n+\tgit reflog show --format=%gs copy-abort-old >copy-old-log &&\n+\tgit reflog show --format=%gs copy-abort-new >copy-new-log &&\n+\ttest_hook reference-transaction <<-\\EOF &&\n+\t\ttest \"$1\" != prepared\n+\tEOF\n+\ttest_must_fail git branch -C copy-abort-old copy-abort-new &&\n+\ttest_cmp_rev PRE refs/heads/copy-abort-old &&\n+\ttest_cmp_rev POST refs/heads/copy-abort-new &&\n+\tgit reflog show --format=%gs copy-abort-old >copy-old-log-after &&\n+\tgit reflog show --format=%gs copy-abort-new >copy-new-log-after &&\n+\ttest_cmp copy-old-log copy-old-log-after &&\n+\ttest_cmp copy-new-log copy-new-log-after\n+'\n+\n+test_expect_success 'branch rename detects an update during preparing hook' '\n+\tgit branch race-old PRE &&\n+\tgit branch race-new POST &&\n+\ttest_hook reference-transaction <<-\\EOF &&\n+\t\tmarker=$(git rev-parse --git-path rename-race-once)\n+\t\tif test \"$1\" = preparing && test ! -e \"$marker\"\n+\t\tthen\n+\t\t\t>\"$marker\" &&\n+\t\t\tgit update-ref refs/heads/race-old POST\n+\t\tfi\n+\tEOF\n+\ttest_must_fail git branch -M race-old race-new &&\n+\ttest_cmp_rev POST refs/heads/race-old &&\n+\ttest_cmp_rev POST refs/heads/race-new\n+'\n+\n test_expect_success 'hook gets all queued updates in aborted state' '\n \ttest_when_finished \"rm actual\" &&\n \tgit reset --hard PRE &&\n\nbase-commit: d38352cd43ab9745686d697872408bc3249a153f\n-- \n2.39.3 (Apple Git-146)\n\n"},{"id":"552943","messageId":"xmqqjyoemqvu.fsf@gitster.g","threadId":"66350","inReplyTo":"20260920165037.88524-1-maciej.ciemborowicz@gmail.com","subject":"Re: [PATCH] refs: run copy and rename through transactions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-21T17:54:45Z","receivedAt":"2026-09-21T17:54:47Z","isPatch":true,"body":"Maciej Ciemborowicz writes:\n\n> Reference copy and rename operations currently bypass the transaction API.\n> Consequently, the reference-transaction hook sees only the source deletion\n> with the files backend and no useful update with the reftable backend.\n>\n> Represent both operations as reference transactions containing their\n> logical updates. A rename is a deletion of the old reference and creation\n> of the new reference in the same transaction. Retain backend-specific\n> reflog handling: the files backend stages its existing rename procedure\n> across prepare, finish and abort, while reftable stages an addition while\n> holding the stack lock. Suppress hooks for the files backend's nested\n> deletion transactions so that callers observe one logical transaction.\n>\n> Record and verify the source and destination values after taking backend\n> locks. This rejects concurrent changes instead of applying a rename or copy\n> that differs from the payload shown to the preparing hook. Preserve D/F\n> renames and restore overwritten references and reflogs when a prepared hook\n> rejects the operation.\n>\n> Add coverage for rename, copy, forced updates, both directions of D/F\n> conflicts, concurrent updates and prepared-hook rollback.\n>\n> Helped-by: Karthik Nayak \n> Signed-off-by: Maciej Ciemborowicz \n> ---\n\nDrop unnecessary \"currently\" to the first sentence, and add \"test\"\nto the laste sentence somewhere, and this would be perfect.\n\nVery pleasing to see an exceptionally well-written proposed commit\nlog message by a new contributor.\n\n> refs.c | 137 ++++++++++++---\n> refs.h | 3 +\n> refs/debug.c | 25 ---\n> refs/files-backend.c | 276 +++++++++++++++++++++++++++----\n> refs/packed-backend.c | 2 -\n> refs/refs-internal.h | 38 +++--\n> refs/reftable-backend.c | 194 ++++++++++++++++------\n> t/t1416-ref-transaction-hooks.sh | 142 ++++++++++++++++\n> 8 files changed, 679 insertions(+), 138 deletions(-)\n>\n> diff --git a/refs.c b/refs.c\n> index 92d5df5b7..22c000f7f 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -1004,15 +1004,17 @@ long get_files_ref_lock_timeout_ms(struct repository *repo)\n> \treturn timeout_ms;\n> }\n> \n> -int refs_delete_ref(struct ref_store *refs, const char *msg,\n> -\t\t const char *refname,\n> -\t\t const struct object_id *old_oid,\n> -\t\t unsigned int flags)\n> +int refs_delete_ref_with_transaction_flags(struct ref_store *refs,\n> +\t\t\t\t\t const char *msg,\n> +\t\t\t\t\t const char *refname,\n> +\t\t\t\t\t const struct object_id *old_oid,\n> +\t\t\t\t\t unsigned int flags,\n> +\t\t\t\t\t unsigned int transaction_flags)\n> {\n> \tstruct ref_transaction *transaction;\n> \tstruct strbuf err = STRBUF_INIT;\n> \n> -\ttransaction = ref_store_transaction_begin(refs, 0, &err);\n> +\ttransaction = ref_store_transaction_begin(refs, transaction_flags, &err);\n> \tif (!transaction ||\n> \t ref_transaction_delete(transaction, refname, old_oid,\n> \t\t\t\t NULL, flags, msg, &err) ||\n> @@ -1027,6 +1029,15 @@ int refs_delete_ref(struct ref_store *refs, const char *msg,\n> \treturn 0;\n> }\n> \n> +int refs_delete_ref(struct ref_store *refs, const char *msg,\n> +\t\t const char *refname,\n> +\t\t const struct object_id *old_oid,\n> +\t\t unsigned int flags)\n> +{\n> +\treturn refs_delete_ref_with_transaction_flags(refs, msg, refname,\n> +\t\t\t\t\t\t old_oid, flags, 0);\n> +}\n> +\n> static void copy_reflog_msg(struct strbuf *sb, const char *msg)\n> {\n> \tchar c;\n> @@ -1270,6 +1281,10 @@ void ref_transaction_free(struct ref_transaction *transaction)\n> \n> \tstring_list_clear(&transaction->refnames, 0);\n> \tfree(transaction->updates);\n> +\tfree(transaction->old_refname);\n> +\tfree(transaction->new_refname);\n> +\tfree(transaction->logmsg);\n> +\tfree(transaction->destination_target);\n> \tfree(transaction);\n> }\n> \n> @@ -2710,7 +2725,8 @@ int ref_transaction_prepare(struct ref_transaction *transaction,\n> \t\treturn REF_TRANSACTION_ERROR_GENERIC;\n> \n> \t/* Preparing checks before locking references */\n> -\tret = run_transaction_hook(transaction, \"preparing\");\n> +\tret = transaction->flags & REF_TRANSACTION_FLAG_SKIP_HOOK ? 0 :\n> +\t\trun_transaction_hook(transaction, \"preparing\");\n> \tif (ret) {\n> \t\tref_transaction_abort(transaction, err);\n> \t\tdie(_(abort_by_ref_transaction_hook), \"preparing\");\n> @@ -2720,7 +2736,8 @@ int ref_transaction_prepare(struct ref_transaction *transaction,\n> \tif (ret)\n> \t\treturn ret;\n> \n> -\tret = run_transaction_hook(transaction, \"prepared\");\n> +\tret = transaction->flags & REF_TRANSACTION_FLAG_SKIP_HOOK ? 0 :\n> +\t\trun_transaction_hook(transaction, \"prepared\");\n> \tif (ret) {\n> \t\tref_transaction_abort(transaction, err);\n> \t\tdie(_(abort_by_ref_transaction_hook), \"prepared\");\n> @@ -2750,7 +2767,8 @@ int ref_transaction_abort(struct ref_transaction *transaction,\n> \t\tbreak;\n> \t}\n> \n> -\trun_transaction_hook(transaction, \"aborted\");\n> +\tif (!(transaction->flags & REF_TRANSACTION_FLAG_SKIP_HOOK))\n> +\t\trun_transaction_hook(transaction, \"aborted\");\n> \n> \tref_transaction_free(transaction);\n> \treturn ret;\n> @@ -2781,7 +2799,8 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n> \t}\n> \n> \tret = refs->be->transaction_finish(refs, transaction, err);\n> -\tif (!ret && !(transaction->flags & REF_TRANSACTION_FLAG_INITIAL))\n> +\tif (!ret && !(transaction->flags & (REF_TRANSACTION_FLAG_INITIAL |\n> +\t\t\t\t\t REF_TRANSACTION_FLAG_SKIP_HOOK)))\n> \t\trun_transaction_hook(transaction, \"committed\");\n> \treturn ret;\n> }\n> @@ -3123,28 +3142,100 @@ int refs_delete_refs(struct ref_store *refs, const char *logmsg,\n> \treturn ret;\n> }\n> \n> -int refs_rename_ref(struct ref_store *refs, const char *oldref,\n> -\t\t const char *newref, const char *logmsg)\n\nIt is annoying that we have to give random callers an unrestricted\nway to skip calling hooks. I suspect it may come from \"this\nfunction should call hook when invoked as the top-level operation,\nbut when it is used as a subroutine for a different top-level\noperation, we want to skip hooks\" kind of reasoning, but is this\nsomething we can avoid by rearranging the call chain?\n\n> +static int refs_copy_or_rename_ref(struct ref_store *refs, const char *oldref,\n> +\t\t\t\t const char *newref, const char *logmsg,\n> +\t\t\t\t int copy)\n\nWill this function ever gain a third mode of operation other than\ncopy or rename? If not, perhaps \"bool copy\"?\n\n> {\n> -\tchar *msg;\n> -\tint retval;\n> +\tstruct ref_transaction *transaction = NULL;\n> +\tstruct object_id old_oid, new_oid;\n> +\tstruct strbuf new_target = STRBUF_INIT;\n> +\tstruct strbuf err = STRBUF_INIT;\n> +\tchar *msg = normalize_reflog_message(logmsg);\n> +\tint old_flags, new_flags = 0, new_exists = 0, ret = 1;\n> \n> -\tmsg = normalize_reflog_message(logmsg);\n> -\tretval = refs->be->rename_ref(refs, oldref, newref, msg);\n> +\tif (!strcmp(oldref, newref)) {\n> +\t\tret = 0;\n> +\t\tgoto out;\n> +\t}\n> +\n> +\tif (!refs_resolve_ref_unsafe(refs, oldref,\n> +\t\t\t\t RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n> +\t\t\t\t &old_oid, &old_flags)) {\n> +\t\terror(\"refname %s not found\", oldref);\n> +\t\tgoto out;\n> +\t}\n> +\tif (old_flags & REF_ISSYMREF) {\n> +\t\terror(\"refname %s is a symbolic ref, %s it is not supported\",\n> +\t\t oldref, copy ? \"copying\" : \"renaming\");\n> +\t\tgoto out;\n> +\t}\n> +\n> +\ttransaction = ref_store_transaction_begin(refs, 0, &err);\n> +\tif (!transaction)\n> +\t\tgoto error;\n> +\ttransaction->type = copy ? REF_TRANSACTION_TYPE_COPY :\n> +\t\tREF_TRANSACTION_TYPE_RENAME;\n> +\ttransaction->old_refname = xstrdup(oldref);\n> +\ttransaction->new_refname = xstrdup(newref);\n> +\ttransaction->logmsg = xstrdup(msg);\n> +\toidcpy(&transaction->source_oid, &old_oid);\n> +\n> +\tif (!copy && ref_transaction_delete(transaction, oldref, &old_oid, NULL,\n> +\t\t\t\t\t REF_NO_DEREF, msg, &err))\n> +\t\tgoto error;\n> +\n> +\tif (refs_resolve_ref_unsafe(refs, newref,\n> +\t\t\t\t RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n> +\t\t\t\t &new_oid, &new_flags)) {\n> +\t\tnew_exists = 1;\n> +\t\tif ((new_flags & REF_ISSYMREF) &&\n> +\t\t refs_read_symbolic_ref(refs, newref, &new_target) < 0) {\n> +\t\t\tstrbuf_addf(&err, \"unable to read symbolic ref %s\", newref);\n> +\t\t\tgoto error;\n> +\t\t}\n> +\t} else {\n> +\t\toidclr(&new_oid, refs->repo->hash_algo);\n> +\t}\n> +\ttransaction->destination_exists = new_exists;\n> +\tif (new_flags & REF_ISSYMREF)\n> +\t\ttransaction->destination_target = xstrdup(new_target.buf);\n> +\telse if (transaction->destination_exists)\n> +\t\toidcpy(&transaction->destination_oid, &new_oid);\n> +\n> +\tif (ref_transaction_update(transaction, newref, &old_oid,\n> +\t\t\t\t (new_flags & REF_ISSYMREF) ? NULL : &new_oid,\n> +\t\t\t\t NULL,\n> +\t\t\t\t (new_flags & REF_ISSYMREF) ? new_target.buf : NULL,\n> +\t\t\t\t REF_NO_DEREF | REF_SKIP_CREATE_REFLOG,\n> +\t\t\t\t NULL, &err))\n> +\t\tgoto error;\n> +\n> +\tif (ref_transaction_commit(transaction, &err))\n> +\t\tgoto error;\n> +\n> +\tret = 0;\n> +\tgoto out;\n> +\n> +error:\n> +\terror(\"%s\", err.buf);\n> +out:\n> +\tref_transaction_free(transaction);\n> +\tstrbuf_release(&new_target);\n> +\tstrbuf_release(&err);\n> \tfree(msg);\n> -\treturn retval;\n> +\treturn ret;\n> }\n\nThat's quite a lot of new code. I see ref_transaction_delete(),\nref_transaction_update() and others are already reused from existing\ncode paths, which is good.\n\n> +struct files_copy_or_rename_transaction_data {\n> +\tstruct ref_lock *lock;\n> +\tstruct object_id orig_oid;\n> +\tstruct object_id destination_oid;\n> +\tchar *destination_target;\n> +\tint logmoved;\n> +\tint destination_exists;\n> +\tint destination_log_backed_up;\n> +};\n\nGood to have a type that can be used to hold pieces of information\nspecific to the operation. Can't we do without rename/copy specific\naddition to the generic ref_transaction struct by following the same\nprinciple?\n\nThe comment above the members does make it understandable, but ...\n\n> @@ -240,6 +253,21 @@ struct ref_transaction {\n> \tvoid *backend_data;\n> \tunsigned int flags;\n> \tuint64_t max_index;\n> +\n> +\t/*\n> +\t * Rename and copy operations need backend-specific reflog handling.\n> +\t * Their logical updates still live in `updates`, so hooks see the\n> +\t * operation like any other reference transaction. The fields below\n> +\t * retain the state that backends verify after taking their locks.\n> +\t */\n> +\tenum ref_transaction_type type;\n> +\tchar *old_refname;\n> +\tchar *new_refname;\n> +\tchar *logmsg;\n> +\tstruct object_id source_oid;\n> +\tstruct object_id destination_oid;\n> +\tchar *destination_target;\n> +\tunsigned int destination_exists:1;\n> };\n\n... is it the best we can do to contaminate a rather generic data\nstructure for such a details relevant only to one specific\noperation?\n\nThanks.\n"},{"id":"552960","messageId":"xmqqh5jijiar.fsf@gitster.g","threadId":"66350","inReplyTo":"xmqqjyoemqvu.fsf@gitster.g","subject":"Re: [PATCH] refs: run copy and rename through transactions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-21T23:28:28Z","receivedAt":"2026-09-21T23:28:33Z","isPatch":true,"body":"Junio C Hamano writes:\n\n>> +struct files_copy_or_rename_transaction_data {\n>> +\tstruct ref_lock *lock;\n>> +\tstruct object_id orig_oid;\n>> +\tstruct object_id destination_oid;\n>> +\tchar *destination_target;\n>> +\tint logmoved;\n>> +\tint destination_exists;\n>> +\tint destination_log_backed_up;\n>> +};\n>\n> Good to have a type that can be used to hold pieces of information\n> specific to the operation. Can't we do without rename/copy specific\n> addition to the generic ref_transaction struct by following the same\n> principle?\n>\n> The comment above the members does make it understandable, but ...\n>\n>> @@ -240,6 +253,21 @@ struct ref_transaction {\n>> \tvoid *backend_data;\n>> \tunsigned int flags;\n>> \tuint64_t max_index;\n>> +\n>> +\t/*\n>> +\t * Rename and copy operations need backend-specific reflog handling.\n>> +\t * Their logical updates still live in `updates`, so hooks see the\n>> +\t * operation like any other reference transaction. The fields below\n>> +\t * retain the state that backends verify after taking their locks.\n>> +\t */\n>> +\tenum ref_transaction_type type;\n>> +\tchar *old_refname;\n>> +\tchar *new_refname;\n>> +\tchar *logmsg;\n>> +\tstruct object_id source_oid;\n>> +\tstruct object_id destination_oid;\n>> +\tchar *destination_target;\n>> +\tunsigned int destination_exists:1;\n>> };\n>\n> ... is it the best we can do to contaminate a rather generic data\n> structure for such a details relevant only to one specific\n> operation?\n\nMore importantly, this structure suggests to me that you can have a\nsingle rename (or copy) from one source to one destination in a\nsingle transaction. Is that correct or am I misunderstanding the\nway this data structure is used? How would one rename A, B and C\nto X, Y and Z in a single transaction? Or perhaps rename A to B\nand copy C to D in a single transaction?\n"},{"id":"552986","messageId":"CACQ=SRGicdcnyP6mFifdZgzCgoSgDPA=yrkr2gSPQ6_s7GJKsw@mail.gmail.com","threadId":"66350","inReplyTo":"xmqqh5jijiar.fsf@gitster.g","subject":"Re: [PATCH] refs: run copy and rename through transactions","fromName":"Maciej Ciemborowicz","fromEmail":"maciej.ciemborowicz@gmail.com","sentAt":"2026-09-22T13:08:24Z","receivedAt":"2026-09-22T13:08:38Z","isPatch":true,"body":"On Tue, Sep 22, 2026 at 1:28 AM Junio C Hamano wrote:\n> More importantly, this structure suggests to me that you can have a\n> single rename (or copy) from one source to one destination in a\n> single transaction. Is that correct or am I misunderstanding the\n> way this data structure is used? How would one rename A, B and C\n> to X, Y and Z in a single transaction? Or perhaps rename A to B\n> and copy C to D in a single transaction?\n\nGood point. The current design indeed makes copy/rename a property of\nthe whole transacton, so it does not compose with multiple such\noperations. I'll rework it so that state belongs to individual\nupdates instead. Unfortunately, I'll have to put this aside for a few\ndays until I finish this first:\nhttps://lore.kernel.org/git/CACQ=SRHthWOLVXmY6wgknOPgpQ+oB1vV-Q0AL=mK9mXb2Xy9Nw@mail.gmail.com/T/#t\n\nThanks,\nMaciej Ciemborowicz\n"},{"id":"553046","messageId":"20260923124931.39424-1-maciej.ciemborowicz@gmail.com","threadId":"66350","inReplyTo":"CAOLa=ZTN1TU2A1sgEhiw=ymMYr6Ge11cMEubSaeKqr4WNU=2EQ@mail.gmail.com","subject":"Re: [PATCH v4 0/3] refs: report old OIDs for batched deletions","fromName":"Maciej Ciemborowicz","fromEmail":"maciej.ciemborowicz@gmail.com","sentAt":"2026-09-23T12:49:31Z","receivedAt":"2026-09-23T12:49:34Z","isPatch":true,"body":"Hi Junio,\n\n> Can we avoid REF_TRANSACTION_FLAG_SKIP_HOOK by changing the call chain\n> instead of exposing a general mechanism for skipping hooks?\n\nI looked into that, but I do not think it removes the distinction\ncleanly. The packed-refs transaction is an internal, physical part of\none logical files-backend update: it needs the normal transaction\nmachinery, while the reference-transaction hook must observe only the\nouter logical update.\n\nRestructuring the call chain would either duplicate the\nprepare/finish/abort lifecycle or hide the same hook-suppression\ndecision in a less explicit helper. I do agree that this should not be\na public escape hatch, though. I will keep it internal to the refs\nimplementation and limit its use to these internal transactions.\n\n> Should the int copy parameter simply be bool copy, given that the\n> function has only two modes?\n\nYes, agreed. It is a two-state mode, so bool copy expresses the\ncontract more clearly. I will change it in the next reroll.\n\nThanks,\nMaciej\n"},{"id":"553060","messageId":"20260923133651.74120-1-maciej.ciemborowicz@gmail.com","threadId":"66350","inReplyTo":"20260920165037.88524-1-maciej.ciemborowicz@gmail.com","subject":"[PATCH v2] refs: run copy and rename through transactions","fromName":"Maciej Ciemborowicz","fromEmail":"maciej.ciemborowicz@gmail.com","sentAt":"2026-09-23T13:36:51Z","receivedAt":"2026-09-23T13:36:56Z","isPatch":true,"body":"Reference copy and rename operations bypass the transaction API.\nConsequently, the reference-transaction hook sees only the source deletion\nwith the files backend and no useful update with the reftable backend.\n\nRepresent both operations as reference transactions containing their\nlogical updates. A rename is a deletion of the old reference and creation\nof the new reference in the same transaction. Attach operation-specific\nstate to the destination update instead of making copy or rename a property\nof the entire transaction.\n\nRetain backend-specific reflog handling: the files backend stages its\nexisting rename procedure across prepare, finish and abort, while reftable\nstages an addition while holding the stack lock. Suppress hooks for the\nfiles backend's nested deletion transactions so that callers observe one\nlogical transaction.\n\nRecord and verify the source and destination values after taking backend\nlocks. This rejects concurrent changes instead of applying a rename or copy\nthat differs from the payload shown to the preparing hook. Preserve D/F\nrenames and restore overwritten references and reflogs when a prepared hook\nrejects the operation.\n\nAdd tests covering rename, copy, forced updates, both directions of D/F\nconflicts, concurrent updates and prepared-hook rollback.\n\nHelped-by: Karthik Nayak \nSigned-off-by: Maciej Ciemborowicz \n---\nApologies for the unrelated Subject header on my earlier reply. This\nreroll incorporates the points discussed there.\n\nChanges since v1:\n\n * Keep copy/rename state on the destination ref update instead of the\n generic transaction, so the operation is no longer a transaction-wide\n property.\n * Keep REF_TRANSACTION_FLAG_SKIP_HOOK private to the refs implementation.\n * Use bool for the two-state copy parameter.\n * Apply Junio's commit-message wording suggestions.\n\nRange-diff against v1:\n1: de0a5a9f7 ! 1: d852537d8 refs: run copy and rename through transactions\n @@ Metadata\n ## Commit message ##\n refs: run copy and rename through transactions\n \n - Reference copy and rename operations currently bypass the transaction API.\n + Reference copy and rename operations bypass the transaction API.\n Consequently, the reference-transaction hook sees only the source deletion\n with the files backend and no useful update with the reftable backend.\n \n Represent both operations as reference transactions containing their\n logical updates. A rename is a deletion of the old reference and creation\n - of the new reference in the same transaction. Retain backend-specific\n - reflog handling: the files backend stages its existing rename procedure\n - across prepare, finish and abort, while reftable stages an addition while\n - holding the stack lock. Suppress hooks for the files backend's nested\n - deletion transactions so that callers observe one logical transaction.\n + of the new reference in the same transaction. Attach operation-specific\n + state to the destination update instead of making copy or rename a property\n + of the entire transaction.\n +\n + Retain backend-specific reflog handling: the files backend stages its\n + existing rename procedure across prepare, finish and abort, while reftable\n + stages an addition while holding the stack lock. Suppress hooks for the\n + files backend's nested deletion transactions so that callers observe one\n + logical transaction.\n \n Record and verify the source and destination values after taking backend\n locks. This rejects concurrent changes instead of applying a rename or copy\n @@ Commit message\n renames and restore overwritten references and reflogs when a prepared hook\n rejects the operation.\n \n - Add coverage for rename, copy, forced updates, both directions of D/F\n + Add tests covering rename, copy, forced updates, both directions of D/F\n conflicts, concurrent updates and prepared-hook rollback.\n \n Helped-by: Karthik Nayak \n @@ refs.c: int refs_delete_ref(struct ref_store *refs, const char *msg,\n {\n \tchar c;\n @@ refs.c: void ref_transaction_free(struct ref_transaction *transaction)\n + \t}\n + \n + \tfor (i = 0; i < transaction->nr; i++) {\n ++\t\tstruct ref_copy_or_rename_update *operation =\n ++\t\t\ttransaction->updates[i]->copy_or_rename;\n ++\n + \t\tfree(transaction->updates[i]->msg);\n + \t\tfree(transaction->updates[i]->committer_info);\n + \t\tfree((char *)transaction->updates[i]->new_target);\n + \t\tfree((char *)transaction->updates[i]->old_target);\n + \t\tfree((char *)transaction->updates[i]->rejection_details);\n ++\t\tif (operation) {\n ++\t\t\tfree(operation->old_refname);\n ++\t\t\tfree(operation->logmsg);\n ++\t\t\tfree(operation->destination_target);\n ++\t\t\tfree(operation);\n ++\t\t}\n + \t\tfree(transaction->updates[i]);\n + \t}\n \n - \tstring_list_clear(&transaction->refnames, 0);\n - \tfree(transaction->updates);\n -+\tfree(transaction->old_refname);\n -+\tfree(transaction->new_refname);\n -+\tfree(transaction->logmsg);\n -+\tfree(transaction->destination_target);\n +@@ refs.c: void ref_transaction_free(struct ref_transaction *transaction)\n \tfree(transaction);\n }\n \n ++struct ref_update *ref_transaction_copy_or_rename_update(\n ++\tstruct ref_transaction *transaction)\n ++{\n ++\tstruct ref_update *operation = NULL;\n ++\tsize_t i;\n ++\n ++\tfor (i = 0; i < transaction->nr; i++) {\n ++\t\tif (!transaction->updates[i]->copy_or_rename)\n ++\t\t\tcontinue;\n ++\t\tif (operation)\n ++\t\t\tBUG(\"multiple copy or rename updates in one transaction\");\n ++\t\toperation = transaction->updates[i];\n ++\t}\n ++\n ++\treturn operation;\n ++}\n ++\n + int ref_transaction_maybe_set_rejected(struct ref_transaction *transaction,\n + \t\t\t\t size_t update_idx,\n + \t\t\t\t enum ref_transaction_error err,\n @@ refs.c: int ref_transaction_prepare(struct ref_transaction *transaction,\n \t\treturn REF_TRANSACTION_ERROR_GENERIC;\n \n @@ refs.c: int refs_delete_refs(struct ref_store *refs, const char *logmsg,\n -\t\t const char *newref, const char *logmsg)\n +static int refs_copy_or_rename_ref(struct ref_store *refs, const char *oldref,\n +\t\t\t\t const char *newref, const char *logmsg,\n -+\t\t\t\t int copy)\n ++\t\t\t\t bool copy)\n {\n -\tchar *msg;\n -\tint retval;\n +\tstruct ref_transaction *transaction = NULL;\n ++\tstruct ref_copy_or_rename_update *operation = NULL;\n ++\tstruct ref_update *destination_update;\n +\tstruct object_id old_oid, new_oid;\n +\tstruct strbuf new_target = STRBUF_INIT;\n +\tstruct strbuf err = STRBUF_INIT;\n @@ refs.c: int refs_delete_refs(struct ref_store *refs, const char *logmsg,\n +\ttransaction = ref_store_transaction_begin(refs, 0, &err);\n +\tif (!transaction)\n +\t\tgoto error;\n -+\ttransaction->type = copy ? REF_TRANSACTION_TYPE_COPY :\n -+\t\tREF_TRANSACTION_TYPE_RENAME;\n -+\ttransaction->old_refname = xstrdup(oldref);\n -+\ttransaction->new_refname = xstrdup(newref);\n -+\ttransaction->logmsg = xstrdup(msg);\n -+\toidcpy(&transaction->source_oid, &old_oid);\n -+\n +\tif (!copy && ref_transaction_delete(transaction, oldref, &old_oid, NULL,\n +\t\t\t\t\t REF_NO_DEREF, msg, &err))\n +\t\tgoto error;\n @@ refs.c: int refs_delete_refs(struct ref_store *refs, const char *logmsg,\n +\t} else {\n +\t\toidclr(&new_oid, refs->repo->hash_algo);\n +\t}\n -+\ttransaction->destination_exists = new_exists;\n -+\tif (new_flags & REF_ISSYMREF)\n -+\t\ttransaction->destination_target = xstrdup(new_target.buf);\n -+\telse if (transaction->destination_exists)\n -+\t\toidcpy(&transaction->destination_oid, &new_oid);\n -+\n +\tif (ref_transaction_update(transaction, newref, &old_oid,\n +\t\t\t\t (new_flags & REF_ISSYMREF) ? NULL : &new_oid,\n +\t\t\t\t NULL,\n @@ refs.c: int refs_delete_refs(struct ref_store *refs, const char *logmsg,\n +\t\t\t\t NULL, &err))\n +\t\tgoto error;\n +\n ++\tdestination_update = transaction->updates[transaction->nr - 1];\n ++\tCALLOC_ARRAY(operation, 1);\n ++\toperation->type = copy ? REF_UPDATE_COPY : REF_UPDATE_RENAME;\n ++\toperation->old_refname = xstrdup(oldref);\n ++\toperation->logmsg = xstrdup(msg);\n ++\toidcpy(&operation->source_oid, &old_oid);\n ++\toperation->destination_exists = new_exists;\n ++\tif (new_flags & REF_ISSYMREF)\n ++\t\toperation->destination_target = xstrdup(new_target.buf);\n ++\telse if (operation->destination_exists)\n ++\t\toidcpy(&operation->destination_oid, &new_oid);\n ++\tdestination_update->copy_or_rename = operation;\n ++\n +\tif (ref_transaction_commit(transaction, &err))\n +\t\tgoto error;\n +\n @@ refs.c: int refs_delete_refs(struct ref_store *refs, const char *logmsg,\n \n const char *ref_update_original_update_refname(struct ref_update *update)\n \n - ## refs.h ##\n -@@ refs.h: enum ref_transaction_flag {\n - \t * while rejecting updates which do not match the expected state.\n - \t */\n - \tREF_TRANSACTION_ALLOW_FAILURE = (1 << 1),\n -+\n -+\t/* Suppress hooks for an update nested in another transaction. */\n -+\tREF_TRANSACTION_FLAG_SKIP_HOOK = (1 << 2),\n - };\n - \n - /*\n -\n ## refs/debug.c ##\n @@ refs/debug.c: static int debug_optimize_required(struct ref_store *ref_store,\n \treturn res;\n @@ refs/files-backend.c: static int refs_rename_ref_available(struct ref_store *ref\n +};\n +\n static int files_copy_or_rename_ref(struct ref_store *ref_store,\n - \t\t\t const char *oldrefname, const char *newrefname,\n +-\t\t\t const char *oldrefname, const char *newrefname,\n -\t\t\t const char *logmsg, int copy)\n -+\t\t\t const char *logmsg, int copy,\n -+\t\t\t struct ref_transaction *transaction)\n ++\t\t\t\t struct ref_update *update,\n ++\t\t\t\t struct ref_transaction *transaction)\n {\n \tstruct files_ref_store *refs =\n -\t\tfiles_downcast(ref_store, REF_STORE_WRITE, \"rename_ref\");\n +\t\tfiles_downcast(ref_store, REF_STORE_WRITE,\n +\t\t\t \"ref_transaction_prepare\");\n ++\tstruct ref_copy_or_rename_update *operation = update->copy_or_rename;\n ++\tconst char *oldrefname = operation->old_refname;\n ++\tconst char *newrefname = update->refname;\n ++\tconst char *logmsg = operation->logmsg;\n ++\tbool copy = operation->type == REF_UPDATE_COPY;\n \tstruct object_id orig_oid;\n \tint flag = 0, logmoved = 0;\n \tstruct ref_lock *lock;\n @@ refs/files-backend.c: static int files_copy_or_rename_ref(struct ref_store *ref_\n \t\t\t\t oldrefname);\n \t\tgoto out;\n \t}\n -+\tif (!oideq(&orig_oid, &transaction->source_oid)) {\n ++\tif (!oideq(&orig_oid, &operation->source_oid)) {\n +\t\tret = error(\"refname %s is at %s but expected %s\",\n +\t\t\t oldrefname, oid_to_hex(&orig_oid),\n -+\t\t\t oid_to_hex(&transaction->source_oid));\n ++\t\t\t oid_to_hex(&operation->source_oid));\n +\t\tgoto out;\n +\t}\n \tif (!refs_rename_ref_available(&refs->base, oldrefname, newrefname)) {\n @@ refs/files-backend.c: static int files_copy_or_rename_ref(struct ref_store *ref_\n +\t\t\tgoto out;\n +\t\t}\n +\t}\n -+\tif (destination_exists != transaction->destination_exists) {\n ++\tif (destination_exists != operation->destination_exists) {\n +\t\tret = error(\"refname %s changed while renaming\", newrefname);\n +\t\tgoto out;\n +\t}\n +\tif (destination_exists) {\n +\t\tif (destination_flags & REF_ISSYMREF) {\n -+\t\t\tif (!transaction->destination_target ||\n ++\t\t\tif (!operation->destination_target ||\n +\t\t\t strcmp(destination_target.buf,\n -+\t\t\t\t transaction->destination_target)) {\n ++\t\t\t\t operation->destination_target)) {\n +\t\t\t\tret = error(\"refname %s changed while renaming\",\n +\t\t\t\t\t newrefname);\n +\t\t\t\tgoto out;\n +\t\t\t}\n -+\t\t} else if (transaction->destination_target ||\n ++\t\t} else if (operation->destination_target ||\n +\t\t\t !oideq(&destination_oid,\n -+\t\t\t\t &transaction->destination_oid)) {\n ++\t\t\t\t &operation->destination_oid)) {\n +\t\t\tret = error(\"refname %s changed while renaming\", newrefname);\n +\t\t\tgoto out;\n +\t\t}\n @@ refs/files-backend.c: static int files_transaction_prepare(struct ref_store *ref\n \tstruct ref_transaction *packed_transaction = NULL;\n \n \tassert(err);\n -+\tif (transaction->type != REF_TRANSACTION_TYPE_NORMAL)\n -+\t\treturn files_copy_or_rename_ref(ref_store,\n -+\t\t\t\ttransaction->old_refname,\n -+\t\t\t\ttransaction->new_refname,\n -+\t\t\t\ttransaction->logmsg,\n -+\t\t\t\ttransaction->type == REF_TRANSACTION_TYPE_COPY,\n -+\t\t\t\ttransaction);\n ++\t{\n ++\t\tstruct ref_update *operation =\n ++\t\t\tref_transaction_copy_or_rename_update(transaction);\n ++\n ++\t\tif (operation)\n ++\t\t\treturn files_copy_or_rename_ref(ref_store, operation,\n ++\t\t\t\t\t\t\ttransaction);\n ++\t}\n \n \tif (transaction->flags & REF_TRANSACTION_FLAG_INITIAL)\n \t\tgoto cleanup;\n @@ refs/files-backend.c: static int files_transaction_finish(struct ref_store *ref_\n \n \n \tassert(err);\n -+\tif (transaction->type != REF_TRANSACTION_TYPE_NORMAL) {\n -+\t\tstruct files_copy_or_rename_transaction_data *data =\n -+\t\t\ttransaction->backend_data;\n -+\t\tint special_ret;\n ++\t{\n ++\t\tstruct ref_update *update =\n ++\t\t\tref_transaction_copy_or_rename_update(transaction);\n +\n -+\t\tspecial_ret = commit_ref_update(refs, data->lock, &data->orig_oid,\n -+\t\t\t\t\t\ttransaction->logmsg, 0, err);\n -+\t\tif (special_ret) {\n -+\t\t\terror(\"unable to write current sha1 into %s: %s\",\n -+\t\t\t transaction->new_refname, err->buf);\n -+\t\t\tdata->lock = NULL;\n -+\t\t\tfiles_transaction_abort(ref_store, transaction, err);\n -+\t\t\treturn special_ret;\n -+\t\t} else if (data->destination_log_backed_up) {\n -+\t\t\tstruct strbuf path = STRBUF_INIT;\n ++\t\tif (update) {\n ++\t\t\tstruct ref_copy_or_rename_update *operation =\n ++\t\t\t\tupdate->copy_or_rename;\n ++\t\t\tstruct files_copy_or_rename_transaction_data *data =\n ++\t\t\t\ttransaction->backend_data;\n ++\t\t\tint special_ret;\n ++\n ++\t\t\tspecial_ret = commit_ref_update(refs, data->lock, &data->orig_oid,\n ++\t\t\t\t\t\t\toperation->logmsg, 0, err);\n ++\t\t\tif (special_ret) {\n ++\t\t\t\terror(\"unable to write current sha1 into %s: %s\",\n ++\t\t\t\t update->refname, err->buf);\n ++\t\t\t\tdata->lock = NULL;\n ++\t\t\t\tfiles_transaction_abort(ref_store, transaction, err);\n ++\t\t\t\treturn special_ret;\n ++\t\t\t} else if (data->destination_log_backed_up) {\n ++\t\t\t\tstruct strbuf path = STRBUF_INIT;\n +\n -+\t\t\tfiles_reflog_path(refs, &path, TMP_RENAMED_LOG_DESTINATION);\n -+\t\t\tif (unlink(path.buf) < 0 && errno != ENOENT)\n -+\t\t\t\twarning_errno(\"unable to remove '%s'\", path.buf);\n -+\t\t\tstrbuf_release(&path);\n ++\t\t\t\tfiles_reflog_path(refs, &path, TMP_RENAMED_LOG_DESTINATION);\n ++\t\t\t\tif (unlink(path.buf) < 0 && errno != ENOENT)\n ++\t\t\t\t\twarning_errno(\"unable to remove '%s'\", path.buf);\n ++\t\t\t\tstrbuf_release(&path);\n ++\t\t\t}\n ++\t\t\tfree(data->destination_target);\n ++\t\t\tfree(data);\n ++\t\t\ttransaction->backend_data = NULL;\n ++\t\t\ttransaction->state = REF_TRANSACTION_CLOSED;\n ++\t\t\treturn special_ret;\n +\t\t}\n -+\t\tfree(data->destination_target);\n -+\t\tfree(data);\n -+\t\ttransaction->backend_data = NULL;\n -+\t\ttransaction->state = REF_TRANSACTION_CLOSED;\n -+\t\treturn special_ret;\n +\t}\n \n \tif (transaction->flags & REF_TRANSACTION_FLAG_INITIAL)\n @@ refs/files-backend.c: static int files_transaction_finish(struct ref_store *ref_\n \tstruct files_ref_store *refs =\n \t\tfiles_downcast(ref_store, 0, \"ref_transaction_abort\");\n \n -+\tif (transaction->type != REF_TRANSACTION_TYPE_NORMAL) {\n -+\t\tstruct files_copy_or_rename_transaction_data *data =\n -+\t\t\ttransaction->backend_data;\n -+\t\tstruct strbuf new_log = STRBUF_INIT;\n -+\t\tstruct strbuf destination_log = STRBUF_INIT;\n -+\t\tstruct strbuf temporary_log = STRBUF_INIT;\n -+\t\tstruct ref_transaction *restore_transaction = NULL;\n -+\t\tstruct ref_lock *lock;\n -+\t\tint ret = 0;\n ++\t{\n ++\t\tstruct ref_update *update =\n ++\t\t\tref_transaction_copy_or_rename_update(transaction);\n +\n -+\t\tif (data->lock)\n -+\t\t\tunlock_ref(data->lock);\n -+\t\tif (transaction->type == REF_TRANSACTION_TYPE_RENAME) {\n -+\t\t\tlock = lock_ref_oid_basic(refs, transaction->old_refname, err);\n -+\t\t\tif (!lock ||\n -+\t\t\t write_ref_to_lockfile(refs, lock, &data->orig_oid, err) ||\n -+\t\t\t commit_ref_update(refs, lock, &data->orig_oid, NULL,\n -+\t\t\t\t\t REF_SKIP_CREATE_REFLOG, err))\n -+\t\t\t\tret = -1;\n -+\t\t}\n ++\t\tif (update) {\n ++\t\t\tstruct ref_copy_or_rename_update *operation =\n ++\t\t\t\tupdate->copy_or_rename;\n ++\t\t\tstruct files_copy_or_rename_transaction_data *data =\n ++\t\t\t\ttransaction->backend_data;\n ++\t\t\tstruct strbuf new_log = STRBUF_INIT;\n ++\t\t\tstruct strbuf destination_log = STRBUF_INIT;\n ++\t\t\tstruct strbuf temporary_log = STRBUF_INIT;\n ++\t\t\tstruct ref_transaction *restore_transaction = NULL;\n ++\t\t\tstruct ref_lock *lock;\n ++\t\t\tint ret = 0;\n +\n -+\t\tif (data->logmoved) {\n -+\t\t\tfiles_reflog_path(refs, &new_log, transaction->new_refname);\n -+\t\t\tif (transaction->type == REF_TRANSACTION_TYPE_RENAME) {\n -+\t\t\t\tfiles_reflog_path(refs, &temporary_log, TMP_RENAMED_LOG);\n -+\t\t\t\tif (rename(new_log.buf, temporary_log.buf) < 0) {\n -+\t\t\t\t\tstrbuf_addf(err, \"unable to restore logfile %s: %s\",\n -+\t\t\t\t\t\t transaction->old_refname, strerror(errno));\n ++\t\t\tif (data->lock)\n ++\t\t\t\tunlock_ref(data->lock);\n ++\t\t\tif (operation->type == REF_UPDATE_RENAME) {\n ++\t\t\t\tlock = lock_ref_oid_basic(refs, operation->old_refname, err);\n ++\t\t\t\tif (!lock ||\n ++\t\t\t\t write_ref_to_lockfile(refs, lock, &data->orig_oid, err) ||\n ++\t\t\t\t commit_ref_update(refs, lock, &data->orig_oid, NULL,\n ++\t\t\t\t\t\t REF_SKIP_CREATE_REFLOG, err))\n +\t\t\t\t\tret = -1;\n -+\t\t\t\t} else {\n -+\t\t\t\t\ttry_remove_empty_parents(refs,\n -+\t\t\t\t\t\t\t transaction->new_refname,\n -+\t\t\t\t\t\t\t REMOVE_EMPTY_PARENTS_REFLOG);\n -+\t\t\t\t\tif (rename_tmp_log(refs,\n -+\t\t\t\t\t\t\t transaction->old_refname)) {\n ++\t\t\t}\n ++\n ++\t\t\tif (data->logmoved) {\n ++\t\t\t\tfiles_reflog_path(refs, &new_log, update->refname);\n ++\t\t\t\tif (operation->type == REF_UPDATE_RENAME) {\n ++\t\t\t\t\tfiles_reflog_path(refs, &temporary_log, TMP_RENAMED_LOG);\n ++\t\t\t\t\tif (rename(new_log.buf, temporary_log.buf) < 0) {\n +\t\t\t\t\t\tstrbuf_addf(err, \"unable to restore logfile %s: %s\",\n -+\t\t\t\t\t\t\t transaction->old_refname,\n -+\t\t\t\t\t\t\t strerror(errno));\n ++\t\t\t\t\t\t\t operation->old_refname, strerror(errno));\n +\t\t\t\t\t\tret = -1;\n ++\t\t\t\t\t} else {\n ++\t\t\t\t\t\ttry_remove_empty_parents(refs,\n ++\t\t\t\t\t\t\t\t\t update->refname,\n ++\t\t\t\t\t\t\t\t\t REMOVE_EMPTY_PARENTS_REFLOG);\n ++\t\t\t\t\t\tif (rename_tmp_log(refs,\n ++\t\t\t\t\t\t\t\t operation->old_refname)) {\n ++\t\t\t\t\t\t\tstrbuf_addf(err, \"unable to restore logfile %s: %s\",\n ++\t\t\t\t\t\t\t\t operation->old_refname,\n ++\t\t\t\t\t\t\t\t strerror(errno));\n ++\t\t\t\t\t\t\tret = -1;\n ++\t\t\t\t\t\t}\n +\t\t\t\t\t}\n ++\t\t\t\t} else if (unlink(new_log.buf) < 0 && errno != ENOENT) {\n ++\t\t\t\t\tstrbuf_addf(err, \"unable to remove logfile %s: %s\",\n ++\t\t\t\t\t\t update->refname, strerror(errno));\n ++\t\t\t\t\tret = -1;\n +\t\t\t\t}\n -+\t\t\t} else if (unlink(new_log.buf) < 0 && errno != ENOENT) {\n -+\t\t\t\tstrbuf_addf(err, \"unable to remove logfile %s: %s\",\n -+\t\t\t\t\t transaction->new_refname, strerror(errno));\n -+\t\t\t\tret = -1;\n +\t\t\t}\n -+\t\t}\n -+\t\tif (data->destination_log_backed_up) {\n -+\t\t\tfiles_reflog_path(refs, &destination_log,\n -+\t\t\t\t\t TMP_RENAMED_LOG_DESTINATION);\n -+\t\t\tif (rename(destination_log.buf, new_log.buf) < 0) {\n -+\t\t\t\tstrbuf_addf(err, \"unable to restore logfile %s: %s\",\n -+\t\t\t\t\t transaction->new_refname, strerror(errno));\n -+\t\t\t\tret = -1;\n ++\t\t\tif (data->destination_log_backed_up) {\n ++\t\t\t\tfiles_reflog_path(refs, &destination_log,\n ++\t\t\t\t\t\t TMP_RENAMED_LOG_DESTINATION);\n ++\t\t\t\tif (rename(destination_log.buf, new_log.buf) < 0) {\n ++\t\t\t\t\tstrbuf_addf(err, \"unable to restore logfile %s: %s\",\n ++\t\t\t\t\t\t update->refname, strerror(errno));\n ++\t\t\t\t\tret = -1;\n ++\t\t\t\t}\n +\t\t\t}\n -+\t\t}\n +\n -+\t\tif (transaction->type == REF_TRANSACTION_TYPE_RENAME &&\n -+\t\t data->destination_exists) {\n -+\t\t\trestore_transaction = ref_store_transaction_begin(\n ++\t\t\tif (operation->type == REF_UPDATE_RENAME &&\n ++\t\t\t data->destination_exists) {\n ++\t\t\t\trestore_transaction = ref_store_transaction_begin(\n +\t\t\t\t\t&refs->base, REF_TRANSACTION_FLAG_SKIP_HOOK, err);\n -+\t\t\tif (!restore_transaction ||\n -+\t\t\t ref_transaction_update(restore_transaction,\n -+\t\t\t\t\t\t transaction->new_refname,\n -+\t\t\t\t\t\t data->destination_target ? NULL :\n -+\t\t\t\t\t\t\t&data->destination_oid,\n -+\t\t\t\t\t\t NULL,\n -+\t\t\t\t\t\t data->destination_target,\n -+\t\t\t\t\t\t NULL,\n -+\t\t\t\t\t\t REF_NO_DEREF |\n -+\t\t\t\t\t\t\tREF_SKIP_CREATE_REFLOG,\n -+\t\t\t\t\t\t NULL, err) ||\n -+\t\t\t ref_transaction_commit(restore_transaction, err))\n -+\t\t\t\tret = -1;\n -+\t\t\tref_transaction_free(restore_transaction);\n -+\t\t}\n ++\t\t\t\tif (!restore_transaction ||\n ++\t\t\t\t ref_transaction_update(restore_transaction,\n ++\t\t\t\t\t\t\t update->refname,\n ++\t\t\t\t\t\t\t data->destination_target ? NULL :\n ++\t\t\t\t\t\t\t\t\t\t &data->destination_oid,\n ++\t\t\t\t\t\t\t NULL,\n ++\t\t\t\t\t\t\t data->destination_target,\n ++\t\t\t\t\t\t\t NULL,\n ++\t\t\t\t\t\t\t REF_NO_DEREF |\n ++\t\t\t\t\t\t\t\t REF_SKIP_CREATE_REFLOG,\n ++\t\t\t\t\t\t\t NULL, err) ||\n ++\t\t\t\t ref_transaction_commit(restore_transaction, err))\n ++\t\t\t\t\tret = -1;\n ++\t\t\t\tref_transaction_free(restore_transaction);\n ++\t\t\t}\n +\n -+\t\tstrbuf_release(&destination_log);\n -+\t\tstrbuf_release(&temporary_log);\n -+\t\tstrbuf_release(&new_log);\n -+\t\tfree(data->destination_target);\n -+\t\tfree(data);\n -+\t\ttransaction->backend_data = NULL;\n -+\t\ttransaction->state = REF_TRANSACTION_CLOSED;\n -+\t\treturn ret;\n ++\t\t\tstrbuf_release(&destination_log);\n ++\t\t\tstrbuf_release(&temporary_log);\n ++\t\t\tstrbuf_release(&new_log);\n ++\t\t\tfree(data->destination_target);\n ++\t\t\tfree(data);\n ++\t\t\ttransaction->backend_data = NULL;\n ++\t\t\ttransaction->state = REF_TRANSACTION_CLOSED;\n ++\t\t\treturn ret;\n ++\t\t}\n +\t}\n +\n \tfiles_transaction_cleanup(refs, transaction);\n @@ refs/packed-backend.c: struct ref_storage_be refs_be_packed = {\n \t.read_raw_ref = packed_read_raw_ref,\n \n ## refs/refs-internal.h ##\n +@@ refs/refs-internal.h: struct ref_update {\n + \t */\n + \tstruct ref_update *parent_update;\n + \n ++\t/*\n ++\t * Copy and rename operations require backend-specific handling while\n ++\t * still exposing their logical updates to transaction hooks. Keep that\n ++\t * state on the destination update so it composes with other updates in\n ++\t * the transaction instead of making copy or rename a transaction-wide\n ++\t * property.\n ++\t */\n ++\tstruct ref_copy_or_rename_update *copy_or_rename;\n ++\n + \tconst char refname[FLEX_ARRAY];\n + };\n + \n ++enum ref_copy_or_rename_type {\n ++\tREF_UPDATE_RENAME,\n ++\tREF_UPDATE_COPY,\n ++};\n ++\n ++struct ref_copy_or_rename_update {\n ++\tenum ref_copy_or_rename_type type;\n ++\tchar *old_refname;\n ++\tchar *logmsg;\n ++\tstruct object_id source_oid;\n ++\tstruct object_id destination_oid;\n ++\tchar *destination_target;\n ++\tunsigned int destination_exists:1;\n ++};\n ++\n + int refs_read_raw_ref(struct ref_store *ref_store, const char *refname,\n + \t\t struct object_id *oid, struct strbuf *referent,\n + \t\t unsigned int *type, int *failure_errno);\n @@ refs/refs-internal.h: struct ref_update *ref_transaction_add_update(\n \t\tconst char *committer_info,\n \t\tconst char *msg);\n @@ refs/refs-internal.h: struct ref_update *ref_transaction_add_update(\n /*\n * Transaction states.\n *\n -@@ refs/refs-internal.h: enum ref_transaction_state {\n - \tREF_TRANSACTION_CLOSED = 2\n - };\n - \n -+enum ref_transaction_type {\n -+\tREF_TRANSACTION_TYPE_NORMAL = 0,\n -+\tREF_TRANSACTION_TYPE_RENAME,\n -+\tREF_TRANSACTION_TYPE_COPY,\n -+};\n -+\n - /*\n - * Data structure to hold indices of updates which were rejected, for batched\n - * reference updates. While the updates themselves hold the rejection error,\n @@ refs/refs-internal.h: struct ref_transaction {\n - \tvoid *backend_data;\n - \tunsigned int flags;\n \tuint64_t max_index;\n -+\n -+\t/*\n -+\t * Rename and copy operations need backend-specific reflog handling.\n -+\t * Their logical updates still live in `updates`, so hooks see the\n -+\t * operation like any other reference transaction. The fields below\n -+\t * retain the state that backends verify after taking their locks.\n -+\t */\n -+\tenum ref_transaction_type type;\n -+\tchar *old_refname;\n -+\tchar *new_refname;\n -+\tchar *logmsg;\n -+\tstruct object_id source_oid;\n -+\tstruct object_id destination_oid;\n -+\tchar *destination_target;\n -+\tunsigned int destination_exists:1;\n };\n \n ++/* Suppress hooks for a transaction nested inside another refs operation. */\n ++#define REF_TRANSACTION_FLAG_SKIP_HOOK (1 << 2)\n ++\n ++struct ref_update *ref_transaction_copy_or_rename_update(\n ++\tstruct ref_transaction *transaction);\n ++\n /*\n + * Check for entries in extras that are within the specified\n + * directory, where dirname is a reference directory name including\n @@ refs/refs-internal.h: typedef int optimize_required_fn(struct ref_store *ref_store,\n \t\t\t\t struct refs_optimize_opts *opts,\n \t\t\t\t bool *required);\n @@ refs/reftable-backend.c: static int reftable_be_transaction_prepare(struct ref_s\n \tsize_t i;\n \tint ret;\n \n -+\tif (transaction->type != REF_TRANSACTION_TYPE_NORMAL)\n ++\tif (ref_transaction_copy_or_rename_update(transaction))\n +\t\treturn reftable_be_copy_or_rename_prepare(ref_store, transaction,\n +\t\t\t\t\t\t\t err);\n +\n @@ refs/reftable-backend.c: static int reftable_be_transaction_abort(struct ref_sto\n -\tstruct reftable_transaction_data *tx_data = transaction->backend_data;\n +\tstruct reftable_transaction_data *tx_data;\n +\n -+\tif (transaction->type != REF_TRANSACTION_TYPE_NORMAL) {\n ++\tif (ref_transaction_copy_or_rename_update(transaction)) {\n +\t\tstruct reftable_copy_or_rename_transaction_data *data =\n +\t\t\ttransaction->backend_data;\n +\n @@ refs/reftable-backend.c: static int reftable_be_transaction_finish(struct ref_st\n +\tstruct reftable_transaction_data *tx_data;\n \tint ret = 0;\n \n -+\tif (transaction->type != REF_TRANSACTION_TYPE_NORMAL) {\n ++\tif (ref_transaction_copy_or_rename_update(transaction)) {\n +\t\tstruct reftable_copy_or_rename_transaction_data *data =\n +\t\t\ttransaction->backend_data;\n +\t\tint special_ret = reftable_addition_commit(data->addition);\n @@ refs/reftable-backend.c: struct write_create_symref_arg {\n \tconst char *newname;\n \tconst char *logmsg;\n \tint delete_old;\n -+\tstruct ref_transaction *transaction;\n ++\tstruct ref_copy_or_rename_update *operation;\n };\n \n static int write_copy_table(struct reftable_writer *writer, void *cb_data)\n @@ refs/reftable-backend.c: static int write_copy_table(struct reftable_writer *wri\n +\t\tret = -1;\n \t\tgoto done;\n \t}\n -+\tif (arg->transaction) {\n ++\tif (arg->operation) {\n +\t\tstruct object_id oid;\n +\n +\t\tif (old_ref.value_type == REFTABLE_REF_VAL2)\n @@ refs/reftable-backend.c: static int write_copy_table(struct reftable_writer *wri\n +\t\telse\n +\t\t\toidread(&oid, old_ref.value.val1,\n +\t\t\t\targ->refs->base.repo->hash_algo);\n -+\t\tif (!oideq(&oid, &arg->transaction->source_oid)) {\n ++\t\tif (!oideq(&oid, &arg->operation->source_oid)) {\n +\t\t\tstrbuf_addf(arg->err,\n +\t\t\t\t _(\"refname %s is at %s but expected %s\"),\n +\t\t\t\t arg->oldname, oid_to_hex(&oid),\n -+\t\t\t\t oid_to_hex(&arg->transaction->source_oid));\n ++\t\t\t\t oid_to_hex(&arg->operation->source_oid));\n +\t\t\tret = -1;\n +\t\t\tgoto done;\n +\t\t}\n @@ refs/reftable-backend.c: static int write_copy_table(struct reftable_writer *wri\n +\t\t\t\t\t &destination_ref);\n +\t\tif (ret < 0)\n +\t\t\tgoto done;\n -+\t\tif (arg->transaction->destination_exists != !ret) {\n ++\t\tif (arg->operation->destination_exists != !ret) {\n +\t\t\tstrbuf_addf(arg->err,\n +\t\t\t\t _(\"refname %s changed while renaming\"),\n +\t\t\t\t arg->newname);\n @@ refs/reftable-backend.c: static int write_copy_table(struct reftable_writer *wri\n +\t\t}\n +\t\tif (!ret) {\n +\t\t\tif (destination_ref.value_type == REFTABLE_REF_SYMREF) {\n -+\t\t\t\tif (!arg->transaction->destination_target ||\n ++\t\t\t\tif (!arg->operation->destination_target ||\n +\t\t\t\t strcmp(destination_ref.value.symref,\n -+\t\t\t\t\t arg->transaction->destination_target)) {\n ++\t\t\t\t\t arg->operation->destination_target)) {\n +\t\t\t\t\tstrbuf_addf(arg->err,\n +\t\t\t\t\t\t _(\"refname %s changed while renaming\"),\n +\t\t\t\t\t\t arg->newname);\n @@ refs/reftable-backend.c: static int write_copy_table(struct reftable_writer *wri\n +\t\t\t\telse\n +\t\t\t\t\toidread(&oid, destination_ref.value.val1,\n +\t\t\t\t\t\targ->refs->base.repo->hash_algo);\n -+\t\t\t\tif (arg->transaction->destination_target ||\n -+\t\t\t\t !oideq(&oid, &arg->transaction->destination_oid)) {\n ++\t\t\t\tif (arg->operation->destination_target ||\n ++\t\t\t\t !oideq(&oid, &arg->operation->destination_oid)) {\n +\t\t\t\t\tstrbuf_addf(arg->err,\n +\t\t\t\t\t\t _(\"refname %s changed while renaming\"),\n +\t\t\t\t\t\t arg->newname);\n @@ refs/reftable-backend.c: static int write_copy_table(struct reftable_writer *wri\n +\t\treftable_be_downcast(ref_store, REF_STORE_WRITE,\n +\t\t\t\t \"ref_transaction_prepare\");\n +\tstruct reftable_copy_or_rename_transaction_data *data = NULL;\n ++\tstruct ref_update *update =\n ++\t\tref_transaction_copy_or_rename_update(transaction);\n ++\tstruct ref_copy_or_rename_update *operation = update->copy_or_rename;\n \tstruct write_copy_arg arg = {\n \t\t.refs = refs,\n -\t\t.oldname = oldrefname,\n @@ refs/reftable-backend.c: static int write_copy_table(struct reftable_writer *wri\n -\t\t.logmsg = logmsg,\n -\t\t.delete_old = 1,\n +\t\t.err = err,\n -+\t\t.oldname = transaction->old_refname,\n -+\t\t.newname = transaction->new_refname,\n -+\t\t.logmsg = transaction->logmsg,\n -+\t\t.delete_old = transaction->type == REF_TRANSACTION_TYPE_RENAME,\n -+\t\t.transaction = transaction,\n ++\t\t.oldname = operation->old_refname,\n ++\t\t.newname = update->refname,\n ++\t\t.logmsg = operation->logmsg,\n ++\t\t.delete_old = operation->type == REF_UPDATE_RENAME,\n ++\t\t.operation = operation,\n \t};\n \tint ret;\n \n @@ refs/reftable-backend.c: static int write_copy_table(struct reftable_writer *wri\n \t\tgoto done;\n -\n -\tret = backend_for(&arg.be, refs, newrefname, &newrefname, 1);\n -+\tret = backend_for(&arg.be, refs, transaction->new_refname,\n ++\tret = backend_for(&arg.be, refs, update->refname,\n +\t\t\t &arg.newname, 1);\n \tif (ret)\n \t\tgoto done;\n\n refs.c | 161 ++++++++++++++---\n refs/debug.c | 25 ---\n refs/files-backend.c | 297 +++++++++++++++++++++++++++----\n refs/packed-backend.c | 2 -\n refs/refs-internal.h | 47 +++--\n refs/reftable-backend.c | 197 +++++++++++++++-----\n t/t1416-ref-transaction-hooks.sh | 142 +++++++++++++++\n 7 files changed, 732 insertions(+), 139 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 92d5df5b7..f036ae4b9 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1004,15 +1004,17 @@ long get_files_ref_lock_timeout_ms(struct repository *repo)\n \treturn timeout_ms;\n }\n \n-int refs_delete_ref(struct ref_store *refs, const char *msg,\n-\t\t const char *refname,\n-\t\t const struct object_id *old_oid,\n-\t\t unsigned int flags)\n+int refs_delete_ref_with_transaction_flags(struct ref_store *refs,\n+\t\t\t\t\t const char *msg,\n+\t\t\t\t\t const char *refname,\n+\t\t\t\t\t const struct object_id *old_oid,\n+\t\t\t\t\t unsigned int flags,\n+\t\t\t\t\t unsigned int transaction_flags)\n {\n \tstruct ref_transaction *transaction;\n \tstruct strbuf err = STRBUF_INIT;\n \n-\ttransaction = ref_store_transaction_begin(refs, 0, &err);\n+\ttransaction = ref_store_transaction_begin(refs, transaction_flags, &err);\n \tif (!transaction ||\n \t ref_transaction_delete(transaction, refname, old_oid,\n \t\t\t\t NULL, flags, msg, &err) ||\n@@ -1027,6 +1029,15 @@ int refs_delete_ref(struct ref_store *refs, const char *msg,\n \treturn 0;\n }\n \n+int refs_delete_ref(struct ref_store *refs, const char *msg,\n+\t\t const char *refname,\n+\t\t const struct object_id *old_oid,\n+\t\t unsigned int flags)\n+{\n+\treturn refs_delete_ref_with_transaction_flags(refs, msg, refname,\n+\t\t\t\t\t\t old_oid, flags, 0);\n+}\n+\n static void copy_reflog_msg(struct strbuf *sb, const char *msg)\n {\n \tchar c;\n@@ -1256,11 +1267,20 @@ void ref_transaction_free(struct ref_transaction *transaction)\n \t}\n \n \tfor (i = 0; i < transaction->nr; i++) {\n+\t\tstruct ref_copy_or_rename_update *operation =\n+\t\t\ttransaction->updates[i]->copy_or_rename;\n+\n \t\tfree(transaction->updates[i]->msg);\n \t\tfree(transaction->updates[i]->committer_info);\n \t\tfree((char *)transaction->updates[i]->new_target);\n \t\tfree((char *)transaction->updates[i]->old_target);\n \t\tfree((char *)transaction->updates[i]->rejection_details);\n+\t\tif (operation) {\n+\t\t\tfree(operation->old_refname);\n+\t\t\tfree(operation->logmsg);\n+\t\t\tfree(operation->destination_target);\n+\t\t\tfree(operation);\n+\t\t}\n \t\tfree(transaction->updates[i]);\n \t}\n \n@@ -1273,6 +1293,23 @@ void ref_transaction_free(struct ref_transaction *transaction)\n \tfree(transaction);\n }\n \n+struct ref_update *ref_transaction_copy_or_rename_update(\n+\tstruct ref_transaction *transaction)\n+{\n+\tstruct ref_update *operation = NULL;\n+\tsize_t i;\n+\n+\tfor (i = 0; i < transaction->nr; i++) {\n+\t\tif (!transaction->updates[i]->copy_or_rename)\n+\t\t\tcontinue;\n+\t\tif (operation)\n+\t\t\tBUG(\"multiple copy or rename updates in one transaction\");\n+\t\toperation = transaction->updates[i];\n+\t}\n+\n+\treturn operation;\n+}\n+\n int ref_transaction_maybe_set_rejected(struct ref_transaction *transaction,\n \t\t\t\t size_t update_idx,\n \t\t\t\t enum ref_transaction_error err,\n@@ -2710,7 +2747,8 @@ int ref_transaction_prepare(struct ref_transaction *transaction,\n \t\treturn REF_TRANSACTION_ERROR_GENERIC;\n \n \t/* Preparing checks before locking references */\n-\tret = run_transaction_hook(transaction, \"preparing\");\n+\tret = transaction->flags & REF_TRANSACTION_FLAG_SKIP_HOOK ? 0 :\n+\t\trun_transaction_hook(transaction, \"preparing\");\n \tif (ret) {\n \t\tref_transaction_abort(transaction, err);\n \t\tdie(_(abort_by_ref_transaction_hook), \"preparing\");\n@@ -2720,7 +2758,8 @@ int ref_transaction_prepare(struct ref_transaction *transaction,\n \tif (ret)\n \t\treturn ret;\n \n-\tret = run_transaction_hook(transaction, \"prepared\");\n+\tret = transaction->flags & REF_TRANSACTION_FLAG_SKIP_HOOK ? 0 :\n+\t\trun_transaction_hook(transaction, \"prepared\");\n \tif (ret) {\n \t\tref_transaction_abort(transaction, err);\n \t\tdie(_(abort_by_ref_transaction_hook), \"prepared\");\n@@ -2750,7 +2789,8 @@ int ref_transaction_abort(struct ref_transaction *transaction,\n \t\tbreak;\n \t}\n \n-\trun_transaction_hook(transaction, \"aborted\");\n+\tif (!(transaction->flags & REF_TRANSACTION_FLAG_SKIP_HOOK))\n+\t\trun_transaction_hook(transaction, \"aborted\");\n \n \tref_transaction_free(transaction);\n \treturn ret;\n@@ -2781,7 +2821,8 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \t}\n \n \tret = refs->be->transaction_finish(refs, transaction, err);\n-\tif (!ret && !(transaction->flags & REF_TRANSACTION_FLAG_INITIAL))\n+\tif (!ret && !(transaction->flags & (REF_TRANSACTION_FLAG_INITIAL |\n+\t\t\t\t\t REF_TRANSACTION_FLAG_SKIP_HOOK)))\n \t\trun_transaction_hook(transaction, \"committed\");\n \treturn ret;\n }\n@@ -3123,28 +3164,102 @@ int refs_delete_refs(struct ref_store *refs, const char *logmsg,\n \treturn ret;\n }\n \n-int refs_rename_ref(struct ref_store *refs, const char *oldref,\n-\t\t const char *newref, const char *logmsg)\n+static int refs_copy_or_rename_ref(struct ref_store *refs, const char *oldref,\n+\t\t\t\t const char *newref, const char *logmsg,\n+\t\t\t\t bool copy)\n {\n-\tchar *msg;\n-\tint retval;\n+\tstruct ref_transaction *transaction = NULL;\n+\tstruct ref_copy_or_rename_update *operation = NULL;\n+\tstruct ref_update *destination_update;\n+\tstruct object_id old_oid, new_oid;\n+\tstruct strbuf new_target = STRBUF_INIT;\n+\tstruct strbuf err = STRBUF_INIT;\n+\tchar *msg = normalize_reflog_message(logmsg);\n+\tint old_flags, new_flags = 0, new_exists = 0, ret = 1;\n \n-\tmsg = normalize_reflog_message(logmsg);\n-\tretval = refs->be->rename_ref(refs, oldref, newref, msg);\n+\tif (!strcmp(oldref, newref)) {\n+\t\tret = 0;\n+\t\tgoto out;\n+\t}\n+\n+\tif (!refs_resolve_ref_unsafe(refs, oldref,\n+\t\t\t\t RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n+\t\t\t\t &old_oid, &old_flags)) {\n+\t\terror(\"refname %s not found\", oldref);\n+\t\tgoto out;\n+\t}\n+\tif (old_flags & REF_ISSYMREF) {\n+\t\terror(\"refname %s is a symbolic ref, %s it is not supported\",\n+\t\t oldref, copy ? \"copying\" : \"renaming\");\n+\t\tgoto out;\n+\t}\n+\n+\ttransaction = ref_store_transaction_begin(refs, 0, &err);\n+\tif (!transaction)\n+\t\tgoto error;\n+\tif (!copy && ref_transaction_delete(transaction, oldref, &old_oid, NULL,\n+\t\t\t\t\t REF_NO_DEREF, msg, &err))\n+\t\tgoto error;\n+\n+\tif (refs_resolve_ref_unsafe(refs, newref,\n+\t\t\t\t RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n+\t\t\t\t &new_oid, &new_flags)) {\n+\t\tnew_exists = 1;\n+\t\tif ((new_flags & REF_ISSYMREF) &&\n+\t\t refs_read_symbolic_ref(refs, newref, &new_target) < 0) {\n+\t\t\tstrbuf_addf(&err, \"unable to read symbolic ref %s\", newref);\n+\t\t\tgoto error;\n+\t\t}\n+\t} else {\n+\t\toidclr(&new_oid, refs->repo->hash_algo);\n+\t}\n+\tif (ref_transaction_update(transaction, newref, &old_oid,\n+\t\t\t\t (new_flags & REF_ISSYMREF) ? NULL : &new_oid,\n+\t\t\t\t NULL,\n+\t\t\t\t (new_flags & REF_ISSYMREF) ? new_target.buf : NULL,\n+\t\t\t\t REF_NO_DEREF | REF_SKIP_CREATE_REFLOG,\n+\t\t\t\t NULL, &err))\n+\t\tgoto error;\n+\n+\tdestination_update = transaction->updates[transaction->nr - 1];\n+\tCALLOC_ARRAY(operation, 1);\n+\toperation->type = copy ? REF_UPDATE_COPY : REF_UPDATE_RENAME;\n+\toperation->old_refname = xstrdup(oldref);\n+\toperation->logmsg = xstrdup(msg);\n+\toidcpy(&operation->source_oid, &old_oid);\n+\toperation->destination_exists = new_exists;\n+\tif (new_flags & REF_ISSYMREF)\n+\t\toperation->destination_target = xstrdup(new_target.buf);\n+\telse if (operation->destination_exists)\n+\t\toidcpy(&operation->destination_oid, &new_oid);\n+\tdestination_update->copy_or_rename = operation;\n+\n+\tif (ref_transaction_commit(transaction, &err))\n+\t\tgoto error;\n+\n+\tret = 0;\n+\tgoto out;\n+\n+error:\n+\terror(\"%s\", err.buf);\n+out:\n+\tref_transaction_free(transaction);\n+\tstrbuf_release(&new_target);\n+\tstrbuf_release(&err);\n \tfree(msg);\n-\treturn retval;\n+\treturn ret;\n }\n \n-int refs_copy_existing_ref(struct ref_store *refs, const char *oldref,\n+int refs_rename_ref(struct ref_store *refs, const char *oldref,\n \t\t const char *newref, const char *logmsg)\n {\n-\tchar *msg;\n-\tint retval;\n+\treturn refs_copy_or_rename_ref(refs, oldref, newref, logmsg, 0);\n+}\n \n-\tmsg = normalize_reflog_message(logmsg);\n-\tretval = refs->be->copy_ref(refs, oldref, newref, msg);\n-\tfree(msg);\n-\treturn retval;\n+int refs_copy_existing_ref(struct ref_store *refs, const char *oldref,\n+\t\t const char *newref, const char *logmsg)\n+{\n+\treturn refs_copy_or_rename_ref(refs, oldref, newref, logmsg, 1);\n }\n \n const char *ref_update_original_update_refname(struct ref_update *update)\ndiff --git a/refs/debug.c b/refs/debug.c\nindex 639db0f26..87b84e767 100644\n--- a/refs/debug.c\n+++ b/refs/debug.c\n@@ -143,28 +143,6 @@ static int debug_optimize_required(struct ref_store *ref_store,\n \treturn res;\n }\n \n-static int debug_rename_ref(struct ref_store *ref_store, const char *oldref,\n-\t\t\t const char *newref, const char *logmsg)\n-{\n-\tstruct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;\n-\tint res = drefs->refs->be->rename_ref(drefs->refs, oldref, newref,\n-\t\t\t\t\t logmsg);\n-\ttrace_printf_key(&trace_refs, \"rename_ref: %s -> %s \\\"%s\\\": %d\\n\", oldref, newref,\n-\t\tlogmsg, res);\n-\treturn res;\n-}\n-\n-static int debug_copy_ref(struct ref_store *ref_store, const char *oldref,\n-\t\t\t const char *newref, const char *logmsg)\n-{\n-\tstruct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;\n-\tint res =\n-\t\tdrefs->refs->be->copy_ref(drefs->refs, oldref, newref, logmsg);\n-\ttrace_printf_key(&trace_refs, \"copy_ref: %s -> %s \\\"%s\\\": %d\\n\", oldref, newref,\n-\t\tlogmsg, res);\n-\treturn res;\n-}\n-\n struct debug_ref_iterator {\n \tstruct ref_iterator base;\n \tstruct ref_iterator *iter;\n@@ -453,9 +431,6 @@ struct ref_storage_be refs_be_debug = {\n \t.optimize = debug_optimize,\n \t.optimize_required = debug_optimize_required,\n \n-\t.rename_ref = debug_rename_ref,\n-\t.copy_ref = debug_copy_ref,\n-\n \t.iterator_begin = debug_ref_iterator_begin,\n \t.read_raw_ref = debug_read_raw_ref,\n \t.read_symbolic_ref = debug_read_symbolic_ref,\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 71628550f..c28228116 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -1594,6 +1594,7 @@ static int files_optimize_required(struct ref_store *ref_store,\n * live into logs/refs.\n */\n #define TMP_RENAMED_LOG \"refs/.tmp-renamed-log\"\n+#define TMP_RENAMED_LOG_DESTINATION \"refs/.tmp-renamed-log-destination\"\n \n struct rename_cb {\n \tconst char *tmp_renamed_log;\n@@ -1685,12 +1686,28 @@ static int refs_rename_ref_available(struct ref_store *refs,\n \treturn ok;\n }\n \n+struct files_copy_or_rename_transaction_data {\n+\tstruct ref_lock *lock;\n+\tstruct object_id orig_oid;\n+\tstruct object_id destination_oid;\n+\tchar *destination_target;\n+\tint logmoved;\n+\tint destination_exists;\n+\tint destination_log_backed_up;\n+};\n+\n static int files_copy_or_rename_ref(struct ref_store *ref_store,\n-\t\t\t const char *oldrefname, const char *newrefname,\n-\t\t\t const char *logmsg, int copy)\n+\t\t\t\t struct ref_update *update,\n+\t\t\t\t struct ref_transaction *transaction)\n {\n \tstruct files_ref_store *refs =\n-\t\tfiles_downcast(ref_store, REF_STORE_WRITE, \"rename_ref\");\n+\t\tfiles_downcast(ref_store, REF_STORE_WRITE,\n+\t\t\t \"ref_transaction_prepare\");\n+\tstruct ref_copy_or_rename_update *operation = update->copy_or_rename;\n+\tconst char *oldrefname = operation->old_refname;\n+\tconst char *newrefname = update->refname;\n+\tconst char *logmsg = operation->logmsg;\n+\tbool copy = operation->type == REF_UPDATE_COPY;\n \tstruct object_id orig_oid;\n \tint flag = 0, logmoved = 0;\n \tstruct ref_lock *lock;\n@@ -1698,12 +1715,19 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,\n \tstruct strbuf sb_oldref = STRBUF_INIT;\n \tstruct strbuf sb_newref = STRBUF_INIT;\n \tstruct strbuf tmp_renamed_log = STRBUF_INIT;\n+\tstruct strbuf tmp_destination_log = STRBUF_INIT;\n+\tstruct strbuf destination_target = STRBUF_INIT;\n \tint log, ret;\n+\tint destination_exists = 0, destination_flags = 0;\n+\tint destination_log_backed_up = 0;\n+\tstruct object_id destination_oid;\n+\tstruct files_copy_or_rename_transaction_data *data;\n \tstruct strbuf err = STRBUF_INIT;\n \n \tfiles_reflog_path(refs, &sb_oldref, oldrefname);\n \tfiles_reflog_path(refs, &sb_newref, newrefname);\n \tfiles_reflog_path(refs, &tmp_renamed_log, TMP_RENAMED_LOG);\n+\tfiles_reflog_path(refs, &tmp_destination_log, TMP_RENAMED_LOG_DESTINATION);\n \n \tlog = !lstat(sb_oldref.buf, &loginfo);\n \tif (log && S_ISLNK(loginfo.st_mode)) {\n@@ -1727,11 +1751,67 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,\n \t\t\t\t oldrefname);\n \t\tgoto out;\n \t}\n+\tif (!oideq(&orig_oid, &operation->source_oid)) {\n+\t\tret = error(\"refname %s is at %s but expected %s\",\n+\t\t\t oldrefname, oid_to_hex(&orig_oid),\n+\t\t\t oid_to_hex(&operation->source_oid));\n+\t\tgoto out;\n+\t}\n \tif (!refs_rename_ref_available(&refs->base, oldrefname, newrefname)) {\n \t\tret = 1;\n \t\tgoto out;\n \t}\n \n+\tif (refs_resolve_ref_unsafe(&refs->base, newrefname,\n+\t\t\t\t RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n+\t\t\t\t &destination_oid, &destination_flags)) {\n+\t\tdestination_exists = 1;\n+\t\tif ((destination_flags & REF_ISSYMREF) &&\n+\t\t refs_read_symbolic_ref(&refs->base, newrefname,\n+\t\t\t\t\t &destination_target) < 0) {\n+\t\t\tret = error(\"unable to read symbolic ref %s\", newrefname);\n+\t\t\tgoto out;\n+\t\t}\n+\t}\n+\tif (destination_exists != operation->destination_exists) {\n+\t\tret = error(\"refname %s changed while renaming\", newrefname);\n+\t\tgoto out;\n+\t}\n+\tif (destination_exists) {\n+\t\tif (destination_flags & REF_ISSYMREF) {\n+\t\t\tif (!operation->destination_target ||\n+\t\t\t strcmp(destination_target.buf,\n+\t\t\t\t operation->destination_target)) {\n+\t\t\t\tret = error(\"refname %s changed while renaming\",\n+\t\t\t\t\t newrefname);\n+\t\t\t\tgoto out;\n+\t\t\t}\n+\t\t} else if (operation->destination_target ||\n+\t\t\t !oideq(&destination_oid,\n+\t\t\t\t &operation->destination_oid)) {\n+\t\t\tret = error(\"refname %s changed while renaming\", newrefname);\n+\t\t\tgoto out;\n+\t\t}\n+\t}\n+\n+\tif (!lstat(sb_newref.buf, &loginfo)) {\n+\t\tif (S_ISLNK(loginfo.st_mode)) {\n+\t\t\tret = error(\"reflog for %s is a symlink\", newrefname);\n+\t\t\tgoto out;\n+\t\t}\n+\t\tif (S_ISREG(loginfo.st_mode)) {\n+\t\t\tif (copy_file(refs->base.repo, tmp_destination_log.buf,\n+\t\t\t\t sb_newref.buf, 0644)) {\n+\t\t\t\tif (errno != EEXIST)\n+\t\t\t\t\tunlink(tmp_destination_log.buf);\n+\t\t\t\tret = error(\"unable to back up logfile logs/%s: %s\",\n+\t\t\t\t\t newrefname, strerror(errno));\n+\t\t\t\tgoto out;\n+\t\t\t}\n+\t\t\tdestination_log_backed_up = 1;\n+\t\t}\n+\t}\n+\n \tif (!copy && log && rename(sb_oldref.buf, tmp_renamed_log.buf)) {\n \t\tret = error(\"unable to move logfile logs/%s to logs/\"TMP_RENAMED_LOG\": %s\",\n \t\t\t oldrefname, strerror(errno));\n@@ -1744,8 +1824,10 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,\n \t\tgoto out;\n \t}\n \n-\tif (!copy && refs_delete_ref(&refs->base, logmsg, oldrefname,\n-\t\t\t &orig_oid, REF_NO_DEREF)) {\n+\tif (!copy && refs_delete_ref_with_transaction_flags(&refs->base, logmsg,\n+\t\t\t\t\t\t\t oldrefname, &orig_oid,\n+\t\t\t\t\t\t\t REF_NO_DEREF,\n+\t\t\t\t\t\t\t REF_TRANSACTION_FLAG_SKIP_HOOK)) {\n \t\terror(\"unable to delete old %s\", oldrefname);\n \t\tgoto rollback;\n \t}\n@@ -1760,8 +1842,9 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,\n \tif (!copy && refs_resolve_ref_unsafe(&refs->base, newrefname,\n \t\t\t\t\t RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n \t\t\t\t\t NULL, NULL) &&\n-\t refs_delete_ref(&refs->base, NULL, newrefname,\n-\t\t\t NULL, REF_NO_DEREF)) {\n+\t refs_delete_ref_with_transaction_flags(&refs->base, NULL, newrefname,\n+\t\t\t\t\t\t NULL, REF_NO_DEREF,\n+\t\t\t\t\t\t REF_TRANSACTION_FLAG_SKIP_HOOK)) {\n \t\tif (errno == EISDIR) {\n \t\t\tstruct strbuf path = STRBUF_INIT;\n \t\t\tint result;\n@@ -1796,13 +1879,25 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,\n \t}\n \toidcpy(&lock->old_oid, &orig_oid);\n \n-\tif (write_ref_to_lockfile(refs, lock, &orig_oid, &err) ||\n-\t commit_ref_update(refs, lock, &orig_oid, logmsg, 0, &err)) {\n+\tif (write_ref_to_lockfile(refs, lock, &orig_oid, &err)) {\n \t\terror(\"unable to write current sha1 into %s: %s\", newrefname, err.buf);\n \t\tstrbuf_release(&err);\n \t\tgoto rollback;\n \t}\n \n+\tCALLOC_ARRAY(data, 1);\n+\tdata->lock = lock;\n+\toidcpy(&data->orig_oid, &orig_oid);\n+\tdata->logmoved = logmoved;\n+\tdata->destination_exists = destination_exists;\n+\tdata->destination_log_backed_up = destination_log_backed_up;\n+\tif (destination_exists && !(destination_flags & REF_ISSYMREF))\n+\t\toidcpy(&data->destination_oid, &destination_oid);\n+\tif (destination_flags & REF_ISSYMREF)\n+\t\tdata->destination_target = strbuf_detach(&destination_target, NULL);\n+\ttransaction->backend_data = data;\n+\ttransaction->state = REF_TRANSACTION_PREPARED;\n+\n \tret = 0;\n \tgoto out;\n \n@@ -1821,38 +1916,40 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,\n \t}\n \n rollbacklog:\n-\tif (logmoved && rename(sb_newref.buf, sb_oldref.buf))\n-\t\terror(\"unable to restore logfile %s from %s: %s\",\n-\t\t\toldrefname, newrefname, strerror(errno));\n+\tif (logmoved) {\n+\t\tif (rename(sb_newref.buf, tmp_renamed_log.buf)) {\n+\t\t\terror(\"unable to restore logfile %s from %s: %s\",\n+\t\t\t oldrefname, newrefname, strerror(errno));\n+\t\t} else {\n+\t\t\ttry_remove_empty_parents(refs, newrefname,\n+\t\t\t\t\t\t REMOVE_EMPTY_PARENTS_REFLOG);\n+\t\t\tif (rename_tmp_log(refs, oldrefname))\n+\t\t\t\terror(\"unable to restore logfile %s from logs/\"\n+\t\t\t\t TMP_RENAMED_LOG \": %s\",\n+\t\t\t\t oldrefname, strerror(errno));\n+\t\t}\n+\t}\n \tif (!logmoved && log &&\n \t rename(tmp_renamed_log.buf, sb_oldref.buf))\n \t\terror(\"unable to restore logfile %s from logs/\"TMP_RENAMED_LOG\": %s\",\n \t\t\toldrefname, strerror(errno));\n+\tif (destination_log_backed_up &&\n+\t rename(tmp_destination_log.buf, sb_newref.buf))\n+\t\terror(\"unable to restore logfile %s: %s\",\n+\t\t newrefname, strerror(errno));\n \tret = 1;\n out:\n+\tif (ret && destination_log_backed_up)\n+\t\tunlink(tmp_destination_log.buf);\n \tstrbuf_release(&sb_newref);\n \tstrbuf_release(&sb_oldref);\n \tstrbuf_release(&tmp_renamed_log);\n+\tstrbuf_release(&tmp_destination_log);\n+\tstrbuf_release(&destination_target);\n \n \treturn ret;\n }\n \n-static int files_rename_ref(struct ref_store *ref_store,\n-\t\t\t const char *oldrefname, const char *newrefname,\n-\t\t\t const char *logmsg)\n-{\n-\treturn files_copy_or_rename_ref(ref_store, oldrefname,\n-\t\t\t\t newrefname, logmsg, 0);\n-}\n-\n-static int files_copy_ref(struct ref_store *ref_store,\n-\t\t\t const char *oldrefname, const char *newrefname,\n-\t\t\t const char *logmsg)\n-{\n-\treturn files_copy_or_rename_ref(ref_store, oldrefname,\n-\t\t\t\t newrefname, logmsg, 1);\n-}\n-\n static int close_ref_gently(struct ref_lock *lock)\n {\n \tif (close_lock_file_gently(&lock->lk))\n@@ -2962,6 +3059,14 @@ static int files_transaction_prepare(struct ref_store *ref_store,\n \tstruct ref_transaction *packed_transaction = NULL;\n \n \tassert(err);\n+\t{\n+\t\tstruct ref_update *operation =\n+\t\t\tref_transaction_copy_or_rename_update(transaction);\n+\n+\t\tif (operation)\n+\t\t\treturn files_copy_or_rename_ref(ref_store, operation,\n+\t\t\t\t\t\t\ttransaction);\n+\t}\n \n \tif (transaction->flags & REF_TRANSACTION_FLAG_INITIAL)\n \t\tgoto cleanup;\n@@ -3318,6 +3423,10 @@ static int files_transaction_finish_initial(struct files_ref_store *refs,\n \treturn ret;\n }\n \n+static int files_transaction_abort(struct ref_store *ref_store,\n+\t\t\t\t struct ref_transaction *transaction,\n+\t\t\t\t struct strbuf *err);\n+\n static int files_transaction_finish(struct ref_store *ref_store,\n \t\t\t\t struct ref_transaction *transaction,\n \t\t\t\t struct strbuf *err)\n@@ -3333,6 +3442,40 @@ static int files_transaction_finish(struct ref_store *ref_store,\n \n \n \tassert(err);\n+\t{\n+\t\tstruct ref_update *update =\n+\t\t\tref_transaction_copy_or_rename_update(transaction);\n+\n+\t\tif (update) {\n+\t\t\tstruct ref_copy_or_rename_update *operation =\n+\t\t\t\tupdate->copy_or_rename;\n+\t\t\tstruct files_copy_or_rename_transaction_data *data =\n+\t\t\t\ttransaction->backend_data;\n+\t\t\tint special_ret;\n+\n+\t\t\tspecial_ret = commit_ref_update(refs, data->lock, &data->orig_oid,\n+\t\t\t\t\t\t\toperation->logmsg, 0, err);\n+\t\t\tif (special_ret) {\n+\t\t\t\terror(\"unable to write current sha1 into %s: %s\",\n+\t\t\t\t update->refname, err->buf);\n+\t\t\t\tdata->lock = NULL;\n+\t\t\t\tfiles_transaction_abort(ref_store, transaction, err);\n+\t\t\t\treturn special_ret;\n+\t\t\t} else if (data->destination_log_backed_up) {\n+\t\t\t\tstruct strbuf path = STRBUF_INIT;\n+\n+\t\t\t\tfiles_reflog_path(refs, &path, TMP_RENAMED_LOG_DESTINATION);\n+\t\t\t\tif (unlink(path.buf) < 0 && errno != ENOENT)\n+\t\t\t\t\twarning_errno(\"unable to remove '%s'\", path.buf);\n+\t\t\t\tstrbuf_release(&path);\n+\t\t\t}\n+\t\t\tfree(data->destination_target);\n+\t\t\tfree(data);\n+\t\t\ttransaction->backend_data = NULL;\n+\t\t\ttransaction->state = REF_TRANSACTION_CLOSED;\n+\t\t\treturn special_ret;\n+\t\t}\n+\t}\n \n \tif (transaction->flags & REF_TRANSACTION_FLAG_INITIAL)\n \t\treturn files_transaction_finish_initial(refs, transaction, err);\n@@ -3476,11 +3619,105 @@ static int files_transaction_finish(struct ref_store *ref_store,\n \n static int files_transaction_abort(struct ref_store *ref_store,\n \t\t\t\t struct ref_transaction *transaction,\n-\t\t\t\t struct strbuf *err UNUSED)\n+\t\t\t\t struct strbuf *err)\n {\n \tstruct files_ref_store *refs =\n \t\tfiles_downcast(ref_store, 0, \"ref_transaction_abort\");\n \n+\t{\n+\t\tstruct ref_update *update =\n+\t\t\tref_transaction_copy_or_rename_update(transaction);\n+\n+\t\tif (update) {\n+\t\t\tstruct ref_copy_or_rename_update *operation =\n+\t\t\t\tupdate->copy_or_rename;\n+\t\t\tstruct files_copy_or_rename_transaction_data *data =\n+\t\t\t\ttransaction->backend_data;\n+\t\t\tstruct strbuf new_log = STRBUF_INIT;\n+\t\t\tstruct strbuf destination_log = STRBUF_INIT;\n+\t\t\tstruct strbuf temporary_log = STRBUF_INIT;\n+\t\t\tstruct ref_transaction *restore_transaction = NULL;\n+\t\t\tstruct ref_lock *lock;\n+\t\t\tint ret = 0;\n+\n+\t\t\tif (data->lock)\n+\t\t\t\tunlock_ref(data->lock);\n+\t\t\tif (operation->type == REF_UPDATE_RENAME) {\n+\t\t\t\tlock = lock_ref_oid_basic(refs, operation->old_refname, err);\n+\t\t\t\tif (!lock ||\n+\t\t\t\t write_ref_to_lockfile(refs, lock, &data->orig_oid, err) ||\n+\t\t\t\t commit_ref_update(refs, lock, &data->orig_oid, NULL,\n+\t\t\t\t\t\t REF_SKIP_CREATE_REFLOG, err))\n+\t\t\t\t\tret = -1;\n+\t\t\t}\n+\n+\t\t\tif (data->logmoved) {\n+\t\t\t\tfiles_reflog_path(refs, &new_log, update->refname);\n+\t\t\t\tif (operation->type == REF_UPDATE_RENAME) {\n+\t\t\t\t\tfiles_reflog_path(refs, &temporary_log, TMP_RENAMED_LOG);\n+\t\t\t\t\tif (rename(new_log.buf, temporary_log.buf) < 0) {\n+\t\t\t\t\t\tstrbuf_addf(err, \"unable to restore logfile %s: %s\",\n+\t\t\t\t\t\t\t operation->old_refname, strerror(errno));\n+\t\t\t\t\t\tret = -1;\n+\t\t\t\t\t} else {\n+\t\t\t\t\t\ttry_remove_empty_parents(refs,\n+\t\t\t\t\t\t\t\t\t update->refname,\n+\t\t\t\t\t\t\t\t\t REMOVE_EMPTY_PARENTS_REFLOG);\n+\t\t\t\t\t\tif (rename_tmp_log(refs,\n+\t\t\t\t\t\t\t\t operation->old_refname)) {\n+\t\t\t\t\t\t\tstrbuf_addf(err, \"unable to restore logfile %s: %s\",\n+\t\t\t\t\t\t\t\t operation->old_refname,\n+\t\t\t\t\t\t\t\t strerror(errno));\n+\t\t\t\t\t\t\tret = -1;\n+\t\t\t\t\t\t}\n+\t\t\t\t\t}\n+\t\t\t\t} else if (unlink(new_log.buf) < 0 && errno != ENOENT) {\n+\t\t\t\t\tstrbuf_addf(err, \"unable to remove logfile %s: %s\",\n+\t\t\t\t\t\t update->refname, strerror(errno));\n+\t\t\t\t\tret = -1;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t\tif (data->destination_log_backed_up) {\n+\t\t\t\tfiles_reflog_path(refs, &destination_log,\n+\t\t\t\t\t\t TMP_RENAMED_LOG_DESTINATION);\n+\t\t\t\tif (rename(destination_log.buf, new_log.buf) < 0) {\n+\t\t\t\t\tstrbuf_addf(err, \"unable to restore logfile %s: %s\",\n+\t\t\t\t\t\t update->refname, strerror(errno));\n+\t\t\t\t\tret = -1;\n+\t\t\t\t}\n+\t\t\t}\n+\n+\t\t\tif (operation->type == REF_UPDATE_RENAME &&\n+\t\t\t data->destination_exists) {\n+\t\t\t\trestore_transaction = ref_store_transaction_begin(\n+\t\t\t\t\t&refs->base, REF_TRANSACTION_FLAG_SKIP_HOOK, err);\n+\t\t\t\tif (!restore_transaction ||\n+\t\t\t\t ref_transaction_update(restore_transaction,\n+\t\t\t\t\t\t\t update->refname,\n+\t\t\t\t\t\t\t data->destination_target ? NULL :\n+\t\t\t\t\t\t\t\t\t\t &data->destination_oid,\n+\t\t\t\t\t\t\t NULL,\n+\t\t\t\t\t\t\t data->destination_target,\n+\t\t\t\t\t\t\t NULL,\n+\t\t\t\t\t\t\t REF_NO_DEREF |\n+\t\t\t\t\t\t\t\t REF_SKIP_CREATE_REFLOG,\n+\t\t\t\t\t\t\t NULL, err) ||\n+\t\t\t\t ref_transaction_commit(restore_transaction, err))\n+\t\t\t\t\tret = -1;\n+\t\t\t\tref_transaction_free(restore_transaction);\n+\t\t\t}\n+\n+\t\t\tstrbuf_release(&destination_log);\n+\t\t\tstrbuf_release(&temporary_log);\n+\t\t\tstrbuf_release(&new_log);\n+\t\t\tfree(data->destination_target);\n+\t\t\tfree(data);\n+\t\t\ttransaction->backend_data = NULL;\n+\t\t\ttransaction->state = REF_TRANSACTION_CLOSED;\n+\t\t\treturn ret;\n+\t\t}\n+\t}\n+\n \tfiles_transaction_cleanup(refs, transaction);\n \treturn 0;\n }\n@@ -4095,8 +4332,6 @@ struct ref_storage_be refs_be_files = {\n \n \t.optimize = files_optimize,\n \t.optimize_required = files_optimize_required,\n-\t.rename_ref = files_rename_ref,\n-\t.copy_ref = files_copy_ref,\n \n \t.iterator_begin = files_ref_iterator_begin,\n \t.read_raw_ref = files_read_raw_ref,\ndiff --git a/refs/packed-backend.c b/refs/packed-backend.c\nindex a73fc6aca..364a91291 100644\n--- a/refs/packed-backend.c\n+++ b/refs/packed-backend.c\n@@ -2164,8 +2164,6 @@ struct ref_storage_be refs_be_packed = {\n \t.optimize = packed_optimize,\n \t.optimize_required = packed_optimize_required,\n \n-\t.rename_ref = NULL,\n-\t.copy_ref = NULL,\n \n \t.iterator_begin = packed_ref_iterator_begin,\n \t.read_raw_ref = packed_read_raw_ref,\ndiff --git a/refs/refs-internal.h b/refs/refs-internal.h\nindex c3ac7b556..5d4dc0171 100644\n--- a/refs/refs-internal.h\n+++ b/refs/refs-internal.h\n@@ -155,9 +155,33 @@ struct ref_update {\n \t */\n \tstruct ref_update *parent_update;\n \n+\t/*\n+\t * Copy and rename operations require backend-specific handling while\n+\t * still exposing their logical updates to transaction hooks. Keep that\n+\t * state on the destination update so it composes with other updates in\n+\t * the transaction instead of making copy or rename a transaction-wide\n+\t * property.\n+\t */\n+\tstruct ref_copy_or_rename_update *copy_or_rename;\n+\n \tconst char refname[FLEX_ARRAY];\n };\n \n+enum ref_copy_or_rename_type {\n+\tREF_UPDATE_RENAME,\n+\tREF_UPDATE_COPY,\n+};\n+\n+struct ref_copy_or_rename_update {\n+\tenum ref_copy_or_rename_type type;\n+\tchar *old_refname;\n+\tchar *logmsg;\n+\tstruct object_id source_oid;\n+\tstruct object_id destination_oid;\n+\tchar *destination_target;\n+\tunsigned int destination_exists:1;\n+};\n+\n int refs_read_raw_ref(struct ref_store *ref_store, const char *refname,\n \t\t struct object_id *oid, struct strbuf *referent,\n \t\t unsigned int *type, int *failure_errno);\n@@ -187,6 +211,13 @@ struct ref_update *ref_transaction_add_update(\n \t\tconst char *committer_info,\n \t\tconst char *msg);\n \n+int refs_delete_ref_with_transaction_flags(struct ref_store *refs,\n+\t\t\t\t\t const char *msg,\n+\t\t\t\t\t const char *refname,\n+\t\t\t\t\t const struct object_id *old_oid,\n+\t\t\t\t\t unsigned int flags,\n+\t\t\t\t\t unsigned int transaction_flags);\n+\n /*\n * Transaction states.\n *\n@@ -242,6 +273,12 @@ struct ref_transaction {\n \tuint64_t max_index;\n };\n \n+/* Suppress hooks for a transaction nested inside another refs operation. */\n+#define REF_TRANSACTION_FLAG_SKIP_HOOK (1 << 2)\n+\n+struct ref_update *ref_transaction_copy_or_rename_update(\n+\tstruct ref_transaction *transaction);\n+\n /*\n * Check for entries in extras that are within the specified\n * directory, where dirname is a reference directory name including\n@@ -451,13 +488,6 @@ typedef int optimize_required_fn(struct ref_store *ref_store,\n \t\t\t\t struct refs_optimize_opts *opts,\n \t\t\t\t bool *required);\n \n-typedef int rename_ref_fn(struct ref_store *ref_store,\n-\t\t\t const char *oldref, const char *newref,\n-\t\t\t const char *logmsg);\n-typedef int copy_ref_fn(struct ref_store *ref_store,\n-\t\t\t const char *oldref, const char *newref,\n-\t\t\t const char *logmsg);\n-\n /*\n * Iterate over the references in `ref_store` whose names start with\n * `prefix`. `prefix` is matched as a literal string, without regard\n@@ -577,9 +607,6 @@ struct ref_storage_be {\n \n \toptimize_fn *optimize;\n \toptimize_required_fn *optimize_required;\n-\trename_ref_fn *rename_ref;\n-\tcopy_ref_fn *copy_ref;\n-\n \tref_iterator_begin_fn *iterator_begin;\n \tread_raw_ref_fn *read_raw_ref;\n \ndiff --git a/refs/reftable-backend.c b/refs/reftable-backend.c\nindex 10db03991..589fcc998 100644\n--- a/refs/reftable-backend.c\n+++ b/refs/reftable-backend.c\n@@ -953,6 +953,14 @@ struct reftable_transaction_data {\n \tsize_t args_nr, args_alloc;\n };\n \n+struct reftable_copy_or_rename_transaction_data {\n+\tstruct reftable_addition *addition;\n+};\n+\n+static int reftable_be_copy_or_rename_prepare(struct ref_store *ref_store,\n+\t\t\t\t\t struct ref_transaction *transaction,\n+\t\t\t\t\t struct strbuf *err);\n+\n static void free_transaction_data(struct reftable_transaction_data *tx_data)\n {\n \tif (!tx_data)\n@@ -1326,6 +1334,10 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,\n \tsize_t i;\n \tint ret;\n \n+\tif (ref_transaction_copy_or_rename_update(transaction))\n+\t\treturn reftable_be_copy_or_rename_prepare(ref_store, transaction,\n+\t\t\t\t\t\t\t err);\n+\n \tret = refs->err;\n \tif (ret < 0)\n \t\tgoto done;\n@@ -1419,7 +1431,20 @@ static int reftable_be_transaction_abort(struct ref_store *ref_store UNUSED,\n \t\t\t\t\t struct ref_transaction *transaction,\n \t\t\t\t\t struct strbuf *err UNUSED)\n {\n-\tstruct reftable_transaction_data *tx_data = transaction->backend_data;\n+\tstruct reftable_transaction_data *tx_data;\n+\n+\tif (ref_transaction_copy_or_rename_update(transaction)) {\n+\t\tstruct reftable_copy_or_rename_transaction_data *data =\n+\t\t\ttransaction->backend_data;\n+\n+\t\treftable_addition_destroy(data->addition);\n+\t\tfree(data);\n+\t\ttransaction->backend_data = NULL;\n+\t\ttransaction->state = REF_TRANSACTION_CLOSED;\n+\t\treturn 0;\n+\t}\n+\n+\ttx_data = transaction->backend_data;\n \tfree_transaction_data(tx_data);\n \ttransaction->state = REF_TRANSACTION_CLOSED;\n \treturn 0;\n@@ -1667,9 +1692,28 @@ static int reftable_be_transaction_finish(struct ref_store *ref_store UNUSED,\n \t\t\t\t\t struct ref_transaction *transaction,\n \t\t\t\t\t struct strbuf *err)\n {\n-\tstruct reftable_transaction_data *tx_data = transaction->backend_data;\n+\tstruct reftable_transaction_data *tx_data;\n \tint ret = 0;\n \n+\tif (ref_transaction_copy_or_rename_update(transaction)) {\n+\t\tstruct reftable_copy_or_rename_transaction_data *data =\n+\t\t\ttransaction->backend_data;\n+\t\tint special_ret = reftable_addition_commit(data->addition);\n+\n+\t\treftable_addition_destroy(data->addition);\n+\t\tfree(data);\n+\t\ttransaction->backend_data = NULL;\n+\t\ttransaction->state = REF_TRANSACTION_CLOSED;\n+\t\tif (special_ret < 0) {\n+\t\t\tstrbuf_addf(err, _(\"reftable: transaction failure: %s\"),\n+\t\t\t\t reftable_error_str(special_ret));\n+\t\t\treturn -1;\n+\t\t}\n+\t\treturn 0;\n+\t}\n+\n+\ttx_data = transaction->backend_data;\n+\n \tfor (size_t i = 0; i < tx_data->args_nr; i++) {\n \t\ttx_data->args[i].max_index = transaction->max_index;\n \n@@ -1764,17 +1808,20 @@ struct write_create_symref_arg {\n struct write_copy_arg {\n \tstruct reftable_ref_store *refs;\n \tstruct reftable_backend *be;\n+\tstruct strbuf *err;\n \tconst char *oldname;\n \tconst char *newname;\n \tconst char *logmsg;\n \tint delete_old;\n+\tstruct ref_copy_or_rename_update *operation;\n };\n \n static int write_copy_table(struct reftable_writer *writer, void *cb_data)\n {\n \tstruct write_copy_arg *arg = cb_data;\n \tuint64_t deletion_ts, creation_ts;\n-\tstruct reftable_ref_record old_ref = {0}, refs[2] = {0};\n+\tstruct reftable_ref_record old_ref = {0}, destination_ref = {0};\n+\tstruct reftable_ref_record refs[2] = {0};\n \tstruct reftable_log_record old_log = {0}, *logs = NULL;\n \tstruct reftable_iterator it = {0};\n \tstruct string_list skip = STRING_LIST_INIT_NODUP;\n@@ -1789,14 +1836,75 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data)\n \t\tBUG(\"failed splitting committer info\");\n \n \tif (reftable_stack_read_ref(arg->be->stack, arg->oldname, &old_ref)) {\n-\t\tret = error(_(\"refname %s not found\"), arg->oldname);\n+\t\tstrbuf_addf(arg->err, _(\"refname %s not found\"), arg->oldname);\n+\t\tret = -1;\n \t\tgoto done;\n \t}\n \tif (old_ref.value_type == REFTABLE_REF_SYMREF) {\n-\t\tret = error(_(\"refname %s is a symbolic ref, copying it is not supported\"),\n+\t\tstrbuf_addf(arg->err,\n+\t\t\t _(\"refname %s is a symbolic ref, copying it is not supported\"),\n \t\t\t arg->oldname);\n+\t\tret = -1;\n \t\tgoto done;\n \t}\n+\tif (arg->operation) {\n+\t\tstruct object_id oid;\n+\n+\t\tif (old_ref.value_type == REFTABLE_REF_VAL2)\n+\t\t\toidread(&oid, old_ref.value.val2.value,\n+\t\t\t\targ->refs->base.repo->hash_algo);\n+\t\telse\n+\t\t\toidread(&oid, old_ref.value.val1,\n+\t\t\t\targ->refs->base.repo->hash_algo);\n+\t\tif (!oideq(&oid, &arg->operation->source_oid)) {\n+\t\t\tstrbuf_addf(arg->err,\n+\t\t\t\t _(\"refname %s is at %s but expected %s\"),\n+\t\t\t\t arg->oldname, oid_to_hex(&oid),\n+\t\t\t\t oid_to_hex(&arg->operation->source_oid));\n+\t\t\tret = -1;\n+\t\t\tgoto done;\n+\t\t}\n+\n+\t\tret = reftable_stack_read_ref(arg->be->stack, arg->newname,\n+\t\t\t\t\t &destination_ref);\n+\t\tif (ret < 0)\n+\t\t\tgoto done;\n+\t\tif (arg->operation->destination_exists != !ret) {\n+\t\t\tstrbuf_addf(arg->err,\n+\t\t\t\t _(\"refname %s changed while renaming\"),\n+\t\t\t\t arg->newname);\n+\t\t\tret = -1;\n+\t\t\tgoto done;\n+\t\t}\n+\t\tif (!ret) {\n+\t\t\tif (destination_ref.value_type == REFTABLE_REF_SYMREF) {\n+\t\t\t\tif (!arg->operation->destination_target ||\n+\t\t\t\t strcmp(destination_ref.value.symref,\n+\t\t\t\t\t arg->operation->destination_target)) {\n+\t\t\t\t\tstrbuf_addf(arg->err,\n+\t\t\t\t\t\t _(\"refname %s changed while renaming\"),\n+\t\t\t\t\t\t arg->newname);\n+\t\t\t\t\tret = -1;\n+\t\t\t\t\tgoto done;\n+\t\t\t\t}\n+\t\t\t} else {\n+\t\t\t\tif (destination_ref.value_type == REFTABLE_REF_VAL2)\n+\t\t\t\t\toidread(&oid, destination_ref.value.val2.value,\n+\t\t\t\t\t\targ->refs->base.repo->hash_algo);\n+\t\t\t\telse\n+\t\t\t\t\toidread(&oid, destination_ref.value.val1,\n+\t\t\t\t\t\targ->refs->base.repo->hash_algo);\n+\t\t\t\tif (arg->operation->destination_target ||\n+\t\t\t\t !oideq(&oid, &arg->operation->destination_oid)) {\n+\t\t\t\t\tstrbuf_addf(arg->err,\n+\t\t\t\t\t\t _(\"refname %s changed while renaming\"),\n+\t\t\t\t\t\t arg->newname);\n+\t\t\t\t\tret = -1;\n+\t\t\t\t\tgoto done;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t}\n+\t}\n \n \t/*\n \t * There's nothing to do in case the old and new name are the same, so\n@@ -1815,7 +1923,7 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data)\n \tret = refs_verify_refname_available(&arg->refs->base, arg->newname,\n \t\t\t\t\t NULL, &skip, 0, &errbuf);\n \tif (ret < 0) {\n-\t\terror(\"%s\", errbuf.buf);\n+\t\tstrbuf_addbuf(arg->err, &errbuf);\n \t\tgoto done;\n \t}\n \n@@ -1980,68 +2088,63 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data)\n \tfor (i = 0; i < ARRAY_SIZE(refs); i++)\n \t\treftable_ref_record_release(&refs[i]);\n \treftable_ref_record_release(&old_ref);\n+\treftable_ref_record_release(&destination_ref);\n \treftable_log_record_release(&old_log);\n \treturn ret;\n }\n \n-static int reftable_be_rename_ref(struct ref_store *ref_store,\n-\t\t\t\t const char *oldrefname,\n-\t\t\t\t const char *newrefname,\n-\t\t\t\t const char *logmsg)\n+static int reftable_be_copy_or_rename_prepare(struct ref_store *ref_store,\n+\t\t\t\t\t struct ref_transaction *transaction,\n+\t\t\t\t\t struct strbuf *err)\n {\n \tstruct reftable_ref_store *refs =\n-\t\treftable_be_downcast(ref_store, REF_STORE_WRITE, \"rename_ref\");\n+\t\treftable_be_downcast(ref_store, REF_STORE_WRITE,\n+\t\t\t\t \"ref_transaction_prepare\");\n+\tstruct reftable_copy_or_rename_transaction_data *data = NULL;\n+\tstruct ref_update *update =\n+\t\tref_transaction_copy_or_rename_update(transaction);\n+\tstruct ref_copy_or_rename_update *operation = update->copy_or_rename;\n \tstruct write_copy_arg arg = {\n \t\t.refs = refs,\n-\t\t.oldname = oldrefname,\n-\t\t.newname = newrefname,\n-\t\t.logmsg = logmsg,\n-\t\t.delete_old = 1,\n+\t\t.err = err,\n+\t\t.oldname = operation->old_refname,\n+\t\t.newname = update->refname,\n+\t\t.logmsg = operation->logmsg,\n+\t\t.delete_old = operation->type == REF_UPDATE_RENAME,\n+\t\t.operation = operation,\n \t};\n \tint ret;\n \n+\tCALLOC_ARRAY(data, 1);\n \tret = refs->err;\n \tif (ret < 0)\n \t\tgoto done;\n-\n-\tret = backend_for(&arg.be, refs, newrefname, &newrefname, 1);\n+\tret = backend_for(&arg.be, refs, update->refname,\n+\t\t\t &arg.newname, 1);\n \tif (ret)\n \t\tgoto done;\n-\tret = reftable_stack_add(arg.be->stack, &write_copy_table, &arg,\n-\t\t\t\t &reftable_be_write_options(refs)->opts);\n-\n-done:\n-\tassert(ret != REFTABLE_API_ERROR);\n-\treturn ret;\n-}\n-\n-static int reftable_be_copy_ref(struct ref_store *ref_store,\n-\t\t\t\tconst char *oldrefname,\n-\t\t\t\tconst char *newrefname,\n-\t\t\t\tconst char *logmsg)\n-{\n-\tstruct reftable_ref_store *refs =\n-\t\treftable_be_downcast(ref_store, REF_STORE_WRITE, \"copy_ref\");\n-\tstruct write_copy_arg arg = {\n-\t\t.refs = refs,\n-\t\t.oldname = oldrefname,\n-\t\t.newname = newrefname,\n-\t\t.logmsg = logmsg,\n-\t};\n-\tint ret;\n-\n-\tret = refs->err;\n-\tif (ret < 0)\n+\tret = reftable_stack_addition_new(&data->addition, arg.be->stack,\n+\t\t\t\t\t &reftable_be_write_options(refs)->opts);\n+\tif (ret)\n \t\tgoto done;\n-\n-\tret = backend_for(&arg.be, refs, newrefname, &newrefname, 1);\n+\tret = reftable_addition_add(data->addition, &write_copy_table, &arg);\n \tif (ret)\n \t\tgoto done;\n-\tret = reftable_stack_add(arg.be->stack, &write_copy_table, &arg,\n-\t\t\t\t &reftable_be_write_options(refs)->opts);\n+\n+\ttransaction->backend_data = data;\n+\ttransaction->state = REF_TRANSACTION_PREPARED;\n+\treturn 0;\n \n done:\n \tassert(ret != REFTABLE_API_ERROR);\n+\tif (data) {\n+\t\treftable_addition_destroy(data->addition);\n+\t\tfree(data);\n+\t}\n+\ttransaction->state = REF_TRANSACTION_CLOSED;\n+\tif (ret && !err->len)\n+\t\tstrbuf_addf(err, _(\"reftable: transaction prepare: %s\"),\n+\t\t\t reftable_error_str(ret));\n \treturn ret;\n }\n \n@@ -2872,8 +2975,6 @@ struct ref_storage_be refs_be_reftable = {\n \t.optimize = reftable_be_optimize,\n \t.optimize_required = reftable_be_optimize_required,\n \n-\t.rename_ref = reftable_be_rename_ref,\n-\t.copy_ref = reftable_be_copy_ref,\n \n \t.iterator_begin = reftable_be_iterator_begin,\n \t.read_raw_ref = reftable_be_read_raw_ref,\ndiff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh\nindex 4fe9d9b23..116b2ff07 100755\n--- a/t/t1416-ref-transaction-hooks.sh\n+++ b/t/t1416-ref-transaction-hooks.sh\n@@ -93,6 +93,148 @@ test_expect_success 'hook gets all queued updates in committed state' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'hook gets both updates when renaming a branch' '\n+\ttest_when_finished \"rm -f actual\" &&\n+\tgit branch old PRE &&\n+\ttest_hook reference-transaction <<-\\EOF &&\n+\t\techo \"$1\" >>actual &&\n+\t\tcat >>actual\n+\tEOF\n+\tcat >expect <<-EOF &&\n+\tpreparing\n+\t$PRE_OID $ZERO_OID refs/heads/old\n+\t$ZERO_OID $PRE_OID refs/heads/new\n+\tprepared\n+\t$PRE_OID $ZERO_OID refs/heads/old\n+\t$ZERO_OID $PRE_OID refs/heads/new\n+\tcommitted\n+\t$PRE_OID $ZERO_OID refs/heads/old\n+\t$ZERO_OID $PRE_OID refs/heads/new\n+\tEOF\n+\tgit branch -m old new &&\n+\ttest_cmp expect actual &&\n+\ttest_must_fail git rev-parse --verify refs/heads/old &&\n+\ttest_cmp_rev PRE refs/heads/new\n+'\n+\n+test_expect_success 'hook gets destination update when copying a branch' '\n+\ttest_when_finished \"rm -f actual\" &&\n+\tgit branch copy-source PRE &&\n+\ttest_hook reference-transaction <<-\\EOF &&\n+\t\techo \"$1\" >>actual &&\n+\t\tcat >>actual\n+\tEOF\n+\tcat >expect <<-EOF &&\n+\tpreparing\n+\t$ZERO_OID $PRE_OID refs/heads/copy-destination\n+\tprepared\n+\t$ZERO_OID $PRE_OID refs/heads/copy-destination\n+\tcommitted\n+\t$ZERO_OID $PRE_OID refs/heads/copy-destination\n+\tEOF\n+\tgit branch -c copy-source copy-destination &&\n+\ttest_cmp expect actual &&\n+\ttest_cmp_rev PRE refs/heads/copy-source &&\n+\ttest_cmp_rev PRE refs/heads/copy-destination\n+'\n+\n+test_expect_success 'hook gets overwritten values for forced rename and copy' '\n+\tgit branch force-old PRE &&\n+\tgit branch force-new POST &&\n+\tgit branch force-copy-source PRE &&\n+\tgit branch force-copy-destination POST &&\n+\ttest_hook reference-transaction <<-\\EOF &&\n+\t\tif test \"$1\" = committed\n+\t\tthen\n+\t\t\tcat >>actual\n+\t\tfi\n+\tEOF\n+\tgit branch -M force-old force-new &&\n+\tgit branch -C force-copy-source force-copy-destination &&\n+\tcat >expect <<-EOF &&\n+\t$PRE_OID $ZERO_OID refs/heads/force-old\n+\t$POST_OID $PRE_OID refs/heads/force-new\n+\t$POST_OID $PRE_OID refs/heads/force-copy-destination\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'hook can abort a branch rename after preparation' '\n+\tgit branch abort-old PRE &&\n+\tgit branch abort-new POST &&\n+\tgit reflog show --format=%gs abort-old >old-log &&\n+\tgit reflog show --format=%gs abort-new >new-log &&\n+\ttest_hook reference-transaction <<-\\EOF &&\n+\t\ttest \"$1\" != prepared\n+\tEOF\n+\ttest_must_fail git branch -M abort-old abort-new &&\n+\ttest_cmp_rev PRE refs/heads/abort-old &&\n+\ttest_cmp_rev POST refs/heads/abort-new &&\n+\tgit reflog show --format=%gs abort-old >old-log-after &&\n+\tgit reflog show --format=%gs abort-new >new-log-after &&\n+\ttest_cmp old-log old-log-after &&\n+\ttest_cmp new-log new-log-after\n+'\n+\n+test_expect_success 'hook can abort a D/F branch rename after preparation' '\n+\tgit branch df-old PRE &&\n+\tgit reflog show --format=%gs df-old >df-log &&\n+\ttest_hook reference-transaction <<-\\EOF &&\n+\t\ttest \"$1\" != prepared\n+\tEOF\n+\ttest_must_fail git branch -m df-old df-old/child &&\n+\ttest_cmp_rev PRE refs/heads/df-old &&\n+\ttest_must_fail git rev-parse --verify refs/heads/df-old/child &&\n+\tgit reflog show --format=%gs df-old >df-log-after &&\n+\ttest_cmp df-log df-log-after\n+'\n+\n+test_expect_success 'hook can abort a reverse D/F rename after preparation' '\n+\tgit branch reverse/old PRE &&\n+\tgit reflog show --format=%gs reverse/old >reverse-log &&\n+\ttest_hook reference-transaction <<-\\EOF &&\n+\t\ttest \"$1\" != prepared\n+\tEOF\n+\ttest_must_fail git branch -m reverse/old reverse &&\n+\ttest_cmp_rev PRE refs/heads/reverse/old &&\n+\ttest_must_fail git rev-parse --verify refs/heads/reverse &&\n+\tgit reflog show --format=%gs reverse/old >reverse-log-after &&\n+\ttest_cmp reverse-log reverse-log-after\n+'\n+\n+test_expect_success 'hook can abort a forced branch copy after preparation' '\n+\tgit branch copy-abort-old PRE &&\n+\tgit branch copy-abort-new POST &&\n+\tgit reflog show --format=%gs copy-abort-old >copy-old-log &&\n+\tgit reflog show --format=%gs copy-abort-new >copy-new-log &&\n+\ttest_hook reference-transaction <<-\\EOF &&\n+\t\ttest \"$1\" != prepared\n+\tEOF\n+\ttest_must_fail git branch -C copy-abort-old copy-abort-new &&\n+\ttest_cmp_rev PRE refs/heads/copy-abort-old &&\n+\ttest_cmp_rev POST refs/heads/copy-abort-new &&\n+\tgit reflog show --format=%gs copy-abort-old >copy-old-log-after &&\n+\tgit reflog show --format=%gs copy-abort-new >copy-new-log-after &&\n+\ttest_cmp copy-old-log copy-old-log-after &&\n+\ttest_cmp copy-new-log copy-new-log-after\n+'\n+\n+test_expect_success 'branch rename detects an update during preparing hook' '\n+\tgit branch race-old PRE &&\n+\tgit branch race-new POST &&\n+\ttest_hook reference-transaction <<-\\EOF &&\n+\t\tmarker=$(git rev-parse --git-path rename-race-once)\n+\t\tif test \"$1\" = preparing && test ! -e \"$marker\"\n+\t\tthen\n+\t\t\t>\"$marker\" &&\n+\t\t\tgit update-ref refs/heads/race-old POST\n+\t\tfi\n+\tEOF\n+\ttest_must_fail git branch -M race-old race-new &&\n+\ttest_cmp_rev POST refs/heads/race-old &&\n+\ttest_cmp_rev POST refs/heads/race-new\n+'\n+\n test_expect_success 'hook gets all queued updates in aborted state' '\n \ttest_when_finished \"rm actual\" &&\n \tgit reset --hard PRE &&\n-- \n2.39.3 (Apple Git-146)\n"},{"id":"553667","messageId":"CACQ=SRG5ajG-+vHT8TPDif2Y0C9JFaXFuUesyPFHbDpJCq-76Q@mail.gmail.com","threadId":"66350","inReplyTo":"20260923133651.74120-1-maciej.ciemborowicz@gmail.com","subject":"Re: [PATCH v2] refs: run copy and rename through transactions","fromName":"Maciej Ciemborowicz","fromEmail":"maciej.ciemborowicz@gmail.com","sentAt":"2026-09-30T03:32:53Z","receivedAt":"2026-09-30T03:33:07Z","isPatch":true,"body":"I'd appreciate a code review of v2.\n\nThanks,\n- Maciej Ciemborowicz\n\nOn Wed, Sep 23, 2026 at 3:36 PM Maciej Ciemborowicz\n wrote:\n>\n> Reference copy and rename operations bypass the transaction API.\n> Consequently, the reference-transaction hook sees only the source deletion\n> with the files backend and no useful update with the reftable backend.\n>\n> Represent both operations as reference transactions containing their\n> logical updates. A rename is a deletion of the old reference and creation\n> of the new reference in the same transaction. Attach operation-specific\n> state to the destination update instead of making copy or rename a property\n> of the entire transaction.\n>\n> Retain backend-specific reflog handling: the files backend stages its\n> existing rename procedure across prepare, finish and abort, while reftable\n> stages an addition while holding the stack lock. Suppress hooks for the\n> files backend's nested deletion transactions so that callers observe one\n> logical transaction.\n>\n> Record and verify the source and destination values after taking backend\n> locks. This rejects concurrent changes instead of applying a rename or copy\n> that differs from the payload shown to the preparing hook. Preserve D/F\n> renames and restore overwritten references and reflogs when a prepared hook\n> rejects the operation.\n>\n> Add tests covering rename, copy, forced updates, both directions of D/F\n> conflicts, concurrent updates and prepared-hook rollback.\n>\n> Helped-by: Karthik Nayak \n> Signed-off-by: Maciej Ciemborowicz \n> ---\n> Apologies for the unrelated Subject header on my earlier reply. This\n> reroll incorporates the points discussed there.\n>\n> Changes since v1:\n>\n> * Keep copy/rename state on the destination ref update instead of the\n> generic transaction, so the operation is no longer a transaction-wide\n> property.\n> * Keep REF_TRANSACTION_FLAG_SKIP_HOOK private to the refs implementation.\n> * Use bool for the two-state copy parameter.\n> * Apply Junio's commit-message wording suggestions.\n>\n> Range-diff against v1:\n> 1: de0a5a9f7 ! 1: d852537d8 refs: run copy and rename through transactions\n> @@ Metadata\n> ## Commit message ##\n> refs: run copy and rename through transactions\n>\n> - Reference copy and rename operations currently bypass the transaction API.\n> + Reference copy and rename operations bypass the transaction API.\n> Consequently, the reference-transaction hook sees only the source deletion\n> with the files backend and no useful update with the reftable backend.\n>\n> Represent both operations as reference transactions containing their\n> logical updates. A rename is a deletion of the old reference and creation\n> - of the new reference in the same transaction. Retain backend-specific\n> - reflog handling: the files backend stages its existing rename procedure\n> - across prepare, finish and abort, while reftable stages an addition while\n> - holding the stack lock. Suppress hooks for the files backend's nested\n> - deletion transactions so that callers observe one logical transaction.\n> + of the new reference in the same transaction. Attach operation-specific\n> + state to the destination update instead of making copy or rename a property\n> + of the entire transaction.\n> +\n> + Retain backend-specific reflog handling: the files backend stages its\n> + existing rename procedure across prepare, finish and abort, while reftable\n> + stages an addition while holding the stack lock. Suppress hooks for the\n> + files backend's nested deletion transactions so that callers observe one\n> + logical transaction.\n>\n> Record and verify the source and destination values after taking backend\n> locks. This rejects concurrent changes instead of applying a rename or copy\n> @@ Commit message\n> renames and restore overwritten references and reflogs when a prepared hook\n> rejects the operation.\n>\n> - Add coverage for rename, copy, forced updates, both directions of D/F\n> + Add tests covering rename, copy, forced updates, both directions of D/F\n> conflicts, concurrent updates and prepared-hook rollback.\n>\n> Helped-by: Karthik Nayak \n> @@ refs.c: int refs_delete_ref(struct ref_store *refs, const char *msg,\n> {\n> char c;\n> @@ refs.c: void ref_transaction_free(struct ref_transaction *transaction)\n> + }\n> +\n> + for (i = 0; i < transaction->nr; i++) {\n> ++ struct ref_copy_or_rename_update *operation =\n> ++ transaction->updates[i]->copy_or_rename;\n> ++\n> + free(transaction->updates[i]->msg);\n> + free(transaction->updates[i]->committer_info);\n> + free((char *)transaction->updates[i]->new_target);\n> + free((char *)transaction->updates[i]->old_target);\n> + free((char *)transaction->updates[i]->rejection_details);\n> ++ if (operation) {\n> ++ free(operation->old_refname);\n> ++ free(operation->logmsg);\n> ++ free(operation->destination_target);\n> ++ free(operation);\n> ++ }\n> + free(transaction->updates[i]);\n> + }\n>\n> - string_list_clear(&transaction->refnames, 0);\n> - free(transaction->updates);\n> -+ free(transaction->old_refname);\n> -+ free(transaction->new_refname);\n> -+ free(transaction->logmsg);\n> -+ free(transaction->destination_target);\n> +@@ refs.c: void ref_transaction_free(struct ref_transaction *transaction)\n> free(transaction);\n> }\n>\n> ++struct ref_update *ref_transaction_copy_or_rename_update(\n> ++ struct ref_transaction *transaction)\n> ++{\n> ++ struct ref_update *operation = NULL;\n> ++ size_t i;\n> ++\n> ++ for (i = 0; i < transaction->nr; i++) {\n> ++ if (!transaction->updates[i]->copy_or_rename)\n> ++ continue;\n> ++ if (operation)\n> ++ BUG(\"multiple copy or rename updates in one transaction\");\n> ++ operation = transaction->updates[i];\n> ++ }\n> ++\n> ++ return operation;\n> ++}\n> ++\n> + int ref_transaction_maybe_set_rejected(struct ref_transaction *transaction,\n> + size_t update_idx,\n> + enum ref_transaction_error err,\n> @@ refs.c: int ref_transaction_prepare(struct ref_transaction *transaction,\n> return REF_TRANSACTION_ERROR_GENERIC;\n>\n> @@ refs.c: int refs_delete_refs(struct ref_store *refs, const char *logmsg,\n> - const char *newref, const char *logmsg)\n> +static int refs_copy_or_rename_ref(struct ref_store *refs, const char *oldref,\n> + const char *newref, const char *logmsg,\n> -+ int copy)\n> ++ bool copy)\n> {\n> - char *msg;\n> - int retval;\n> + struct ref_transaction *transaction = NULL;\n> ++ struct ref_copy_or_rename_update *operation = NULL;\n> ++ struct ref_update *destination_update;\n> + struct object_id old_oid, new_oid;\n> + struct strbuf new_target = STRBUF_INIT;\n> + struct strbuf err = STRBUF_INIT;\n> @@ refs.c: int refs_delete_refs(struct ref_store *refs, const char *logmsg,\n> + transaction = ref_store_transaction_begin(refs, 0, &err);\n> + if (!transaction)\n> + goto error;\n> -+ transaction->type = copy ? REF_TRANSACTION_TYPE_COPY :\n> -+ REF_TRANSACTION_TYPE_RENAME;\n> -+ transaction->old_refname = xstrdup(oldref);\n> -+ transaction->new_refname = xstrdup(newref);\n> -+ transaction->logmsg = xstrdup(msg);\n> -+ oidcpy(&transaction->source_oid, &old_oid);\n> -+\n> + if (!copy && ref_transaction_delete(transaction, oldref, &old_oid, NULL,\n> + REF_NO_DEREF, msg, &err))\n> + goto error;\n> @@ refs.c: int refs_delete_refs(struct ref_store *refs, const char *logmsg,\n> + } else {\n> + oidclr(&new_oid, refs->repo->hash_algo);\n> + }\n> -+ transaction->destination_exists = new_exists;\n> -+ if (new_flags & REF_ISSYMREF)\n> -+ transaction->destination_target = xstrdup(new_target.buf);\n> -+ else if (transaction->destination_exists)\n> -+ oidcpy(&transaction->destination_oid, &new_oid);\n> -+\n> + if (ref_transaction_update(transaction, newref, &old_oid,\n> + (new_flags & REF_ISSYMREF) ? NULL : &new_oid,\n> + NULL,\n> @@ refs.c: int refs_delete_refs(struct ref_store *refs, const char *logmsg,\n> + NULL, &err))\n> + goto error;\n> +\n> ++ destination_update = transaction->updates[transaction->nr - 1];\n> ++ CALLOC_ARRAY(operation, 1);\n> ++ operation->type = copy ? REF_UPDATE_COPY : REF_UPDATE_RENAME;\n> ++ operation->old_refname = xstrdup(oldref);\n> ++ operation->logmsg = xstrdup(msg);\n> ++ oidcpy(&operation->source_oid, &old_oid);\n> ++ operation->destination_exists = new_exists;\n> ++ if (new_flags & REF_ISSYMREF)\n> ++ operation->destination_target = xstrdup(new_target.buf);\n> ++ else if (operation->destination_exists)\n> ++ oidcpy(&operation->destination_oid, &new_oid);\n> ++ destination_update->copy_or_rename = operation;\n> ++\n> + if (ref_transaction_commit(transaction, &err))\n> + goto error;\n> +\n> @@ refs.c: int refs_delete_refs(struct ref_store *refs, const char *logmsg,\n>\n> const char *ref_update_original_update_refname(struct ref_update *update)\n>\n> - ## refs.h ##\n> -@@ refs.h: enum ref_transaction_flag {\n> - * while rejecting updates which do not match the expected state.\n> - */\n> - REF_TRANSACTION_ALLOW_FAILURE = (1 << 1),\n> -+\n> -+ /* Suppress hooks for an update nested in another transaction. */\n> -+ REF_TRANSACTION_FLAG_SKIP_HOOK = (1 << 2),\n> - };\n> -\n> - /*\n> -\n> ## refs/debug.c ##\n> @@ refs/debug.c: static int debug_optimize_required(struct ref_store *ref_store,\n> return res;\n> @@ refs/files-backend.c: static int refs_rename_ref_available(struct ref_store *ref\n> +};\n> +\n> static int files_copy_or_rename_ref(struct ref_store *ref_store,\n> - const char *oldrefname, const char *newrefname,\n> +- const char *oldrefname, const char *newrefname,\n> - const char *logmsg, int copy)\n> -+ const char *logmsg, int copy,\n> -+ struct ref_transaction *transaction)\n> ++ struct ref_update *update,\n> ++ struct ref_transaction *transaction)\n> {\n> struct files_ref_store *refs =\n> - files_downcast(ref_store, REF_STORE_WRITE, \"rename_ref\");\n> + files_downcast(ref_store, REF_STORE_WRITE,\n> + \"ref_transaction_prepare\");\n> ++ struct ref_copy_or_rename_update *operation = update->copy_or_rename;\n> ++ const char *oldrefname = operation->old_refname;\n> ++ const char *newrefname = update->refname;\n> ++ const char *logmsg = operation->logmsg;\n> ++ bool copy = operation->type == REF_UPDATE_COPY;\n> struct object_id orig_oid;\n> int flag = 0, logmoved = 0;\n> struct ref_lock *lock;\n> @@ refs/files-backend.c: static int files_copy_or_rename_ref(struct ref_store *ref_\n> oldrefname);\n> goto out;\n> }\n> -+ if (!oideq(&orig_oid, &transaction->source_oid)) {\n> ++ if (!oideq(&orig_oid, &operation->source_oid)) {\n> + ret = error(\"refname %s is at %s but expected %s\",\n> + oldrefname, oid_to_hex(&orig_oid),\n> -+ oid_to_hex(&transaction->source_oid));\n> ++ oid_to_hex(&operation->source_oid));\n> + goto out;\n> + }\n> if (!refs_rename_ref_available(&refs->base, oldrefname, newrefname)) {\n> @@ refs/files-backend.c: static int files_copy_or_rename_ref(struct ref_store *ref_\n> + goto out;\n> + }\n> + }\n> -+ if (destination_exists != transaction->destination_exists) {\n> ++ if (destination_exists != operation->destination_exists) {\n> + ret = error(\"refname %s changed while renaming\", newrefname);\n> + goto out;\n> + }\n> + if (destination_exists) {\n> + if (destination_flags & REF_ISSYMREF) {\n> -+ if (!transaction->destination_target ||\n> ++ if (!operation->destination_target ||\n> + strcmp(destination_target.buf,\n> -+ transaction->destination_target)) {\n> ++ operation->destination_target)) {\n> + ret = error(\"refname %s changed while renaming\",\n> + newrefname);\n> + goto out;\n> + }\n> -+ } else if (transaction->destination_target ||\n> ++ } else if (operation->destination_target ||\n> + !oideq(&destination_oid,\n> -+ &transaction->destination_oid)) {\n> ++ &operation->destination_oid)) {\n> + ret = error(\"refname %s changed while renaming\", newrefname);\n> + goto out;\n> + }\n> @@ refs/files-backend.c: static int files_transaction_prepare(struct ref_store *ref\n> struct ref_transaction *packed_transaction = NULL;\n>\n> assert(err);\n> -+ if (transaction->type != REF_TRANSACTION_TYPE_NORMAL)\n> -+ return files_copy_or_rename_ref(ref_store,\n> -+ transaction->old_refname,\n> -+ transaction->new_refname,\n> -+ transaction->logmsg,\n> -+ transaction->type == REF_TRANSACTION_TYPE_COPY,\n> -+ transaction);\n> ++ {\n> ++ struct ref_update *operation =\n> ++ ref_transaction_copy_or_rename_update(transaction);\n> ++\n> ++ if (operation)\n> ++ return files_copy_or_rename_ref(ref_store, operation,\n> ++ transaction);\n> ++ }\n>\n> if (transaction->flags & REF_TRANSACTION_FLAG_INITIAL)\n> goto cleanup;\n> @@ refs/files-backend.c: static int files_transaction_finish(struct ref_store *ref_\n>\n>\n> assert(err);\n> -+ if (transaction->type != REF_TRANSACTION_TYPE_NORMAL) {\n> -+ struct files_copy_or_rename_transaction_data *data =\n> -+ transaction->backend_data;\n> -+ int special_ret;\n> ++ {\n> ++ struct ref_update *update =\n> ++ ref_transaction_copy_or_rename_update(transaction);\n> +\n> -+ special_ret = commit_ref_update(refs, data->lock, &data->orig_oid,\n> -+ transaction->logmsg, 0, err);\n> -+ if (special_ret) {\n> -+ error(\"unable to write current sha1 into %s: %s\",\n> -+ transaction->new_refname, err->buf);\n> -+ data->lock = NULL;\n> -+ files_transaction_abort(ref_store, transaction, err);\n> -+ return special_ret;\n> -+ } else if (data->destination_log_backed_up) {\n> -+ struct strbuf path = STRBUF_INIT;\n> ++ if (update) {\n> ++ struct ref_copy_or_rename_update *operation =\n> ++ update->copy_or_rename;\n> ++ struct files_copy_or_rename_transaction_data *data =\n> ++ transaction->backend_data;\n> ++ int special_ret;\n> ++\n> ++ special_ret = commit_ref_update(refs, data->lock, &data->orig_oid,\n> ++ operation->logmsg, 0, err);\n> ++ if (special_ret) {\n> ++ error(\"unable to write current sha1 into %s: %s\",\n> ++ update->refname, err->buf);\n> ++ data->lock = NULL;\n> ++ files_transaction_abort(ref_store, transaction, err);\n> ++ return special_ret;\n> ++ } else if (data->destination_log_backed_up) {\n> ++ struct strbuf path = STRBUF_INIT;\n> +\n> -+ files_reflog_path(refs, &path, TMP_RENAMED_LOG_DESTINATION);\n> -+ if (unlink(path.buf) < 0 && errno != ENOENT)\n> -+ warning_errno(\"unable to remove '%s'\", path.buf);\n> -+ strbuf_release(&path);\n> ++ files_reflog_path(refs, &path, TMP_RENAMED_LOG_DESTINATION);\n> ++ if (unlink(path.buf) < 0 && errno != ENOENT)\n> ++ warning_errno(\"unable to remove '%s'\", path.buf);\n> ++ strbuf_release(&path);\n> ++ }\n> ++ free(data->destination_target);\n> ++ free(data);\n> ++ transaction->backend_data = NULL;\n> ++ transaction->state = REF_TRANSACTION_CLOSED;\n> ++ return special_ret;\n> + }\n> -+ free(data->destination_target);\n> -+ free(data);\n> -+ transaction->backend_data = NULL;\n> -+ transaction->state = REF_TRANSACTION_CLOSED;\n> -+ return special_ret;\n> + }\n>\n> if (transaction->flags & REF_TRANSACTION_FLAG_INITIAL)\n> @@ refs/files-backend.c: static int files_transaction_finish(struct ref_store *ref_\n> struct files_ref_store *refs =\n> files_downcast(ref_store, 0, \"ref_transaction_abort\");\n>\n> -+ if (transaction->type != REF_TRANSACTION_TYPE_NORMAL) {\n> -+ struct files_copy_or_rename_transaction_data *data =\n> -+ transaction->backend_data;\n> -+ struct strbuf new_log = STRBUF_INIT;\n> -+ struct strbuf destination_log = STRBUF_INIT;\n> -+ struct strbuf temporary_log = STRBUF_INIT;\n> -+ struct ref_transaction *restore_transaction = NULL;\n> -+ struct ref_lock *lock;\n> -+ int ret = 0;\n> ++ {\n> ++ struct ref_update *update =\n> ++ ref_transaction_copy_or_rename_update(transaction);\n> +\n> -+ if (data->lock)\n> -+ unlock_ref(data->lock);\n> -+ if (transaction->type == REF_TRANSACTION_TYPE_RENAME) {\n> -+ lock = lock_ref_oid_basic(refs, transaction->old_refname, err);\n> -+ if (!lock ||\n> -+ write_ref_to_lockfile(refs, lock, &data->orig_oid, err) ||\n> -+ commit_ref_update(refs, lock, &data->orig_oid, NULL,\n> -+ REF_SKIP_CREATE_REFLOG, err))\n> -+ ret = -1;\n> -+ }\n> ++ if (update) {\n> ++ struct ref_copy_or_rename_update *operation =\n> ++ update->copy_or_rename;\n> ++ struct files_copy_or_rename_transaction_data *data =\n> ++ transaction->backend_data;\n> ++ struct strbuf new_log = STRBUF_INIT;\n> ++ struct strbuf destination_log = STRBUF_INIT;\n> ++ struct strbuf temporary_log = STRBUF_INIT;\n> ++ struct ref_transaction *restore_transaction = NULL;\n> ++ struct ref_lock *lock;\n> ++ int ret = 0;\n> +\n> -+ if (data->logmoved) {\n> -+ files_reflog_path(refs, &new_log, transaction->new_refname);\n> -+ if (transaction->type == REF_TRANSACTION_TYPE_RENAME) {\n> -+ files_reflog_path(refs, &temporary_log, TMP_RENAMED_LOG);\n> -+ if (rename(new_log.buf, temporary_log.buf) < 0) {\n> -+ strbuf_addf(err, \"unable to restore logfile %s: %s\",\n> -+ transaction->old_refname, strerror(errno));\n> ++ if (data->lock)\n> ++ unlock_ref(data->lock);\n> ++ if (operation->type == REF_UPDATE_RENAME) {\n> ++ lock = lock_ref_oid_basic(refs, operation->old_refname, err);\n> ++ if (!lock ||\n> ++ write_ref_to_lockfile(refs, lock, &data->orig_oid, err) ||\n> ++ commit_ref_update(refs, lock, &data->orig_oid, NULL,\n> ++ REF_SKIP_CREATE_REFLOG, err))\n> + ret = -1;\n> -+ } else {\n> -+ try_remove_empty_parents(refs,\n> -+ transaction->new_refname,\n> -+ REMOVE_EMPTY_PARENTS_REFLOG);\n> -+ if (rename_tmp_log(refs,\n> -+ transaction->old_refname)) {\n> ++ }\n> ++\n> ++ if (data->logmoved) {\n> ++ files_reflog_path(refs, &new_log, update->refname);\n> ++ if (operation->type == REF_UPDATE_RENAME) {\n> ++ files_reflog_path(refs, &temporary_log, TMP_RENAMED_LOG);\n> ++ if (rename(new_log.buf, temporary_log.buf) < 0) {\n> + strbuf_addf(err, \"unable to restore logfile %s: %s\",\n> -+ transaction->old_refname,\n> -+ strerror(errno));\n> ++ operation->old_refname, strerror(errno));\n> + ret = -1;\n> ++ } else {\n> ++ try_remove_empty_parents(refs,\n> ++ update->refname,\n> ++ REMOVE_EMPTY_PARENTS_REFLOG);\n> ++ if (rename_tmp_log(refs,\n> ++ operation->old_refname)) {\n> ++ strbuf_addf(err, \"unable to restore logfile %s: %s\",\n> ++ operation->old_refname,\n> ++ strerror(errno));\n> ++ ret = -1;\n> ++ }\n> + }\n> ++ } else if (unlink(new_log.buf) < 0 && errno != ENOENT) {\n> ++ strbuf_addf(err, \"unable to remove logfile %s: %s\",\n> ++ update->refname, strerror(errno));\n> ++ ret = -1;\n> + }\n> -+ } else if (unlink(new_log.buf) < 0 && errno != ENOENT) {\n> -+ strbuf_addf(err, \"unable to remove logfile %s: %s\",\n> -+ transaction->new_refname, strerror(errno));\n> -+ ret = -1;\n> + }\n> -+ }\n> -+ if (data->destination_log_backed_up) {\n> -+ files_reflog_path(refs, &destination_log,\n> -+ TMP_RENAMED_LOG_DESTINATION);\n> -+ if (rename(destination_log.buf, new_log.buf) < 0) {\n> -+ strbuf_addf(err, \"unable to restore logfile %s: %s\",\n> -+ transaction->new_refname, strerror(errno));\n> -+ ret = -1;\n> ++ if (data->destination_log_backed_up) {\n> ++ files_reflog_path(refs, &destination_log,\n> ++ TMP_RENAMED_LOG_DESTINATION);\n> ++ if (rename(destination_log.buf, new_log.buf) < 0) {\n> ++ strbuf_addf(err, \"unable to restore logfile %s: %s\",\n> ++ update->refname, strerror(errno));\n> ++ ret = -1;\n> ++ }\n> + }\n> -+ }\n> +\n> -+ if (transaction->type == REF_TRANSACTION_TYPE_RENAME &&\n> -+ data->destination_exists) {\n> -+ restore_transaction = ref_store_transaction_begin(\n> ++ if (operation->type == REF_UPDATE_RENAME &&\n> ++ data->destination_exists) {\n> ++ restore_transaction = ref_store_transaction_begin(\n> + &refs->base, REF_TRANSACTION_FLAG_SKIP_HOOK, err);\n> -+ if (!restore_transaction ||\n> -+ ref_transaction_update(restore_transaction,\n> -+ transaction->new_refname,\n> -+ data->destination_target ? NULL :\n> -+ &data->destination_oid,\n> -+ NULL,\n> -+ data->destination_target,\n> -+ NULL,\n> -+ REF_NO_DEREF |\n> -+ REF_SKIP_CREATE_REFLOG,\n> -+ NULL, err) ||\n> -+ ref_transaction_commit(restore_transaction, err))\n> -+ ret = -1;\n> -+ ref_transaction_free(restore_transaction);\n> -+ }\n> ++ if (!restore_transaction ||\n> ++ ref_transaction_update(restore_transaction,\n> ++ update->refname,\n> ++ data->destination_target ? NULL :\n> ++ &data->destination_oid,\n> ++ NULL,\n> ++ data->destination_target,\n> ++ NULL,\n> ++ REF_NO_DEREF |\n> ++ REF_SKIP_CREATE_REFLOG,\n> ++ NULL, err) ||\n> ++ ref_transaction_commit(restore_transaction, err))\n> ++ ret = -1;\n> ++ ref_transaction_free(restore_transaction);\n> ++ }\n> +\n> -+ strbuf_release(&destination_log);\n> -+ strbuf_release(&temporary_log);\n> -+ strbuf_release(&new_log);\n> -+ free(data->destination_target);\n> -+ free(data);\n> -+ transaction->backend_data = NULL;\n> -+ transaction->state = REF_TRANSACTION_CLOSED;\n> -+ return ret;\n> ++ strbuf_release(&destination_log);\n> ++ strbuf_release(&temporary_log);\n> ++ strbuf_release(&new_log);\n> ++ free(data->destination_target);\n> ++ free(data);\n> ++ transaction->backend_data = NULL;\n> ++ transaction->state = REF_TRANSACTION_CLOSED;\n> ++ return ret;\n> ++ }\n> + }\n> +\n> files_transaction_cleanup(refs, transaction);\n> @@ refs/packed-backend.c: struct ref_storage_be refs_be_packed = {\n> .read_raw_ref = packed_read_raw_ref,\n>\n> ## refs/refs-internal.h ##\n> +@@ refs/refs-internal.h: struct ref_update {\n> + */\n> + struct ref_update *parent_update;\n> +\n> ++ /*\n> ++ * Copy and rename operations require backend-specific handling while\n> ++ * still exposing their logical updates to transaction hooks. Keep that\n> ++ * state on the destination update so it composes with other updates in\n> ++ * the transaction instead of making copy or rename a transaction-wide\n> ++ * property.\n> ++ */\n> ++ struct ref_copy_or_rename_update *copy_or_rename;\n> ++\n> + const char refname[FLEX_ARRAY];\n> + };\n> +\n> ++enum ref_copy_or_rename_type {\n> ++ REF_UPDATE_RENAME,\n> ++ REF_UPDATE_COPY,\n> ++};\n> ++\n> ++struct ref_copy_or_rename_update {\n> ++ enum ref_copy_or_rename_type type;\n> ++ char *old_refname;\n> ++ char *logmsg;\n> ++ struct object_id source_oid;\n> ++ struct object_id destination_oid;\n> ++ char *destination_target;\n> ++ unsigned int destination_exists:1;\n> ++};\n> ++\n> + int refs_read_raw_ref(struct ref_store *ref_store, const char *refname,\n> + struct object_id *oid, struct strbuf *referent,\n> + unsigned int *type, int *failure_errno);\n> @@ refs/refs-internal.h: struct ref_update *ref_transaction_add_update(\n> const char *committer_info,\n> const char *msg);\n> @@ refs/refs-internal.h: struct ref_update *ref_transaction_add_update(\n> /*\n> * Transaction states.\n> *\n> -@@ refs/refs-internal.h: enum ref_transaction_state {\n> - REF_TRANSACTION_CLOSED = 2\n> - };\n> -\n> -+enum ref_transaction_type {\n> -+ REF_TRANSACTION_TYPE_NORMAL = 0,\n> -+ REF_TRANSACTION_TYPE_RENAME,\n> -+ REF_TRANSACTION_TYPE_COPY,\n> -+};\n> -+\n> - /*\n> - * Data structure to hold indices of updates which were rejected, for batched\n> - * reference updates. While the updates themselves hold the rejection error,\n> @@ refs/refs-internal.h: struct ref_transaction {\n> - void *backend_data;\n> - unsigned int flags;\n> uint64_t max_index;\n> -+\n> -+ /*\n> -+ * Rename and copy operations need backend-specific reflog handling.\n> -+ * Their logical updates still live in `updates`, so hooks see the\n> -+ * operation like any other reference transaction. The fields below\n> -+ * retain the state that backends verify after taking their locks.\n> -+ */\n> -+ enum ref_transaction_type type;\n> -+ char *old_refname;\n> -+ char *new_refname;\n> -+ char *logmsg;\n> -+ struct object_id source_oid;\n> -+ struct object_id destination_oid;\n> -+ char *destination_target;\n> -+ unsigned int destination_exists:1;\n> };\n>\n> ++/* Suppress hooks for a transaction nested inside another refs operation. */\n> ++#define REF_TRANSACTION_FLAG_SKIP_HOOK (1 << 2)\n> ++\n> ++struct ref_update *ref_transaction_copy_or_rename_update(\n> ++ struct ref_transaction *transaction);\n> ++\n> /*\n> + * Check for entries in extras that are within the specified\n> + * directory, where dirname is a reference directory name including\n> @@ refs/refs-internal.h: typedef int optimize_required_fn(struct ref_store *ref_store,\n> struct refs_optimize_opts *opts,\n> bool *required);\n> @@ refs/reftable-backend.c: static int reftable_be_transaction_prepare(struct ref_s\n> size_t i;\n> int ret;\n>\n> -+ if (transaction->type != REF_TRANSACTION_TYPE_NORMAL)\n> ++ if (ref_transaction_copy_or_rename_update(transaction))\n> + return reftable_be_copy_or_rename_prepare(ref_store, transaction,\n> + err);\n> +\n> @@ refs/reftable-backend.c: static int reftable_be_transaction_abort(struct ref_sto\n> - struct reftable_transaction_data *tx_data = transaction->backend_data;\n> + struct reftable_transaction_data *tx_data;\n> +\n> -+ if (transaction->type != REF_TRANSACTION_TYPE_NORMAL) {\n> ++ if (ref_transaction_copy_or_rename_update(transaction)) {\n> + struct reftable_copy_or_rename_transaction_data *data =\n> + transaction->backend_data;\n> +\n> @@ refs/reftable-backend.c: static int reftable_be_transaction_finish(struct ref_st\n> + struct reftable_transaction_data *tx_data;\n> int ret = 0;\n>\n> -+ if (transaction->type != REF_TRANSACTION_TYPE_NORMAL) {\n> ++ if (ref_transaction_copy_or_rename_update(transaction)) {\n> + struct reftable_copy_or_rename_transaction_data *data =\n> + transaction->backend_data;\n> + int special_ret = reftable_addition_commit(data->addition);\n> @@ refs/reftable-backend.c: struct write_create_symref_arg {\n> const char *newname;\n> const char *logmsg;\n> int delete_old;\n> -+ struct ref_transaction *transaction;\n> ++ struct ref_copy_or_rename_update *operation;\n> };\n>\n> static int write_copy_table(struct reftable_writer *writer, void *cb_data)\n> @@ refs/reftable-backend.c: static int write_copy_table(struct reftable_writer *wri\n> + ret = -1;\n> goto done;\n> }\n> -+ if (arg->transaction) {\n> ++ if (arg->operation) {\n> + struct object_id oid;\n> +\n> + if (old_ref.value_type == REFTABLE_REF_VAL2)\n> @@ refs/reftable-backend.c: static int write_copy_table(struct reftable_writer *wri\n> + else\n> + oidread(&oid, old_ref.value.val1,\n> + arg->refs->base.repo->hash_algo);\n> -+ if (!oideq(&oid, &arg->transaction->source_oid)) {\n> ++ if (!oideq(&oid, &arg->operation->source_oid)) {\n> + strbuf_addf(arg->err,\n> + _(\"refname %s is at %s but expected %s\"),\n> + arg->oldname, oid_to_hex(&oid),\n> -+ oid_to_hex(&arg->transaction->source_oid));\n> ++ oid_to_hex(&arg->operation->source_oid));\n> + ret = -1;\n> + goto done;\n> + }\n> @@ refs/reftable-backend.c: static int write_copy_table(struct reftable_writer *wri\n> + &destination_ref);\n> + if (ret < 0)\n> + goto done;\n> -+ if (arg->transaction->destination_exists != !ret) {\n> ++ if (arg->operation->destination_exists != !ret) {\n> + strbuf_addf(arg->err,\n> + _(\"refname %s changed while renaming\"),\n> + arg->newname);\n> @@ refs/reftable-backend.c: static int write_copy_table(struct reftable_writer *wri\n> + }\n> + if (!ret) {\n> + if (destination_ref.value_type == REFTABLE_REF_SYMREF) {\n> -+ if (!arg->transaction->destination_target ||\n> ++ if (!arg->operation->destination_target ||\n> + strcmp(destination_ref.value.symref,\n> -+ arg->transaction->destination_target)) {\n> ++ arg->operation->destination_target)) {\n> + strbuf_addf(arg->err,\n> + _(\"refname %s changed while renaming\"),\n> + arg->newname);\n> @@ refs/reftable-backend.c: static int write_copy_table(struct reftable_writer *wri\n> + else\n> + oidread(&oid, destination_ref.value.val1,\n> + arg->refs->base.repo->hash_algo);\n> -+ if (arg->transaction->destination_target ||\n> -+ !oideq(&oid, &arg->transaction->destination_oid)) {\n> ++ if (arg->operation->destination_target ||\n> ++ !oideq(&oid, &arg->operation->destination_oid)) {\n> + strbuf_addf(arg->err,\n> + _(\"refname %s changed while renaming\"),\n> + arg->newname);\n> @@ refs/reftable-backend.c: static int write_copy_table(struct reftable_writer *wri\n> + reftable_be_downcast(ref_store, REF_STORE_WRITE,\n> + \"ref_transaction_prepare\");\n> + struct reftable_copy_or_rename_transaction_data *data = NULL;\n> ++ struct ref_update *update =\n> ++ ref_transaction_copy_or_rename_update(transaction);\n> ++ struct ref_copy_or_rename_update *operation = update->copy_or_rename;\n> struct write_copy_arg arg = {\n> .refs = refs,\n> - .oldname = oldrefname,\n> @@ refs/reftable-backend.c: static int write_copy_table(struct reftable_writer *wri\n> - .logmsg = logmsg,\n> - .delete_old = 1,\n> + .err = err,\n> -+ .oldname = transaction->old_refname,\n> -+ .newname = transaction->new_refname,\n> -+ .logmsg = transaction->logmsg,\n> -+ .delete_old = transaction->type == REF_TRANSACTION_TYPE_RENAME,\n> -+ .transaction = transaction,\n> ++ .oldname = operation->old_refname,\n> ++ .newname = update->refname,\n> ++ .logmsg = operation->logmsg,\n> ++ .delete_old = operation->type == REF_UPDATE_RENAME,\n> ++ .operation = operation,\n> };\n> int ret;\n>\n> @@ refs/reftable-backend.c: static int write_copy_table(struct reftable_writer *wri\n> goto done;\n> -\n> - ret = backend_for(&arg.be, refs, newrefname, &newrefname, 1);\n> -+ ret = backend_for(&arg.be, refs, transaction->new_refname,\n> ++ ret = backend_for(&arg.be, refs, update->refname,\n> + &arg.newname, 1);\n> if (ret)\n> goto done;\n>\n> refs.c | 161 ++++++++++++++---\n> refs/debug.c | 25 ---\n> refs/files-backend.c | 297 +++++++++++++++++++++++++++----\n> refs/packed-backend.c | 2 -\n> refs/refs-internal.h | 47 +++--\n> refs/reftable-backend.c | 197 +++++++++++++++-----\n> t/t1416-ref-transaction-hooks.sh | 142 +++++++++++++++\n> 7 files changed, 732 insertions(+), 139 deletions(-)\n>\n> diff --git a/refs.c b/refs.c\n> index 92d5df5b7..f036ae4b9 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -1004,15 +1004,17 @@ long get_files_ref_lock_timeout_ms(struct repository *repo)\n> return timeout_ms;\n> }\n>\n> -int refs_delete_ref(struct ref_store *refs, const char *msg,\n> - const char *refname,\n> - const struct object_id *old_oid,\n> - unsigned int flags)\n> +int refs_delete_ref_with_transaction_flags(struct ref_store *refs,\n> + const char *msg,\n> + const char *refname,\n> + const struct object_id *old_oid,\n> + unsigned int flags,\n> + unsigned int transaction_flags)\n> {\n> struct ref_transaction *transaction;\n> struct strbuf err = STRBUF_INIT;\n>\n> - transaction = ref_store_transaction_begin(refs, 0, &err);\n> + transaction = ref_store_transaction_begin(refs, transaction_flags, &err);\n> if (!transaction ||\n> ref_transaction_delete(transaction, refname, old_oid,\n> NULL, flags, msg, &err) ||\n> @@ -1027,6 +1029,15 @@ int refs_delete_ref(struct ref_store *refs, const char *msg,\n> return 0;\n> }\n>\n> +int refs_delete_ref(struct ref_store *refs, const char *msg,\n> + const char *refname,\n> + const struct object_id *old_oid,\n> + unsigned int flags)\n> +{\n> + return refs_delete_ref_with_transaction_flags(refs, msg, refname,\n> + old_oid, flags, 0);\n> +}\n> +\n> static void copy_reflog_msg(struct strbuf *sb, const char *msg)\n> {\n> char c;\n> @@ -1256,11 +1267,20 @@ void ref_transaction_free(struct ref_transaction *transaction)\n> }\n>\n> for (i = 0; i < transaction->nr; i++) {\n> + struct ref_copy_or_rename_update *operation =\n> + transaction->updates[i]->copy_or_rename;\n> +\n> free(transaction->updates[i]->msg);\n> free(transaction->updates[i]->committer_info);\n> free((char *)transaction->updates[i]->new_target);\n> free((char *)transaction->updates[i]->old_target);\n> free((char *)transaction->updates[i]->rejection_details);\n> + if (operation) {\n> + free(operation->old_refname);\n> + free(operation->logmsg);\n> + free(operation->destination_target);\n> + free(operation);\n> + }\n> free(transaction->updates[i]);\n> }\n>\n> @@ -1273,6 +1293,23 @@ void ref_transaction_free(struct ref_transaction *transaction)\n> free(transaction);\n> }\n>\n> +struct ref_update *ref_transaction_copy_or_rename_update(\n> + struct ref_transaction *transaction)\n> +{\n> + struct ref_update *operation = NULL;\n> + size_t i;\n> +\n> + for (i = 0; i < transaction->nr; i++) {\n> + if (!transaction->updates[i]->copy_or_rename)\n> + continue;\n> + if (operation)\n> + BUG(\"multiple copy or rename updates in one transaction\");\n> + operation = transaction->updates[i];\n> + }\n> +\n> + return operation;\n> +}\n> +\n> int ref_transaction_maybe_set_rejected(struct ref_transaction *transaction,\n> size_t update_idx,\n> enum ref_transaction_error err,\n> @@ -2710,7 +2747,8 @@ int ref_transaction_prepare(struct ref_transaction *transaction,\n> return REF_TRANSACTION_ERROR_GENERIC;\n>\n> /* Preparing checks before locking references */\n> - ret = run_transaction_hook(transaction, \"preparing\");\n> + ret = transaction->flags & REF_TRANSACTION_FLAG_SKIP_HOOK ? 0 :\n> + run_transaction_hook(transaction, \"preparing\");\n> if (ret) {\n> ref_transaction_abort(transaction, err);\n> die(_(abort_by_ref_transaction_hook), \"preparing\");\n> @@ -2720,7 +2758,8 @@ int ref_transaction_prepare(struct ref_transaction *transaction,\n> if (ret)\n> return ret;\n>\n> - ret = run_transaction_hook(transaction, \"prepared\");\n> + ret = transaction->flags & REF_TRANSACTION_FLAG_SKIP_HOOK ? 0 :\n> + run_transaction_hook(transaction, \"prepared\");\n> if (ret) {\n> ref_transaction_abort(transaction, err);\n> die(_(abort_by_ref_transaction_hook), \"prepared\");\n> @@ -2750,7 +2789,8 @@ int ref_transaction_abort(struct ref_transaction *transaction,\n> break;\n> }\n>\n> - run_transaction_hook(transaction, \"aborted\");\n> + if (!(transaction->flags & REF_TRANSACTION_FLAG_SKIP_HOOK))\n> + run_transaction_hook(transaction, \"aborted\");\n>\n> ref_transaction_free(transaction);\n> return ret;\n> @@ -2781,7 +2821,8 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n> }\n>\n> ret = refs->be->transaction_finish(refs, transaction, err);\n> - if (!ret && !(transaction->flags & REF_TRANSACTION_FLAG_INITIAL))\n> + if (!ret && !(transaction->flags & (REF_TRANSACTION_FLAG_INITIAL |\n> + REF_TRANSACTION_FLAG_SKIP_HOOK)))\n> run_transaction_hook(transaction, \"committed\");\n> return ret;\n> }\n> @@ -3123,28 +3164,102 @@ int refs_delete_refs(struct ref_store *refs, const char *logmsg,\n> return ret;\n> }\n>\n> -int refs_rename_ref(struct ref_store *refs, const char *oldref,\n> - const char *newref, const char *logmsg)\n> +static int refs_copy_or_rename_ref(struct ref_store *refs, const char *oldref,\n> + const char *newref, const char *logmsg,\n> + bool copy)\n> {\n> - char *msg;\n> - int retval;\n> + struct ref_transaction *transaction = NULL;\n> + struct ref_copy_or_rename_update *operation = NULL;\n> + struct ref_update *destination_update;\n> + struct object_id old_oid, new_oid;\n> + struct strbuf new_target = STRBUF_INIT;\n> + struct strbuf err = STRBUF_INIT;\n> + char *msg = normalize_reflog_message(logmsg);\n> + int old_flags, new_flags = 0, new_exists = 0, ret = 1;\n>\n> - msg = normalize_reflog_message(logmsg);\n> - retval = refs->be->rename_ref(refs, oldref, newref, msg);\n> + if (!strcmp(oldref, newref)) {\n> + ret = 0;\n> + goto out;\n> + }\n> +\n> + if (!refs_resolve_ref_unsafe(refs, oldref,\n> + RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n> + &old_oid, &old_flags)) {\n> + error(\"refname %s not found\", oldref);\n> + goto out;\n> + }\n> + if (old_flags & REF_ISSYMREF) {\n> + error(\"refname %s is a symbolic ref, %s it is not supported\",\n> + oldref, copy ? \"copying\" : \"renaming\");\n> + goto out;\n> + }\n> +\n> + transaction = ref_store_transaction_begin(refs, 0, &err);\n> + if (!transaction)\n> + goto error;\n> + if (!copy && ref_transaction_delete(transaction, oldref, &old_oid, NULL,\n> + REF_NO_DEREF, msg, &err))\n> + goto error;\n> +\n> + if (refs_resolve_ref_unsafe(refs, newref,\n> + RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n> + &new_oid, &new_flags)) {\n> + new_exists = 1;\n> + if ((new_flags & REF_ISSYMREF) &&\n> + refs_read_symbolic_ref(refs, newref, &new_target) < 0) {\n> + strbuf_addf(&err, \"unable to read symbolic ref %s\", newref);\n> + goto error;\n> + }\n> + } else {\n> + oidclr(&new_oid, refs->repo->hash_algo);\n> + }\n> + if (ref_transaction_update(transaction, newref, &old_oid,\n> + (new_flags & REF_ISSYMREF) ? NULL : &new_oid,\n> + NULL,\n> + (new_flags & REF_ISSYMREF) ? new_target.buf : NULL,\n> + REF_NO_DEREF | REF_SKIP_CREATE_REFLOG,\n> + NULL, &err))\n> + goto error;\n> +\n> + destination_update = transaction->updates[transaction->nr - 1];\n> + CALLOC_ARRAY(operation, 1);\n> + operation->type = copy ? REF_UPDATE_COPY : REF_UPDATE_RENAME;\n> + operation->old_refname = xstrdup(oldref);\n> + operation->logmsg = xstrdup(msg);\n> + oidcpy(&operation->source_oid, &old_oid);\n> + operation->destination_exists = new_exists;\n> + if (new_flags & REF_ISSYMREF)\n> + operation->destination_target = xstrdup(new_target.buf);\n> + else if (operation->destination_exists)\n> + oidcpy(&operation->destination_oid, &new_oid);\n> + destination_update->copy_or_rename = operation;\n> +\n> + if (ref_transaction_commit(transaction, &err))\n> + goto error;\n> +\n> + ret = 0;\n> + goto out;\n> +\n> +error:\n> + error(\"%s\", err.buf);\n> +out:\n> + ref_transaction_free(transaction);\n> + strbuf_release(&new_target);\n> + strbuf_release(&err);\n> free(msg);\n> - return retval;\n> + return ret;\n> }\n>\n> -int refs_copy_existing_ref(struct ref_store *refs, const char *oldref,\n> +int refs_rename_ref(struct ref_store *refs, const char *oldref,\n> const char *newref, const char *logmsg)\n> {\n> - char *msg;\n> - int retval;\n> + return refs_copy_or_rename_ref(refs, oldref, newref, logmsg, 0);\n> +}\n>\n> - msg = normalize_reflog_message(logmsg);\n> - retval = refs->be->copy_ref(refs, oldref, newref, msg);\n> - free(msg);\n> - return retval;\n> +int refs_copy_existing_ref(struct ref_store *refs, const char *oldref,\n> + const char *newref, const char *logmsg)\n> +{\n> + return refs_copy_or_rename_ref(refs, oldref, newref, logmsg, 1);\n> }\n>\n> const char *ref_update_original_update_refname(struct ref_update *update)\n> diff --git a/refs/debug.c b/refs/debug.c\n> index 639db0f26..87b84e767 100644\n> --- a/refs/debug.c\n> +++ b/refs/debug.c\n> @@ -143,28 +143,6 @@ static int debug_optimize_required(struct ref_store *ref_store,\n> return res;\n> }\n>\n> -static int debug_rename_ref(struct ref_store *ref_store, const char *oldref,\n> - const char *newref, const char *logmsg)\n> -{\n> - struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;\n> - int res = drefs->refs->be->rename_ref(drefs->refs, oldref, newref,\n> - logmsg);\n> - trace_printf_key(&trace_refs, \"rename_ref: %s -> %s \\\"%s\\\": %d\\n\", oldref, newref,\n> - logmsg, res);\n> - return res;\n> -}\n> -\n> -static int debug_copy_ref(struct ref_store *ref_store, const char *oldref,\n> - const char *newref, const char *logmsg)\n> -{\n> - struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;\n> - int res =\n> - drefs->refs->be->copy_ref(drefs->refs, oldref, newref, logmsg);\n> - trace_printf_key(&trace_refs, \"copy_ref: %s -> %s \\\"%s\\\": %d\\n\", oldref, newref,\n> - logmsg, res);\n> - return res;\n> -}\n> -\n> struct debug_ref_iterator {\n> struct ref_iterator base;\n> struct ref_iterator *iter;\n> @@ -453,9 +431,6 @@ struct ref_storage_be refs_be_debug = {\n> .optimize = debug_optimize,\n> .optimize_required = debug_optimize_required,\n>\n> - .rename_ref = debug_rename_ref,\n> - .copy_ref = debug_copy_ref,\n> -\n> .iterator_begin = debug_ref_iterator_begin,\n> .read_raw_ref = debug_read_raw_ref,\n> .read_symbolic_ref = debug_read_symbolic_ref,\n> diff --git a/refs/files-backend.c b/refs/files-backend.c\n> index 71628550f..c28228116 100644\n> --- a/refs/files-backend.c\n> +++ b/refs/files-backend.c\n> @@ -1594,6 +1594,7 @@ static int files_optimize_required(struct ref_store *ref_store,\n> * live into logs/refs.\n> */\n> #define TMP_RENAMED_LOG \"refs/.tmp-renamed-log\"\n> +#define TMP_RENAMED_LOG_DESTINATION \"refs/.tmp-renamed-log-destination\"\n>\n> struct rename_cb {\n> const char *tmp_renamed_log;\n> @@ -1685,12 +1686,28 @@ static int refs_rename_ref_available(struct ref_store *refs,\n> return ok;\n> }\n>\n> +struct files_copy_or_rename_transaction_data {\n> + struct ref_lock *lock;\n> + struct object_id orig_oid;\n> + struct object_id destination_oid;\n> + char *destination_target;\n> + int logmoved;\n> + int destination_exists;\n> + int destination_log_backed_up;\n> +};\n> +\n> static int files_copy_or_rename_ref(struct ref_store *ref_store,\n> - const char *oldrefname, const char *newrefname,\n> - const char *logmsg, int copy)\n> + struct ref_update *update,\n> + struct ref_transaction *transaction)\n> {\n> struct files_ref_store *refs =\n> - files_downcast(ref_store, REF_STORE_WRITE, \"rename_ref\");\n> + files_downcast(ref_store, REF_STORE_WRITE,\n> + \"ref_transaction_prepare\");\n> + struct ref_copy_or_rename_update *operation = update->copy_or_rename;\n> + const char *oldrefname = operation->old_refname;\n> + const char *newrefname = update->refname;\n> + const char *logmsg = operation->logmsg;\n> + bool copy = operation->type == REF_UPDATE_COPY;\n> struct object_id orig_oid;\n> int flag = 0, logmoved = 0;\n> struct ref_lock *lock;\n> @@ -1698,12 +1715,19 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,\n> struct strbuf sb_oldref = STRBUF_INIT;\n> struct strbuf sb_newref = STRBUF_INIT;\n> struct strbuf tmp_renamed_log = STRBUF_INIT;\n> + struct strbuf tmp_destination_log = STRBUF_INIT;\n> + struct strbuf destination_target = STRBUF_INIT;\n> int log, ret;\n> + int destination_exists = 0, destination_flags = 0;\n> + int destination_log_backed_up = 0;\n> + struct object_id destination_oid;\n> + struct files_copy_or_rename_transaction_data *data;\n> struct strbuf err = STRBUF_INIT;\n>\n> files_reflog_path(refs, &sb_oldref, oldrefname);\n> files_reflog_path(refs, &sb_newref, newrefname);\n> files_reflog_path(refs, &tmp_renamed_log, TMP_RENAMED_LOG);\n> + files_reflog_path(refs, &tmp_destination_log, TMP_RENAMED_LOG_DESTINATION);\n>\n> log = !lstat(sb_oldref.buf, &loginfo);\n> if (log && S_ISLNK(loginfo.st_mode)) {\n> @@ -1727,11 +1751,67 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,\n> oldrefname);\n> goto out;\n> }\n> + if (!oideq(&orig_oid, &operation->source_oid)) {\n> + ret = error(\"refname %s is at %s but expected %s\",\n> + oldrefname, oid_to_hex(&orig_oid),\n> + oid_to_hex(&operation->source_oid));\n> + goto out;\n> + }\n> if (!refs_rename_ref_available(&refs->base, oldrefname, newrefname)) {\n> ret = 1;\n> goto out;\n> }\n>\n> + if (refs_resolve_ref_unsafe(&refs->base, newrefname,\n> + RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n> + &destination_oid, &destination_flags)) {\n> + destination_exists = 1;\n> + if ((destination_flags & REF_ISSYMREF) &&\n> + refs_read_symbolic_ref(&refs->base, newrefname,\n> + &destination_target) < 0) {\n> + ret = error(\"unable to read symbolic ref %s\", newrefname);\n> + goto out;\n> + }\n> + }\n> + if (destination_exists != operation->destination_exists) {\n> + ret = error(\"refname %s changed while renaming\", newrefname);\n> + goto out;\n> + }\n> + if (destination_exists) {\n> + if (destination_flags & REF_ISSYMREF) {\n> + if (!operation->destination_target ||\n> + strcmp(destination_target.buf,\n> + operation->destination_target)) {\n> + ret = error(\"refname %s changed while renaming\",\n> + newrefname);\n> + goto out;\n> + }\n> + } else if (operation->destination_target ||\n> + !oideq(&destination_oid,\n> + &operation->destination_oid)) {\n> + ret = error(\"refname %s changed while renaming\", newrefname);\n> + goto out;\n> + }\n> + }\n> +\n> + if (!lstat(sb_newref.buf, &loginfo)) {\n> + if (S_ISLNK(loginfo.st_mode)) {\n> + ret = error(\"reflog for %s is a symlink\", newrefname);\n> + goto out;\n> + }\n> + if (S_ISREG(loginfo.st_mode)) {\n> + if (copy_file(refs->base.repo, tmp_destination_log.buf,\n> + sb_newref.buf, 0644)) {\n> + if (errno != EEXIST)\n> + unlink(tmp_destination_log.buf);\n> + ret = error(\"unable to back up logfile logs/%s: %s\",\n> + newrefname, strerror(errno));\n> + goto out;\n> + }\n> + destination_log_backed_up = 1;\n> + }\n> + }\n> +\n> if (!copy && log && rename(sb_oldref.buf, tmp_renamed_log.buf)) {\n> ret = error(\"unable to move logfile logs/%s to logs/\"TMP_RENAMED_LOG\": %s\",\n> oldrefname, strerror(errno));\n> @@ -1744,8 +1824,10 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,\n> goto out;\n> }\n>\n> - if (!copy && refs_delete_ref(&refs->base, logmsg, oldrefname,\n> - &orig_oid, REF_NO_DEREF)) {\n> + if (!copy && refs_delete_ref_with_transaction_flags(&refs->base, logmsg,\n> + oldrefname, &orig_oid,\n> + REF_NO_DEREF,\n> + REF_TRANSACTION_FLAG_SKIP_HOOK)) {\n> error(\"unable to delete old %s\", oldrefname);\n> goto rollback;\n> }\n> @@ -1760,8 +1842,9 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,\n> if (!copy && refs_resolve_ref_unsafe(&refs->base, newrefname,\n> RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n> NULL, NULL) &&\n> - refs_delete_ref(&refs->base, NULL, newrefname,\n> - NULL, REF_NO_DEREF)) {\n> + refs_delete_ref_with_transaction_flags(&refs->base, NULL, newrefname,\n> + NULL, REF_NO_DEREF,\n> + REF_TRANSACTION_FLAG_SKIP_HOOK)) {\n> if (errno == EISDIR) {\n> struct strbuf path = STRBUF_INIT;\n> int result;\n> @@ -1796,13 +1879,25 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,\n> }\n> oidcpy(&lock->old_oid, &orig_oid);\n>\n> - if (write_ref_to_lockfile(refs, lock, &orig_oid, &err) ||\n> - commit_ref_update(refs, lock, &orig_oid, logmsg, 0, &err)) {\n> + if (write_ref_to_lockfile(refs, lock, &orig_oid, &err)) {\n> error(\"unable to write current sha1 into %s: %s\", newrefname, err.buf);\n> strbuf_release(&err);\n> goto rollback;\n> }\n>\n> + CALLOC_ARRAY(data, 1);\n> + data->lock = lock;\n> + oidcpy(&data->orig_oid, &orig_oid);\n> + data->logmoved = logmoved;\n> + data->destination_exists = destination_exists;\n> + data->destination_log_backed_up = destination_log_backed_up;\n> + if (destination_exists && !(destination_flags & REF_ISSYMREF))\n> + oidcpy(&data->destination_oid, &destination_oid);\n> + if (destination_flags & REF_ISSYMREF)\n> + data->destination_target = strbuf_detach(&destination_target, NULL);\n> + transaction->backend_data = data;\n> + transaction->state = REF_TRANSACTION_PREPARED;\n> +\n> ret = 0;\n> goto out;\n>\n> @@ -1821,38 +1916,40 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,\n> }\n>\n> rollbacklog:\n> - if (logmoved && rename(sb_newref.buf, sb_oldref.buf))\n> - error(\"unable to restore logfile %s from %s: %s\",\n> - oldrefname, newrefname, strerror(errno));\n> + if (logmoved) {\n> + if (rename(sb_newref.buf, tmp_renamed_log.buf)) {\n> + error(\"unable to restore logfile %s from %s: %s\",\n> + oldrefname, newrefname, strerror(errno));\n> + } else {\n> + try_remove_empty_parents(refs, newrefname,\n> + REMOVE_EMPTY_PARENTS_REFLOG);\n> + if (rename_tmp_log(refs, oldrefname))\n> + error(\"unable to restore logfile %s from logs/\"\n> + TMP_RENAMED_LOG \": %s\",\n> + oldrefname, strerror(errno));\n> + }\n> + }\n> if (!logmoved && log &&\n> rename(tmp_renamed_log.buf, sb_oldref.buf))\n> error(\"unable to restore logfile %s from logs/\"TMP_RENAMED_LOG\": %s\",\n> oldrefname, strerror(errno));\n> + if (destination_log_backed_up &&\n> + rename(tmp_destination_log.buf, sb_newref.buf))\n> + error(\"unable to restore logfile %s: %s\",\n> + newrefname, strerror(errno));\n> ret = 1;\n> out:\n> + if (ret && destination_log_backed_up)\n> + unlink(tmp_destination_log.buf);\n> strbuf_release(&sb_newref);\n> strbuf_release(&sb_oldref);\n> strbuf_release(&tmp_renamed_log);\n> + strbuf_release(&tmp_destination_log);\n> + strbuf_release(&destination_target);\n>\n> return ret;\n> }\n>\n> -static int files_rename_ref(struct ref_store *ref_store,\n> - const char *oldrefname, const char *newrefname,\n> - const char *logmsg)\n> -{\n> - return files_copy_or_rename_ref(ref_store, oldrefname,\n> - newrefname, logmsg, 0);\n> -}\n> -\n> -static int files_copy_ref(struct ref_store *ref_store,\n> - const char *oldrefname, const char *newrefname,\n> - const char *logmsg)\n> -{\n> - return files_copy_or_rename_ref(ref_store, oldrefname,\n> - newrefname, logmsg, 1);\n> -}\n> -\n> static int close_ref_gently(struct ref_lock *lock)\n> {\n> if (close_lock_file_gently(&lock->lk))\n> @@ -2962,6 +3059,14 @@ static int files_transaction_prepare(struct ref_store *ref_store,\n> struct ref_transaction *packed_transaction = NULL;\n>\n> assert(err);\n> + {\n> + struct ref_update *operation =\n> + ref_transaction_copy_or_rename_update(transaction);\n> +\n> + if (operation)\n> + return files_copy_or_rename_ref(ref_store, operation,\n> + transaction);\n> + }\n>\n> if (transaction->flags & REF_TRANSACTION_FLAG_INITIAL)\n> goto cleanup;\n> @@ -3318,6 +3423,10 @@ static int files_transaction_finish_initial(struct files_ref_store *refs,\n> return ret;\n> }\n>\n> +static int files_transaction_abort(struct ref_store *ref_store,\n> + struct ref_transaction *transaction,\n> + struct strbuf *err);\n> +\n> static int files_transaction_finish(struct ref_store *ref_store,\n> struct ref_transaction *transaction,\n> struct strbuf *err)\n> @@ -3333,6 +3442,40 @@ static int files_transaction_finish(struct ref_store *ref_store,\n>\n>\n> assert(err);\n> + {\n> + struct ref_update *update =\n> + ref_transaction_copy_or_rename_update(transaction);\n> +\n> + if (update) {\n> + struct ref_copy_or_rename_update *operation =\n> + update->copy_or_rename;\n> + struct files_copy_or_rename_transaction_data *data =\n> + transaction->backend_data;\n> + int special_ret;\n> +\n> + special_ret = commit_ref_update(refs, data->lock, &data->orig_oid,\n> + operation->logmsg, 0, err);\n> + if (special_ret) {\n> + error(\"unable to write current sha1 into %s: %s\",\n> + update->refname, err->buf);\n> + data->lock = NULL;\n> + files_transaction_abort(ref_store, transaction, err);\n> + return special_ret;\n> + } else if (data->destination_log_backed_up) {\n> + struct strbuf path = STRBUF_INIT;\n> +\n> + files_reflog_path(refs, &path, TMP_RENAMED_LOG_DESTINATION);\n> + if (unlink(path.buf) < 0 && errno != ENOENT)\n> + warning_errno(\"unable to remove '%s'\", path.buf);\n> + strbuf_release(&path);\n> + }\n> + free(data->destination_target);\n> + free(data);\n> + transaction->backend_data = NULL;\n> + transaction->state = REF_TRANSACTION_CLOSED;\n> + return special_ret;\n> + }\n> + }\n>\n> if (transaction->flags & REF_TRANSACTION_FLAG_INITIAL)\n> return files_transaction_finish_initial(refs, transaction, err);\n> @@ -3476,11 +3619,105 @@ static int files_transaction_finish(struct ref_store *ref_store,\n>\n> static int files_transaction_abort(struct ref_store *ref_store,\n> struct ref_transaction *transaction,\n> - struct strbuf *err UNUSED)\n> + struct strbuf *err)\n> {\n> struct files_ref_store *refs =\n> files_downcast(ref_store, 0, \"ref_transaction_abort\");\n>\n> + {\n> + struct ref_update *update =\n> + ref_transaction_copy_or_rename_update(transaction);\n> +\n> + if (update) {\n> + struct ref_copy_or_rename_update *operation =\n> + update->copy_or_rename;\n> + struct files_copy_or_rename_transaction_data *data =\n> + transaction->backend_data;\n> + struct strbuf new_log = STRBUF_INIT;\n> + struct strbuf destination_log = STRBUF_INIT;\n> + struct strbuf temporary_log = STRBUF_INIT;\n> + struct ref_transaction *restore_transaction = NULL;\n> + struct ref_lock *lock;\n> + int ret = 0;\n> +\n> + if (data->lock)\n> + unlock_ref(data->lock);\n> + if (operation->type == REF_UPDATE_RENAME) {\n> + lock = lock_ref_oid_basic(refs, operation->old_refname, err);\n> + if (!lock ||\n> + write_ref_to_lockfile(refs, lock, &data->orig_oid, err) ||\n> + commit_ref_update(refs, lock, &data->orig_oid, NULL,\n> + REF_SKIP_CREATE_REFLOG, err))\n> + ret = -1;\n> + }\n> +\n> + if (data->logmoved) {\n> + files_reflog_path(refs, &new_log, update->refname);\n> + if (operation->type == REF_UPDATE_RENAME) {\n> + files_reflog_path(refs, &temporary_log, TMP_RENAMED_LOG);\n> + if (rename(new_log.buf, temporary_log.buf) < 0) {\n> + strbuf_addf(err, \"unable to restore logfile %s: %s\",\n> + operation->old_refname, strerror(errno));\n> + ret = -1;\n> + } else {\n> + try_remove_empty_parents(refs,\n> + update->refname,\n> + REMOVE_EMPTY_PARENTS_REFLOG);\n> + if (rename_tmp_log(refs,\n> + operation->old_refname)) {\n> + strbuf_addf(err, \"unable to restore logfile %s: %s\",\n> + operation->old_refname,\n> + strerror(errno));\n> + ret = -1;\n> + }\n> + }\n> + } else if (unlink(new_log.buf) < 0 && errno != ENOENT) {\n> + strbuf_addf(err, \"unable to remove logfile %s: %s\",\n> + update->refname, strerror(errno));\n> + ret = -1;\n> + }\n> + }\n> + if (data->destination_log_backed_up) {\n> + files_reflog_path(refs, &destination_log,\n> + TMP_RENAMED_LOG_DESTINATION);\n> + if (rename(destination_log.buf, new_log.buf) < 0) {\n> + strbuf_addf(err, \"unable to restore logfile %s: %s\",\n> + update->refname, strerror(errno));\n> + ret = -1;\n> + }\n> + }\n> +\n> + if (operation->type == REF_UPDATE_RENAME &&\n> + data->destination_exists) {\n> + restore_transaction = ref_store_transaction_begin(\n> + &refs->base, REF_TRANSACTION_FLAG_SKIP_HOOK, err);\n> + if (!restore_transaction ||\n> + ref_transaction_update(restore_transaction,\n> + update->refname,\n> + data->destination_target ? NULL :\n> + &data->destination_oid,\n> + NULL,\n> + data->destination_target,\n> + NULL,\n> + REF_NO_DEREF |\n> + REF_SKIP_CREATE_REFLOG,\n> + NULL, err) ||\n> + ref_transaction_commit(restore_transaction, err))\n> + ret = -1;\n> + ref_transaction_free(restore_transaction);\n> + }\n> +\n> + strbuf_release(&destination_log);\n> + strbuf_release(&temporary_log);\n> + strbuf_release(&new_log);\n> + free(data->destination_target);\n> + free(data);\n> + transaction->backend_data = NULL;\n> + transaction->state = REF_TRANSACTION_CLOSED;\n> + return ret;\n> + }\n> + }\n> +\n> files_transaction_cleanup(refs, transaction);\n> return 0;\n> }\n> @@ -4095,8 +4332,6 @@ struct ref_storage_be refs_be_files = {\n>\n> .optimize = files_optimize,\n> .optimize_required = files_optimize_required,\n> - .rename_ref = files_rename_ref,\n> - .copy_ref = files_copy_ref,\n>\n> .iterator_begin = files_ref_iterator_begin,\n> .read_raw_ref = files_read_raw_ref,\n> diff --git a/refs/packed-backend.c b/refs/packed-backend.c\n> index a73fc6aca..364a91291 100644\n> --- a/refs/packed-backend.c\n> +++ b/refs/packed-backend.c\n> @@ -2164,8 +2164,6 @@ struct ref_storage_be refs_be_packed = {\n> .optimize = packed_optimize,\n> .optimize_required = packed_optimize_required,\n>\n> - .rename_ref = NULL,\n> - .copy_ref = NULL,\n>\n> .iterator_begin = packed_ref_iterator_begin,\n> .read_raw_ref = packed_read_raw_ref,\n> diff --git a/refs/refs-internal.h b/refs/refs-internal.h\n> index c3ac7b556..5d4dc0171 100644\n> --- a/refs/refs-internal.h\n> +++ b/refs/refs-internal.h\n> @@ -155,9 +155,33 @@ struct ref_update {\n> */\n> struct ref_update *parent_update;\n>\n> + /*\n> + * Copy and rename operations require backend-specific handling while\n> + * still exposing their logical updates to transaction hooks. Keep that\n> + * state on the destination update so it composes with other updates in\n> + * the transaction instead of making copy or rename a transaction-wide\n> + * property.\n> + */\n> + struct ref_copy_or_rename_update *copy_or_rename;\n> +\n> const char refname[FLEX_ARRAY];\n> };\n>\n> +enum ref_copy_or_rename_type {\n> + REF_UPDATE_RENAME,\n> + REF_UPDATE_COPY,\n> +};\n> +\n> +struct ref_copy_or_rename_update {\n> + enum ref_copy_or_rename_type type;\n> + char *old_refname;\n> + char *logmsg;\n> + struct object_id source_oid;\n> + struct object_id destination_oid;\n> + char *destination_target;\n> + unsigned int destination_exists:1;\n> +};\n> +\n> int refs_read_raw_ref(struct ref_store *ref_store, const char *refname,\n> struct object_id *oid, struct strbuf *referent,\n> unsigned int *type, int *failure_errno);\n> @@ -187,6 +211,13 @@ struct ref_update *ref_transaction_add_update(\n> const char *committer_info,\n> const char *msg);\n>\n> +int refs_delete_ref_with_transaction_flags(struct ref_store *refs,\n> + const char *msg,\n> + const char *refname,\n> + const struct object_id *old_oid,\n> + unsigned int flags,\n> + unsigned int transaction_flags);\n> +\n> /*\n> * Transaction states.\n> *\n> @@ -242,6 +273,12 @@ struct ref_transaction {\n> uint64_t max_index;\n> };\n>\n> +/* Suppress hooks for a transaction nested inside another refs operation. */\n> +#define REF_TRANSACTION_FLAG_SKIP_HOOK (1 << 2)\n> +\n> +struct ref_update *ref_transaction_copy_or_rename_update(\n> + struct ref_transaction *transaction);\n> +\n> /*\n> * Check for entries in extras that are within the specified\n> * directory, where dirname is a reference directory name including\n> @@ -451,13 +488,6 @@ typedef int optimize_required_fn(struct ref_store *ref_store,\n> struct refs_optimize_opts *opts,\n> bool *required);\n>\n> -typedef int rename_ref_fn(struct ref_store *ref_store,\n> - const char *oldref, const char *newref,\n> - const char *logmsg);\n> -typedef int copy_ref_fn(struct ref_store *ref_store,\n> - const char *oldref, const char *newref,\n> - const char *logmsg);\n> -\n> /*\n> * Iterate over the references in `ref_store` whose names start with\n> * `prefix`. `prefix` is matched as a literal string, without regard\n> @@ -577,9 +607,6 @@ struct ref_storage_be {\n>\n> optimize_fn *optimize;\n> optimize_required_fn *optimize_required;\n> - rename_ref_fn *rename_ref;\n> - copy_ref_fn *copy_ref;\n> -\n> ref_iterator_begin_fn *iterator_begin;\n> read_raw_ref_fn *read_raw_ref;\n>\n> diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c\n> index 10db03991..589fcc998 100644\n> --- a/refs/reftable-backend.c\n> +++ b/refs/reftable-backend.c\n> @@ -953,6 +953,14 @@ struct reftable_transaction_data {\n> size_t args_nr, args_alloc;\n> };\n>\n> +struct reftable_copy_or_rename_transaction_data {\n> + struct reftable_addition *addition;\n> +};\n> +\n> +static int reftable_be_copy_or_rename_prepare(struct ref_store *ref_store,\n> + struct ref_transaction *transaction,\n> + struct strbuf *err);\n> +\n> static void free_transaction_data(struct reftable_transaction_data *tx_data)\n> {\n> if (!tx_data)\n> @@ -1326,6 +1334,10 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,\n> size_t i;\n> int ret;\n>\n> + if (ref_transaction_copy_or_rename_update(transaction))\n> + return reftable_be_copy_or_rename_prepare(ref_store, transaction,\n> + err);\n> +\n> ret = refs->err;\n> if (ret < 0)\n> goto done;\n> @@ -1419,7 +1431,20 @@ static int reftable_be_transaction_abort(struct ref_store *ref_store UNUSED,\n> struct ref_transaction *transaction,\n> struct strbuf *err UNUSED)\n> {\n> - struct reftable_transaction_data *tx_data = transaction->backend_data;\n> + struct reftable_transaction_data *tx_data;\n> +\n> + if (ref_transaction_copy_or_rename_update(transaction)) {\n> + struct reftable_copy_or_rename_transaction_data *data =\n> + transaction->backend_data;\n> +\n> + reftable_addition_destroy(data->addition);\n> + free(data);\n> + transaction->backend_data = NULL;\n> + transaction->state = REF_TRANSACTION_CLOSED;\n> + return 0;\n> + }\n> +\n> + tx_data = transaction->backend_data;\n> free_transaction_data(tx_data);\n> transaction->state = REF_TRANSACTION_CLOSED;\n> return 0;\n> @@ -1667,9 +1692,28 @@ static int reftable_be_transaction_finish(struct ref_store *ref_store UNUSED,\n> struct ref_transaction *transaction,\n> struct strbuf *err)\n> {\n> - struct reftable_transaction_data *tx_data = transaction->backend_data;\n> + struct reftable_transaction_data *tx_data;\n> int ret = 0;\n>\n> + if (ref_transaction_copy_or_rename_update(transaction)) {\n> + struct reftable_copy_or_rename_transaction_data *data =\n> + transaction->backend_data;\n> + int special_ret = reftable_addition_commit(data->addition);\n> +\n> + reftable_addition_destroy(data->addition);\n> + free(data);\n> + transaction->backend_data = NULL;\n> + transaction->state = REF_TRANSACTION_CLOSED;\n> + if (special_ret < 0) {\n> + strbuf_addf(err, _(\"reftable: transaction failure: %s\"),\n> + reftable_error_str(special_ret));\n> + return -1;\n> + }\n> + return 0;\n> + }\n> +\n> + tx_data = transaction->backend_data;\n> +\n> for (size_t i = 0; i < tx_data->args_nr; i++) {\n> tx_data->args[i].max_index = transaction->max_index;\n>\n> @@ -1764,17 +1808,20 @@ struct write_create_symref_arg {\n> struct write_copy_arg {\n> struct reftable_ref_store *refs;\n> struct reftable_backend *be;\n> + struct strbuf *err;\n> const char *oldname;\n> const char *newname;\n> const char *logmsg;\n> int delete_old;\n> + struct ref_copy_or_rename_update *operation;\n> };\n>\n> static int write_copy_table(struct reftable_writer *writer, void *cb_data)\n> {\n> struct write_copy_arg *arg = cb_data;\n> uint64_t deletion_ts, creation_ts;\n> - struct reftable_ref_record old_ref = {0}, refs[2] = {0};\n> + struct reftable_ref_record old_ref = {0}, destination_ref = {0};\n> + struct reftable_ref_record refs[2] = {0};\n> struct reftable_log_record old_log = {0}, *logs = NULL;\n> struct reftable_iterator it = {0};\n> struct string_list skip = STRING_LIST_INIT_NODUP;\n> @@ -1789,14 +1836,75 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data)\n> BUG(\"failed splitting committer info\");\n>\n> if (reftable_stack_read_ref(arg->be->stack, arg->oldname, &old_ref)) {\n> - ret = error(_(\"refname %s not found\"), arg->oldname);\n> + strbuf_addf(arg->err, _(\"refname %s not found\"), arg->oldname);\n> + ret = -1;\n> goto done;\n> }\n> if (old_ref.value_type == REFTABLE_REF_SYMREF) {\n> - ret = error(_(\"refname %s is a symbolic ref, copying it is not supported\"),\n> + strbuf_addf(arg->err,\n> + _(\"refname %s is a symbolic ref, copying it is not supported\"),\n> arg->oldname);\n> + ret = -1;\n> goto done;\n> }\n> + if (arg->operation) {\n> + struct object_id oid;\n> +\n> + if (old_ref.value_type == REFTABLE_REF_VAL2)\n> + oidread(&oid, old_ref.value.val2.value,\n> + arg->refs->base.repo->hash_algo);\n> + else\n> + oidread(&oid, old_ref.value.val1,\n> + arg->refs->base.repo->hash_algo);\n> + if (!oideq(&oid, &arg->operation->source_oid)) {\n> + strbuf_addf(arg->err,\n> + _(\"refname %s is at %s but expected %s\"),\n> + arg->oldname, oid_to_hex(&oid),\n> + oid_to_hex(&arg->operation->source_oid));\n> + ret = -1;\n> + goto done;\n> + }\n> +\n> + ret = reftable_stack_read_ref(arg->be->stack, arg->newname,\n> + &destination_ref);\n> + if (ret < 0)\n> + goto done;\n> + if (arg->operation->destination_exists != !ret) {\n> + strbuf_addf(arg->err,\n> + _(\"refname %s changed while renaming\"),\n> + arg->newname);\n> + ret = -1;\n> + goto done;\n> + }\n> + if (!ret) {\n> + if (destination_ref.value_type == REFTABLE_REF_SYMREF) {\n> + if (!arg->operation->destination_target ||\n> + strcmp(destination_ref.value.symref,\n> + arg->operation->destination_target)) {\n> + strbuf_addf(arg->err,\n> + _(\"refname %s changed while renaming\"),\n> + arg->newname);\n> + ret = -1;\n> + goto done;\n> + }\n> + } else {\n> + if (destination_ref.value_type == REFTABLE_REF_VAL2)\n> + oidread(&oid, destination_ref.value.val2.value,\n> + arg->refs->base.repo->hash_algo);\n> + else\n> + oidread(&oid, destination_ref.value.val1,\n> + arg->refs->base.repo->hash_algo);\n> + if (arg->operation->destination_target ||\n> + !oideq(&oid, &arg->operation->destination_oid)) {\n> + strbuf_addf(arg->err,\n> + _(\"refname %s changed while renaming\"),\n> + arg->newname);\n> + ret = -1;\n> + goto done;\n> + }\n> + }\n> + }\n> + }\n>\n> /*\n> * There's nothing to do in case the old and new name are the same, so\n> @@ -1815,7 +1923,7 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data)\n> ret = refs_verify_refname_available(&arg->refs->base, arg->newname,\n> NULL, &skip, 0, &errbuf);\n> if (ret < 0) {\n> - error(\"%s\", errbuf.buf);\n> + strbuf_addbuf(arg->err, &errbuf);\n> goto done;\n> }\n>\n> @@ -1980,68 +2088,63 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data)\n> for (i = 0; i < ARRAY_SIZE(refs); i++)\n> reftable_ref_record_release(&refs[i]);\n> reftable_ref_record_release(&old_ref);\n> + reftable_ref_record_release(&destination_ref);\n> reftable_log_record_release(&old_log);\n> return ret;\n> }\n>\n> -static int reftable_be_rename_ref(struct ref_store *ref_store,\n> - const char *oldrefname,\n> - const char *newrefname,\n> - const char *logmsg)\n> +static int reftable_be_copy_or_rename_prepare(struct ref_store *ref_store,\n> + struct ref_transaction *transaction,\n> + struct strbuf *err)\n> {\n> struct reftable_ref_store *refs =\n> - reftable_be_downcast(ref_store, REF_STORE_WRITE, \"rename_ref\");\n> + reftable_be_downcast(ref_store, REF_STORE_WRITE,\n> + \"ref_transaction_prepare\");\n> + struct reftable_copy_or_rename_transaction_data *data = NULL;\n> + struct ref_update *update =\n> + ref_transaction_copy_or_rename_update(transaction);\n> + struct ref_copy_or_rename_update *operation = update->copy_or_rename;\n> struct write_copy_arg arg = {\n> .refs = refs,\n> - .oldname = oldrefname,\n> - .newname = newrefname,\n> - .logmsg = logmsg,\n> - .delete_old = 1,\n> + .err = err,\n> + .oldname = operation->old_refname,\n> + .newname = update->refname,\n> + .logmsg = operation->logmsg,\n> + .delete_old = operation->type == REF_UPDATE_RENAME,\n> + .operation = operation,\n> };\n> int ret;\n>\n> + CALLOC_ARRAY(data, 1);\n> ret = refs->err;\n> if (ret < 0)\n> goto done;\n> -\n> - ret = backend_for(&arg.be, refs, newrefname, &newrefname, 1);\n> + ret = backend_for(&arg.be, refs, update->refname,\n> + &arg.newname, 1);\n> if (ret)\n> goto done;\n> - ret = reftable_stack_add(arg.be->stack, &write_copy_table, &arg,\n> - &reftable_be_write_options(refs)->opts);\n> -\n> -done:\n> - assert(ret != REFTABLE_API_ERROR);\n> - return ret;\n> -}\n> -\n> -static int reftable_be_copy_ref(struct ref_store *ref_store,\n> - const char *oldrefname,\n> - const char *newrefname,\n> - const char *logmsg)\n> -{\n> - struct reftable_ref_store *refs =\n> - reftable_be_downcast(ref_store, REF_STORE_WRITE, \"copy_ref\");\n> - struct write_copy_arg arg = {\n> - .refs = refs,\n> - .oldname = oldrefname,\n> - .newname = newrefname,\n> - .logmsg = logmsg,\n> - };\n> - int ret;\n> -\n> - ret = refs->err;\n> - if (ret < 0)\n> + ret = reftable_stack_addition_new(&data->addition, arg.be->stack,\n> + &reftable_be_write_options(refs)->opts);\n> + if (ret)\n> goto done;\n> -\n> - ret = backend_for(&arg.be, refs, newrefname, &newrefname, 1);\n> + ret = reftable_addition_add(data->addition, &write_copy_table, &arg);\n> if (ret)\n> goto done;\n> - ret = reftable_stack_add(arg.be->stack, &write_copy_table, &arg,\n> - &reftable_be_write_options(refs)->opts);\n> +\n> + transaction->backend_data = data;\n> + transaction->state = REF_TRANSACTION_PREPARED;\n> + return 0;\n>\n> done:\n> assert(ret != REFTABLE_API_ERROR);\n> + if (data) {\n> + reftable_addition_destroy(data->addition);\n> + free(data);\n> + }\n> + transaction->state = REF_TRANSACTION_CLOSED;\n> + if (ret && !err->len)\n> + strbuf_addf(err, _(\"reftable: transaction prepare: %s\"),\n> + reftable_error_str(ret));\n> return ret;\n> }\n>\n> @@ -2872,8 +2975,6 @@ struct ref_storage_be refs_be_reftable = {\n> .optimize = reftable_be_optimize,\n> .optimize_required = reftable_be_optimize_required,\n>\n> - .rename_ref = reftable_be_rename_ref,\n> - .copy_ref = reftable_be_copy_ref,\n>\n> .iterator_begin = reftable_be_iterator_begin,\n> .read_raw_ref = reftable_be_read_raw_ref,\n> diff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh\n> index 4fe9d9b23..116b2ff07 100755\n> --- a/t/t1416-ref-transaction-hooks.sh\n> +++ b/t/t1416-ref-transaction-hooks.sh\n> @@ -93,6 +93,148 @@ test_expect_success 'hook gets all queued updates in committed state' '\n> test_cmp expect actual\n> '\n>\n> +test_expect_success 'hook gets both updates when renaming a branch' '\n> + test_when_finished \"rm -f actual\" &&\n> + git branch old PRE &&\n> + test_hook reference-transaction <<-\\EOF &&\n> + echo \"$1\" >>actual &&\n> + cat >>actual\n> + EOF\n> + cat >expect <<-EOF &&\n> + preparing\n> + $PRE_OID $ZERO_OID refs/heads/old\n> + $ZERO_OID $PRE_OID refs/heads/new\n> + prepared\n> + $PRE_OID $ZERO_OID refs/heads/old\n> + $ZERO_OID $PRE_OID refs/heads/new\n> + committed\n> + $PRE_OID $ZERO_OID refs/heads/old\n> + $ZERO_OID $PRE_OID refs/heads/new\n> + EOF\n> + git branch -m old new &&\n> + test_cmp expect actual &&\n> + test_must_fail git rev-parse --verify refs/heads/old &&\n> + test_cmp_rev PRE refs/heads/new\n> +'\n> +\n> +test_expect_success 'hook gets destination update when copying a branch' '\n> + test_when_finished \"rm -f actual\" &&\n> + git branch copy-source PRE &&\n> + test_hook reference-transaction <<-\\EOF &&\n> + echo \"$1\" >>actual &&\n> + cat >>actual\n> + EOF\n> + cat >expect <<-EOF &&\n> + preparing\n> + $ZERO_OID $PRE_OID refs/heads/copy-destination\n> + prepared\n> + $ZERO_OID $PRE_OID refs/heads/copy-destination\n> + committed\n> + $ZERO_OID $PRE_OID refs/heads/copy-destination\n> + EOF\n> + git branch -c copy-source copy-destination &&\n> + test_cmp expect actual &&\n> + test_cmp_rev PRE refs/heads/copy-source &&\n> + test_cmp_rev PRE refs/heads/copy-destination\n> +'\n> +\n> +test_expect_success 'hook gets overwritten values for forced rename and copy' '\n> + git branch force-old PRE &&\n> + git branch force-new POST &&\n> + git branch force-copy-source PRE &&\n> + git branch force-copy-destination POST &&\n> + test_hook reference-transaction <<-\\EOF &&\n> + if test \"$1\" = committed\n> + then\n> + cat >>actual\n> + fi\n> + EOF\n> + git branch -M force-old force-new &&\n> + git branch -C force-copy-source force-copy-destination &&\n> + cat >expect <<-EOF &&\n> + $PRE_OID $ZERO_OID refs/heads/force-old\n> + $POST_OID $PRE_OID refs/heads/force-new\n> + $POST_OID $PRE_OID refs/heads/force-copy-destination\n> + EOF\n> + test_cmp expect actual\n> +'\n> +\n> +test_expect_success 'hook can abort a branch rename after preparation' '\n> + git branch abort-old PRE &&\n> + git branch abort-new POST &&\n> + git reflog show --format=%gs abort-old >old-log &&\n> + git reflog show --format=%gs abort-new >new-log &&\n> + test_hook reference-transaction <<-\\EOF &&\n> + test \"$1\" != prepared\n> + EOF\n> + test_must_fail git branch -M abort-old abort-new &&\n> + test_cmp_rev PRE refs/heads/abort-old &&\n> + test_cmp_rev POST refs/heads/abort-new &&\n> + git reflog show --format=%gs abort-old >old-log-after &&\n> + git reflog show --format=%gs abort-new >new-log-after &&\n> + test_cmp old-log old-log-after &&\n> + test_cmp new-log new-log-after\n> +'\n> +\n> +test_expect_success 'hook can abort a D/F branch rename after preparation' '\n> + git branch df-old PRE &&\n> + git reflog show --format=%gs df-old >df-log &&\n> + test_hook reference-transaction <<-\\EOF &&\n> + test \"$1\" != prepared\n> + EOF\n> + test_must_fail git branch -m df-old df-old/child &&\n> + test_cmp_rev PRE refs/heads/df-old &&\n> + test_must_fail git rev-parse --verify refs/heads/df-old/child &&\n> + git reflog show --format=%gs df-old >df-log-after &&\n> + test_cmp df-log df-log-after\n> +'\n> +\n> +test_expect_success 'hook can abort a reverse D/F rename after preparation' '\n> + git branch reverse/old PRE &&\n> + git reflog show --format=%gs reverse/old >reverse-log &&\n> + test_hook reference-transaction <<-\\EOF &&\n> + test \"$1\" != prepared\n> + EOF\n> + test_must_fail git branch -m reverse/old reverse &&\n> + test_cmp_rev PRE refs/heads/reverse/old &&\n> + test_must_fail git rev-parse --verify refs/heads/reverse &&\n> + git reflog show --format=%gs reverse/old >reverse-log-after &&\n> + test_cmp reverse-log reverse-log-after\n> +'\n> +\n> +test_expect_success 'hook can abort a forced branch copy after preparation' '\n> + git branch copy-abort-old PRE &&\n> + git branch copy-abort-new POST &&\n> + git reflog show --format=%gs copy-abort-old >copy-old-log &&\n> + git reflog show --format=%gs copy-abort-new >copy-new-log &&\n> + test_hook reference-transaction <<-\\EOF &&\n> + test \"$1\" != prepared\n> + EOF\n> + test_must_fail git branch -C copy-abort-old copy-abort-new &&\n> + test_cmp_rev PRE refs/heads/copy-abort-old &&\n> + test_cmp_rev POST refs/heads/copy-abort-new &&\n> + git reflog show --format=%gs copy-abort-old >copy-old-log-after &&\n> + git reflog show --format=%gs copy-abort-new >copy-new-log-after &&\n> + test_cmp copy-old-log copy-old-log-after &&\n> + test_cmp copy-new-log copy-new-log-after\n> +'\n> +\n> +test_expect_success 'branch rename detects an update during preparing hook' '\n> + git branch race-old PRE &&\n> + git branch race-new POST &&\n> + test_hook reference-transaction <<-\\EOF &&\n> + marker=$(git rev-parse --git-path rename-race-once)\n> + if test \"$1\" = preparing && test ! -e \"$marker\"\n> + then\n> + >\"$marker\" &&\n> + git update-ref refs/heads/race-old POST\n> + fi\n> + EOF\n> + test_must_fail git branch -M race-old race-new &&\n> + test_cmp_rev POST refs/heads/race-old &&\n> + test_cmp_rev POST refs/heads/race-new\n> +'\n> +\n> test_expect_success 'hook gets all queued updates in aborted state' '\n> test_when_finished \"rm actual\" &&\n> git reset --hard PRE &&\n> --\n> 2.39.3 (Apple Git-146)\n"},{"id":"553948","messageId":"ar-N7SA63fN_xx9P@pks.im","threadId":"66350","inReplyTo":"20260923133651.74120-1-maciej.ciemborowicz@gmail.com","subject":"Re: [PATCH v2] refs: run copy and rename through transactions","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-10-02T10:56:45Z","receivedAt":"2026-10-02T10:56:53Z","isPatch":true,"body":"On Wed, Sep 23, 2026 at 03:36:51PM +0200, Maciej Ciemborowicz wrote:\n> Reference copy and rename operations bypass the transaction API.\n> Consequently, the reference-transaction hook sees only the source deletion\n> with the files backend and no useful update with the reftable backend.\n> \n> Represent both operations as reference transactions containing their\n> logical updates. A rename is a deletion of the old reference and creation\n> of the new reference in the same transaction. Attach operation-specific\n> state to the destination update instead of making copy or rename a property\n> of the entire transaction.\n\nSorry, but what does this last sentence mean? What is the consequence\nof it?\n\n> Retain backend-specific reflog handling: the files backend stages its\n> existing rename procedure across prepare, finish and abort, while reftable\n> stages an addition while holding the stack lock. Suppress hooks for the\n> files backend's nested deletion transactions so that callers observe one\n> logical transaction.\n\nThe fact that we retain the backend-specific logic is not really\ninteresting by itself. The way more interesting question is _why_ we\nretain it. Or asked differently, why can't we make this whole mechanism\ncompletely agnostic of the backend and implement this via pure\ntransactions?\n\n> Record and verify the source and destination values after taking backend\n> locks. This rejects concurrent changes instead of applying a rename or copy\n> that differs from the payload shown to the preparing hook. Preserve D/F\n> renames and restore overwritten references and reflogs when a prepared hook\n> rejects the operation.\n\nIs this new behaviour? Is this retaining old behaviour? I have no clue.\n\n> Add tests covering rename, copy, forced updates, both directions of D/F\n> conflicts, concurrent updates and prepared-hook rollback.\n\nThis sentence doesn't really add much value to the message.\n\nHow does all of this impact performance?\n\n> diff --git a/refs.c b/refs.c\n> index 92d5df5b7..f036ae4b9 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -1027,6 +1029,15 @@ int refs_delete_ref(struct ref_store *refs, const char *msg,\n> \treturn 0;\n> }\n> \n> +int refs_delete_ref(struct ref_store *refs, const char *msg,\n> +\t\t const char *refname,\n> +\t\t const struct object_id *old_oid,\n> +\t\t unsigned int flags)\n> +{\n> +\treturn refs_delete_ref_with_transaction_flags(refs, msg, refname,\n> +\t\t\t\t\t\t old_oid, flags, 0);\n> +}\n> +\n> static void copy_reflog_msg(struct strbuf *sb, const char *msg)\n> {\n> \tchar c;\n\nRefactorings like these could easily go into a separate commit to make\nthis easier to review.\n\n> @@ -2710,7 +2747,8 @@ int ref_transaction_prepare(struct ref_transaction *transaction,\n> \t\treturn REF_TRANSACTION_ERROR_GENERIC;\n> \n> \t/* Preparing checks before locking references */\n> -\tret = run_transaction_hook(transaction, \"preparing\");\n> +\tret = transaction->flags & REF_TRANSACTION_FLAG_SKIP_HOOK ? 0 :\n> +\t\trun_transaction_hook(transaction, \"preparing\");\n> \tif (ret) {\n> \t\tref_transaction_abort(transaction, err);\n> \t\tdie(_(abort_by_ref_transaction_hook), \"preparing\");\n\nInstead of teaching every site to conditionally call\n`run_transaction_hook()` only when the flag is not set, can't we adapt\nthe function itself to skip?\n\nIn any case, this is another change that could easily be split out into\na separate commit.\n\n> diff --git a/refs/files-backend.c b/refs/files-backend.c\n> index 71628550f..c28228116 100644\n> --- a/refs/files-backend.c\n> +++ b/refs/files-backend.c\n> @@ -2962,6 +3059,14 @@ static int files_transaction_prepare(struct ref_store *ref_store,\n> \tstruct ref_transaction *packed_transaction = NULL;\n> \n> \tassert(err);\n> +\t{\n> +\t\tstruct ref_update *operation =\n> +\t\t\tref_transaction_copy_or_rename_update(transaction);\n> +\n> +\t\tif (operation)\n> +\t\t\treturn files_copy_or_rename_ref(ref_store, operation,\n> +\t\t\t\t\t\t\ttransaction);\n> +\t}\n> \n> \tif (transaction->flags & REF_TRANSACTION_FLAG_INITIAL)\n> \t\tgoto cleanup;\n\nI know this is a construct that AI loves, but that's not following our\ncoding style.\n\n> @@ -3333,6 +3442,40 @@ static int files_transaction_finish(struct ref_store *ref_store,\n> \n> \n> \tassert(err);\n> +\t{\n> +\t\tstruct ref_update *update =\n> +\t\t\tref_transaction_copy_or_rename_update(transaction);\n> +\n> +\t\tif (update) {\n> +\t\t\tstruct ref_copy_or_rename_update *operation =\n> +\t\t\t\tupdate->copy_or_rename;\n> +\t\t\tstruct files_copy_or_rename_transaction_data *data =\n> +\t\t\t\ttransaction->backend_data;\n> +\t\t\tint special_ret;\n> +\n> +\t\t\tspecial_ret = commit_ref_update(refs, data->lock, &data->orig_oid,\n> +\t\t\t\t\t\t\toperation->logmsg, 0, err);\n> +\t\t\tif (special_ret) {\n> +\t\t\t\terror(\"unable to write current sha1 into %s: %s\",\n> +\t\t\t\t update->refname, err->buf);\n> +\t\t\t\tdata->lock = NULL;\n> +\t\t\t\tfiles_transaction_abort(ref_store, transaction, err);\n> +\t\t\t\treturn special_ret;\n> +\t\t\t} else if (data->destination_log_backed_up) {\n> +\t\t\t\tstruct strbuf path = STRBUF_INIT;\n> +\n> +\t\t\t\tfiles_reflog_path(refs, &path, TMP_RENAMED_LOG_DESTINATION);\n> +\t\t\t\tif (unlink(path.buf) < 0 && errno != ENOENT)\n> +\t\t\t\t\twarning_errno(\"unable to remove '%s'\", path.buf);\n> +\t\t\t\tstrbuf_release(&path);\n> +\t\t\t}\n> +\t\t\tfree(data->destination_target);\n> +\t\t\tfree(data);\n> +\t\t\ttransaction->backend_data = NULL;\n> +\t\t\ttransaction->state = REF_TRANSACTION_CLOSED;\n> +\t\t\treturn special_ret;\n> +\t\t}\n> +\t}\n> \n> \tif (transaction->flags & REF_TRANSACTION_FLAG_INITIAL)\n> \t\treturn files_transaction_finish_initial(refs, transaction, err);\n\nYeah...\n\n> @@ -3476,11 +3619,105 @@ static int files_transaction_finish(struct ref_store *ref_store,\n> \n> static int files_transaction_abort(struct ref_store *ref_store,\n> \t\t\t\t struct ref_transaction *transaction,\n> -\t\t\t\t struct strbuf *err UNUSED)\n> +\t\t\t\t struct strbuf *err)\n> {\n> \tstruct files_ref_store *refs =\n> \t\tfiles_downcast(ref_store, 0, \"ref_transaction_abort\");\n> \n> +\t{\n> +\t\tstruct ref_update *update =\n> +\t\t\tref_transaction_copy_or_rename_update(transaction);\n\n... really?\n\nSorry, but I'm going to stop reading here. This is not in a state that\nis reviewable and has way too much stuff that is obviously generated by\nan AI without much thought being put into it by the author. I don't want\nto invest my time into a topic where the author has obviously not spent\ntheir time thinking about it, either.\n\nPatrick\n"},{"id":"553970","messageId":"CACQ=SRGSEsbNz3v3obd3JUOs2MrROnvuHkx1Dm51seCcv+12Cw@mail.gmail.com","threadId":"66350","inReplyTo":"ar-N7SA63fN_xx9P@pks.im","subject":"Re: [PATCH v2] refs: run copy and rename through transactions","fromName":"Maciej Ciemborowicz","fromEmail":"maciej.ciemborowicz@gmail.com","sentAt":"2026-10-02T14:16:08Z","receivedAt":"2026-10-02T14:16:23Z","isPatch":true,"body":"On Fri, Oct 2, 2026 at 12:56 PM Patrick Steinhardt wrote:\n\n> Sorry, but what does this last sentence mean? What is the consequence\n> of it?\n\nThe intent was to address Junio's comment about making copy/rename a\nproperty of the whole ref_transaction. In v2 the extra state is\nattached to the destination ref_update instead.\n\n> why can't we make this whole mechanism completely agnostic of the\n> backend and implement this via pure transactions?\n\nThe part I was trying to preserve is the existing reflog semantics. A\nnormal ref transaction can express the logical ref updates. In example\ndeleting the old ref and creating/updating the destination. But branch\nrename/copy also moves or copies the existing reflog history. For the\nfiles backend that currently involves filesystem-level reflog\nrename/copy and D/F handling, while reftable represents the same\noperation differently. So my assumption was that the logical ref\nupdates could go through the generic transaction API, while the\nreflog-history operation would remain backend-specific.\n\n> Is this new behaviour? Is this retaining old behaviour?\n\nThe source/destination revalidation is new validation required by\nintroducing the preparing hook before the backend locks are taken. The\nhook can itself change one of the refs. Without revalidation, the hook\npayload could describe one state while the rename/copy later operates\non another state. The intention is therefore to reject an operation\nwhen the state observed by the preparing hook is no longer the state\nbeing committed.\n\n> How does all of this impact performance?\n\nEnabling reference-transaction for rename/copy naturally adds the cost\nof invoking the hook when one is installed. I measured `git branch -m`\nand `git branch -c`. Each result is the median of five blocks of 40\ncommands per version:\n\n Hook Before After Change\nfiles branch -m no 5.089 ms 5.234 ms +3.8%\nfiles branch -m yes 12.608 ms 11.134 ms -10.8%\nfiles branch -c no 4.753 ms 4.916 ms +3.4%\nfiles branch -c yes 5.509 ms 12.732 ms +137.7%\nreftable branch -m no 6.478 ms 6.998 ms +7.8%\nreftable branch -m yes 5.918 ms 13.229 ms +114.6%\nreftable branch -c no 5.620 ms 6.368 ms +10.9%\nreftable branch -c yes 6.261 ms 12.354 ms +97.0%\n\n> Sorry, but I'm going to stop reading here. This is not in a state that\n> is reviewable and has way too much stuff that is obviously generated by\n> an AI without much thought being put into it by the author. I don't want\n> to invest my time into a topic where the author has obviously not spent\n> their time thinking about it, either.\n\nI'm really sorry to hear that. Yes, the patches I prepared were\nAI-assisted, but I do feel that I understand what I am doing. I would\nappreciate some understanding, though, as I do not work with C on a\ndaily basis. The bug report and my attempt to fix it came from the\nfact that I am working on a Ruby gem for per-branch and per-worktree\ncontainerization. That is why I had to write git-hooks-ext, which is\nhow I ended up running into this bug in the first place.\n\nI am not insisting that my patch should be merged. I simply thought\nthat submitting a patch might help get the bug fixed faster, and\ngetting the bug fixed is what I care about most. Karthik Nayak offered\nto help fix it, so perhaps it would be better for someone who works\nwith C on a daily basis to take it over.\n\nI can, of course, also prepare a v3, split it into more commits, and\nexplain my reasoning more clearly. But I cannot guarantee that it will\nmeet your standards, simply because I am not yet familiar with them.\n"},{"id":"554144","messageId":"asM9m-_ZoX_5UQ-I@pks.im","threadId":"66350","inReplyTo":"CACQ=SRGSEsbNz3v3obd3JUOs2MrROnvuHkx1Dm51seCcv+12Cw@mail.gmail.com","subject":"Re: [PATCH v2] refs: run copy and rename through transactions","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-10-05T06:03:07Z","receivedAt":"2026-10-05T06:03:18Z","isPatch":true,"body":"On Fri, Oct 02, 2026 at 04:16:08PM +0200, Maciej Ciemborowicz wrote:\n> On Fri, Oct 2, 2026 at 12:56 PM Patrick Steinhardt wrote:\n> > why can't we make this whole mechanism completely agnostic of the\n> > backend and implement this via pure transactions?\n> \n> The part I was trying to preserve is the existing reflog semantics. A\n> normal ref transaction can express the logical ref updates. In example\n> deleting the old ref and creating/updating the destination. But branch\n> rename/copy also moves or copies the existing reflog history. For the\n> files backend that currently involves filesystem-level reflog\n> rename/copy and D/F handling, while reftable represents the same\n> operation differently. So my assumption was that the logical ref\n> updates could go through the generic transaction API, while the\n> reflog-history operation would remain backend-specific.\n\nYes, the reflog semantics should of course stay the same. But nowadays,\nthis would also be achievable with only backend-agnostic logic as the\nreference transactions have learned to write many reflog entries for a\nsingle reference. This was added back when we introduced the migration\nlogic to convert between two different backends.\n\nNow there's potentially two caveats:\n\n - I don't think we have a way to delete many old reflog entries yet.\n\n - There may be a significant impact on performance.\n\nThe question thus is whether we can avoid or fix those caveats somehow\nand thus arrive at a more future-proof mechanism.\n\n> > Is this new behaviour? Is this retaining old behaviour?\n> \n> The source/destination revalidation is new validation required by\n> introducing the preparing hook before the backend locks are taken. The\n> hook can itself change one of the refs. Without revalidation, the hook\n> payload could describe one state while the rename/copy later operates\n> on another state. The intention is therefore to reject an operation\n> when the state observed by the preparing hook is no longer the state\n> being committed.\n\nI don't feel like that's sensible. The \"preparing\" hook is explicitly\nrun before we perform locking and is documented as such. So it is fully\nexpected that the on-disk state may still change between executing this\nand the \"prepared\" phase. It is the responsibility of the hook author to\nhandle such cases, we shouldn't do this ourselves as we're now starting\nto assume semantics of the hook itself.\n\n> > Sorry, but I'm going to stop reading here. This is not in a state that\n> > is reviewable and has way too much stuff that is obviously generated by\n> > an AI without much thought being put into it by the author. I don't want\n> > to invest my time into a topic where the author has obviously not spent\n> > their time thinking about it, either.\n> \n> I'm really sorry to hear that. Yes, the patches I prepared were\n> AI-assisted, but I do feel that I understand what I am doing. I would\n> appreciate some understanding, though, as I do not work with C on a\n> daily basis. The bug report and my attempt to fix it came from the\n> fact that I am working on a Ruby gem for per-branch and per-worktree\n> containerization. That is why I had to write git-hooks-ext, which is\n> how I ended up running into this bug in the first place.\n> \n> I am not insisting that my patch should be merged. I simply thought\n> that submitting a patch might help get the bug fixed faster, and\n> getting the bug fixed is what I care about most. Karthik Nayak offered\n> to help fix it, so perhaps it would be better for someone who works\n> with C on a daily basis to take it over.\n> \n> I can, of course, also prepare a v3, split it into more commits, and\n> explain my reasoning more clearly. But I cannot guarantee that it will\n> meet your standards, simply because I am not yet familiar with them.\n\nI'd suggest to iterate then. In the current version this patch is not in\na shape that is ready for review. The patch needs to be split up, and\nthere are a lot of gaps in the commit message. Taken together that gives\nthe signal that you don't really understand what you are doing.\n\nThat doesn't mean that you cannot fix that with another iteration\nthough. But I'd suggest to take your time prepping the next iteration to\nread through the code, understand the concepts and doubt what AI spits\nout.\n\nThanks!\n\nPatrick\n"}]}