[PATCH v4 0/6] refs: provide detailed error messages when using batched update
- From
Karthik Nayak <karthik.188@gmail.com>
- Date
- Jan 22, 2026, 12:04 UTC
- Message-ID
- <20260122-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v4-0-2ddba0832440@gmail.com>
- In-Reply-To
- <20260114-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v1-0-f5f8b173c501@gmail.com>
The refs namespace uses an error buffer to capture details about failed reference updates. However when we added batched update support to reference transactions, these messages were never propagated, instead only an error code pertaining to the type of failure was propagated.
Currently, there are three regions which utilize batched updates:
- git update-ref --batch-updates - git fetch - git receive-pack
While 'git update-ref --batch-updates' was a newly introduced flag, both 'git fetch' and 'git receive-pack' were pre-existing. Before using batched updates, they provided more detailed error messages to the user, but this changed with the introduction of batched updates. This is a regression in their workings.
This patch series fixes this, by passing the detailed error message and utilizing it whenever available. The regression was reported by Elijah Newren [1] and based on the patch submitted by Jeff King [2].
[1]: https://lore.kernel.org/all/CABPp-BGL2tJR4dPidQuFcp-X0_VkVTknCY-0Zgo=jHVGv_P=wA@mail.gmail.com/ [2]: https://lore.kernel.org/all/20251224081214.GA1879908@coredump.intra.peff.net/
---
Changes in v4:
- In the last commit, instead of propagating {*list, count}, propagate
an array with {*list, nr, count} and use ALLOC_GROW. This simplifies
the variables passed and cleanups the code.
- Link to v3: https://patch.msgid.link/20260120-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v3-0-e0edb29acbef@gmail.comChanges in v3: - Drop the first commit. - For the last commit, where we delay 'git fetch' status information, delay all information to the end. Also use a list to compliment the existing strmap, this ensures that the order is maintained. - Link to v2: https://patch.msgid.link/20260116-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v2-0-925a0e9c7f32@gmail.com
Changes in v2: - Updates to the commit messages to be more descriptive. - Instead of passing the char pointer for the error description, pass the 'strbuf' itself. This makes the API a lot cleaner to deal with. Also avoids having to remember to reset the strbuf after usage. - Chalk out a separate commit for using a 'goto next_ref' in `refs_verify_refnames_available()`. This makes the intention much clearer. - For git-update-ref(1), keep the existing implementation as is and only output the detailed error message to stderr. - For git-receive-pack(1), use 'rp_error()' for detailed error message while keeping the current implementation as is. - Added a separate patch to handle missing information in git-fetch(1)'s status table. This involves delaying updates to the end, where update success/failure information is available. I'm not too confident about this approach though, we could also drop it from the series and I could pick that up independently. This is still 1.19 ± 0.02 times faster than non-batched version (v2.50.0) in the files backend. - Link to v1: https://patch.msgid.link/20260114-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v1-0-f5f8b173c501@gmail.com
--- builtin/fetch.c | 255 +++++++++++++++++++++++++++++++++++++----------- builtin/receive-pack.c | 7 +- builtin/update-ref.c | 7 +- refs.c | 46 +++++---- refs.h | 1 + refs/files-backend.c | 5 +- refs/packed-backend.c | 12 +-- refs/refs-internal.h | 4 +- refs/reftable-backend.c | 5 +- t/t1400-update-ref.sh | 71 ++++++++------ t/t5510-fetch.sh | 8 +- t/t5516-fetch-push.sh | 16 +++ 12 files changed, 312 insertions(+), 125 deletions(-)
Karthik Nayak (6):
refs: skip to next ref when current ref is rejected
refs: add rejection detail to the callback function
update-ref: utilize rejected error details if available
fetch: utilize rejected ref error details
receive-pack: utilize rejected ref error details
fetch: delay user information post committing of transactionRange-diff versus v3:
1: f5fa12101b = 1: f89b8a3526 refs: skip to next ref when current ref is rejected
2: 6911cab2c7 = 2: c988070f5f refs: add rejection detail to the callback function
3: c20fc32d3e = 3: f419704bca update-ref: utilize rejected error details if available
4: 8b5ce22c65 = 4: 0c50af08f3 fetch: utilize rejected ref error details
5: 73a43ddeeb = 5: 758a265930 receive-pack: utilize rejected ref error details
6: f9b76d57f8 ! 6: c22d759ae8 fetch: delay user information post committing of transaction
@@ builtin/fetch.c: static void display_ref_update(struct display_state *display_st
+ struct object_id new_oid;
+};
+
++struct ref_update_display_info_array {
++ struct ref_update_display_info *info;
++ size_t alloc, nr;
++};
++
+static struct ref_update_display_info *ref_update_display_info_append(
-+ struct ref_update_display_info **list,
-+ size_t *count,
++ struct ref_update_display_info_array *array,
+ char success_code,
+ char fail_code,
+ const char *summary,
@@ builtin/fetch.c: static void display_ref_update(struct display_state *display_st
+ const struct object_id *new_oid)
+{
+ struct ref_update_display_info *info;
-+ size_t index = *count;
-+
-+ (*count)++;
-+ REALLOC_ARRAY(*list, *count);
+
-+ info = &(*list)[index];
++ ALLOC_GROW(array->info, array->nr + 1, array->alloc);
++ info = &array->info[array->nr++];
+
+ info->failed = false;
+ info->success_code = success_code;
@@ builtin/fetch.c: static void display_ref_update(struct display_state *display_st
- int summary_width,
- const struct fetch_config *config)
+ const struct fetch_config *config,
-+ struct ref_update_display_info **display_list,
-+ size_t *display_count)
++ struct ref_update_display_info_array *display_array)
{
struct commit *current = NULL, *updated;
int fast_forward = 0;
@@ builtin/fetch.c: static int update_local_ref(struct ref *ref,
- display_ref_update(display_state, '=', _("[up to date]"), NULL,
- remote_ref->name, ref->name,
- &ref->old_oid, &ref->new_oid, summary_width);
-+ ref_update_display_info_append(display_list, display_count,
-+ '=', '=', _("[up to date]"),
-+ NULL, NULL, ref->name,
++ ref_update_display_info_append(display_array, '=', '=',
++ _("[up to date]"), NULL,
++ NULL, ref->name,
+ remote_ref->name, &ref->old_oid,
+ &ref->new_oid);
return 0;
@@ builtin/fetch.c: static int update_local_ref(struct ref *ref,
- _("can't fetch into checked-out branch"),
- remote_ref->name, ref->name,
- &ref->old_oid, &ref->new_oid, summary_width);
-+ info = ref_update_display_info_append(display_list, display_count,
-+ '!', '!', _("[rejected]"),
-+ NULL, _("can't fetch into checked-out branch"),
++ info = ref_update_display_info_append(display_array, '!', '!',
++ _("[rejected]"), NULL,
++ _("can't fetch into checked-out branch"),
+ ref->name, remote_ref->name,
+ &ref->old_oid, &ref->new_oid);
+ ref_update_display_info_set_failed(info);
@@ builtin/fetch.c: static int update_local_ref(struct ref *ref,
- remote_ref->name, ref->name,
- &ref->old_oid, &ref->new_oid, summary_width);
+
-+ info = ref_update_display_info_append(display_list, display_count,
-+ 't', '!', _("[tag update]"), NULL,
++ info = ref_update_display_info_append(display_array, 't', '!',
++ _("[tag update]"), NULL,
+ _("unable to update local ref"),
+ ref->name, remote_ref->name,
+ &ref->old_oid, &ref->new_oid);
@@ builtin/fetch.c: static int update_local_ref(struct ref *ref,
- _("would clobber existing tag"),
- remote_ref->name, ref->name,
- &ref->old_oid, &ref->new_oid, summary_width);
-+ info = ref_update_display_info_append(display_list, display_count,
-+ '!', '!', _("[rejected]"), NULL,
++ info = ref_update_display_info_append(display_array, '!', '!',
++ _("[rejected]"), NULL,
+ _("would clobber existing tag"),
+ ref->name, remote_ref->name,
+ &ref->old_oid, &ref->new_oid);
@@ builtin/fetch.c: static int update_local_ref(struct ref *ref,
- remote_ref->name, ref->name,
- &ref->old_oid, &ref->new_oid, summary_width);
+
-+ info = ref_update_display_info_append(display_list, display_count,
-+ '*', '!', what, NULL,
++ info = ref_update_display_info_append(display_array, '*', '!',
++ what, NULL,
+ _("unable to update local ref"),
+ ref->name, remote_ref->name,
+ &ref->old_oid, &ref->new_oid);
@@ builtin/fetch.c: static int update_local_ref(struct ref *ref,
- remote_ref->name, ref->name,
- &ref->old_oid, &ref->new_oid, summary_width);
+
-+ info = ref_update_display_info_append(display_list, display_count,
-+ ' ', '!', quickref.buf, NULL,
++ info = ref_update_display_info_append(display_array, ' ', '!',
++ quickref.buf, NULL,
+ _("unable to update local ref"),
+ ref->name, remote_ref->name,
+ &ref->old_oid, &ref->new_oid);
@@ builtin/fetch.c: static int update_local_ref(struct ref *ref,
- remote_ref->name, ref->name,
- &ref->old_oid, &ref->new_oid, summary_width);
+
-+ info = ref_update_display_info_append(display_list, display_count,
-+ '+', '!', quickref.buf, _("forced update"),
++ info = ref_update_display_info_append(display_array, '+', '!',
++ quickref.buf, _("forced update"),
+ _("unable to update local ref"),
+ ref->name, remote_ref->name,
+ &ref->old_oid, &ref->new_oid);
@@ builtin/fetch.c: static int update_local_ref(struct ref *ref,
- remote_ref->name, ref->name,
- &ref->old_oid, &ref->new_oid, summary_width);
+ struct ref_update_display_info *info;
-+ info = ref_update_display_info_append(display_list, display_count,
-+ '!', '!', _("[rejected]"), NULL,
++ info = ref_update_display_info_append(display_array, '!', '!',
++ _("[rejected]"), NULL,
+ _("non-fast-forward"),
+ ref->name, remote_ref->name,
+ &ref->old_oid, &ref->new_oid);
@@ builtin/fetch.c: static int store_updated_refs(struct display_state *display_sta
struct fetch_head *fetch_head,
- const struct fetch_config *config)
+ const struct fetch_config *config,
-+ struct ref_update_display_info **display_list,
-+ size_t *display_count)
++ struct ref_update_display_info_array *display_array)
{
int rc = 0;
struct strbuf note = STRBUF_INIT;
@@ builtin/fetch.c: static int store_updated_refs(struct display_state *display_sta
- rc |= update_local_ref(ref, transaction, display_state,
- rm, summary_width, config);
+ rc |= update_local_ref(ref, transaction, rm,
-+ config, display_list,
-+ display_count);
++ config, display_array);
free(ref);
} else if (write_fetch_head || dry_run) {
/*
@@ builtin/fetch.c: static int store_updated_refs(struct display_state *display_sta
- &rm->new_oid, &rm->old_oid,
- summary_width);
+
-+ ref_update_display_info_append(display_list, display_count,
-+ '*', '*', *kind ? kind : "branch",
-+ NULL, NULL, "FETCH_HEAD", rm->name,
-+ &rm->new_oid, &rm->old_oid);
++ ref_update_display_info_append(display_array, '*', '*',
++ *kind ? kind : "branch",
++ NULL, NULL, "FETCH_HEAD",
++ rm->name, &rm->new_oid,
++ &rm->old_oid);
}
}
}
@@ builtin/fetch.c: static int fetch_and_consume_refs(struct display_state *display
struct fetch_head *fetch_head,
- const struct fetch_config *config)
+ const struct fetch_config *config,
-+ struct ref_update_display_info **display_list,
-+ size_t *display_count)
++ struct ref_update_display_info_array *display_array)
{
int connectivity_checked = 1;
int ret;
@@ builtin/fetch.c: static int fetch_and_consume_refs(struct display_state *display
ret = store_updated_refs(display_state, connectivity_checked,
- transaction, ref_map, fetch_head, config);
+ transaction, ref_map, fetch_head, config,
-+ display_list, display_count);
++ display_array);
trace2_region_leave("fetch", "consume_refs", the_repository);
out:
@@ builtin/fetch.c: static int backfill_tags(struct display_state *display_state,
struct fetch_head *fetch_head,
- const struct fetch_config *config)
+ const struct fetch_config *config,
-+ struct ref_update_display_info **display_list,
-+ size_t *display_count)
++ struct ref_update_display_info_array *display_array)
{
int retcode, cannot_reuse;
@@ builtin/fetch.c: static int backfill_tags(struct display_state *display_state,
transport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL);
retcode = fetch_and_consume_refs(display_state, transport, transaction, ref_map,
- fetch_head, config);
-+ fetch_head, config, display_list, display_count);
++ fetch_head, config, display_array);
if (gsecondary) {
transport_disconnect(gsecondary);
@@ builtin/fetch.c: static int do_fetch(struct transport *transport,
struct fetch_head fetch_head = { 0 };
struct strbuf err = STRBUF_INIT;
int do_set_head = 0;
-+ struct ref_update_display_info *display_list = NULL;
++ struct ref_update_display_info_array display_array = { 0 };
+ struct strmap rejected_refs = STRMAP_INIT;
-+ size_t display_count = 0;
+ int summary_width = 0;
if (tags == TAGS_DEFAULT) {
@@ builtin/fetch.c: static int do_fetch(struct transport *transport,
if (fetch_and_consume_refs(&display_state, transport, transaction, ref_map,
- &fetch_head, config)) {
-+ &fetch_head, config, &display_list, &display_count)) {
++ &fetch_head, config, &display_array)) {
retcode = 1;
goto cleanup;
}
@@ builtin/fetch.c: static int do_fetch(struct transport *transport,
*/
if (backfill_tags(&display_state, transport, transaction, tags_ref_map,
- &fetch_head, config))
-+ &fetch_head, config, &display_list, &display_count))
++ &fetch_head, config, &display_array))
retcode = 1;
}
@@ builtin/fetch.c: static int do_fetch(struct transport *transport,
+ transport->remote->name,
+ &rejected_refs, &err);
+
-+ for (size_t i = 0; i < display_count; i++) {
-+ struct ref_update_display_info *info = &display_list[i];
++ for (size_t i = 0; i < display_array.nr; i++) {
++ struct ref_update_display_info *info = &display_array.info[i];
+
+ if (!info->failed && strmap_contains(&rejected_refs, info->ref))
+ ref_update_display_info_set_failed(info);
@@ builtin/fetch.c: static int do_fetch(struct transport *transport,
if (transaction)
ref_transaction_free(transaction);
+
-+ free(display_list);
++ free(display_array.info);
+ strmap_clear(&rejected_refs, 0);
display_state_release(&display_state);
close_fetch_head(&fetch_head);base-commit: 8745eae506f700657882b9e32b2aa00f234a6fb6 change-id: 20260113-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-17786b20894a
Thanks - Karthik