From 8a349a1d7970ed2c6fcac8ea0c1d641384ad082d Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Mon, 4 May 2026 19:44:05 +0200 Subject: [PATCH 1/9] refs: remove unused typedef 'ref_transaction_commit_fn' The typedef 'ref_transaction_commit_fn' is not used anywhere in our code, let's remove it. Signed-off-by: Karthik Nayak Signed-off-by: Junio C Hamano --- refs/refs-internal.h | 4 ---- 1 file changed, 4 deletions(-) diff --git a/refs/refs-internal.h b/refs/refs-internal.h index d79e35fd26..2d963cc4f4 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -421,10 +421,6 @@ typedef int ref_transaction_abort_fn(struct ref_store *refs, struct ref_transaction *transaction, struct strbuf *err); -typedef int ref_transaction_commit_fn(struct ref_store *refs, - struct ref_transaction *transaction, - struct strbuf *err); - typedef int optimize_fn(struct ref_store *ref_store, struct refs_optimize_opts *opts); From d194dffcfd3ad26105149f8e0fbd3b6537bf1986 Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Mon, 4 May 2026 19:44:06 +0200 Subject: [PATCH 2/9] refs: introduce `ref_store_init_options` Reference backends are initiated via the `init()` function. When initiating the function, the backend is also provided flags which denote the access levels of the initiator. Create a new structure `ref_store_init_options` to house such options and move the access flags to this structure. This allows easier extension of providing further options to the backends. In the following commit, we'll also provide config around reflog creation to the backends via the same structure. Signed-off-by: Karthik Nayak Signed-off-by: Junio C Hamano --- refs.c | 6 +++++- refs/files-backend.c | 8 +++++--- refs/packed-backend.c | 4 ++-- refs/packed-backend.h | 3 ++- refs/refs-internal.h | 11 ++++++++++- refs/reftable-backend.c | 4 ++-- 6 files changed, 26 insertions(+), 10 deletions(-) diff --git a/refs.c b/refs.c index bfcb9c7ac3..8992dd6ae8 100644 --- a/refs.c +++ b/refs.c @@ -2295,6 +2295,9 @@ static struct ref_store *ref_store_init(struct repository *repo, { const struct ref_storage_be *be; struct ref_store *refs; + struct ref_store_init_options opts = { + .access_flags = flags, + }; be = find_ref_storage_backend(format); if (!be) @@ -2304,7 +2307,8 @@ static struct ref_store *ref_store_init(struct repository *repo, * TODO Send in a 'struct worktree' instead of a 'gitdir', and * allow the backend to handle how it wants to deal with worktrees. */ - refs = be->init(repo, repo->ref_storage_payload, gitdir, flags); + refs = be->init(repo, repo->ref_storage_payload, gitdir, &opts); + return refs; } diff --git a/refs/files-backend.c b/refs/files-backend.c index b3b0c25f84..72afe62cee 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -108,7 +108,7 @@ static void clear_loose_ref_cache(struct files_ref_store *refs) static struct ref_store *files_ref_store_init(struct repository *repo, const char *payload, const char *gitdir, - unsigned int flags) + const struct ref_store_init_options *opts) { struct files_ref_store *refs = xcalloc(1, sizeof(*refs)); struct ref_store *ref_store = (struct ref_store *)refs; @@ -120,11 +120,13 @@ static struct ref_store *files_ref_store_init(struct repository *repo, &ref_common_dir); base_ref_store_init(ref_store, repo, refdir.buf, &refs_be_files); - refs->store_flags = flags; + refs->gitcommondir = strbuf_detach(&ref_common_dir, NULL); refs->packed_ref_store = - packed_ref_store_init(repo, NULL, refs->gitcommondir, flags); + packed_ref_store_init(repo, NULL, refs->gitcommondir, opts); + refs->store_flags = opts->access_flags; refs->log_all_ref_updates = repo_settings_get_log_all_ref_updates(repo); + repo_config_get_bool(repo, "core.prefersymlinkrefs", &refs->prefer_symlink_refs); chdir_notify_reparent("files-backend $GIT_DIR", &refs->base.gitdir); diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 23ed62984b..35a0f32e1c 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -218,14 +218,14 @@ static size_t snapshot_hexsz(const struct snapshot *snapshot) struct ref_store *packed_ref_store_init(struct repository *repo, const char *payload UNUSED, const char *gitdir, - unsigned int store_flags) + const struct ref_store_init_options *opts) { struct packed_ref_store *refs = xcalloc(1, sizeof(*refs)); struct ref_store *ref_store = (struct ref_store *)refs; struct strbuf sb = STRBUF_INIT; base_ref_store_init(ref_store, repo, gitdir, &refs_be_packed); - refs->store_flags = store_flags; + refs->store_flags = opts->access_flags; strbuf_addf(&sb, "%s/packed-refs", gitdir); refs->path = strbuf_detach(&sb, NULL); diff --git a/refs/packed-backend.h b/refs/packed-backend.h index 2c2377a356..1db48e801d 100644 --- a/refs/packed-backend.h +++ b/refs/packed-backend.h @@ -3,6 +3,7 @@ struct repository; struct ref_transaction; +struct ref_store_init_options; /* * Support for storing references in a `packed-refs` file. @@ -16,7 +17,7 @@ struct ref_transaction; struct ref_store *packed_ref_store_init(struct repository *repo, const char *payload, const char *gitdir, - unsigned int store_flags); + const struct ref_store_init_options *options); /* * Lock the packed-refs file for writing. Flags is passed to diff --git a/refs/refs-internal.h b/refs/refs-internal.h index 2d963cc4f4..f49b3807bf 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -385,6 +385,15 @@ struct ref_store; REF_STORE_ODB | \ REF_STORE_MAIN) +/* + * Options for initializing the ref backend. All backend-agnostic information + * which backends required will be held here. + */ +struct ref_store_init_options { + /* The kind of operations that the ref_store is allowed to perform. */ + unsigned int access_flags; +}; + /* * Initialize the ref_store for the specified gitdir. These functions * should call base_ref_store_init() to initialize the shared part of @@ -393,7 +402,7 @@ struct ref_store; typedef struct ref_store *ref_store_init_fn(struct repository *repo, const char *payload, const char *gitdir, - unsigned int flags); + const struct ref_store_init_options *opts); /* * Release all memory and resources associated with the ref store. */ diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index daea30a5b4..ad4ee2627c 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -369,7 +369,7 @@ static int reftable_be_config(const char *var, const char *value, static struct ref_store *reftable_be_init(struct repository *repo, const char *payload, const char *gitdir, - unsigned int store_flags) + const struct ref_store_init_options *opts) { struct reftable_ref_store *refs = xcalloc(1, sizeof(*refs)); struct strbuf ref_common_dir = STRBUF_INIT; @@ -386,8 +386,8 @@ static struct ref_store *reftable_be_init(struct repository *repo, base_ref_store_init(&refs->base, repo, refdir.buf, &refs_be_reftable); strmap_init(&refs->worktree_backends); - refs->store_flags = store_flags; refs->log_all_ref_updates = repo_settings_get_log_all_ref_updates(repo); + refs->store_flags = opts->access_flags; switch (repo->hash_algo->format_id) { case GIT_SHA1_FORMAT_ID: From cc42c88945753363d67d130c79640e3c682e1334 Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Mon, 4 May 2026 19:44:07 +0200 Subject: [PATCH 3/9] refs: extract out reflog config to generic layer The reference backends need to know when to create reflog entries, this is dictated by the 'core.logallrefupdates' config. Instead of relying on the backends to call `repo_settings_get_log_all_ref_updates()` to obtain this config value, let's do this in the generic layer and pass down the value to the backends. Signed-off-by: Karthik Nayak Signed-off-by: Junio C Hamano --- refs.c | 1 + refs/files-backend.c | 2 +- refs/refs-internal.h | 6 ++++++ refs/reftable-backend.c | 2 +- 4 files changed, 9 insertions(+), 2 deletions(-) diff --git a/refs.c b/refs.c index 8992dd6ae8..6b506aeea3 100644 --- a/refs.c +++ b/refs.c @@ -2297,6 +2297,7 @@ static struct ref_store *ref_store_init(struct repository *repo, struct ref_store *refs; struct ref_store_init_options opts = { .access_flags = flags, + .log_all_ref_updates = repo_settings_get_log_all_ref_updates(repo), }; be = find_ref_storage_backend(format); diff --git a/refs/files-backend.c b/refs/files-backend.c index 72afe62cee..4b2faf4777 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -125,7 +125,7 @@ static struct ref_store *files_ref_store_init(struct repository *repo, refs->packed_ref_store = packed_ref_store_init(repo, NULL, refs->gitcommondir, opts); refs->store_flags = opts->access_flags; - refs->log_all_ref_updates = repo_settings_get_log_all_ref_updates(repo); + refs->log_all_ref_updates = opts->log_all_ref_updates; repo_config_get_bool(repo, "core.prefersymlinkrefs", &refs->prefer_symlink_refs); diff --git a/refs/refs-internal.h b/refs/refs-internal.h index f49b3807bf..d103387ebf 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -392,6 +392,12 @@ struct ref_store; struct ref_store_init_options { /* The kind of operations that the ref_store is allowed to perform. */ unsigned int access_flags; + + /* + * Denotes under what conditions reflogs should be created when updating + * references. + */ + enum log_refs_config log_all_ref_updates; }; /* diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index ad4ee2627c..93374d25c2 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -386,7 +386,7 @@ static struct ref_store *reftable_be_init(struct repository *repo, base_ref_store_init(&refs->base, repo, refdir.buf, &refs_be_reftable); strmap_init(&refs->worktree_backends); - refs->log_all_ref_updates = repo_settings_get_log_all_ref_updates(repo); + refs->log_all_ref_updates = opts->log_all_ref_updates; refs->store_flags = opts->access_flags; switch (repo->hash_algo->format_id) { From e99e98e600181ddf431b267c2887358b3556e45c Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Mon, 4 May 2026 19:44:08 +0200 Subject: [PATCH 4/9] refs: return `ref_transaction_error` from `ref_transaction_update()` The `ref_transaction_update()` function is used to add updates to a given reference transactions. In the following commit, we'll add more validation to this function. As such, it would be beneficial if the function returns specific error types, so callers can differentiate between different errors. To facilitate this, return `enum ref_transaction_error` from the function and covert the existing '-1' returns to 'REF_TRANSACTION_ERROR_GENERIC'. Since this retains the existing behavior, no changes are made to any of the callers but this sets the necessary infrastructure for introduction of other errors. Signed-off-by: Karthik Nayak Signed-off-by: Junio C Hamano --- refs.c | 20 ++++++++++---------- refs.h | 16 ++++++++-------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/refs.c b/refs.c index 6b506aeea3..efa16b739d 100644 --- a/refs.c +++ b/refs.c @@ -1383,25 +1383,25 @@ static int transaction_refname_valid(const char *refname, return 1; } -int ref_transaction_update(struct ref_transaction *transaction, - const char *refname, - const struct object_id *new_oid, - const struct object_id *old_oid, - const char *new_target, - const char *old_target, - unsigned int flags, const char *msg, - struct strbuf *err) +enum ref_transaction_error ref_transaction_update(struct ref_transaction *transaction, + const char *refname, + const struct object_id *new_oid, + const struct object_id *old_oid, + const char *new_target, + const char *old_target, + unsigned int flags, const char *msg, + struct strbuf *err) { assert(err); if ((flags & REF_FORCE_CREATE_REFLOG) && (flags & REF_SKIP_CREATE_REFLOG)) { strbuf_addstr(err, _("refusing to force and skip creation of reflog")); - return -1; + return REF_TRANSACTION_ERROR_GENERIC; } if (!transaction_refname_valid(refname, new_oid, flags, err)) - return -1; + return REF_TRANSACTION_ERROR_GENERIC; if (flags & ~REF_TRANSACTION_UPDATE_ALLOWED_FLAGS) BUG("illegal flags 0x%x passed to ref_transaction_update()", flags); diff --git a/refs.h b/refs.h index d65de6ab5f..71d5c186d0 100644 --- a/refs.h +++ b/refs.h @@ -905,14 +905,14 @@ struct ref_transaction *ref_store_transaction_begin(struct ref_store *refs, * See the above comment "Reference transaction updates" for more * information. */ -int ref_transaction_update(struct ref_transaction *transaction, - const char *refname, - const struct object_id *new_oid, - const struct object_id *old_oid, - const char *new_target, - const char *old_target, - unsigned int flags, const char *msg, - struct strbuf *err); +enum ref_transaction_error ref_transaction_update(struct ref_transaction *transaction, + const char *refname, + const struct object_id *new_oid, + const struct object_id *old_oid, + const char *new_target, + const char *old_target, + unsigned int flags, const char *msg, + struct strbuf *err); /* * Similar to `ref_transaction_update`, but this function is only for adding From 637989cdec77b95fb0743cf433873253cec5ae0f Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Mon, 4 May 2026 19:44:09 +0200 Subject: [PATCH 5/9] update-ref: move `print_rejected_refs()` up The `print_rejected_refs()` function is used to print any rejected refs when using git-updated-ref(1) with the '--batch-updates' option. In the following commit, we'll need to use this function in another place, so move the function up to avoid a separate forward declaration. Signed-off-by: Karthik Nayak Signed-off-by: Junio C Hamano --- builtin/update-ref.c | 45 ++++++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/builtin/update-ref.c b/builtin/update-ref.c index 2d68c40ecb..5259cc7226 100644 --- a/builtin/update-ref.c +++ b/builtin/update-ref.c @@ -234,6 +234,28 @@ static int parse_next_oid(const char **next, const char *end, command, refname); } +static void print_rejected_refs(const char *refname, + const struct object_id *old_oid, + const struct object_id *new_oid, + const char *old_target, + const char *new_target, + enum ref_transaction_error err, + const char *details, + void *cb_data UNUSED) +{ + struct strbuf sb = STRBUF_INIT; + + if (details && *details) + error("%s", details); + + strbuf_addf(&sb, "rejected %s %s %s %s\n", refname, + new_oid ? oid_to_hex(new_oid) : new_target, + old_oid ? oid_to_hex(old_oid) : old_target, + ref_transaction_error_msg(err)); + + fwrite(sb.buf, sb.len, 1, stdout); + strbuf_release(&sb); +} /* * The following five parse_cmd_*() functions parse the corresponding @@ -567,29 +589,6 @@ static void parse_cmd_abort(struct ref_transaction *transaction, report_ok("abort"); } -static void print_rejected_refs(const char *refname, - const struct object_id *old_oid, - const struct object_id *new_oid, - const char *old_target, - const char *new_target, - enum ref_transaction_error err, - const char *details, - void *cb_data UNUSED) -{ - struct strbuf sb = STRBUF_INIT; - - if (details && *details) - error("%s", details); - - strbuf_addf(&sb, "rejected %s %s %s %s\n", refname, - new_oid ? oid_to_hex(new_oid) : new_target, - old_oid ? oid_to_hex(old_oid) : old_target, - ref_transaction_error_msg(err)); - - fwrite(sb.buf, sb.len, 1, stdout); - strbuf_release(&sb); -} - static void parse_cmd_commit(struct ref_transaction *transaction, const char *next, const char *end UNUSED) { From e31a10418a4c2270651bab326f4715892db9c3ee Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Mon, 4 May 2026 19:44:10 +0200 Subject: [PATCH 6/9] update-ref: handle rejections while adding updates When using git-update-ref(1) with the '--batch-updates' flag, updates rejected by the reference backend are displayed to the user while other updates are applied. This only applies during the commit phase of the transaction. In the following commits, we'll also extend `ref_transaction_update()` to reject updates before a transaction is prepared/committed. In preparation, modify the code in update-ref to also handle non-generic rejections from `ref_transaction_update()`. This involves propagating information to each of the commands on whether updates are allowed to be rejected, and also checking for rejections and only dying for generic failures. Errors encountered during updates will be shown to the user immediately unlike other errors encountered only when the transaction is prepared/committed. As the verification of object IDs and peeled tag objects will move into `ref_transaction_update()` in the following commit, this means that those errors will be shown to the user before other errors, this changes the order of errors, but the functionality remains the same. Signed-off-by: Karthik Nayak Signed-off-by: Junio C Hamano --- builtin/update-ref.c | 137 +++++++++++++++++++++++++++++++------------ 1 file changed, 98 insertions(+), 39 deletions(-) diff --git a/builtin/update-ref.c b/builtin/update-ref.c index 5259cc7226..6355c3dd3e 100644 --- a/builtin/update-ref.c +++ b/builtin/update-ref.c @@ -25,6 +25,15 @@ static unsigned int default_flags; static unsigned create_reflog_flag; static const char *msg; +struct command_options { + /* + * Individual updates are allowed to fail without causing + * update-ref to exit. This is set when using the + * '--batch-updates' flag. + */ + bool allow_update_failures; +}; + /* * Parse one whitespace- or NUL-terminated, possibly C-quoted argument * and append the result to arg. Return a pointer to the terminator. @@ -257,6 +266,31 @@ static void print_rejected_refs(const char *refname, strbuf_release(&sb); } +/* + * Handle transaction errors. If we're using batches updates, we want to only + * die for generic errors and print the remaining to the user. + */ +static void handle_ref_transaction_error(const char *refname, + struct object_id *new_oid, + struct object_id *old_oid, + const char *new_target, + const char *old_target, + enum ref_transaction_error tx_err, + struct strbuf *err, + struct command_options *opts) +{ + if (!tx_err) + return; + + if (tx_err != REF_TRANSACTION_ERROR_GENERIC && opts->allow_update_failures) { + print_rejected_refs(refname, old_oid, new_oid, old_target, + new_target, tx_err, err->buf, NULL); + return; + } + + die("%s", err->buf); +} + /* * The following five parse_cmd_*() functions parse the corresponding * command. In each case, next points at the character following the @@ -268,11 +302,13 @@ static void print_rejected_refs(const char *refname, */ static void parse_cmd_update(struct ref_transaction *transaction, - const char *next, const char *end) + const char *next, const char *end, + struct command_options *opts) { struct strbuf err = STRBUF_INIT; char *refname; struct object_id new_oid, old_oid; + enum ref_transaction_error tx_err; int have_old; refname = parse_refname(&next); @@ -289,12 +325,14 @@ static void parse_cmd_update(struct ref_transaction *transaction, if (*next != line_termination) die("update %s: extra input: %s", refname, next); - if (ref_transaction_update(transaction, refname, - &new_oid, have_old ? &old_oid : NULL, - NULL, NULL, - update_flags | create_reflog_flag, - msg, &err)) - die("%s", err.buf); + tx_err = ref_transaction_update(transaction, refname, + &new_oid, have_old ? &old_oid : NULL, + NULL, NULL, + update_flags | create_reflog_flag, + msg, &err); + handle_ref_transaction_error(refname, &new_oid, have_old ? &old_oid : NULL, + NULL, NULL, tx_err, &err, opts); + update_flags = default_flags; free(refname); @@ -302,9 +340,11 @@ static void parse_cmd_update(struct ref_transaction *transaction, } static void parse_cmd_symref_update(struct ref_transaction *transaction, - const char *next, const char *end UNUSED) + const char *next, const char *end UNUSED, + struct command_options *opts) { char *refname, *new_target, *old_arg; + enum ref_transaction_error tx_err; char *old_target = NULL; struct strbuf err = STRBUF_INIT; struct object_id old_oid; @@ -341,13 +381,15 @@ static void parse_cmd_symref_update(struct ref_transaction *transaction, if (*next != line_termination) die("symref-update %s: extra input: %s", refname, next); - if (ref_transaction_update(transaction, refname, NULL, - have_old_oid ? &old_oid : NULL, - new_target, - have_old_oid ? NULL : old_target, - update_flags | create_reflog_flag, - msg, &err)) - die("%s", err.buf); + tx_err = ref_transaction_update(transaction, refname, NULL, + have_old_oid ? &old_oid : NULL, + new_target, + have_old_oid ? NULL : old_target, + update_flags | create_reflog_flag, + msg, &err); + handle_ref_transaction_error(refname, NULL, have_old_oid ? &old_oid : NULL, + new_target, have_old_oid ? NULL : old_target, + tx_err, &err, opts); update_flags = default_flags; free(refname); @@ -358,11 +400,13 @@ static void parse_cmd_symref_update(struct ref_transaction *transaction, } static void parse_cmd_create(struct ref_transaction *transaction, - const char *next, const char *end) + const char *next, const char *end, + struct command_options *opts) { struct strbuf err = STRBUF_INIT; char *refname; struct object_id new_oid; + enum ref_transaction_error tx_err; refname = parse_refname(&next); if (!refname) @@ -377,22 +421,24 @@ static void parse_cmd_create(struct ref_transaction *transaction, if (*next != line_termination) die("create %s: extra input: %s", refname, next); - if (ref_transaction_create(transaction, refname, &new_oid, NULL, - update_flags | create_reflog_flag, - msg, &err)) - die("%s", err.buf); + tx_err = ref_transaction_create(transaction, refname, &new_oid, NULL, + update_flags | create_reflog_flag, + msg, &err); + handle_ref_transaction_error(refname, &new_oid, NULL, NULL, NULL, tx_err, + &err, opts); update_flags = default_flags; free(refname); strbuf_release(&err); } - static void parse_cmd_symref_create(struct ref_transaction *transaction, - const char *next, const char *end UNUSED) + const char *next, const char *end UNUSED, + struct command_options *opts) { struct strbuf err = STRBUF_INIT; char *refname, *new_target; + enum ref_transaction_error tx_err; refname = parse_refname(&next); if (!refname) @@ -405,10 +451,11 @@ static void parse_cmd_symref_create(struct ref_transaction *transaction, if (*next != line_termination) die("symref-create %s: extra input: %s", refname, next); - if (ref_transaction_create(transaction, refname, NULL, new_target, - update_flags | create_reflog_flag, - msg, &err)) - die("%s", err.buf); + tx_err = ref_transaction_create(transaction, refname, NULL, new_target, + update_flags | create_reflog_flag, + msg, &err); + handle_ref_transaction_error(refname, NULL, NULL, new_target, NULL, + tx_err, &err, opts); update_flags = default_flags; free(refname); @@ -417,7 +464,8 @@ static void parse_cmd_symref_create(struct ref_transaction *transaction, } static void parse_cmd_delete(struct ref_transaction *transaction, - const char *next, const char *end) + const char *next, const char *end, + struct command_options *opts UNUSED) { struct strbuf err = STRBUF_INIT; char *refname; @@ -450,9 +498,9 @@ static void parse_cmd_delete(struct ref_transaction *transaction, strbuf_release(&err); } - static void parse_cmd_symref_delete(struct ref_transaction *transaction, - const char *next, const char *end UNUSED) + const char *next, const char *end UNUSED, + struct command_options *opts UNUSED) { struct strbuf err = STRBUF_INIT; char *refname, *old_target; @@ -479,9 +527,9 @@ static void parse_cmd_symref_delete(struct ref_transaction *transaction, strbuf_release(&err); } - static void parse_cmd_verify(struct ref_transaction *transaction, - const char *next, const char *end) + const char *next, const char *end, + struct command_options *opts UNUSED) { struct strbuf err = STRBUF_INIT; char *refname; @@ -508,7 +556,8 @@ static void parse_cmd_verify(struct ref_transaction *transaction, } static void parse_cmd_symref_verify(struct ref_transaction *transaction, - const char *next, const char *end UNUSED) + const char *next, const char *end UNUSED, + struct command_options *opts UNUSED) { struct strbuf err = STRBUF_INIT; struct object_id old_oid; @@ -550,7 +599,8 @@ static void report_ok(const char *command) } static void parse_cmd_option(struct ref_transaction *transaction UNUSED, - const char *next, const char *end UNUSED) + const char *next, const char *end UNUSED, + struct command_options *opts UNUSED) { const char *rest; if (skip_prefix(next, "no-deref", &rest) && *rest == line_termination) @@ -560,7 +610,8 @@ static void parse_cmd_option(struct ref_transaction *transaction UNUSED, } static void parse_cmd_start(struct ref_transaction *transaction UNUSED, - const char *next, const char *end UNUSED) + const char *next, const char *end UNUSED, + struct command_options *opts UNUSED) { if (*next != line_termination) die("start: extra input: %s", next); @@ -568,7 +619,8 @@ static void parse_cmd_start(struct ref_transaction *transaction UNUSED, } static void parse_cmd_prepare(struct ref_transaction *transaction, - const char *next, const char *end UNUSED) + const char *next, const char *end UNUSED, + struct command_options *opts UNUSED) { struct strbuf error = STRBUF_INIT; if (*next != line_termination) @@ -579,7 +631,8 @@ static void parse_cmd_prepare(struct ref_transaction *transaction, } static void parse_cmd_abort(struct ref_transaction *transaction, - const char *next, const char *end UNUSED) + const char *next, const char *end UNUSED, + struct command_options *opts UNUSED) { struct strbuf error = STRBUF_INIT; if (*next != line_termination) @@ -590,7 +643,8 @@ static void parse_cmd_abort(struct ref_transaction *transaction, } static void parse_cmd_commit(struct ref_transaction *transaction, - const char *next, const char *end UNUSED) + const char *next, const char *end UNUSED, + struct command_options *opts UNUSED) { struct strbuf error = STRBUF_INIT; if (*next != line_termination) @@ -618,7 +672,8 @@ enum update_refs_state { static const struct parse_cmd { const char *prefix; - void (*fn)(struct ref_transaction *, const char *, const char *); + void (*fn)(struct ref_transaction *, const char *, const char *, + struct command_options *); unsigned args; enum update_refs_state state; } command[] = { @@ -644,6 +699,10 @@ static void update_refs_stdin(unsigned int flags) struct ref_transaction *transaction; int i, j; + struct command_options opts = { + .allow_update_failures = flags & REF_TRANSACTION_ALLOW_FAILURE, + }; + transaction = ref_store_transaction_begin(get_main_ref_store(the_repository), flags, &err); if (!transaction) @@ -721,7 +780,7 @@ static void update_refs_stdin(unsigned int flags) } cmd->fn(transaction, input.buf + strlen(cmd->prefix) + !!cmd->args, - input.buf + input.len); + input.buf + input.len, &opts); } switch (state) { From b32c23be3bf444ad8d56e8daee4a704ff8cae0ea Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Mon, 4 May 2026 19:44:11 +0200 Subject: [PATCH 7/9] refs: move object parsing to the generic layer Regular reference updates made via reference transactions validate that the provided object ID exists in the object database, which is done by calling 'parse_object()'. This check is done independently by the backends which leads to duplicated logic. Let's move this to the generic layer, ensuring the backends only have to care about reference storage and not about validation of the object IDs. With this also remove the 'REF_TRANSACTION_ERROR_INVALID_NEW_VALUE' error type as its no longer used. Since we don't iterate over individual references in `ref_transaction_prepare()`, we add this check to `ref_transaction_update()`. This means that the validation is done as soon as an update is queued, without needing to prepare the transaction. It can be argued that this is more ideal, since this validation has no dependency on the reference transaction being prepared. It must be noted that the change in behavior means that this error cannot be ignored even with usage of batched updates, since this happens when the update is being added to the transaction. But since the caller gets specific error codes, they can either abort the transaction or continue adding other updates to the transaction. Modify 'builtin/receive-pack.c' to now capture the error type so that the error propagated to the client stays the same. Also remove two of the tests which validates batch-updates with invalid new_oid. Signed-off-by: Karthik Nayak Signed-off-by: Junio C Hamano --- builtin/receive-pack.c | 22 +++++++++++++--------- refs.c | 18 ++++++++++++++++++ refs/files-backend.c | 28 ++-------------------------- refs/reftable-backend.c | 19 ------------------- t/t1400-update-ref.sh | 14 ++++++++++++++ 5 files changed, 47 insertions(+), 54 deletions(-) diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 878aa7f0ed..376e755e97 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -1641,8 +1641,8 @@ static const char *update(struct command *cmd, struct shallow_info *si) ret = NULL; /* good */ } strbuf_release(&err); - } - else { + } else { + enum ref_transaction_error tx_err; struct strbuf err = STRBUF_INIT; if (shallow_update && si->shallow_ref[cmd->index] && update_shallow_ref(cmd, si)) { @@ -1650,14 +1650,18 @@ static const char *update(struct command *cmd, struct shallow_info *si) goto out; } - if (ref_transaction_update(transaction, - namespaced_name, - new_oid, old_oid, - NULL, NULL, - 0, "push", - &err)) { + tx_err = ref_transaction_update(transaction, + namespaced_name, + new_oid, old_oid, + NULL, NULL, + 0, "push", + &err); + if (tx_err) { rp_error("%s", err.buf); - ret = "failed to update ref"; + if (tx_err == REF_TRANSACTION_ERROR_GENERIC) + ret = "failed to update ref"; + else + ret = ref_transaction_error_msg(tx_err); } else { ret = NULL; /* good */ } diff --git a/refs.c b/refs.c index efa16b739d..662a9e6f9e 100644 --- a/refs.c +++ b/refs.c @@ -1416,6 +1416,24 @@ enum ref_transaction_error ref_transaction_update(struct ref_transaction *transa flags |= (new_oid ? REF_HAVE_NEW : 0) | (old_oid ? REF_HAVE_OLD : 0); flags |= (new_target ? REF_HAVE_NEW : 0) | (old_target ? REF_HAVE_OLD : 0); + if ((flags & REF_HAVE_NEW) && !new_target && !is_null_oid(new_oid) && + !(flags & REF_SKIP_OID_VERIFICATION) && !(flags & REF_LOG_ONLY)) { + struct object *o = parse_object(transaction->ref_store->repo, new_oid); + + if (!o) { + strbuf_addf(err, + _("trying to write ref '%s' with nonexistent object %s"), + refname, oid_to_hex(new_oid)); + return REF_TRANSACTION_ERROR_INVALID_NEW_VALUE; + } + + if (o->type != OBJ_COMMIT && is_branch(refname)) { + strbuf_addf(err, _("trying to write non-commit object %s to branch '%s'"), + oid_to_hex(new_oid), refname); + return REF_TRANSACTION_ERROR_INVALID_NEW_VALUE; + } + } + ref_transaction_add_update(transaction, refname, flags, new_oid, old_oid, new_target, old_target, NULL, msg); diff --git a/refs/files-backend.c b/refs/files-backend.c index 4b2faf4777..f20f580fbc 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -19,7 +19,6 @@ #include "../iterator.h" #include "../dir-iterator.h" #include "../lockfile.h" -#include "../object.h" #include "../path.h" #include "../dir.h" #include "../chdir-notify.h" @@ -1589,7 +1588,6 @@ static int rename_tmp_log(struct files_ref_store *refs, const char *newrefname) static enum ref_transaction_error write_ref_to_lockfile(struct files_ref_store *refs, struct ref_lock *lock, const struct object_id *oid, - int skip_oid_verification, struct strbuf *err); static int commit_ref_update(struct files_ref_store *refs, struct ref_lock *lock, @@ -1737,7 +1735,7 @@ 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, 0, &err) || + if (write_ref_to_lockfile(refs, lock, &orig_oid, &err) || commit_ref_update(refs, lock, &orig_oid, logmsg, 0, &err)) { error("unable to write current sha1 into %s: %s", newrefname, err.buf); strbuf_release(&err); @@ -1755,7 +1753,7 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store, goto rollbacklog; } - if (write_ref_to_lockfile(refs, lock, &orig_oid, 0, &err) || + if (write_ref_to_lockfile(refs, lock, &orig_oid, &err) || commit_ref_update(refs, lock, &orig_oid, NULL, REF_SKIP_CREATE_REFLOG, &err)) { error("unable to write current sha1 into %s: %s", oldrefname, err.buf); strbuf_release(&err); @@ -1999,32 +1997,11 @@ static int files_log_ref_write(struct files_ref_store *refs, static enum ref_transaction_error write_ref_to_lockfile(struct files_ref_store *refs, struct ref_lock *lock, const struct object_id *oid, - int skip_oid_verification, struct strbuf *err) { static char term = '\n'; - struct object *o; int fd; - if (!skip_oid_verification) { - o = parse_object(refs->base.repo, oid); - if (!o) { - strbuf_addf( - err, - "trying to write ref '%s' with nonexistent object %s", - lock->ref_name, oid_to_hex(oid)); - unlock_ref(lock); - return REF_TRANSACTION_ERROR_INVALID_NEW_VALUE; - } - if (o->type != OBJ_COMMIT && is_branch(lock->ref_name)) { - strbuf_addf( - err, - "trying to write non-commit object %s to branch '%s'", - oid_to_hex(oid), lock->ref_name); - unlock_ref(lock); - return REF_TRANSACTION_ERROR_INVALID_NEW_VALUE; - } - } fd = get_lock_file_fd(&lock->lk); if (write_in_full(fd, oid_to_hex(oid), refs->base.repo->hash_algo->hexsz) < 0 || write_in_full(fd, &term, 1) < 0 || @@ -2828,7 +2805,6 @@ static enum ref_transaction_error lock_ref_for_update(struct files_ref_store *re } else { ret = write_ref_to_lockfile( refs, lock, &update->new_oid, - update->flags & REF_SKIP_OID_VERIFICATION, err); if (ret) { char *write_err = strbuf_detach(err, NULL); diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index 93374d25c2..444b0c24e5 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -1081,25 +1081,6 @@ static enum ref_transaction_error prepare_single_update(struct reftable_ref_stor return 0; } - /* Verify that the new object ID is valid. */ - if ((u->flags & REF_HAVE_NEW) && !is_null_oid(&u->new_oid) && - !(u->flags & REF_SKIP_OID_VERIFICATION) && - !(u->flags & REF_LOG_ONLY)) { - struct object *o = parse_object(refs->base.repo, &u->new_oid); - if (!o) { - strbuf_addf(err, - _("trying to write ref '%s' with nonexistent object %s"), - u->refname, oid_to_hex(&u->new_oid)); - return REF_TRANSACTION_ERROR_INVALID_NEW_VALUE; - } - - if (o->type != OBJ_COMMIT && is_branch(u->refname)) { - strbuf_addf(err, _("trying to write non-commit object %s to branch '%s'"), - oid_to_hex(&u->new_oid), u->refname); - return REF_TRANSACTION_ERROR_INVALID_NEW_VALUE; - } - } - /* * When we update the reference that HEAD points to we enqueue * a second log-only update for HEAD so that its reflog is diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh index b2858a9061..1015f335e3 100755 --- a/t/t1400-update-ref.sh +++ b/t/t1400-update-ref.sh @@ -1196,6 +1196,20 @@ test_expect_success 'stdin -z create ref fails with empty new value' ' test_must_fail git rev-parse --verify -q $c ' +test_expect_success 'stdin -z create ref fails with non commit object' ' + printf $F "create $c" "$(test_oid 001)" >stdin && + test_must_fail git update-ref -z --stdin err && + grep "fatal: trying to write ref ${SQ}$c${SQ} with nonexistent object" err && + test_must_fail git rev-parse --verify -q $c +' + +test_expect_success 'stdin -z update ref fails with non commit object' ' + printf $F "update $b" "$(test_oid 001)" "" >stdin && + test_must_fail git update-ref -z --stdin err && + grep "fatal: trying to write ref ${SQ}$b${SQ} with nonexistent object" err && + test_must_fail git rev-parse --verify -q $c +' + test_expect_success 'stdin -z update ref works with right old value' ' printf $F "update $b" "$m~1" "$m" >stdin && git update-ref -z --stdin Date: Mon, 4 May 2026 19:44:12 +0200 Subject: [PATCH 8/9] refs: add peeled object ID to the `ref_update` struct Certain reference backends {packed, reftable}, have the ability to also store the peeled object ID for a reference pointing to a tag object. This has the added benefit that during retrieval of such references, we also obtain the peeled object ID without having to use the ODB. To provide this functionality, each backend independently calls the ODB to obtain the peeled OID. To move this functionality to the generic layer, there must be support infrastructure to pass in a peeled OID for reference updates. Add a `peeled` field to the `ref_update` structure and modify `ref_transaction_add_update()` to receive and copy this object ID to the `ref_update` structure. Finally, modify `ref_transaction_update()` to peel tag objects and pass the peeled OID to `ref_transaction_add_update()`. Update all callers of these functions with the new function parameters. Callers which only add reflog updates, need to only pass in NULL, since for reflogs, we don't store peeled OIDs. Reference deletions also only need to pass in NULL. For others, pass along the peeled OID if available. In a following commit, we'll modify the backends to use this peeled OID instead of parsing it themselves. Signed-off-by: Karthik Nayak Signed-off-by: Junio C Hamano --- refs.c | 15 +++++++++++++-- refs/files-backend.c | 20 ++++++++++++-------- refs/refs-internal.h | 14 ++++++++++++++ refs/reftable-backend.c | 6 +++--- 4 files changed, 42 insertions(+), 13 deletions(-) diff --git a/refs.c b/refs.c index 662a9e6f9e..0648df2b6c 100644 --- a/refs.c +++ b/refs.c @@ -1307,6 +1307,7 @@ struct ref_update *ref_transaction_add_update( const char *refname, unsigned int flags, const struct object_id *new_oid, const struct object_id *old_oid, + const struct object_id *peeled, const char *new_target, const char *old_target, const char *committer_info, const char *msg) @@ -1339,6 +1340,8 @@ struct ref_update *ref_transaction_add_update( update->committer_info = xstrdup_or_null(committer_info); update->msg = normalize_reflog_message(msg); } + if (flags & REF_HAVE_PEELED) + oidcpy(&update->peeled, peeled); /* * This list is generally used by the backends to avoid duplicates. @@ -1392,6 +1395,8 @@ enum ref_transaction_error ref_transaction_update(struct ref_transaction *transa unsigned int flags, const char *msg, struct strbuf *err) { + struct object_id peeled; + assert(err); if ((flags & REF_FORCE_CREATE_REFLOG) && @@ -1432,10 +1437,16 @@ enum ref_transaction_error ref_transaction_update(struct ref_transaction *transa oid_to_hex(new_oid), refname); return REF_TRANSACTION_ERROR_INVALID_NEW_VALUE; } + + if (o->type == OBJ_TAG) { + if (!peel_object(transaction->ref_store->repo, new_oid, &peeled, + PEEL_OBJECT_VERIFY_TAGGED_OBJECT_TYPE)) + flags |= REF_HAVE_PEELED; + } } ref_transaction_add_update(transaction, refname, flags, - new_oid, old_oid, new_target, + new_oid, old_oid, &peeled, new_target, old_target, NULL, msg); return 0; @@ -1462,7 +1473,7 @@ int ref_transaction_update_reflog(struct ref_transaction *transaction, return -1; update = ref_transaction_add_update(transaction, refname, flags, - new_oid, old_oid, NULL, NULL, + new_oid, old_oid, NULL, NULL, NULL, committer_info, msg); update->index = index; diff --git a/refs/files-backend.c b/refs/files-backend.c index f20f580fbc..d0896d0e37 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1325,7 +1325,8 @@ static void prune_ref(struct files_ref_store *refs, struct ref_to_prune *r) ref_transaction_add_update( transaction, r->name, REF_NO_DEREF | REF_HAVE_NEW | REF_HAVE_OLD | REF_IS_PRUNING, - null_oid(the_hash_algo), &r->oid, NULL, NULL, NULL, NULL); + null_oid(the_hash_algo), &r->oid, NULL, NULL, NULL, + NULL, NULL); if (ref_transaction_commit(transaction, &err)) goto cleanup; @@ -2468,7 +2469,7 @@ static enum ref_transaction_error split_head_update(struct ref_update *update, new_update = ref_transaction_add_update( transaction, "HEAD", update->flags | REF_LOG_ONLY | REF_NO_DEREF | REF_LOG_VIA_SPLIT, - &update->new_oid, &update->old_oid, + &update->new_oid, &update->old_oid, &update->peeled, NULL, NULL, update->committer_info, update->msg); new_update->parent_update = update; @@ -2530,8 +2531,8 @@ static enum ref_transaction_error split_symref_update(struct ref_update *update, transaction, referent, new_flags, update->new_target ? NULL : &update->new_oid, update->old_target ? NULL : &update->old_oid, - update->new_target, update->old_target, NULL, - update->msg); + &update->peeled, update->new_target, update->old_target, + NULL, update->msg); new_update->parent_update = update; @@ -2994,7 +2995,7 @@ static int files_transaction_prepare(struct ref_store *ref_store, ref_transaction_add_update( packed_transaction, update->refname, REF_HAVE_NEW | REF_NO_DEREF, - &update->new_oid, NULL, + &update->new_oid, NULL, NULL, NULL, NULL, NULL, NULL); } } @@ -3200,19 +3201,22 @@ static int files_transaction_finish_initial(struct files_ref_store *refs, if (update->flags & REF_LOG_ONLY) ref_transaction_add_update(loose_transaction, update->refname, update->flags, &update->new_oid, - &update->old_oid, NULL, NULL, + &update->old_oid, &update->peeled, + NULL, NULL, update->committer_info, update->msg); else ref_transaction_add_update(loose_transaction, update->refname, update->flags & ~REF_HAVE_OLD, update->new_target ? NULL : &update->new_oid, NULL, - update->new_target, NULL, update->committer_info, + &update->peeled, update->new_target, + NULL, update->committer_info, NULL); } else { ref_transaction_add_update(packed_transaction, update->refname, update->flags & ~REF_HAVE_OLD, &update->new_oid, &update->old_oid, - NULL, NULL, update->committer_info, NULL); + &update->peeled, NULL, NULL, + update->committer_info, NULL); } } diff --git a/refs/refs-internal.h b/refs/refs-internal.h index d103387ebf..307dcb277b 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -39,6 +39,13 @@ struct ref_transaction; */ #define REF_LOG_ONLY (1 << 7) +/* + * The reference contains a peeled object ID. This is used when the + * new_oid is pointing to a tag object and the reference backend + * wants to also store the peeled value for optimized retrieval. + */ +#define REF_HAVE_PEELED (1 << 15) + /* * Return the length of time to retry acquiring a loose reference lock * before giving up, in milliseconds: @@ -92,6 +99,12 @@ struct ref_update { */ struct object_id old_oid; + /* + * If the new_oid points to a tag object, set this to the peeled + * object ID for optimized retrieval without needed to hit the odb. + */ + struct object_id peeled; + /* * If set, point the reference to this value. This can also be * used to convert regular references to become symbolic refs. @@ -169,6 +182,7 @@ struct ref_update *ref_transaction_add_update( const char *refname, unsigned int flags, const struct object_id *new_oid, const struct object_id *old_oid, + const struct object_id *peeled, const char *new_target, const char *old_target, const char *committer_info, const char *msg); diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index 444b0c24e5..b0c010387d 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -1107,8 +1107,8 @@ static enum ref_transaction_error prepare_single_update(struct reftable_ref_stor ref_transaction_add_update( transaction, "HEAD", u->flags | REF_LOG_ONLY | REF_NO_DEREF, - &u->new_oid, &u->old_oid, NULL, NULL, NULL, - u->msg); + &u->new_oid, &u->old_oid, &u->peeled, NULL, NULL, + NULL, u->msg); } ret = reftable_backend_read_ref(be, rewritten_ref, @@ -1194,7 +1194,7 @@ static enum ref_transaction_error prepare_single_update(struct reftable_ref_stor transaction, referent->buf, new_flags, u->new_target ? NULL : &u->new_oid, u->old_target ? NULL : &u->old_oid, - u->new_target, u->old_target, + &u->peeled, u->new_target, u->old_target, u->committer_info, u->msg); new_update->parent_update = u; From 3ab3d5077dc7048f9a0209e06f7de30971a303d7 Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Mon, 4 May 2026 19:44:13 +0200 Subject: [PATCH 9/9] refs: use peeled tag values in reference backends The reference backends peel tag objects when storing references to them. This is to provide optimized reads which avoids hitting the odb. The previous commits ensures that the peeled value is now propagated via the generic layer. So modify the packed and reftable backend to directly use this value instead of calling `peel_object()` independently. Signed-off-by: Karthik Nayak Signed-off-by: Junio C Hamano --- refs/packed-backend.c | 6 ++---- refs/reftable-backend.c | 9 ++------- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 35a0f32e1c..0acde48c45 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1531,13 +1531,11 @@ static enum ref_transaction_error write_with_updates(struct packed_ref_store *re */ i++; } else { - struct object_id peeled; - int peel_error = peel_object(refs->base.repo, &update->new_oid, - &peeled, PEEL_OBJECT_VERIFY_TAGGED_OBJECT_TYPE); + bool peeled = update->flags & REF_HAVE_PEELED; if (write_packed_entry(out, update->refname, &update->new_oid, - peel_error ? NULL : &peeled)) + peeled ? &update->peeled : NULL)) goto write_error; i++; diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index b0c010387d..8b4ac2e618 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -12,7 +12,6 @@ #include "../hex.h" #include "../ident.h" #include "../iterator.h" -#include "../object.h" #include "../parse.h" #include "../path.h" #include "../refs.h" @@ -1584,17 +1583,13 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data goto done; } else if (u->flags & REF_HAVE_NEW) { struct reftable_ref_record ref = {0}; - struct object_id peeled; - int peel_error; ref.refname = (char *)u->refname; ref.update_index = ts; - peel_error = peel_object(arg->refs->base.repo, &u->new_oid, &peeled, - PEEL_OBJECT_VERIFY_TAGGED_OBJECT_TYPE); - if (!peel_error) { + if (u->flags & REF_HAVE_PEELED) { ref.value_type = REFTABLE_REF_VAL2; - memcpy(ref.value.val2.target_value, peeled.hash, GIT_MAX_RAWSZ); + memcpy(ref.value.val2.target_value, u->peeled.hash, GIT_MAX_RAWSZ); memcpy(ref.value.val2.value, u->new_oid.hash, GIT_MAX_RAWSZ); } else if (!is_null_oid(&u->new_oid)) { ref.value_type = REFTABLE_REF_VAL1;