Show 2045 quoted lines
>
> Reference copy and rename operations bypass the transaction API.
> Consequently, the reference-transaction hook sees only the source deletion
> with the files backend and no useful update with the reftable backend.
>
> Represent both operations as reference transactions containing their
> logical updates. A rename is a deletion of the old reference and creation
> of the new reference in the same transaction. Attach operation-specific
> state to the destination update instead of making copy or rename a property
> of the entire transaction.
>
> Retain backend-specific reflog handling: the files backend stages its
> existing rename procedure across prepare, finish and abort, while reftable
> stages an addition while holding the stack lock. Suppress hooks for the
> files backend's nested deletion transactions so that callers observe one
> logical transaction.
>
> Record and verify the source and destination values after taking backend
> locks. This rejects concurrent changes instead of applying a rename or copy
> that differs from the payload shown to the preparing hook. Preserve D/F
> renames and restore overwritten references and reflogs when a prepared hook
> rejects the operation.
>
> Add tests covering rename, copy, forced updates, both directions of D/F
> conflicts, concurrent updates and prepared-hook rollback.
>
> Helped-by: Karthik Nayak <karthik.188@gmail.com>
> Signed-off-by: Maciej Ciemborowicz <maciej.ciemborowicz@gmail.com>
> ---
> Apologies for the unrelated Subject header on my earlier reply. This
> reroll incorporates the points discussed there.
>
> Changes since v1:
>
> * Keep copy/rename state on the destination ref update instead of the
> generic transaction, so the operation is no longer a transaction-wide
> property.
> * Keep REF_TRANSACTION_FLAG_SKIP_HOOK private to the refs implementation.
> * Use bool for the two-state copy parameter.
> * Apply Junio's commit-message wording suggestions.
>
> Range-diff against v1:
> 1: de0a5a9f7 ! 1: d852537d8 refs: run copy and rename through transactions
> @@ Metadata
> ## Commit message ##
> refs: run copy and rename through transactions
>
> - Reference copy and rename operations currently bypass the transaction API.
> + Reference copy and rename operations bypass the transaction API.
> Consequently, the reference-transaction hook sees only the source deletion
> with the files backend and no useful update with the reftable backend.
>
> Represent both operations as reference transactions containing their
> logical updates. A rename is a deletion of the old reference and creation
> - of the new reference in the same transaction. Retain backend-specific
> - reflog handling: the files backend stages its existing rename procedure
> - across prepare, finish and abort, while reftable stages an addition while
> - holding the stack lock. Suppress hooks for the files backend's nested
> - deletion transactions so that callers observe one logical transaction.
> + of the new reference in the same transaction. Attach operation-specific
> + state to the destination update instead of making copy or rename a property
> + of the entire transaction.
> +
> + Retain backend-specific reflog handling: the files backend stages its
> + existing rename procedure across prepare, finish and abort, while reftable
> + stages an addition while holding the stack lock. Suppress hooks for the
> + files backend's nested deletion transactions so that callers observe one
> + logical transaction.
>
> Record and verify the source and destination values after taking backend
> locks. This rejects concurrent changes instead of applying a rename or copy
> @@ Commit message
> renames and restore overwritten references and reflogs when a prepared hook
> rejects the operation.
>
> - Add coverage for rename, copy, forced updates, both directions of D/F
> + Add tests covering rename, copy, forced updates, both directions of D/F
> conflicts, concurrent updates and prepared-hook rollback.
>
> Helped-by: Karthik Nayak <karthik.188@gmail.com>
> @@ refs.c: int refs_delete_ref(struct ref_store *refs, const char *msg,
> {
> char c;
> @@ refs.c: void ref_transaction_free(struct ref_transaction *transaction)
> + }
> +
> + for (i = 0; i < transaction->nr; i++) {
> ++ struct ref_copy_or_rename_update *operation =
> ++ transaction->updates[i]->copy_or_rename;
> ++
> + free(transaction->updates[i]->msg);
> + free(transaction->updates[i]->committer_info);
> + free((char *)transaction->updates[i]->new_target);
> + free((char *)transaction->updates[i]->old_target);
> + free((char *)transaction->updates[i]->rejection_details);
> ++ if (operation) {
> ++ free(operation->old_refname);
> ++ free(operation->logmsg);
> ++ free(operation->destination_target);
> ++ free(operation);
> ++ }
> + free(transaction->updates[i]);
> + }
>
> - string_list_clear(&transaction->refnames, 0);
> - free(transaction->updates);
> -+ free(transaction->old_refname);
> -+ free(transaction->new_refname);
> -+ free(transaction->logmsg);
> -+ free(transaction->destination_target);
> +@@ refs.c: void ref_transaction_free(struct ref_transaction *transaction)
> free(transaction);
> }
>
> ++struct ref_update *ref_transaction_copy_or_rename_update(
> ++ struct ref_transaction *transaction)
> ++{
> ++ struct ref_update *operation = NULL;
> ++ size_t i;
> ++
> ++ for (i = 0; i < transaction->nr; i++) {
> ++ if (!transaction->updates[i]->copy_or_rename)
> ++ continue;
> ++ if (operation)
> ++ BUG("multiple copy or rename updates in one transaction");
> ++ operation = transaction->updates[i];
> ++ }
> ++
> ++ return operation;
> ++}
> ++
> + int ref_transaction_maybe_set_rejected(struct ref_transaction *transaction,
> + size_t update_idx,
> + enum ref_transaction_error err,
> @@ refs.c: int ref_transaction_prepare(struct ref_transaction *transaction,
> return REF_TRANSACTION_ERROR_GENERIC;
>
> @@ refs.c: int refs_delete_refs(struct ref_store *refs, const char *logmsg,
> - const char *newref, const char *logmsg)
> +static int refs_copy_or_rename_ref(struct ref_store *refs, const char *oldref,
> + const char *newref, const char *logmsg,
> -+ int copy)
> ++ bool copy)
> {
> - char *msg;
> - int retval;
> + struct ref_transaction *transaction = NULL;
> ++ struct ref_copy_or_rename_update *operation = NULL;
> ++ struct ref_update *destination_update;
> + struct object_id old_oid, new_oid;
> + struct strbuf new_target = STRBUF_INIT;
> + struct strbuf err = STRBUF_INIT;
> @@ refs.c: int refs_delete_refs(struct ref_store *refs, const char *logmsg,
> + transaction = ref_store_transaction_begin(refs, 0, &err);
> + if (!transaction)
> + goto error;
> -+ transaction->type = copy ? REF_TRANSACTION_TYPE_COPY :
> -+ REF_TRANSACTION_TYPE_RENAME;
> -+ transaction->old_refname = xstrdup(oldref);
> -+ transaction->new_refname = xstrdup(newref);
> -+ transaction->logmsg = xstrdup(msg);
> -+ oidcpy(&transaction->source_oid, &old_oid);
> -+
> + if (!copy && ref_transaction_delete(transaction, oldref, &old_oid, NULL,
> + REF_NO_DEREF, msg, &err))
> + goto error;
> @@ refs.c: int refs_delete_refs(struct ref_store *refs, const char *logmsg,
> + } else {
> + oidclr(&new_oid, refs->repo->hash_algo);
> + }
> -+ transaction->destination_exists = new_exists;
> -+ if (new_flags & REF_ISSYMREF)
> -+ transaction->destination_target = xstrdup(new_target.buf);
> -+ else if (transaction->destination_exists)
> -+ oidcpy(&transaction->destination_oid, &new_oid);
> -+
> + if (ref_transaction_update(transaction, newref, &old_oid,
> + (new_flags & REF_ISSYMREF) ? NULL : &new_oid,
> + NULL,
> @@ refs.c: int refs_delete_refs(struct ref_store *refs, const char *logmsg,
> + NULL, &err))
> + goto error;
> +
> ++ destination_update = transaction->updates[transaction->nr - 1];
> ++ CALLOC_ARRAY(operation, 1);
> ++ operation->type = copy ? REF_UPDATE_COPY : REF_UPDATE_RENAME;
> ++ operation->old_refname = xstrdup(oldref);
> ++ operation->logmsg = xstrdup(msg);
> ++ oidcpy(&operation->source_oid, &old_oid);
> ++ operation->destination_exists = new_exists;
> ++ if (new_flags & REF_ISSYMREF)
> ++ operation->destination_target = xstrdup(new_target.buf);
> ++ else if (operation->destination_exists)
> ++ oidcpy(&operation->destination_oid, &new_oid);
> ++ destination_update->copy_or_rename = operation;
> ++
> + if (ref_transaction_commit(transaction, &err))
> + goto error;
> +
> @@ refs.c: int refs_delete_refs(struct ref_store *refs, const char *logmsg,
>
> const char *ref_update_original_update_refname(struct ref_update *update)
>
> - ## refs.h ##
> -@@ refs.h: enum ref_transaction_flag {
> - * while rejecting updates which do not match the expected state.
> - */
> - REF_TRANSACTION_ALLOW_FAILURE = (1 << 1),
> -+
> -+ /* Suppress hooks for an update nested in another transaction. */
> -+ REF_TRANSACTION_FLAG_SKIP_HOOK = (1 << 2),
> - };
> -
> - /*
> -
> ## refs/debug.c ##
> @@ refs/debug.c: static int debug_optimize_required(struct ref_store *ref_store,
> return res;
> @@ refs/files-backend.c: static int refs_rename_ref_available(struct ref_store *ref
> +};
> +
> static int files_copy_or_rename_ref(struct ref_store *ref_store,
> - const char *oldrefname, const char *newrefname,
> +- const char *oldrefname, const char *newrefname,
> - const char *logmsg, int copy)
> -+ const char *logmsg, int copy,
> -+ struct ref_transaction *transaction)
> ++ struct ref_update *update,
> ++ struct ref_transaction *transaction)
> {
> struct files_ref_store *refs =
> - files_downcast(ref_store, REF_STORE_WRITE, "rename_ref");
> + files_downcast(ref_store, REF_STORE_WRITE,
> + "ref_transaction_prepare");
> ++ struct ref_copy_or_rename_update *operation = update->copy_or_rename;
> ++ const char *oldrefname = operation->old_refname;
> ++ const char *newrefname = update->refname;
> ++ const char *logmsg = operation->logmsg;
> ++ bool copy = operation->type == REF_UPDATE_COPY;
> struct object_id orig_oid;
> int flag = 0, logmoved = 0;
> struct ref_lock *lock;
> @@ refs/files-backend.c: static int files_copy_or_rename_ref(struct ref_store *ref_
> oldrefname);
> goto out;
> }
> -+ if (!oideq(&orig_oid, &transaction->source_oid)) {
> ++ if (!oideq(&orig_oid, &operation->source_oid)) {
> + ret = error("refname %s is at %s but expected %s",
> + oldrefname, oid_to_hex(&orig_oid),
> -+ oid_to_hex(&transaction->source_oid));
> ++ oid_to_hex(&operation->source_oid));
> + goto out;
> + }
> if (!refs_rename_ref_available(&refs->base, oldrefname, newrefname)) {
> @@ refs/files-backend.c: static int files_copy_or_rename_ref(struct ref_store *ref_
> + goto out;
> + }
> + }
> -+ if (destination_exists != transaction->destination_exists) {
> ++ if (destination_exists != operation->destination_exists) {
> + ret = error("refname %s changed while renaming", newrefname);
> + goto out;
> + }
> + if (destination_exists) {
> + if (destination_flags & REF_ISSYMREF) {
> -+ if (!transaction->destination_target ||
> ++ if (!operation->destination_target ||
> + strcmp(destination_target.buf,
> -+ transaction->destination_target)) {
> ++ operation->destination_target)) {
> + ret = error("refname %s changed while renaming",
> + newrefname);
> + goto out;
> + }
> -+ } else if (transaction->destination_target ||
> ++ } else if (operation->destination_target ||
> + !oideq(&destination_oid,
> -+ &transaction->destination_oid)) {
> ++ &operation->destination_oid)) {
> + ret = error("refname %s changed while renaming", newrefname);
> + goto out;
> + }
> @@ refs/files-backend.c: static int files_transaction_prepare(struct ref_store *ref
> struct ref_transaction *packed_transaction = NULL;
>
> assert(err);
> -+ if (transaction->type != REF_TRANSACTION_TYPE_NORMAL)
> -+ return files_copy_or_rename_ref(ref_store,
> -+ transaction->old_refname,
> -+ transaction->new_refname,
> -+ transaction->logmsg,
> -+ transaction->type == REF_TRANSACTION_TYPE_COPY,
> -+ transaction);
> ++ {
> ++ struct ref_update *operation =
> ++ ref_transaction_copy_or_rename_update(transaction);
> ++
> ++ if (operation)
> ++ return files_copy_or_rename_ref(ref_store, operation,
> ++ transaction);
> ++ }
>
> if (transaction->flags & REF_TRANSACTION_FLAG_INITIAL)
> goto cleanup;
> @@ refs/files-backend.c: static int files_transaction_finish(struct ref_store *ref_
>
>
> assert(err);
> -+ if (transaction->type != REF_TRANSACTION_TYPE_NORMAL) {
> -+ struct files_copy_or_rename_transaction_data *data =
> -+ transaction->backend_data;
> -+ int special_ret;
> ++ {
> ++ struct ref_update *update =
> ++ ref_transaction_copy_or_rename_update(transaction);
> +
> -+ special_ret = commit_ref_update(refs, data->lock, &data->orig_oid,
> -+ transaction->logmsg, 0, err);
> -+ if (special_ret) {
> -+ error("unable to write current sha1 into %s: %s",
> -+ transaction->new_refname, err->buf);
> -+ data->lock = NULL;
> -+ files_transaction_abort(ref_store, transaction, err);
> -+ return special_ret;
> -+ } else if (data->destination_log_backed_up) {
> -+ struct strbuf path = STRBUF_INIT;
> ++ if (update) {
> ++ struct ref_copy_or_rename_update *operation =
> ++ update->copy_or_rename;
> ++ struct files_copy_or_rename_transaction_data *data =
> ++ transaction->backend_data;
> ++ int special_ret;
> ++
> ++ special_ret = commit_ref_update(refs, data->lock, &data->orig_oid,
> ++ operation->logmsg, 0, err);
> ++ if (special_ret) {
> ++ error("unable to write current sha1 into %s: %s",
> ++ update->refname, err->buf);
> ++ data->lock = NULL;
> ++ files_transaction_abort(ref_store, transaction, err);
> ++ return special_ret;
> ++ } else if (data->destination_log_backed_up) {
> ++ struct strbuf path = STRBUF_INIT;
> +
> -+ files_reflog_path(refs, &path, TMP_RENAMED_LOG_DESTINATION);
> -+ if (unlink(path.buf) < 0 && errno != ENOENT)
> -+ warning_errno("unable to remove '%s'", path.buf);
> -+ strbuf_release(&path);
> ++ files_reflog_path(refs, &path, TMP_RENAMED_LOG_DESTINATION);
> ++ if (unlink(path.buf) < 0 && errno != ENOENT)
> ++ warning_errno("unable to remove '%s'", path.buf);
> ++ strbuf_release(&path);
> ++ }
> ++ free(data->destination_target);
> ++ free(data);
> ++ transaction->backend_data = NULL;
> ++ transaction->state = REF_TRANSACTION_CLOSED;
> ++ return special_ret;
> + }
> -+ free(data->destination_target);
> -+ free(data);
> -+ transaction->backend_data = NULL;
> -+ transaction->state = REF_TRANSACTION_CLOSED;
> -+ return special_ret;
> + }
>
> if (transaction->flags & REF_TRANSACTION_FLAG_INITIAL)
> @@ refs/files-backend.c: static int files_transaction_finish(struct ref_store *ref_
> struct files_ref_store *refs =
> files_downcast(ref_store, 0, "ref_transaction_abort");
>
> -+ if (transaction->type != REF_TRANSACTION_TYPE_NORMAL) {
> -+ struct files_copy_or_rename_transaction_data *data =
> -+ transaction->backend_data;
> -+ struct strbuf new_log = STRBUF_INIT;
> -+ struct strbuf destination_log = STRBUF_INIT;
> -+ struct strbuf temporary_log = STRBUF_INIT;
> -+ struct ref_transaction *restore_transaction = NULL;
> -+ struct ref_lock *lock;
> -+ int ret = 0;
> ++ {
> ++ struct ref_update *update =
> ++ ref_transaction_copy_or_rename_update(transaction);
> +
> -+ if (data->lock)
> -+ unlock_ref(data->lock);
> -+ if (transaction->type == REF_TRANSACTION_TYPE_RENAME) {
> -+ lock = lock_ref_oid_basic(refs, transaction->old_refname, err);
> -+ if (!lock ||
> -+ write_ref_to_lockfile(refs, lock, &data->orig_oid, err) ||
> -+ commit_ref_update(refs, lock, &data->orig_oid, NULL,
> -+ REF_SKIP_CREATE_REFLOG, err))
> -+ ret = -1;
> -+ }
> ++ if (update) {
> ++ struct ref_copy_or_rename_update *operation =
> ++ update->copy_or_rename;
> ++ struct files_copy_or_rename_transaction_data *data =
> ++ transaction->backend_data;
> ++ struct strbuf new_log = STRBUF_INIT;
> ++ struct strbuf destination_log = STRBUF_INIT;
> ++ struct strbuf temporary_log = STRBUF_INIT;
> ++ struct ref_transaction *restore_transaction = NULL;
> ++ struct ref_lock *lock;
> ++ int ret = 0;
> +
> -+ if (data->logmoved) {
> -+ files_reflog_path(refs, &new_log, transaction->new_refname);
> -+ if (transaction->type == REF_TRANSACTION_TYPE_RENAME) {
> -+ files_reflog_path(refs, &temporary_log, TMP_RENAMED_LOG);
> -+ if (rename(new_log.buf, temporary_log.buf) < 0) {
> -+ strbuf_addf(err, "unable to restore logfile %s: %s",
> -+ transaction->old_refname, strerror(errno));
> ++ if (data->lock)
> ++ unlock_ref(data->lock);
> ++ if (operation->type == REF_UPDATE_RENAME) {
> ++ lock = lock_ref_oid_basic(refs, operation->old_refname, err);
> ++ if (!lock ||
> ++ write_ref_to_lockfile(refs, lock, &data->orig_oid, err) ||
> ++ commit_ref_update(refs, lock, &data->orig_oid, NULL,
> ++ REF_SKIP_CREATE_REFLOG, err))
> + ret = -1;
> -+ } else {
> -+ try_remove_empty_parents(refs,
> -+ transaction->new_refname,
> -+ REMOVE_EMPTY_PARENTS_REFLOG);
> -+ if (rename_tmp_log(refs,
> -+ transaction->old_refname)) {
> ++ }
> ++
> ++ if (data->logmoved) {
> ++ files_reflog_path(refs, &new_log, update->refname);
> ++ if (operation->type == REF_UPDATE_RENAME) {
> ++ files_reflog_path(refs, &temporary_log, TMP_RENAMED_LOG);
> ++ if (rename(new_log.buf, temporary_log.buf) < 0) {
> + strbuf_addf(err, "unable to restore logfile %s: %s",
> -+ transaction->old_refname,
> -+ strerror(errno));
> ++ operation->old_refname, strerror(errno));
> + ret = -1;
> ++ } else {
> ++ try_remove_empty_parents(refs,
> ++ update->refname,
> ++ REMOVE_EMPTY_PARENTS_REFLOG);
> ++ if (rename_tmp_log(refs,
> ++ operation->old_refname)) {
> ++ strbuf_addf(err, "unable to restore logfile %s: %s",
> ++ operation->old_refname,
> ++ strerror(errno));
> ++ ret = -1;
> ++ }
> + }
> ++ } else if (unlink(new_log.buf) < 0 && errno != ENOENT) {
> ++ strbuf_addf(err, "unable to remove logfile %s: %s",
> ++ update->refname, strerror(errno));
> ++ ret = -1;
> + }
> -+ } else if (unlink(new_log.buf) < 0 && errno != ENOENT) {
> -+ strbuf_addf(err, "unable to remove logfile %s: %s",
> -+ transaction->new_refname, strerror(errno));
> -+ ret = -1;
> + }
> -+ }
> -+ if (data->destination_log_backed_up) {
> -+ files_reflog_path(refs, &destination_log,
> -+ TMP_RENAMED_LOG_DESTINATION);
> -+ if (rename(destination_log.buf, new_log.buf) < 0) {
> -+ strbuf_addf(err, "unable to restore logfile %s: %s",
> -+ transaction->new_refname, strerror(errno));
> -+ ret = -1;
> ++ if (data->destination_log_backed_up) {
> ++ files_reflog_path(refs, &destination_log,
> ++ TMP_RENAMED_LOG_DESTINATION);
> ++ if (rename(destination_log.buf, new_log.buf) < 0) {
> ++ strbuf_addf(err, "unable to restore logfile %s: %s",
> ++ update->refname, strerror(errno));
> ++ ret = -1;
> ++ }
> + }
> -+ }
> +
> -+ if (transaction->type == REF_TRANSACTION_TYPE_RENAME &&
> -+ data->destination_exists) {
> -+ restore_transaction = ref_store_transaction_begin(
> ++ if (operation->type == REF_UPDATE_RENAME &&
> ++ data->destination_exists) {
> ++ restore_transaction = ref_store_transaction_begin(
> + &refs->base, REF_TRANSACTION_FLAG_SKIP_HOOK, err);
> -+ if (!restore_transaction ||
> -+ ref_transaction_update(restore_transaction,
> -+ transaction->new_refname,
> -+ data->destination_target ? NULL :
> -+ &data->destination_oid,
> -+ NULL,
> -+ data->destination_target,
> -+ NULL,
> -+ REF_NO_DEREF |
> -+ REF_SKIP_CREATE_REFLOG,
> -+ NULL, err) ||
> -+ ref_transaction_commit(restore_transaction, err))
> -+ ret = -1;
> -+ ref_transaction_free(restore_transaction);
> -+ }
> ++ if (!restore_transaction ||
> ++ ref_transaction_update(restore_transaction,
> ++ update->refname,
> ++ data->destination_target ? NULL :
> ++ &data->destination_oid,
> ++ NULL,
> ++ data->destination_target,
> ++ NULL,
> ++ REF_NO_DEREF |
> ++ REF_SKIP_CREATE_REFLOG,
> ++ NULL, err) ||
> ++ ref_transaction_commit(restore_transaction, err))
> ++ ret = -1;
> ++ ref_transaction_free(restore_transaction);
> ++ }
> +
> -+ strbuf_release(&destination_log);
> -+ strbuf_release(&temporary_log);
> -+ strbuf_release(&new_log);
> -+ free(data->destination_target);
> -+ free(data);
> -+ transaction->backend_data = NULL;
> -+ transaction->state = REF_TRANSACTION_CLOSED;
> -+ return ret;
> ++ strbuf_release(&destination_log);
> ++ strbuf_release(&temporary_log);
> ++ strbuf_release(&new_log);
> ++ free(data->destination_target);
> ++ free(data);
> ++ transaction->backend_data = NULL;
> ++ transaction->state = REF_TRANSACTION_CLOSED;
> ++ return ret;
> ++ }
> + }
> +
> files_transaction_cleanup(refs, transaction);
> @@ refs/packed-backend.c: struct ref_storage_be refs_be_packed = {
> .read_raw_ref = packed_read_raw_ref,
>
> ## refs/refs-internal.h ##
> +@@ refs/refs-internal.h: struct ref_update {
> + */
> + struct ref_update *parent_update;
> +
> ++ /*
> ++ * Copy and rename operations require backend-specific handling while
> ++ * still exposing their logical updates to transaction hooks. Keep that
> ++ * state on the destination update so it composes with other updates in
> ++ * the transaction instead of making copy or rename a transaction-wide
> ++ * property.
> ++ */
> ++ struct ref_copy_or_rename_update *copy_or_rename;
> ++
> + const char refname[FLEX_ARRAY];
> + };
> +
> ++enum ref_copy_or_rename_type {
> ++ REF_UPDATE_RENAME,
> ++ REF_UPDATE_COPY,
> ++};
> ++
> ++struct ref_copy_or_rename_update {
> ++ enum ref_copy_or_rename_type type;
> ++ char *old_refname;
> ++ char *logmsg;
> ++ struct object_id source_oid;
> ++ struct object_id destination_oid;
> ++ char *destination_target;
> ++ unsigned int destination_exists:1;
> ++};
> ++
> + int refs_read_raw_ref(struct ref_store *ref_store, const char *refname,
> + struct object_id *oid, struct strbuf *referent,
> + unsigned int *type, int *failure_errno);
> @@ refs/refs-internal.h: struct ref_update *ref_transaction_add_update(
> const char *committer_info,
> const char *msg);
> @@ refs/refs-internal.h: struct ref_update *ref_transaction_add_update(
> /*
> * Transaction states.
> *
> -@@ refs/refs-internal.h: enum ref_transaction_state {
> - REF_TRANSACTION_CLOSED = 2
> - };
> -
> -+enum ref_transaction_type {
> -+ REF_TRANSACTION_TYPE_NORMAL = 0,
> -+ REF_TRANSACTION_TYPE_RENAME,
> -+ REF_TRANSACTION_TYPE_COPY,
> -+};
> -+
> - /*
> - * Data structure to hold indices of updates which were rejected, for batched
> - * reference updates. While the updates themselves hold the rejection error,
> @@ refs/refs-internal.h: struct ref_transaction {
> - void *backend_data;
> - unsigned int flags;
> uint64_t max_index;
> -+
> -+ /*
> -+ * Rename and copy operations need backend-specific reflog handling.
> -+ * Their logical updates still live in `updates`, so hooks see the
> -+ * operation like any other reference transaction. The fields below
> -+ * retain the state that backends verify after taking their locks.
> -+ */
> -+ enum ref_transaction_type type;
> -+ char *old_refname;
> -+ char *new_refname;
> -+ char *logmsg;
> -+ struct object_id source_oid;
> -+ struct object_id destination_oid;
> -+ char *destination_target;
> -+ unsigned int destination_exists:1;
> };
>
> ++/* Suppress hooks for a transaction nested inside another refs operation. */
> ++#define REF_TRANSACTION_FLAG_SKIP_HOOK (1 << 2)
> ++
> ++struct ref_update *ref_transaction_copy_or_rename_update(
> ++ struct ref_transaction *transaction);
> ++
> /*
> + * Check for entries in extras that are within the specified
> + * directory, where dirname is a reference directory name including
> @@ refs/refs-internal.h: typedef int optimize_required_fn(struct ref_store *ref_store,
> struct refs_optimize_opts *opts,
> bool *required);
> @@ refs/reftable-backend.c: static int reftable_be_transaction_prepare(struct ref_s
> size_t i;
> int ret;
>
> -+ if (transaction->type != REF_TRANSACTION_TYPE_NORMAL)
> ++ if (ref_transaction_copy_or_rename_update(transaction))
> + return reftable_be_copy_or_rename_prepare(ref_store, transaction,
> + err);
> +
> @@ refs/reftable-backend.c: static int reftable_be_transaction_abort(struct ref_sto
> - struct reftable_transaction_data *tx_data = transaction->backend_data;
> + struct reftable_transaction_data *tx_data;
> +
> -+ if (transaction->type != REF_TRANSACTION_TYPE_NORMAL) {
> ++ if (ref_transaction_copy_or_rename_update(transaction)) {
> + struct reftable_copy_or_rename_transaction_data *data =
> + transaction->backend_data;
> +
> @@ refs/reftable-backend.c: static int reftable_be_transaction_finish(struct ref_st
> + struct reftable_transaction_data *tx_data;
> int ret = 0;
>
> -+ if (transaction->type != REF_TRANSACTION_TYPE_NORMAL) {
> ++ if (ref_transaction_copy_or_rename_update(transaction)) {
> + struct reftable_copy_or_rename_transaction_data *data =
> + transaction->backend_data;
> + int special_ret = reftable_addition_commit(data->addition);
> @@ refs/reftable-backend.c: struct write_create_symref_arg {
> const char *newname;
> const char *logmsg;
> int delete_old;
> -+ struct ref_transaction *transaction;
> ++ struct ref_copy_or_rename_update *operation;
> };
>
> static int write_copy_table(struct reftable_writer *writer, void *cb_data)
> @@ refs/reftable-backend.c: static int write_copy_table(struct reftable_writer *wri
> + ret = -1;
> goto done;
> }
> -+ if (arg->transaction) {
> ++ if (arg->operation) {
> + struct object_id oid;
> +
> + if (old_ref.value_type == REFTABLE_REF_VAL2)
> @@ refs/reftable-backend.c: static int write_copy_table(struct reftable_writer *wri
> + else
> + oidread(&oid, old_ref.value.val1,
> + arg->refs->base.repo->hash_algo);
> -+ if (!oideq(&oid, &arg->transaction->source_oid)) {
> ++ if (!oideq(&oid, &arg->operation->source_oid)) {
> + strbuf_addf(arg->err,
> + _("refname %s is at %s but expected %s"),
> + arg->oldname, oid_to_hex(&oid),
> -+ oid_to_hex(&arg->transaction->source_oid));
> ++ oid_to_hex(&arg->operation->source_oid));
> + ret = -1;
> + goto done;
> + }
> @@ refs/reftable-backend.c: static int write_copy_table(struct reftable_writer *wri
> + &destination_ref);
> + if (ret < 0)
> + goto done;
> -+ if (arg->transaction->destination_exists != !ret) {
> ++ if (arg->operation->destination_exists != !ret) {
> + strbuf_addf(arg->err,
> + _("refname %s changed while renaming"),
> + arg->newname);
> @@ refs/reftable-backend.c: static int write_copy_table(struct reftable_writer *wri
> + }
> + if (!ret) {
> + if (destination_ref.value_type == REFTABLE_REF_SYMREF) {
> -+ if (!arg->transaction->destination_target ||
> ++ if (!arg->operation->destination_target ||
> + strcmp(destination_ref.value.symref,
> -+ arg->transaction->destination_target)) {
> ++ arg->operation->destination_target)) {
> + strbuf_addf(arg->err,
> + _("refname %s changed while renaming"),
> + arg->newname);
> @@ refs/reftable-backend.c: static int write_copy_table(struct reftable_writer *wri
> + else
> + oidread(&oid, destination_ref.value.val1,
> + arg->refs->base.repo->hash_algo);
> -+ if (arg->transaction->destination_target ||
> -+ !oideq(&oid, &arg->transaction->destination_oid)) {
> ++ if (arg->operation->destination_target ||
> ++ !oideq(&oid, &arg->operation->destination_oid)) {
> + strbuf_addf(arg->err,
> + _("refname %s changed while renaming"),
> + arg->newname);
> @@ refs/reftable-backend.c: static int write_copy_table(struct reftable_writer *wri
> + reftable_be_downcast(ref_store, REF_STORE_WRITE,
> + "ref_transaction_prepare");
> + struct reftable_copy_or_rename_transaction_data *data = NULL;
> ++ struct ref_update *update =
> ++ ref_transaction_copy_or_rename_update(transaction);
> ++ struct ref_copy_or_rename_update *operation = update->copy_or_rename;
> struct write_copy_arg arg = {
> .refs = refs,
> - .oldname = oldrefname,
> @@ refs/reftable-backend.c: static int write_copy_table(struct reftable_writer *wri
> - .logmsg = logmsg,
> - .delete_old = 1,
> + .err = err,
> -+ .oldname = transaction->old_refname,
> -+ .newname = transaction->new_refname,
> -+ .logmsg = transaction->logmsg,
> -+ .delete_old = transaction->type == REF_TRANSACTION_TYPE_RENAME,
> -+ .transaction = transaction,
> ++ .oldname = operation->old_refname,
> ++ .newname = update->refname,
> ++ .logmsg = operation->logmsg,
> ++ .delete_old = operation->type == REF_UPDATE_RENAME,
> ++ .operation = operation,
> };
> int ret;
>
> @@ refs/reftable-backend.c: static int write_copy_table(struct reftable_writer *wri
> goto done;
> -
> - ret = backend_for(&arg.be, refs, newrefname, &newrefname, 1);
> -+ ret = backend_for(&arg.be, refs, transaction->new_refname,
> ++ ret = backend_for(&arg.be, refs, update->refname,
> + &arg.newname, 1);
> if (ret)
> goto done;
>
> refs.c | 161 ++++++++++++++---
> refs/debug.c | 25 ---
> refs/files-backend.c | 297 +++++++++++++++++++++++++++----
> refs/packed-backend.c | 2 -
> refs/refs-internal.h | 47 +++--
> refs/reftable-backend.c | 197 +++++++++++++++-----
> t/t1416-ref-transaction-hooks.sh | 142 +++++++++++++++
> 7 files changed, 732 insertions(+), 139 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index 92d5df5b7..f036ae4b9 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -1004,15 +1004,17 @@ long get_files_ref_lock_timeout_ms(struct repository *repo)
> return timeout_ms;
> }
>
> -int refs_delete_ref(struct ref_store *refs, const char *msg,
> - const char *refname,
> - const struct object_id *old_oid,
> - unsigned int flags)
> +int refs_delete_ref_with_transaction_flags(struct ref_store *refs,
> + const char *msg,
> + const char *refname,
> + const struct object_id *old_oid,
> + unsigned int flags,
> + unsigned int transaction_flags)
> {
> struct ref_transaction *transaction;
> struct strbuf err = STRBUF_INIT;
>
> - transaction = ref_store_transaction_begin(refs, 0, &err);
> + transaction = ref_store_transaction_begin(refs, transaction_flags, &err);
> if (!transaction ||
> ref_transaction_delete(transaction, refname, old_oid,
> NULL, flags, msg, &err) ||
> @@ -1027,6 +1029,15 @@ int refs_delete_ref(struct ref_store *refs, const char *msg,
> return 0;
> }
>
> +int refs_delete_ref(struct ref_store *refs, const char *msg,
> + const char *refname,
> + const struct object_id *old_oid,
> + unsigned int flags)
> +{
> + return refs_delete_ref_with_transaction_flags(refs, msg, refname,
> + old_oid, flags, 0);
> +}
> +
> static void copy_reflog_msg(struct strbuf *sb, const char *msg)
> {
> char c;
> @@ -1256,11 +1267,20 @@ void ref_transaction_free(struct ref_transaction *transaction)
> }
>
> for (i = 0; i < transaction->nr; i++) {
> + struct ref_copy_or_rename_update *operation =
> + transaction->updates[i]->copy_or_rename;
> +
> free(transaction->updates[i]->msg);
> free(transaction->updates[i]->committer_info);
> free((char *)transaction->updates[i]->new_target);
> free((char *)transaction->updates[i]->old_target);
> free((char *)transaction->updates[i]->rejection_details);
> + if (operation) {
> + free(operation->old_refname);
> + free(operation->logmsg);
> + free(operation->destination_target);
> + free(operation);
> + }
> free(transaction->updates[i]);
> }
>
> @@ -1273,6 +1293,23 @@ void ref_transaction_free(struct ref_transaction *transaction)
> free(transaction);
> }
>
> +struct ref_update *ref_transaction_copy_or_rename_update(
> + struct ref_transaction *transaction)
> +{
> + struct ref_update *operation = NULL;
> + size_t i;
> +
> + for (i = 0; i < transaction->nr; i++) {
> + if (!transaction->updates[i]->copy_or_rename)
> + continue;
> + if (operation)
> + BUG("multiple copy or rename updates in one transaction");
> + operation = transaction->updates[i];
> + }
> +
> + return operation;
> +}
> +
> int ref_transaction_maybe_set_rejected(struct ref_transaction *transaction,
> size_t update_idx,
> enum ref_transaction_error err,
> @@ -2710,7 +2747,8 @@ int ref_transaction_prepare(struct ref_transaction *transaction,
> return REF_TRANSACTION_ERROR_GENERIC;
>
> /* Preparing checks before locking references */
> - ret = run_transaction_hook(transaction, "preparing");
> + ret = transaction->flags & REF_TRANSACTION_FLAG_SKIP_HOOK ? 0 :
> + run_transaction_hook(transaction, "preparing");
> if (ret) {
> ref_transaction_abort(transaction, err);
> die(_(abort_by_ref_transaction_hook), "preparing");
> @@ -2720,7 +2758,8 @@ int ref_transaction_prepare(struct ref_transaction *transaction,
> if (ret)
> return ret;
>
> - ret = run_transaction_hook(transaction, "prepared");
> + ret = transaction->flags & REF_TRANSACTION_FLAG_SKIP_HOOK ? 0 :
> + run_transaction_hook(transaction, "prepared");
> if (ret) {
> ref_transaction_abort(transaction, err);
> die(_(abort_by_ref_transaction_hook), "prepared");
> @@ -2750,7 +2789,8 @@ int ref_transaction_abort(struct ref_transaction *transaction,
> break;
> }
>
> - run_transaction_hook(transaction, "aborted");
> + if (!(transaction->flags & REF_TRANSACTION_FLAG_SKIP_HOOK))
> + run_transaction_hook(transaction, "aborted");
>
> ref_transaction_free(transaction);
> return ret;
> @@ -2781,7 +2821,8 @@ int ref_transaction_commit(struct ref_transaction *transaction,
> }
>
> ret = refs->be->transaction_finish(refs, transaction, err);
> - if (!ret && !(transaction->flags & REF_TRANSACTION_FLAG_INITIAL))
> + if (!ret && !(transaction->flags & (REF_TRANSACTION_FLAG_INITIAL |
> + REF_TRANSACTION_FLAG_SKIP_HOOK)))
> run_transaction_hook(transaction, "committed");
> return ret;
> }
> @@ -3123,28 +3164,102 @@ int refs_delete_refs(struct ref_store *refs, const char *logmsg,
> return ret;
> }
>
> -int refs_rename_ref(struct ref_store *refs, const char *oldref,
> - const char *newref, const char *logmsg)
> +static int refs_copy_or_rename_ref(struct ref_store *refs, const char *oldref,
> + const char *newref, const char *logmsg,
> + bool copy)
> {
> - char *msg;
> - int retval;
> + struct ref_transaction *transaction = NULL;
> + struct ref_copy_or_rename_update *operation = NULL;
> + struct ref_update *destination_update;
> + struct object_id old_oid, new_oid;
> + struct strbuf new_target = STRBUF_INIT;
> + struct strbuf err = STRBUF_INIT;
> + char *msg = normalize_reflog_message(logmsg);
> + int old_flags, new_flags = 0, new_exists = 0, ret = 1;
>
> - msg = normalize_reflog_message(logmsg);
> - retval = refs->be->rename_ref(refs, oldref, newref, msg);
> + if (!strcmp(oldref, newref)) {
> + ret = 0;
> + goto out;
> + }
> +
> + if (!refs_resolve_ref_unsafe(refs, oldref,
> + RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,
> + &old_oid, &old_flags)) {
> + error("refname %s not found", oldref);
> + goto out;
> + }
> + if (old_flags & REF_ISSYMREF) {
> + error("refname %s is a symbolic ref, %s it is not supported",
> + oldref, copy ? "copying" : "renaming");
> + goto out;
> + }
> +
> + transaction = ref_store_transaction_begin(refs, 0, &err);
> + if (!transaction)
> + goto error;
> + if (!copy && ref_transaction_delete(transaction, oldref, &old_oid, NULL,
> + REF_NO_DEREF, msg, &err))
> + goto error;
> +
> + if (refs_resolve_ref_unsafe(refs, newref,
> + RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,
> + &new_oid, &new_flags)) {
> + new_exists = 1;
> + if ((new_flags & REF_ISSYMREF) &&
> + refs_read_symbolic_ref(refs, newref, &new_target) < 0) {
> + strbuf_addf(&err, "unable to read symbolic ref %s", newref);
> + goto error;
> + }
> + } else {
> + oidclr(&new_oid, refs->repo->hash_algo);
> + }
> + if (ref_transaction_update(transaction, newref, &old_oid,
> + (new_flags & REF_ISSYMREF) ? NULL : &new_oid,
> + NULL,
> + (new_flags & REF_ISSYMREF) ? new_target.buf : NULL,
> + REF_NO_DEREF | REF_SKIP_CREATE_REFLOG,
> + NULL, &err))
> + goto error;
> +
> + destination_update = transaction->updates[transaction->nr - 1];
> + CALLOC_ARRAY(operation, 1);
> + operation->type = copy ? REF_UPDATE_COPY : REF_UPDATE_RENAME;
> + operation->old_refname = xstrdup(oldref);
> + operation->logmsg = xstrdup(msg);
> + oidcpy(&operation->source_oid, &old_oid);
> + operation->destination_exists = new_exists;
> + if (new_flags & REF_ISSYMREF)
> + operation->destination_target = xstrdup(new_target.buf);
> + else if (operation->destination_exists)
> + oidcpy(&operation->destination_oid, &new_oid);
> + destination_update->copy_or_rename = operation;
> +
> + if (ref_transaction_commit(transaction, &err))
> + goto error;
> +
> + ret = 0;
> + goto out;
> +
> +error:
> + error("%s", err.buf);
> +out:
> + ref_transaction_free(transaction);
> + strbuf_release(&new_target);
> + strbuf_release(&err);
> free(msg);
> - return retval;
> + return ret;
> }
>
> -int refs_copy_existing_ref(struct ref_store *refs, const char *oldref,
> +int refs_rename_ref(struct ref_store *refs, const char *oldref,
> const char *newref, const char *logmsg)
> {
> - char *msg;
> - int retval;
> + return refs_copy_or_rename_ref(refs, oldref, newref, logmsg, 0);
> +}
>
> - msg = normalize_reflog_message(logmsg);
> - retval = refs->be->copy_ref(refs, oldref, newref, msg);
> - free(msg);
> - return retval;
> +int refs_copy_existing_ref(struct ref_store *refs, const char *oldref,
> + const char *newref, const char *logmsg)
> +{
> + return refs_copy_or_rename_ref(refs, oldref, newref, logmsg, 1);
> }
>
> const char *ref_update_original_update_refname(struct ref_update *update)
> diff --git a/refs/debug.c b/refs/debug.c
> index 639db0f26..87b84e767 100644
> --- a/refs/debug.c
> +++ b/refs/debug.c
> @@ -143,28 +143,6 @@ static int debug_optimize_required(struct ref_store *ref_store,
> return res;
> }
>
> -static int debug_rename_ref(struct ref_store *ref_store, const char *oldref,
> - const char *newref, const char *logmsg)
> -{
> - struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;
> - int res = drefs->refs->be->rename_ref(drefs->refs, oldref, newref,
> - logmsg);
> - trace_printf_key(&trace_refs, "rename_ref: %s -> %s \"%s\": %d\n", oldref, newref,
> - logmsg, res);
> - return res;
> -}
> -
> -static int debug_copy_ref(struct ref_store *ref_store, const char *oldref,
> - const char *newref, const char *logmsg)
> -{
> - struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;
> - int res =
> - drefs->refs->be->copy_ref(drefs->refs, oldref, newref, logmsg);
> - trace_printf_key(&trace_refs, "copy_ref: %s -> %s \"%s\": %d\n", oldref, newref,
> - logmsg, res);
> - return res;
> -}
> -
> struct debug_ref_iterator {
> struct ref_iterator base;
> struct ref_iterator *iter;
> @@ -453,9 +431,6 @@ struct ref_storage_be refs_be_debug = {
> .optimize = debug_optimize,
> .optimize_required = debug_optimize_required,
>
> - .rename_ref = debug_rename_ref,
> - .copy_ref = debug_copy_ref,
> -
> .iterator_begin = debug_ref_iterator_begin,
> .read_raw_ref = debug_read_raw_ref,
> .read_symbolic_ref = debug_read_symbolic_ref,
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index 71628550f..c28228116 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -1594,6 +1594,7 @@ static int files_optimize_required(struct ref_store *ref_store,
> * live into logs/refs.
> */
> #define TMP_RENAMED_LOG "refs/.tmp-renamed-log"
> +#define TMP_RENAMED_LOG_DESTINATION "refs/.tmp-renamed-log-destination"
>
> struct rename_cb {
> const char *tmp_renamed_log;
> @@ -1685,12 +1686,28 @@ static int refs_rename_ref_available(struct ref_store *refs,
> return ok;
> }
>
> +struct files_copy_or_rename_transaction_data {
> + struct ref_lock *lock;
> + struct object_id orig_oid;
> + struct object_id destination_oid;
> + char *destination_target;
> + int logmoved;
> + int destination_exists;
> + int destination_log_backed_up;
> +};
> +
> static int files_copy_or_rename_ref(struct ref_store *ref_store,
> - const char *oldrefname, const char *newrefname,
> - const char *logmsg, int copy)
> + struct ref_update *update,
> + struct ref_transaction *transaction)
> {
> struct files_ref_store *refs =
> - files_downcast(ref_store, REF_STORE_WRITE, "rename_ref");
> + files_downcast(ref_store, REF_STORE_WRITE,
> + "ref_transaction_prepare");
> + struct ref_copy_or_rename_update *operation = update->copy_or_rename;
> + const char *oldrefname = operation->old_refname;
> + const char *newrefname = update->refname;
> + const char *logmsg = operation->logmsg;
> + bool copy = operation->type == REF_UPDATE_COPY;
> struct object_id orig_oid;
> int flag = 0, logmoved = 0;
> struct ref_lock *lock;
> @@ -1698,12 +1715,19 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,
> struct strbuf sb_oldref = STRBUF_INIT;
> struct strbuf sb_newref = STRBUF_INIT;
> struct strbuf tmp_renamed_log = STRBUF_INIT;
> + struct strbuf tmp_destination_log = STRBUF_INIT;
> + struct strbuf destination_target = STRBUF_INIT;
> int log, ret;
> + int destination_exists = 0, destination_flags = 0;
> + int destination_log_backed_up = 0;
> + struct object_id destination_oid;
> + struct files_copy_or_rename_transaction_data *data;
> struct strbuf err = STRBUF_INIT;
>
> files_reflog_path(refs, &sb_oldref, oldrefname);
> files_reflog_path(refs, &sb_newref, newrefname);
> files_reflog_path(refs, &tmp_renamed_log, TMP_RENAMED_LOG);
> + files_reflog_path(refs, &tmp_destination_log, TMP_RENAMED_LOG_DESTINATION);
>
> log = !lstat(sb_oldref.buf, &loginfo);
> if (log && S_ISLNK(loginfo.st_mode)) {
> @@ -1727,11 +1751,67 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,
> oldrefname);
> goto out;
> }
> + if (!oideq(&orig_oid, &operation->source_oid)) {
> + ret = error("refname %s is at %s but expected %s",
> + oldrefname, oid_to_hex(&orig_oid),
> + oid_to_hex(&operation->source_oid));
> + goto out;
> + }
> if (!refs_rename_ref_available(&refs->base, oldrefname, newrefname)) {
> ret = 1;
> goto out;
> }
>
> + if (refs_resolve_ref_unsafe(&refs->base, newrefname,
> + RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,
> + &destination_oid, &destination_flags)) {
> + destination_exists = 1;
> + if ((destination_flags & REF_ISSYMREF) &&
> + refs_read_symbolic_ref(&refs->base, newrefname,
> + &destination_target) < 0) {
> + ret = error("unable to read symbolic ref %s", newrefname);
> + goto out;
> + }
> + }
> + if (destination_exists != operation->destination_exists) {
> + ret = error("refname %s changed while renaming", newrefname);
> + goto out;
> + }
> + if (destination_exists) {
> + if (destination_flags & REF_ISSYMREF) {
> + if (!operation->destination_target ||
> + strcmp(destination_target.buf,
> + operation->destination_target)) {
> + ret = error("refname %s changed while renaming",
> + newrefname);
> + goto out;
> + }
> + } else if (operation->destination_target ||
> + !oideq(&destination_oid,
> + &operation->destination_oid)) {
> + ret = error("refname %s changed while renaming", newrefname);
> + goto out;
> + }
> + }
> +
> + if (!lstat(sb_newref.buf, &loginfo)) {
> + if (S_ISLNK(loginfo.st_mode)) {
> + ret = error("reflog for %s is a symlink", newrefname);
> + goto out;
> + }
> + if (S_ISREG(loginfo.st_mode)) {
> + if (copy_file(refs->base.repo, tmp_destination_log.buf,
> + sb_newref.buf, 0644)) {
> + if (errno != EEXIST)
> + unlink(tmp_destination_log.buf);
> + ret = error("unable to back up logfile logs/%s: %s",
> + newrefname, strerror(errno));
> + goto out;
> + }
> + destination_log_backed_up = 1;
> + }
> + }
> +
> if (!copy && log && rename(sb_oldref.buf, tmp_renamed_log.buf)) {
> ret = error("unable to move logfile logs/%s to logs/"TMP_RENAMED_LOG": %s",
> oldrefname, strerror(errno));
> @@ -1744,8 +1824,10 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,
> goto out;
> }
>
> - if (!copy && refs_delete_ref(&refs->base, logmsg, oldrefname,
> - &orig_oid, REF_NO_DEREF)) {
> + if (!copy && refs_delete_ref_with_transaction_flags(&refs->base, logmsg,
> + oldrefname, &orig_oid,
> + REF_NO_DEREF,
> + REF_TRANSACTION_FLAG_SKIP_HOOK)) {
> error("unable to delete old %s", oldrefname);
> goto rollback;
> }
> @@ -1760,8 +1842,9 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,
> if (!copy && refs_resolve_ref_unsafe(&refs->base, newrefname,
> RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,
> NULL, NULL) &&
> - refs_delete_ref(&refs->base, NULL, newrefname,
> - NULL, REF_NO_DEREF)) {
> + refs_delete_ref_with_transaction_flags(&refs->base, NULL, newrefname,
> + NULL, REF_NO_DEREF,
> + REF_TRANSACTION_FLAG_SKIP_HOOK)) {
> if (errno == EISDIR) {
> struct strbuf path = STRBUF_INIT;
> int result;
> @@ -1796,13 +1879,25 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,
> }
> oidcpy(&lock->old_oid, &orig_oid);
>
> - if (write_ref_to_lockfile(refs, lock, &orig_oid, &err) ||
> - commit_ref_update(refs, lock, &orig_oid, logmsg, 0, &err)) {
> + if (write_ref_to_lockfile(refs, lock, &orig_oid, &err)) {
> error("unable to write current sha1 into %s: %s", newrefname, err.buf);
> strbuf_release(&err);
> goto rollback;
> }
>
> + CALLOC_ARRAY(data, 1);
> + data->lock = lock;
> + oidcpy(&data->orig_oid, &orig_oid);
> + data->logmoved = logmoved;
> + data->destination_exists = destination_exists;
> + data->destination_log_backed_up = destination_log_backed_up;
> + if (destination_exists && !(destination_flags & REF_ISSYMREF))
> + oidcpy(&data->destination_oid, &destination_oid);
> + if (destination_flags & REF_ISSYMREF)
> + data->destination_target = strbuf_detach(&destination_target, NULL);
> + transaction->backend_data = data;
> + transaction->state = REF_TRANSACTION_PREPARED;
> +
> ret = 0;
> goto out;
>
> @@ -1821,38 +1916,40 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,
> }
>
> rollbacklog:
> - if (logmoved && rename(sb_newref.buf, sb_oldref.buf))
> - error("unable to restore logfile %s from %s: %s",
> - oldrefname, newrefname, strerror(errno));
> + if (logmoved) {
> + if (rename(sb_newref.buf, tmp_renamed_log.buf)) {
> + error("unable to restore logfile %s from %s: %s",
> + oldrefname, newrefname, strerror(errno));
> + } else {
> + try_remove_empty_parents(refs, newrefname,
> + REMOVE_EMPTY_PARENTS_REFLOG);
> + if (rename_tmp_log(refs, oldrefname))
> + error("unable to restore logfile %s from logs/"
> + TMP_RENAMED_LOG ": %s",
> + oldrefname, strerror(errno));
> + }
> + }
> if (!logmoved && log &&
> rename(tmp_renamed_log.buf, sb_oldref.buf))
> error("unable to restore logfile %s from logs/"TMP_RENAMED_LOG": %s",
> oldrefname, strerror(errno));
> + if (destination_log_backed_up &&
> + rename(tmp_destination_log.buf, sb_newref.buf))
> + error("unable to restore logfile %s: %s",
> + newrefname, strerror(errno));
> ret = 1;
> out:
> + if (ret && destination_log_backed_up)
> + unlink(tmp_destination_log.buf);
> strbuf_release(&sb_newref);
> strbuf_release(&sb_oldref);
> strbuf_release(&tmp_renamed_log);
> + strbuf_release(&tmp_destination_log);
> + strbuf_release(&destination_target);
>
> return ret;
> }
>
> -static int files_rename_ref(struct ref_store *ref_store,
> - const char *oldrefname, const char *newrefname,
> - const char *logmsg)
> -{
> - return files_copy_or_rename_ref(ref_store, oldrefname,
> - newrefname, logmsg, 0);
> -}
> -
> -static int files_copy_ref(struct ref_store *ref_store,
> - const char *oldrefname, const char *newrefname,
> - const char *logmsg)
> -{
> - return files_copy_or_rename_ref(ref_store, oldrefname,
> - newrefname, logmsg, 1);
> -}
> -
> static int close_ref_gently(struct ref_lock *lock)
> {
> if (close_lock_file_gently(&lock->lk))
> @@ -2962,6 +3059,14 @@ static int files_transaction_prepare(struct ref_store *ref_store,
> struct ref_transaction *packed_transaction = NULL;
>
> assert(err);
> + {
> + struct ref_update *operation =
> + ref_transaction_copy_or_rename_update(transaction);
> +
> + if (operation)
> + return files_copy_or_rename_ref(ref_store, operation,
> + transaction);
> + }
>
> if (transaction->flags & REF_TRANSACTION_FLAG_INITIAL)
> goto cleanup;
> @@ -3318,6 +3423,10 @@ static int files_transaction_finish_initial(struct files_ref_store *refs,
> return ret;
> }
>
> +static int files_transaction_abort(struct ref_store *ref_store,
> + struct ref_transaction *transaction,
> + struct strbuf *err);
> +
> static int files_transaction_finish(struct ref_store *ref_store,
> struct ref_transaction *transaction,
> struct strbuf *err)
> @@ -3333,6 +3442,40 @@ static int files_transaction_finish(struct ref_store *ref_store,
>
>
> assert(err);
> + {
> + struct ref_update *update =
> + ref_transaction_copy_or_rename_update(transaction);
> +
> + if (update) {
> + struct ref_copy_or_rename_update *operation =
> + update->copy_or_rename;
> + struct files_copy_or_rename_transaction_data *data =
> + transaction->backend_data;
> + int special_ret;
> +
> + special_ret = commit_ref_update(refs, data->lock, &data->orig_oid,
> + operation->logmsg, 0, err);
> + if (special_ret) {
> + error("unable to write current sha1 into %s: %s",
> + update->refname, err->buf);
> + data->lock = NULL;
> + files_transaction_abort(ref_store, transaction, err);
> + return special_ret;
> + } else if (data->destination_log_backed_up) {
> + struct strbuf path = STRBUF_INIT;
> +
> + files_reflog_path(refs, &path, TMP_RENAMED_LOG_DESTINATION);
> + if (unlink(path.buf) < 0 && errno != ENOENT)
> + warning_errno("unable to remove '%s'", path.buf);
> + strbuf_release(&path);
> + }
> + free(data->destination_target);
> + free(data);
> + transaction->backend_data = NULL;
> + transaction->state = REF_TRANSACTION_CLOSED;
> + return special_ret;
> + }
> + }
>
> if (transaction->flags & REF_TRANSACTION_FLAG_INITIAL)
> return files_transaction_finish_initial(refs, transaction, err);
> @@ -3476,11 +3619,105 @@ static int files_transaction_finish(struct ref_store *ref_store,
>
> static int files_transaction_abort(struct ref_store *ref_store,
> struct ref_transaction *transaction,
> - struct strbuf *err UNUSED)
> + struct strbuf *err)
> {
> struct files_ref_store *refs =
> files_downcast(ref_store, 0, "ref_transaction_abort");
>
> + {
> + struct ref_update *update =
> + ref_transaction_copy_or_rename_update(transaction);
> +
> + if (update) {
> + struct ref_copy_or_rename_update *operation =
> + update->copy_or_rename;
> + struct files_copy_or_rename_transaction_data *data =
> + transaction->backend_data;
> + struct strbuf new_log = STRBUF_INIT;
> + struct strbuf destination_log = STRBUF_INIT;
> + struct strbuf temporary_log = STRBUF_INIT;
> + struct ref_transaction *restore_transaction = NULL;
> + struct ref_lock *lock;
> + int ret = 0;
> +
> + if (data->lock)
> + unlock_ref(data->lock);
> + if (operation->type == REF_UPDATE_RENAME) {
> + lock = lock_ref_oid_basic(refs, operation->old_refname, err);
> + if (!lock ||
> + write_ref_to_lockfile(refs, lock, &data->orig_oid, err) ||
> + commit_ref_update(refs, lock, &data->orig_oid, NULL,
> + REF_SKIP_CREATE_REFLOG, err))
> + ret = -1;
> + }
> +
> + if (data->logmoved) {
> + files_reflog_path(refs, &new_log, update->refname);
> + if (operation->type == REF_UPDATE_RENAME) {
> + files_reflog_path(refs, &temporary_log, TMP_RENAMED_LOG);
> + if (rename(new_log.buf, temporary_log.buf) < 0) {
> + strbuf_addf(err, "unable to restore logfile %s: %s",
> + operation->old_refname, strerror(errno));
> + ret = -1;
> + } else {
> + try_remove_empty_parents(refs,
> + update->refname,
> + REMOVE_EMPTY_PARENTS_REFLOG);
> + if (rename_tmp_log(refs,
> + operation->old_refname)) {
> + strbuf_addf(err, "unable to restore logfile %s: %s",
> + operation->old_refname,
> + strerror(errno));
> + ret = -1;
> + }
> + }
> + } else if (unlink(new_log.buf) < 0 && errno != ENOENT) {
> + strbuf_addf(err, "unable to remove logfile %s: %s",
> + update->refname, strerror(errno));
> + ret = -1;
> + }
> + }
> + if (data->destination_log_backed_up) {
> + files_reflog_path(refs, &destination_log,
> + TMP_RENAMED_LOG_DESTINATION);
> + if (rename(destination_log.buf, new_log.buf) < 0) {
> + strbuf_addf(err, "unable to restore logfile %s: %s",
> + update->refname, strerror(errno));
> + ret = -1;
> + }
> + }
> +
> + if (operation->type == REF_UPDATE_RENAME &&
> + data->destination_exists) {
> + restore_transaction = ref_store_transaction_begin(
> + &refs->base, REF_TRANSACTION_FLAG_SKIP_HOOK, err);
> + if (!restore_transaction ||
> + ref_transaction_update(restore_transaction,
> + update->refname,
> + data->destination_target ? NULL :
> + &data->destination_oid,
> + NULL,
> + data->destination_target,
> + NULL,
> + REF_NO_DEREF |
> + REF_SKIP_CREATE_REFLOG,
> + NULL, err) ||
> + ref_transaction_commit(restore_transaction, err))
> + ret = -1;
> + ref_transaction_free(restore_transaction);
> + }
> +
> + strbuf_release(&destination_log);
> + strbuf_release(&temporary_log);
> + strbuf_release(&new_log);
> + free(data->destination_target);
> + free(data);
> + transaction->backend_data = NULL;
> + transaction->state = REF_TRANSACTION_CLOSED;
> + return ret;
> + }
> + }
> +
> files_transaction_cleanup(refs, transaction);
> return 0;
> }
> @@ -4095,8 +4332,6 @@ struct ref_storage_be refs_be_files = {
>
> .optimize = files_optimize,
> .optimize_required = files_optimize_required,
> - .rename_ref = files_rename_ref,
> - .copy_ref = files_copy_ref,
>
> .iterator_begin = files_ref_iterator_begin,
> .read_raw_ref = files_read_raw_ref,
> diff --git a/refs/packed-backend.c b/refs/packed-backend.c
> index a73fc6aca..364a91291 100644
> --- a/refs/packed-backend.c
> +++ b/refs/packed-backend.c
> @@ -2164,8 +2164,6 @@ struct ref_storage_be refs_be_packed = {
> .optimize = packed_optimize,
> .optimize_required = packed_optimize_required,
>
> - .rename_ref = NULL,
> - .copy_ref = NULL,
>
> .iterator_begin = packed_ref_iterator_begin,
> .read_raw_ref = packed_read_raw_ref,
> diff --git a/refs/refs-internal.h b/refs/refs-internal.h
> index c3ac7b556..5d4dc0171 100644
> --- a/refs/refs-internal.h
> +++ b/refs/refs-internal.h
> @@ -155,9 +155,33 @@ struct ref_update {
> */
> struct ref_update *parent_update;
>
> + /*
> + * Copy and rename operations require backend-specific handling while
> + * still exposing their logical updates to transaction hooks. Keep that
> + * state on the destination update so it composes with other updates in
> + * the transaction instead of making copy or rename a transaction-wide
> + * property.
> + */
> + struct ref_copy_or_rename_update *copy_or_rename;
> +
> const char refname[FLEX_ARRAY];
> };
>
> +enum ref_copy_or_rename_type {
> + REF_UPDATE_RENAME,
> + REF_UPDATE_COPY,
> +};
> +
> +struct ref_copy_or_rename_update {
> + enum ref_copy_or_rename_type type;
> + char *old_refname;
> + char *logmsg;
> + struct object_id source_oid;
> + struct object_id destination_oid;
> + char *destination_target;
> + unsigned int destination_exists:1;
> +};
> +
> int refs_read_raw_ref(struct ref_store *ref_store, const char *refname,
> struct object_id *oid, struct strbuf *referent,
> unsigned int *type, int *failure_errno);
> @@ -187,6 +211,13 @@ struct ref_update *ref_transaction_add_update(
> const char *committer_info,
> const char *msg);
>
> +int refs_delete_ref_with_transaction_flags(struct ref_store *refs,
> + const char *msg,
> + const char *refname,
> + const struct object_id *old_oid,
> + unsigned int flags,
> + unsigned int transaction_flags);
> +
> /*
> * Transaction states.
> *
> @@ -242,6 +273,12 @@ struct ref_transaction {
> uint64_t max_index;
> };
>
> +/* Suppress hooks for a transaction nested inside another refs operation. */
> +#define REF_TRANSACTION_FLAG_SKIP_HOOK (1 << 2)
> +
> +struct ref_update *ref_transaction_copy_or_rename_update(
> + struct ref_transaction *transaction);
> +
> /*
> * Check for entries in extras that are within the specified
> * directory, where dirname is a reference directory name including
> @@ -451,13 +488,6 @@ typedef int optimize_required_fn(struct ref_store *ref_store,
> struct refs_optimize_opts *opts,
> bool *required);
>
> -typedef int rename_ref_fn(struct ref_store *ref_store,
> - const char *oldref, const char *newref,
> - const char *logmsg);
> -typedef int copy_ref_fn(struct ref_store *ref_store,
> - const char *oldref, const char *newref,
> - const char *logmsg);
> -
> /*
> * Iterate over the references in `ref_store` whose names start with
> * `prefix`. `prefix` is matched as a literal string, without regard
> @@ -577,9 +607,6 @@ struct ref_storage_be {
>
> optimize_fn *optimize;
> optimize_required_fn *optimize_required;
> - rename_ref_fn *rename_ref;
> - copy_ref_fn *copy_ref;
> -
> ref_iterator_begin_fn *iterator_begin;
> read_raw_ref_fn *read_raw_ref;
>
> diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
> index 10db03991..589fcc998 100644
> --- a/refs/reftable-backend.c
> +++ b/refs/reftable-backend.c
> @@ -953,6 +953,14 @@ struct reftable_transaction_data {
> size_t args_nr, args_alloc;
> };
>
> +struct reftable_copy_or_rename_transaction_data {
> + struct reftable_addition *addition;
> +};
> +
> +static int reftable_be_copy_or_rename_prepare(struct ref_store *ref_store,
> + struct ref_transaction *transaction,
> + struct strbuf *err);
> +
> static void free_transaction_data(struct reftable_transaction_data *tx_data)
> {
> if (!tx_data)
> @@ -1326,6 +1334,10 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,
> size_t i;
> int ret;
>
> + if (ref_transaction_copy_or_rename_update(transaction))
> + return reftable_be_copy_or_rename_prepare(ref_store, transaction,
> + err);
> +
> ret = refs->err;
> if (ret < 0)
> goto done;
> @@ -1419,7 +1431,20 @@ static int reftable_be_transaction_abort(struct ref_store *ref_store UNUSED,
> struct ref_transaction *transaction,
> struct strbuf *err UNUSED)
> {
> - struct reftable_transaction_data *tx_data = transaction->backend_data;
> + struct reftable_transaction_data *tx_data;
> +
> + if (ref_transaction_copy_or_rename_update(transaction)) {
> + struct reftable_copy_or_rename_transaction_data *data =
> + transaction->backend_data;
> +
> + reftable_addition_destroy(data->addition);
> + free(data);
> + transaction->backend_data = NULL;
> + transaction->state = REF_TRANSACTION_CLOSED;
> + return 0;
> + }
> +
> + tx_data = transaction->backend_data;
> free_transaction_data(tx_data);
> transaction->state = REF_TRANSACTION_CLOSED;
> return 0;
> @@ -1667,9 +1692,28 @@ static int reftable_be_transaction_finish(struct ref_store *ref_store UNUSED,
> struct ref_transaction *transaction,
> struct strbuf *err)
> {
> - struct reftable_transaction_data *tx_data = transaction->backend_data;
> + struct reftable_transaction_data *tx_data;
> int ret = 0;
>
> + if (ref_transaction_copy_or_rename_update(transaction)) {
> + struct reftable_copy_or_rename_transaction_data *data =
> + transaction->backend_data;
> + int special_ret = reftable_addition_commit(data->addition);
> +
> + reftable_addition_destroy(data->addition);
> + free(data);
> + transaction->backend_data = NULL;
> + transaction->state = REF_TRANSACTION_CLOSED;
> + if (special_ret < 0) {
> + strbuf_addf(err, _("reftable: transaction failure: %s"),
> + reftable_error_str(special_ret));
> + return -1;
> + }
> + return 0;
> + }
> +
> + tx_data = transaction->backend_data;
> +
> for (size_t i = 0; i < tx_data->args_nr; i++) {
> tx_data->args[i].max_index = transaction->max_index;
>
> @@ -1764,17 +1808,20 @@ struct write_create_symref_arg {
> struct write_copy_arg {
> struct reftable_ref_store *refs;
> struct reftable_backend *be;
> + struct strbuf *err;
> const char *oldname;
> const char *newname;
> const char *logmsg;
> int delete_old;
> + struct ref_copy_or_rename_update *operation;
> };
>
> static int write_copy_table(struct reftable_writer *writer, void *cb_data)
> {
> struct write_copy_arg *arg = cb_data;
> uint64_t deletion_ts, creation_ts;
> - struct reftable_ref_record old_ref = {0}, refs[2] = {0};
> + struct reftable_ref_record old_ref = {0}, destination_ref = {0};
> + struct reftable_ref_record refs[2] = {0};
> struct reftable_log_record old_log = {0}, *logs = NULL;
> struct reftable_iterator it = {0};
> struct string_list skip = STRING_LIST_INIT_NODUP;
> @@ -1789,14 +1836,75 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data)
> BUG("failed splitting committer info");
>
> if (reftable_stack_read_ref(arg->be->stack, arg->oldname, &old_ref)) {
> - ret = error(_("refname %s not found"), arg->oldname);
> + strbuf_addf(arg->err, _("refname %s not found"), arg->oldname);
> + ret = -1;
> goto done;
> }
> if (old_ref.value_type == REFTABLE_REF_SYMREF) {
> - ret = error(_("refname %s is a symbolic ref, copying it is not supported"),
> + strbuf_addf(arg->err,
> + _("refname %s is a symbolic ref, copying it is not supported"),
> arg->oldname);
> + ret = -1;
> goto done;
> }
> + if (arg->operation) {
> + struct object_id oid;
> +
> + if (old_ref.value_type == REFTABLE_REF_VAL2)
> + oidread(&oid, old_ref.value.val2.value,
> + arg->refs->base.repo->hash_algo);
> + else
> + oidread(&oid, old_ref.value.val1,
> + arg->refs->base.repo->hash_algo);
> + if (!oideq(&oid, &arg->operation->source_oid)) {
> + strbuf_addf(arg->err,
> + _("refname %s is at %s but expected %s"),
> + arg->oldname, oid_to_hex(&oid),
> + oid_to_hex(&arg->operation->source_oid));
> + ret = -1;
> + goto done;
> + }
> +
> + ret = reftable_stack_read_ref(arg->be->stack, arg->newname,
> + &destination_ref);
> + if (ret < 0)
> + goto done;
> + if (arg->operation->destination_exists != !ret) {
> + strbuf_addf(arg->err,
> + _("refname %s changed while renaming"),
> + arg->newname);
> + ret = -1;
> + goto done;
> + }
> + if (!ret) {
> + if (destination_ref.value_type == REFTABLE_REF_SYMREF) {
> + if (!arg->operation->destination_target ||
> + strcmp(destination_ref.value.symref,
> + arg->operation->destination_target)) {
> + strbuf_addf(arg->err,
> + _("refname %s changed while renaming"),
> + arg->newname);
> + ret = -1;
> + goto done;
> + }
> + } else {
> + if (destination_ref.value_type == REFTABLE_REF_VAL2)
> + oidread(&oid, destination_ref.value.val2.value,
> + arg->refs->base.repo->hash_algo);
> + else
> + oidread(&oid, destination_ref.value.val1,
> + arg->refs->base.repo->hash_algo);
> + if (arg->operation->destination_target ||
> + !oideq(&oid, &arg->operation->destination_oid)) {
> + strbuf_addf(arg->err,
> + _("refname %s changed while renaming"),
> + arg->newname);
> + ret = -1;
> + goto done;
> + }
> + }
> + }
> + }
>
> /*
> * There's nothing to do in case the old and new name are the same, so
> @@ -1815,7 +1923,7 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data)
> ret = refs_verify_refname_available(&arg->refs->base, arg->newname,
> NULL, &skip, 0, &errbuf);
> if (ret < 0) {
> - error("%s", errbuf.buf);
> + strbuf_addbuf(arg->err, &errbuf);
> goto done;
> }
>
> @@ -1980,68 +2088,63 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data)
> for (i = 0; i < ARRAY_SIZE(refs); i++)
> reftable_ref_record_release(&refs[i]);
> reftable_ref_record_release(&old_ref);
> + reftable_ref_record_release(&destination_ref);
> reftable_log_record_release(&old_log);
> return ret;
> }
>
> -static int reftable_be_rename_ref(struct ref_store *ref_store,
> - const char *oldrefname,
> - const char *newrefname,
> - const char *logmsg)
> +static int reftable_be_copy_or_rename_prepare(struct ref_store *ref_store,
> + struct ref_transaction *transaction,
> + struct strbuf *err)
> {
> struct reftable_ref_store *refs =
> - reftable_be_downcast(ref_store, REF_STORE_WRITE, "rename_ref");
> + reftable_be_downcast(ref_store, REF_STORE_WRITE,
> + "ref_transaction_prepare");
> + struct reftable_copy_or_rename_transaction_data *data = NULL;
> + struct ref_update *update =
> + ref_transaction_copy_or_rename_update(transaction);
> + struct ref_copy_or_rename_update *operation = update->copy_or_rename;
> struct write_copy_arg arg = {
> .refs = refs,
> - .oldname = oldrefname,
> - .newname = newrefname,
> - .logmsg = logmsg,
> - .delete_old = 1,
> + .err = err,
> + .oldname = operation->old_refname,
> + .newname = update->refname,
> + .logmsg = operation->logmsg,
> + .delete_old = operation->type == REF_UPDATE_RENAME,
> + .operation = operation,
> };
> int ret;
>
> + CALLOC_ARRAY(data, 1);
> ret = refs->err;
> if (ret < 0)
> goto done;
> -
> - ret = backend_for(&arg.be, refs, newrefname, &newrefname, 1);
> + ret = backend_for(&arg.be, refs, update->refname,
> + &arg.newname, 1);
> if (ret)
> goto done;
> - ret = reftable_stack_add(arg.be->stack, &write_copy_table, &arg,
> - &reftable_be_write_options(refs)->opts);
> -
> -done:
> - assert(ret != REFTABLE_API_ERROR);
> - return ret;
> -}
> -
> -static int reftable_be_copy_ref(struct ref_store *ref_store,
> - const char *oldrefname,
> - const char *newrefname,
> - const char *logmsg)
> -{
> - struct reftable_ref_store *refs =
> - reftable_be_downcast(ref_store, REF_STORE_WRITE, "copy_ref");
> - struct write_copy_arg arg = {
> - .refs = refs,
> - .oldname = oldrefname,
> - .newname = newrefname,
> - .logmsg = logmsg,
> - };
> - int ret;
> -
> - ret = refs->err;
> - if (ret < 0)
> + ret = reftable_stack_addition_new(&data->addition, arg.be->stack,
> + &reftable_be_write_options(refs)->opts);
> + if (ret)
> goto done;
> -
> - ret = backend_for(&arg.be, refs, newrefname, &newrefname, 1);
> + ret = reftable_addition_add(data->addition, &write_copy_table, &arg);
> if (ret)
> goto done;
> - ret = reftable_stack_add(arg.be->stack, &write_copy_table, &arg,
> - &reftable_be_write_options(refs)->opts);
> +
> + transaction->backend_data = data;
> + transaction->state = REF_TRANSACTION_PREPARED;
> + return 0;
>
> done:
> assert(ret != REFTABLE_API_ERROR);
> + if (data) {
> + reftable_addition_destroy(data->addition);
> + free(data);
> + }
> + transaction->state = REF_TRANSACTION_CLOSED;
> + if (ret && !err->len)
> + strbuf_addf(err, _("reftable: transaction prepare: %s"),
> + reftable_error_str(ret));
> return ret;
> }
>
> @@ -2872,8 +2975,6 @@ struct ref_storage_be refs_be_reftable = {
> .optimize = reftable_be_optimize,
> .optimize_required = reftable_be_optimize_required,
>
> - .rename_ref = reftable_be_rename_ref,
> - .copy_ref = reftable_be_copy_ref,
>
> .iterator_begin = reftable_be_iterator_begin,
> .read_raw_ref = reftable_be_read_raw_ref,
> diff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh
> index 4fe9d9b23..116b2ff07 100755
> --- a/t/t1416-ref-transaction-hooks.sh
> +++ b/t/t1416-ref-transaction-hooks.sh
> @@ -93,6 +93,148 @@ test_expect_success 'hook gets all queued updates in committed state' '
> test_cmp expect actual
> '
>
> +test_expect_success 'hook gets both updates when renaming a branch' '
> + test_when_finished "rm -f actual" &&
> + git branch old PRE &&
> + test_hook reference-transaction <<-\EOF &&
> + echo "$1" >>actual &&
> + cat >>actual
> + EOF
> + cat >expect <<-EOF &&
> + preparing
> + $PRE_OID $ZERO_OID refs/heads/old
> + $ZERO_OID $PRE_OID refs/heads/new
> + prepared
> + $PRE_OID $ZERO_OID refs/heads/old
> + $ZERO_OID $PRE_OID refs/heads/new
> + committed
> + $PRE_OID $ZERO_OID refs/heads/old
> + $ZERO_OID $PRE_OID refs/heads/new
> + EOF
> + git branch -m old new &&
> + test_cmp expect actual &&
> + test_must_fail git rev-parse --verify refs/heads/old &&
> + test_cmp_rev PRE refs/heads/new
> +'
> +
> +test_expect_success 'hook gets destination update when copying a branch' '
> + test_when_finished "rm -f actual" &&
> + git branch copy-source PRE &&
> + test_hook reference-transaction <<-\EOF &&
> + echo "$1" >>actual &&
> + cat >>actual
> + EOF
> + cat >expect <<-EOF &&
> + preparing
> + $ZERO_OID $PRE_OID refs/heads/copy-destination
> + prepared
> + $ZERO_OID $PRE_OID refs/heads/copy-destination
> + committed
> + $ZERO_OID $PRE_OID refs/heads/copy-destination
> + EOF
> + git branch -c copy-source copy-destination &&
> + test_cmp expect actual &&
> + test_cmp_rev PRE refs/heads/copy-source &&
> + test_cmp_rev PRE refs/heads/copy-destination
> +'
> +
> +test_expect_success 'hook gets overwritten values for forced rename and copy' '
> + git branch force-old PRE &&
> + git branch force-new POST &&
> + git branch force-copy-source PRE &&
> + git branch force-copy-destination POST &&
> + test_hook reference-transaction <<-\EOF &&
> + if test "$1" = committed
> + then
> + cat >>actual
> + fi
> + EOF
> + git branch -M force-old force-new &&
> + git branch -C force-copy-source force-copy-destination &&
> + cat >expect <<-EOF &&
> + $PRE_OID $ZERO_OID refs/heads/force-old
> + $POST_OID $PRE_OID refs/heads/force-new
> + $POST_OID $PRE_OID refs/heads/force-copy-destination
> + EOF
> + test_cmp expect actual
> +'
> +
> +test_expect_success 'hook can abort a branch rename after preparation' '
> + git branch abort-old PRE &&
> + git branch abort-new POST &&
> + git reflog show --format=%gs abort-old >old-log &&
> + git reflog show --format=%gs abort-new >new-log &&
> + test_hook reference-transaction <<-\EOF &&
> + test "$1" != prepared
> + EOF
> + test_must_fail git branch -M abort-old abort-new &&
> + test_cmp_rev PRE refs/heads/abort-old &&
> + test_cmp_rev POST refs/heads/abort-new &&
> + git reflog show --format=%gs abort-old >old-log-after &&
> + git reflog show --format=%gs abort-new >new-log-after &&
> + test_cmp old-log old-log-after &&
> + test_cmp new-log new-log-after
> +'
> +
> +test_expect_success 'hook can abort a D/F branch rename after preparation' '
> + git branch df-old PRE &&
> + git reflog show --format=%gs df-old >df-log &&
> + test_hook reference-transaction <<-\EOF &&
> + test "$1" != prepared
> + EOF
> + test_must_fail git branch -m df-old df-old/child &&
> + test_cmp_rev PRE refs/heads/df-old &&
> + test_must_fail git rev-parse --verify refs/heads/df-old/child &&
> + git reflog show --format=%gs df-old >df-log-after &&
> + test_cmp df-log df-log-after
> +'
> +
> +test_expect_success 'hook can abort a reverse D/F rename after preparation' '
> + git branch reverse/old PRE &&
> + git reflog show --format=%gs reverse/old >reverse-log &&
> + test_hook reference-transaction <<-\EOF &&
> + test "$1" != prepared
> + EOF
> + test_must_fail git branch -m reverse/old reverse &&
> + test_cmp_rev PRE refs/heads/reverse/old &&
> + test_must_fail git rev-parse --verify refs/heads/reverse &&
> + git reflog show --format=%gs reverse/old >reverse-log-after &&
> + test_cmp reverse-log reverse-log-after
> +'
> +
> +test_expect_success 'hook can abort a forced branch copy after preparation' '
> + git branch copy-abort-old PRE &&
> + git branch copy-abort-new POST &&
> + git reflog show --format=%gs copy-abort-old >copy-old-log &&
> + git reflog show --format=%gs copy-abort-new >copy-new-log &&
> + test_hook reference-transaction <<-\EOF &&
> + test "$1" != prepared
> + EOF
> + test_must_fail git branch -C copy-abort-old copy-abort-new &&
> + test_cmp_rev PRE refs/heads/copy-abort-old &&
> + test_cmp_rev POST refs/heads/copy-abort-new &&
> + git reflog show --format=%gs copy-abort-old >copy-old-log-after &&
> + git reflog show --format=%gs copy-abort-new >copy-new-log-after &&
> + test_cmp copy-old-log copy-old-log-after &&
> + test_cmp copy-new-log copy-new-log-after
> +'
> +
> +test_expect_success 'branch rename detects an update during preparing hook' '
> + git branch race-old PRE &&
> + git branch race-new POST &&
> + test_hook reference-transaction <<-\EOF &&
> + marker=$(git rev-parse --git-path rename-race-once)
> + if test "$1" = preparing && test ! -e "$marker"
> + then
> + >"$marker" &&
> + git update-ref refs/heads/race-old POST
> + fi
> + EOF
> + test_must_fail git branch -M race-old race-new &&
> + test_cmp_rev POST refs/heads/race-old &&
> + test_cmp_rev POST refs/heads/race-new
> +'
> +
> test_expect_success 'hook gets all queued updates in aborted state' '
> test_when_finished "rm actual" &&
> git reset --hard PRE &&
> --
> 2.39.3 (Apple Git-146)