{"thread":{"id":"64802","subject":"[PATCH 0/6] refs: provide detailed error messages when using batched update","startedAt":"2026-01-14T15:41:07Z","lastAt":"2026-01-25T22:52:52Z","messageCount":32,"participants":["Karthik Nayak","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"533831","messageId":"20260114-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v1-0-f5f8b173c501@gmail.com","threadId":"64802","inReplyTo":null,"subject":"[PATCH 0/6] refs: provide detailed error messages when using batched update","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-14T15:40:41Z","receivedAt":"2026-01-14T15:41:07Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The refs namespace uses an error buffer to capture details about failed\nreference updates. However when we added batched update support to\nreference transactions, these messages were never propagated, instead\nonly an error code pertaining to the type of failure was propagated.\n\nCurrently, there are three regions which utilize batched updates:\n\n - git update-ref --batch-updates\n - git fetch\n - git receive-pack\n\nWhile 'git update-ref --batch-updates' was a newly introduced flag, both\n'git fetch' and 'git receive-pack' were pre-existing. Before using\nbatched updates, they provided more detailed error messages to the user,\nbut this changed with the introduction of batched updates. This is a\nregression in their workings.\n\nThis patch series fixes this, by passing the detailed error message and\nutilizing it whenever available. The regression was reported by Elijah\nNewren [1] and based on the patch submitted by Jeff King [2].\n\n[1]: https://lore.kernel.org/all/CABPp-BGL2tJR4dPidQuFcp-X0_VkVTknCY-0Zgo=jHVGv_P=wA@mail.gmail.com/\n[2]: https://lore.kernel.org/all/20251224081214.GA1879908@coredump.intra.peff.net/\n\n---\n builtin/fetch.c | 9 +++++---\n builtin/receive-pack.c | 9 ++++++--\n builtin/update-ref.c | 13 +++++++-----\n refs.c | 56 ++++++++++++++++++++++++++++++-------------------\n refs.h | 1 +\n refs/files-backend.c | 3 ++-\n refs/packed-backend.c | 9 +++++---\n refs/refs-internal.h | 4 +++-\n refs/reftable-backend.c | 3 ++-\n t/t1400-update-ref.sh | 26 +++++++++++------------\n t/t5510-fetch.sh | 8 +++----\n t/t5516-fetch-push.sh | 15 +++++++++++++\n 12 files changed, 102 insertions(+), 54 deletions(-)\n\nKarthik Nayak (6):\n refs: remove unused header\n refs: attach rejection details to updates\n refs: add rejection detail to the callback function\n update-ref: utilize rejected error details if available\n fetch: utilize rejected ref error details\n receive-pack: utilize rejected ref error details\n\n\n\nbase-commit: 8745eae506f700657882b9e32b2aa00f234a6fb6\nchange-id: 20260113-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-17786b20894a\n\nThanks\n- Karthik\n\n"},{"id":"533832","messageId":"20260114-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v1-1-f5f8b173c501@gmail.com","threadId":"64802","inReplyTo":"20260114-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v1-0-f5f8b173c501@gmail.com","subject":"[PATCH 1/6] refs: remove unused header","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-14T15:40:42Z","receivedAt":"2026-01-14T15:41:07Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Some of the headers in 'refs.c' are no longer required, let's remove\nthem.\n\nSigned-off-by: Karthik Nayak \n---\n refs.c | 2 --\n 1 file changed, 2 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex e06e0cb072..965b232a06 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -15,7 +15,6 @@\n #include \"iterator.h\"\n #include \"refs.h\"\n #include \"refs/refs-internal.h\"\n-#include \"run-command.h\"\n #include \"hook.h\"\n #include \"object-name.h\"\n #include \"odb.h\"\n@@ -26,7 +25,6 @@\n #include \"strvec.h\"\n #include \"repo-settings.h\"\n #include \"setup.h\"\n-#include \"sigchain.h\"\n #include \"date.h\"\n #include \"commit.h\"\n #include \"wildmatch.h\"\n\n-- \n2.51.2\n\n"},{"id":"533833","messageId":"20260114-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v1-2-f5f8b173c501@gmail.com","threadId":"64802","inReplyTo":"20260114-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v1-0-f5f8b173c501@gmail.com","subject":"[PATCH 2/6] refs: attach rejection details to updates","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-14T15:40:43Z","receivedAt":"2026-01-14T15:41:08Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The implementation of batched updates in 23fc8e4f61 (refs: implement\nbatch reference update support, 2025-04-08) added rejection error codes\nto each reference update. This allowed batching of updates, however\nwhile each rejection is linked to a rejection code, the already present\nuser readable error message is simply dropped.\n\nMake necessary changes to ensure that the rejection detail is also added\nto the reference update. In upcoming commits, we'll utilize this field\nto provide better error message to users, namely in:\n\n - git update-ref --batch-updates\n - git fetch\n - git receive-pack\n\nWe move the error message creation right above\n`ref_transaction_maybe_set_rejected()`, so that the error message is\navailable and also reset the error message if utilized to avoid\nun-expected concatination.\n\nCo-authored-by: Jeff King \nSigned-off-by: Karthik Nayak \n---\n refs.c | 52 ++++++++++++++++++++++++++++++++-----------------\n refs/files-backend.c | 3 ++-\n refs/packed-backend.c | 9 ++++++---\n refs/refs-internal.h | 4 +++-\n refs/reftable-backend.c | 3 ++-\n 5 files changed, 47 insertions(+), 24 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 965b232a06..991bd8e6ee 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1222,6 +1222,7 @@ void ref_transaction_free(struct ref_transaction *transaction)\n \t\tfree(transaction->updates[i]->committer_info);\n \t\tfree((char *)transaction->updates[i]->new_target);\n \t\tfree((char *)transaction->updates[i]->old_target);\n+\t\tfree((char *)transaction->updates[i]->rejection_details);\n \t\tfree(transaction->updates[i]);\n \t}\n \n@@ -1236,7 +1237,8 @@ void ref_transaction_free(struct ref_transaction *transaction)\n \n int ref_transaction_maybe_set_rejected(struct ref_transaction *transaction,\n \t\t\t\t size_t update_idx,\n-\t\t\t\t enum ref_transaction_error err)\n+\t\t\t\t enum ref_transaction_error err,\n+\t\t\t\t const char *details)\n {\n \tif (update_idx >= transaction->nr)\n \t\tBUG(\"trying to set rejection on invalid update index\");\n@@ -1262,6 +1264,8 @@ int ref_transaction_maybe_set_rejected(struct ref_transaction *transaction,\n \t\t\t transaction->updates[update_idx]->refname, 0);\n \n \ttransaction->updates[update_idx]->rejection_err = err;\n+\tif (details)\n+\t\ttransaction->updates[update_idx]->rejection_details = xstrdup(details);\n \tALLOC_GROW(transaction->rejections->update_indices,\n \t\t transaction->rejections->nr + 1,\n \t\t transaction->rejections->alloc);\n@@ -2657,30 +2661,35 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs\n \t\t\tif (!initial_transaction &&\n \t\t\t (strset_contains(&conflicting_dirnames, dirname.buf) ||\n \t\t\t !refs_read_raw_ref(refs, dirname.buf, &oid, &referent,\n-\t\t\t\t\t\t &type, &ignore_errno))) {\n+\t\t\t\t\t\t&type, &ignore_errno))) {\n+\n+\t\t\t\tstrbuf_addf(err, _(\"'%s' exists; cannot create '%s'\"),\n+\t\t\t\t\t dirname.buf, refname);\n+\n \t\t\t\tif (transaction && ref_transaction_maybe_set_rejected(\n \t\t\t\t\t transaction, *update_idx,\n-\t\t\t\t\t REF_TRANSACTION_ERROR_NAME_CONFLICT)) {\n+\t\t\t\t\t REF_TRANSACTION_ERROR_NAME_CONFLICT, err->buf)) {\n \t\t\t\t\tstrset_remove(&dirnames, dirname.buf);\n \t\t\t\t\tstrset_add(&conflicting_dirnames, dirname.buf);\n-\t\t\t\t\tcontinue;\n+\t\t\t\t\tstrbuf_reset(err);\n+\t\t\t\t\tgoto next;\n \t\t\t\t}\n \n-\t\t\t\tstrbuf_addf(err, _(\"'%s' exists; cannot create '%s'\"),\n-\t\t\t\t\t dirname.buf, refname);\n \t\t\t\tgoto cleanup;\n \t\t\t}\n \n \t\t\tif (extras && string_list_has_string(extras, dirname.buf)) {\n+\t\t\t\tstrbuf_addf(err, _(\"cannot process '%s' and '%s' at the same time\"),\n+\t\t\t\t\t refname, dirname.buf);\n+\n \t\t\t\tif (transaction && ref_transaction_maybe_set_rejected(\n \t\t\t\t\t transaction, *update_idx,\n-\t\t\t\t\t REF_TRANSACTION_ERROR_NAME_CONFLICT)) {\n+\t\t\t\t\t REF_TRANSACTION_ERROR_NAME_CONFLICT, err->buf)) {\n \t\t\t\t\tstrset_remove(&dirnames, dirname.buf);\n-\t\t\t\t\tcontinue;\n+\t\t\t\t\tstrbuf_reset(err);\n+\t\t\t\t\tgoto next;\n \t\t\t\t}\n \n-\t\t\t\tstrbuf_addf(err, _(\"cannot process '%s' and '%s' at the same time\"),\n-\t\t\t\t\t refname, dirname.buf);\n \t\t\t\tgoto cleanup;\n \t\t\t}\n \t\t}\n@@ -2711,13 +2720,16 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs\n \t\t\t\t string_list_has_string(skip, iter->ref.name))\n \t\t\t\t\tcontinue;\n \n+\t\t\t\tstrbuf_addf(err, _(\"'%s' exists; cannot create '%s'\"),\n+\t\t\t\t\t iter->ref.name, refname);\n+\n \t\t\t\tif (transaction && ref_transaction_maybe_set_rejected(\n \t\t\t\t\t transaction, *update_idx,\n-\t\t\t\t\t REF_TRANSACTION_ERROR_NAME_CONFLICT))\n-\t\t\t\t\tcontinue;\n+\t\t\t\t\t REF_TRANSACTION_ERROR_NAME_CONFLICT, err->buf)) {\n+\t\t\t\t\tstrbuf_reset(err);\n+\t\t\t\t\tgoto next;\n+\t\t\t\t}\n \n-\t\t\t\tstrbuf_addf(err, _(\"'%s' exists; cannot create '%s'\"),\n-\t\t\t\t\t iter->ref.name, refname);\n \t\t\t\tgoto cleanup;\n \t\t\t}\n \n@@ -2727,15 +2739,19 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs\n \n \t\textra_refname = find_descendant_ref(dirname.buf, extras, skip);\n \t\tif (extra_refname) {\n+\t\t\tstrbuf_addf(err, _(\"cannot process '%s' and '%s' at the same time\"),\n+\t\t\t\t refname, extra_refname);\n+\n \t\t\tif (transaction && ref_transaction_maybe_set_rejected(\n \t\t\t\t transaction, *update_idx,\n-\t\t\t\t REF_TRANSACTION_ERROR_NAME_CONFLICT))\n-\t\t\t\tcontinue;\n+\t\t\t\t REF_TRANSACTION_ERROR_NAME_CONFLICT, err->buf)) {\n+\t\t\t\tstrbuf_reset(err);\n+\t\t\t\tgoto next;\n+\t\t\t}\n \n-\t\t\tstrbuf_addf(err, _(\"cannot process '%s' and '%s' at the same time\"),\n-\t\t\t\t refname, extra_refname);\n \t\t\tgoto cleanup;\n \t\t}\n+next:;\n \t}\n \n \tret = 0;\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 6f6f76a8d8..8d22a2e8e3 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -2983,7 +2983,8 @@ static int files_transaction_prepare(struct ref_store *ref_store,\n \t\t\t\t\t head_ref, &refnames_to_check,\n \t\t\t\t\t err);\n \t\tif (ret) {\n-\t\t\tif (ref_transaction_maybe_set_rejected(transaction, i, ret)) {\n+\t\t\tif (ref_transaction_maybe_set_rejected(transaction, i,\n+\t\t\t\t\t\t\t ret, err->buf)) {\n \t\t\t\tstrbuf_reset(err);\n \t\t\t\tret = 0;\n \ndiff --git a/refs/packed-backend.c b/refs/packed-backend.c\nindex 4ea0c12299..535200db01 100644\n--- a/refs/packed-backend.c\n+++ b/refs/packed-backend.c\n@@ -1437,7 +1437,8 @@ static enum ref_transaction_error write_with_updates(struct packed_ref_store *re\n \t\t\t\t\t\t update->refname);\n \t\t\t\t\tret = REF_TRANSACTION_ERROR_CREATE_EXISTS;\n \n-\t\t\t\t\tif (ref_transaction_maybe_set_rejected(transaction, i, ret)) {\n+\t\t\t\t\tif (ref_transaction_maybe_set_rejected(transaction, i,\n+\t\t\t\t\t\t\t\t\t ret, err->buf)) {\n \t\t\t\t\t\tstrbuf_reset(err);\n \t\t\t\t\t\tret = 0;\n \t\t\t\t\t\tcontinue;\n@@ -1452,7 +1453,8 @@ static enum ref_transaction_error write_with_updates(struct packed_ref_store *re\n \t\t\t\t\t\t oid_to_hex(&update->old_oid));\n \t\t\t\t\tret = REF_TRANSACTION_ERROR_INCORRECT_OLD_VALUE;\n \n-\t\t\t\t\tif (ref_transaction_maybe_set_rejected(transaction, i, ret)) {\n+\t\t\t\t\tif (ref_transaction_maybe_set_rejected(transaction, i,\n+\t\t\t\t\t\t\t\t\t ret, err->buf)) {\n \t\t\t\t\t\tstrbuf_reset(err);\n \t\t\t\t\t\tret = 0;\n \t\t\t\t\t\tcontinue;\n@@ -1496,7 +1498,8 @@ static enum ref_transaction_error write_with_updates(struct packed_ref_store *re\n \t\t\t\t\t oid_to_hex(&update->old_oid));\n \t\t\t\tret = REF_TRANSACTION_ERROR_NONEXISTENT_REF;\n \n-\t\t\t\tif (ref_transaction_maybe_set_rejected(transaction, i, ret)) {\n+\t\t\t\tif (ref_transaction_maybe_set_rejected(transaction, i,\n+\t\t\t\t\t\t\t\t ret, err->buf)) {\n \t\t\t\t\tstrbuf_reset(err);\n \t\t\t\t\tret = 0;\n \t\t\t\t\tcontinue;\ndiff --git a/refs/refs-internal.h b/refs/refs-internal.h\nindex c7d2a6e50b..60d9f015cf 100644\n--- a/refs/refs-internal.h\n+++ b/refs/refs-internal.h\n@@ -128,6 +128,7 @@ struct ref_update {\n \t * was rejected.\n \t */\n \tenum ref_transaction_error rejection_err;\n+\tconst char *rejection_details;\n \n \t/*\n \t * If this ref_update was split off of a symref update via\n@@ -153,7 +154,8 @@ int refs_read_raw_ref(struct ref_store *ref_store, const char *refname,\n */\n int ref_transaction_maybe_set_rejected(struct ref_transaction *transaction,\n \t\t\t\t size_t update_idx,\n-\t\t\t\t enum ref_transaction_error err);\n+\t\t\t\t enum ref_transaction_error err,\n+\t\t\t\t const char *details);\n \n /*\n * Add a ref_update with the specified properties to transaction, and\ndiff --git a/refs/reftable-backend.c b/refs/reftable-backend.c\nindex 4319a4eacb..a9c9ceebf3 100644\n--- a/refs/reftable-backend.c\n+++ b/refs/reftable-backend.c\n@@ -1401,7 +1401,8 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,\n \t\t\t\t\t &refnames_to_check, head_type,\n \t\t\t\t\t &head_referent, &referent, err);\n \t\tif (ret) {\n-\t\t\tif (ref_transaction_maybe_set_rejected(transaction, i, ret)) {\n+\t\t\tif (ref_transaction_maybe_set_rejected(transaction, i,\n+\t\t\t\t\t\t\t ret, err->buf)) {\n \t\t\t\tstrbuf_reset(err);\n \t\t\t\tret = 0;\n \n\n-- \n2.51.2\n\n"},{"id":"533834","messageId":"20260114-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v1-3-f5f8b173c501@gmail.com","threadId":"64802","inReplyTo":"20260114-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v1-0-f5f8b173c501@gmail.com","subject":"[PATCH 3/6] refs: add rejection detail to the callback function","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-14T15:40:44Z","receivedAt":"2026-01-14T15:41:09Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The previous commit started storing the rejection details alongside the\nerror code for rejected updates. Pass this along to the callback\nfunction `ref_transaction_for_each_rejected_update()`. Currently the\nfield is unused, but will be integrated in the upcoming commits.\n\nCo-authored-by: Jeff King \nSigned-off-by: Karthik Nayak \n---\n builtin/fetch.c | 1 +\n builtin/receive-pack.c | 1 +\n builtin/update-ref.c | 1 +\n refs.c | 2 +-\n refs.h | 1 +\n 5 files changed, 5 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 288d3772ea..d427adea61 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -1649,6 +1649,7 @@ static void ref_transaction_rejection_handler(const char *refname,\n \t\t\t\t\t const char *old_target UNUSED,\n \t\t\t\t\t const char *new_target UNUSED,\n \t\t\t\t\t enum ref_transaction_error err,\n+\t\t\t\t\t const char *details UNUSED,\n \t\t\t\t\t void *cb_data)\n {\n \tstruct ref_rejection_data *data = cb_data;\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex ef1f77be8c..94d3e73cee 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -1813,6 +1813,7 @@ static void ref_transaction_rejection_handler(const char *refname,\n \t\t\t\t\t const char *old_target UNUSED,\n \t\t\t\t\t const char *new_target UNUSED,\n \t\t\t\t\t enum ref_transaction_error err,\n+\t\t\t\t\t const char *details UNUSED,\n \t\t\t\t\t void *cb_data)\n {\n \tstruct strmap *failed_refs = cb_data;\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 195437e7c6..0046a87c57 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -573,6 +573,7 @@ static void print_rejected_refs(const char *refname,\n \t\t\t\tconst char *old_target,\n \t\t\t\tconst char *new_target,\n \t\t\t\tenum ref_transaction_error err,\n+\t\t\t\tconst char *details UNUSED,\n \t\t\t\tvoid *cb_data UNUSED)\n {\n \tstruct strbuf sb = STRBUF_INIT;\ndiff --git a/refs.c b/refs.c\nindex 991bd8e6ee..ad1898598f 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2880,7 +2880,7 @@ void ref_transaction_for_each_rejected_update(struct ref_transaction *transactio\n \t\t (update->flags & REF_HAVE_OLD) ? &update->old_oid : NULL,\n \t\t (update->flags & REF_HAVE_NEW) ? &update->new_oid : NULL,\n \t\t update->old_target, update->new_target,\n-\t\t update->rejection_err, cb_data);\n+\t\t update->rejection_err, update->rejection_details, cb_data);\n \t}\n }\n \ndiff --git a/refs.h b/refs.h\nindex d9051bbb04..4fbe3da924 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -975,6 +975,7 @@ typedef void ref_transaction_for_each_rejected_update_fn(const char *refname,\n \t\t\t\t\t\t\t const char *old_target,\n \t\t\t\t\t\t\t const char *new_target,\n \t\t\t\t\t\t\t enum ref_transaction_error err,\n+\t\t\t\t\t\t\t const char *details,\n \t\t\t\t\t\t\t void *cb_data);\n void ref_transaction_for_each_rejected_update(struct ref_transaction *transaction,\n \t\t\t\t\t ref_transaction_for_each_rejected_update_fn cb,\n\n-- \n2.51.2\n\n"},{"id":"533835","messageId":"20260114-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v1-4-f5f8b173c501@gmail.com","threadId":"64802","inReplyTo":"20260114-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v1-0-f5f8b173c501@gmail.com","subject":"[PATCH 4/6] update-ref: utilize rejected error details if available","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-14T15:40:45Z","receivedAt":"2026-01-14T15:41:10Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"When git-update-ref(1) received the '--update-ref' flag, the error\ndetails generated in the refs namespace wasn't propagated with failed\nupdates. Instead only an error code pertaining to the type of rejection\nwas noted.\n\nThis missed detailed error message which the user can act upon. The\nprevious commits added the required code to propagate these detailed\nerror messages from the refs namespace. Now that additional details are\navailable, use them instead of the generic error message based of the\nerror code. Fix the tests to also accommodate these error messages.\n\nReported-by: Elijah Newren \nCo-authored-by: Jeff King \nSigned-off-by: Karthik Nayak \n---\n builtin/update-ref.c | 14 ++++++++------\n t/t1400-update-ref.sh | 26 +++++++++++++-------------\n 2 files changed, 21 insertions(+), 19 deletions(-)\n\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 0046a87c57..800e380d32 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -573,16 +573,18 @@ static void print_rejected_refs(const char *refname,\n \t\t\t\tconst char *old_target,\n \t\t\t\tconst char *new_target,\n \t\t\t\tenum ref_transaction_error err,\n-\t\t\t\tconst char *details UNUSED,\n+\t\t\t\tconst char *details,\n \t\t\t\tvoid *cb_data UNUSED)\n {\n \tstruct strbuf sb = STRBUF_INIT;\n-\tconst char *reason = ref_transaction_error_msg(err);\n \n-\tstrbuf_addf(&sb, \"rejected %s %s %s %s\\n\", refname,\n-\t\t new_oid ? oid_to_hex(new_oid) : new_target,\n-\t\t old_oid ? oid_to_hex(old_oid) : old_target,\n-\t\t reason);\n+\tif (details)\n+\t\tstrbuf_addf(&sb, \"%s\\n\", details);\n+\telse\n+\t\tstrbuf_addf(&sb, \"rejected %s %s %s %s\\n\", refname,\n+\t\t\t new_oid ? oid_to_hex(new_oid) : new_target,\n+\t\t\t old_oid ? oid_to_hex(old_oid) : old_target,\n+\t\t\t ref_transaction_error_msg(err));\n \n \tfwrite(sb.buf, sb.len, 1, stdout);\n \tstrbuf_release(&sb);\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex db7f5444da..6cd6b45411 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -2100,7 +2100,7 @@ do\n \t\t\techo $head >expect &&\n \t\t\tgit rev-parse refs/heads/ref2 >actual &&\n \t\t\ttest_cmp expect actual &&\n-\t\t\ttest_grep -q \"invalid new value provided\" stdout\n+\t\t\ttest_grep -q \"trying to write ref ${SQ}refs/heads/ref2${SQ} with nonexistent object\" stdout\n \t\t)\n \t'\n \n@@ -2126,7 +2126,7 @@ do\n \t\t\techo $head >expect &&\n \t\t\tgit rev-parse refs/heads/ref2 >actual &&\n \t\t\ttest_cmp expect actual &&\n-\t\t\ttest_grep -q \"invalid new value provided\" stdout\n+\t\t\ttest_grep -q \"trying to write non-commit object $head_tree to branch ${SQ}refs/heads/ref2${SQ}\" stdout\n \t\t)\n \t'\n \n@@ -2148,7 +2148,7 @@ do\n \t\t\tgit rev-parse refs/heads/ref1 >actual &&\n \t\t\ttest_cmp expect actual &&\n \t\t\ttest_must_fail git rev-parse refs/heads/ref2 &&\n-\t\t\ttest_grep -q \"reference does not exist\" stdout\n+\t\t\ttest_grep -q \"cannot lock ref ${SQ}refs/heads/ref2${SQ}: unable to resolve reference ${SQ}refs/heads/ref2${SQ}\" stdout\n \t\t)\n \t'\n \n@@ -2172,7 +2172,7 @@ do\n \t\t\ttest_cmp expect actual &&\n \t\t\techo $head >expect &&\n \t\t\ttest_must_fail git rev-parse refs/heads/ref2 &&\n-\t\t\ttest_grep -q \"reference does not exist\" stdout\n+\t\t\ttest_grep -q \"cannot lock ref ${SQ}refs/heads/ref2${SQ}: reference is missing but expected $head\" stdout\n \t\t)\n \t'\n \n@@ -2198,7 +2198,7 @@ do\n \t\t\techo $head >expect &&\n \t\t\tgit rev-parse refs/heads/ref2 >actual &&\n \t\t\ttest_cmp expect actual &&\n-\t\t\ttest_grep -q \"expected symref but found regular ref\" stdout\n+\t\t\ttest_grep -q \"cannot lock ref ${SQ}refs/heads/ref2${SQ}: expected symref with target ${SQ}refs/heads/nonexistent${SQ}: but is a regular ref\" stdout\n \t\t)\n \t'\n \n@@ -2223,7 +2223,7 @@ do\n \t\t\techo $head >expect &&\n \t\t\tgit rev-parse refs/heads/ref2 >actual &&\n \t\t\ttest_cmp expect actual &&\n-\t\t\ttest_grep -q \"reference already exists\" stdout\n+\t\t\ttest_grep -q \"cannot lock ref ${SQ}refs/heads/ref2${SQ}: reference already exists\" stdout\n \t\t)\n \t'\n \n@@ -2248,7 +2248,7 @@ do\n \t\t\techo $head >expect &&\n \t\t\tgit rev-parse refs/heads/ref2 >actual &&\n \t\t\ttest_cmp expect actual &&\n-\t\t\ttest_grep -q \"incorrect old value provided\" stdout\n+\t\t\ttest_grep -q \"cannot lock ref ${SQ}refs/heads/ref2${SQ}: is at $head but expected $old_head\" stdout\n \t\t)\n \t'\n \n@@ -2269,7 +2269,7 @@ do\n \t\t\techo $old_head >expect &&\n \t\t\tgit rev-parse refs/heads/ref/foo >actual &&\n \t\t\ttest_cmp expect actual &&\n-\t\t\ttest_grep -q \"refname conflict\" stdout\n+\t\t\ttest_grep -q \"${SQ}refs/heads/ref/foo${SQ} exists; cannot create ${SQ}refs/heads/ref${SQ}\" stdout\n \t\t)\n \t'\n \n@@ -2290,7 +2290,7 @@ do\n \t\t\techo $old_head >expect &&\n \t\t\tgit rev-parse refs/heads/foo >actual &&\n \t\t\ttest_cmp expect actual &&\n-\t\t\ttest_grep -q \"refname conflict\" stdout\n+\t\t\ttest_grep -q \"${SQ}refs/heads/ref/foo${SQ} exists; cannot create ${SQ}refs/heads/ref${SQ}\" stdout\n \t\t)\n \t'\n \n@@ -2316,7 +2316,7 @@ do\n \t\t\techo $old_head >expect &&\n \t\t\tgit rev-parse refs/heads/ref >actual &&\n \t\t\ttest_cmp expect actual &&\n-\t\t\ttest_grep -q \"reference conflict due to case-insensitive filesystem\" stdout\n+\t\t\ttest_grep -e \"cannot lock ref ${SQ}refs/heads/Foo${SQ}: Unable to create\" -e \"Foo.lock\" stdout\n \t\t)\n \t'\n \n@@ -2358,7 +2358,7 @@ do\n \n \t\t\tformat_command $type \"delete refs/heads/symbolic\" \"$head\" >stdin &&\n \t\t\tgit update-ref $type --stdin --batch-updates stdout &&\n-\t\t\ttest_grep \"reference does not exist\" stdout\n+\t\t\ttest_grep \"cannot lock ref ${SQ}refs/heads/symbolic${SQ}: unable to resolve reference ${SQ}refs/heads/non-existent${SQ}\" stdout\n \t\t)\n \t'\n \n@@ -2374,7 +2374,7 @@ do\n \n \t\t\tformat_command $type \"delete refs/heads/new-branch\" \"$head\" >stdin &&\n \t\t\tgit update-ref $type --stdin --batch-updates stdout &&\n-\t\t\ttest_grep \"incorrect old value provided\" stdout\n+\t\t\ttest_grep \"cannot lock ref ${SQ}refs/heads/new-branch${SQ}: is at $(git rev-parse new-branch) but expected $head\" stdout\n \t\t)\n \t'\n \n@@ -2388,7 +2388,7 @@ do\n \n \t\t\tformat_command $type \"delete refs/heads/non-existent\" \"$head\" >stdin &&\n \t\t\tgit update-ref $type --stdin --batch-updates stdout &&\n-\t\t\ttest_grep \"reference does not exist\" stdout\n+\t\t\ttest_grep \"cannot lock ref ${SQ}refs/heads/non-existent${SQ}: unable to resolve reference ${SQ}refs/heads/non-existent${SQ}\" stdout\n \t\t)\n \t'\n done\n\n-- \n2.51.2\n\n"},{"id":"533836","messageId":"20260114-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v1-5-f5f8b173c501@gmail.com","threadId":"64802","inReplyTo":"20260114-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v1-0-f5f8b173c501@gmail.com","subject":"[PATCH 5/6] fetch: utilize rejected ref error details","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-14T15:40:46Z","receivedAt":"2026-01-14T15:41:10Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"In 0e358de64a (fetch: use batched reference updates, 2025-05-19),\ngit-fetch(1) switched to using batched reference updates. This also\nintroduced a regression wherein instead of providing detailed error\nmessages for failed referenced updates, the users were provided generic\nerror messages based on the error type.\n\nSimilar to the previous commit, switch to using detailed error messages\nif present for failed reference updates to fix this regression.\n\nReported-by: Elijah Newren \nCo-authored-by: Jeff King \nSigned-off-by: Karthik Nayak \n---\n builtin/fetch.c | 10 ++++++----\n t/t5510-fetch.sh | 8 ++++----\n 2 files changed, 10 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex d427adea61..49495be0b6 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -1649,7 +1649,7 @@ static void ref_transaction_rejection_handler(const char *refname,\n \t\t\t\t\t const char *old_target UNUSED,\n \t\t\t\t\t const char *new_target UNUSED,\n \t\t\t\t\t enum ref_transaction_error err,\n-\t\t\t\t\t const char *details UNUSED,\n+\t\t\t\t\t const char *details,\n \t\t\t\t\t void *cb_data)\n {\n \tstruct ref_rejection_data *data = cb_data;\n@@ -1674,9 +1674,11 @@ static void ref_transaction_rejection_handler(const char *refname,\n \t\t\t\"branches\"), data->remote_name);\n \t\tdata->conflict_msg_shown = true;\n \t} else {\n-\t\tconst char *reason = ref_transaction_error_msg(err);\n-\n-\t\terror(_(\"fetching ref %s failed: %s\"), refname, reason);\n+\t\tif (details)\n+\t\t\terror(\"%s\", details);\n+\t\telse\n+\t\t\terror(_(\"fetching ref %s failed: %s\"),\n+\t\t\t refname, ref_transaction_error_msg(err));\n \t}\n \n \t*data->retcode = 1;\ndiff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\nindex ce1c23684e..c69afb5a60 100755\n--- a/t/t5510-fetch.sh\n+++ b/t/t5510-fetch.sh\n@@ -1516,7 +1516,7 @@ test_expect_success REFFILES 'existing reference lock in repo' '\n \t\tgit remote add origin ../base &&\n \t\ttouch refs/heads/foo.lock &&\n \t\ttest_must_fail git fetch -f origin \"refs/heads/*:refs/heads/*\" 2>err &&\n-\t\ttest_grep \"error: fetching ref refs/heads/foo failed: reference already exists\" err &&\n+\t\ttest_grep -e \"error: cannot lock ref ${SQ}refs/heads/foo${SQ}: Unable to create\" -e \"refs/heads/foo.lock${SQ}: File exists.\" err &&\n \t\tgit rev-parse refs/heads/main >expect &&\n \t\tgit rev-parse refs/heads/branch >actual &&\n \t\ttest_cmp expect actual\n@@ -1530,7 +1530,7 @@ test_expect_success CASE_INSENSITIVE_FS,REFFILES 'F/D conflict on case insensiti\n \t\tcd case_insensitive &&\n \t\tgit remote add origin -- ../case_sensitive_fd &&\n \t\ttest_must_fail git fetch -f origin \"refs/heads/*:refs/heads/*\" 2>err &&\n-\t\ttest_grep \"failed: refname conflict\" err &&\n+\t\ttest_grep \"cannot process ${SQ}refs/remotes/origin/foo${SQ} and ${SQ}refs/remotes/origin/foo/bar${SQ} at the same time\" err &&\n \t\tgit rev-parse refs/heads/main >expect &&\n \t\tgit rev-parse refs/heads/foo/bar >actual &&\n \t\ttest_cmp expect actual\n@@ -1544,7 +1544,7 @@ test_expect_success CASE_INSENSITIVE_FS,REFFILES 'D/F conflict on case insensiti\n \t\tcd case_insensitive &&\n \t\tgit remote add origin -- ../case_sensitive_df &&\n \t\ttest_must_fail git fetch -f origin \"refs/heads/*:refs/heads/*\" 2>err &&\n-\t\ttest_grep \"failed: refname conflict\" err &&\n+\t\ttest_grep \"cannot lock ref ${SQ}refs/remotes/origin/foo${SQ}: there is a non-empty directory ${SQ}./refs/remotes/origin/foo${SQ} blocking reference ${SQ}refs/remotes/origin/foo${SQ}\" err &&\n \t\tgit rev-parse refs/heads/main >expect &&\n \t\tgit rev-parse refs/heads/Foo/bar >actual &&\n \t\ttest_cmp expect actual\n@@ -1658,7 +1658,7 @@ test_expect_success REFFILES \"FETCH_HEAD is updated even if ref updates fail\" '\n \t\tgit remote add origin ../base &&\n \t\t>refs/heads/foo.lock &&\n \t\ttest_must_fail git fetch -f origin \"refs/heads/*:refs/heads/*\" 2>err &&\n-\t\ttest_grep \"error: fetching ref refs/heads/foo failed: reference already exists\" err &&\n+\t\ttest_grep -e \"error: cannot lock ref ${SQ}refs/heads/foo${SQ}: Unable to create\" -e \"refs/heads/foo.lock${SQ}: File exists.\" err &&\n \t\ttest_grep \"branch ${SQ}branch${SQ} of ../base\" FETCH_HEAD &&\n \t\ttest_grep \"branch ${SQ}foo${SQ} of ../base\" FETCH_HEAD\n \t)\n\n-- \n2.51.2\n\n"},{"id":"533837","messageId":"20260114-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v1-6-f5f8b173c501@gmail.com","threadId":"64802","inReplyTo":"20260114-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v1-0-f5f8b173c501@gmail.com","subject":"[PATCH 6/6] receive-pack: utilize rejected ref error details","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-14T15:40:47Z","receivedAt":"2026-01-14T15:41:11Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"In 9d2962a7c4 (receive-pack: use batched reference updates, 2025-05-19),\ngit-receive-pack(1) switched to using batched reference updates. This also\nintroduced a regression wherein instead of providing detailed error\nmessages for failed referenced updates, the users were provided generic\nerror messages based on the error type.\n\nSimilar to the previous commit, switch to using detailed error messages\nif present for failed reference updates to fix this regression.\n\nOne downside of this is that the messages can be very verbose, for e.g.\nin the files backend, when trying to write a non-commit object to a\nbranch, you would see:\n\n ! [remote rejected] 3eaec9ccf3a53f168362a6b3fdeb73426fb9813d ->\n branch (cannot update ref 'refs/heads/branch': trying to write\n non-commit object 3eaec9ccf3a53f168362a6b3fdeb73426fb9813d to branch\n 'refs/heads/branch')\n\nHere the refname is repeated multiple times due to how error messages\nare propagated and filled over the code stack. This potentially can be\ncleaned up in a future commit.\n\nReported-by: Elijah Newren \nCo-authored-by: Jeff King \nSigned-off-by: Karthik Nayak \n---\n builtin/receive-pack.c | 10 +++++++---\n t/t5516-fetch-push.sh | 15 +++++++++++++++\n 2 files changed, 22 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 94d3e73cee..969d59ae3e 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -1813,12 +1813,15 @@ static void ref_transaction_rejection_handler(const char *refname,\n \t\t\t\t\t const char *old_target UNUSED,\n \t\t\t\t\t const char *new_target UNUSED,\n \t\t\t\t\t enum ref_transaction_error err,\n-\t\t\t\t\t const char *details UNUSED,\n+\t\t\t\t\t const char *details,\n \t\t\t\t\t void *cb_data)\n {\n \tstruct strmap *failed_refs = cb_data;\n \n-\tstrmap_put(failed_refs, refname, (char *)ref_transaction_error_msg(err));\n+\tif (!details)\n+\t\tdetails = ref_transaction_error_msg(err);\n+\n+\tstrmap_put(failed_refs, refname, (char *)details);\n }\n \n static void execute_commands_non_atomic(struct command *commands,\n@@ -1884,6 +1887,7 @@ static void execute_commands_non_atomic(struct command *commands,\n \t\t}\n \n \t\tref_transaction_for_each_rejected_update(transaction,\n+\n \t\t\t\t\t\t\t ref_transaction_rejection_handler,\n \t\t\t\t\t\t\t &failed_refs);\n \n@@ -1895,7 +1899,7 @@ static void execute_commands_non_atomic(struct command *commands,\n \t\t\tif (reported_error)\n \t\t\t\tcmd->error_string = reported_error;\n \t\t\telse if (strmap_contains(&failed_refs, cmd->ref_name))\n-\t\t\t\tcmd->error_string = strmap_get(&failed_refs, cmd->ref_name);\n+\t\t\t\tcmd->error_string = cmd->error_string_owned = xstrdup(strmap_get(&failed_refs, cmd->ref_name));\n \t\t}\n \n \tcleanup:\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 46926e7bbd..45595991c8 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -1882,4 +1882,19 @@ test_expect_success 'push with F/D conflict with deletion and creation' '\n \tgit push testrepo :refs/heads/branch/conflict refs/heads/branch\n '\n \n+test_expect_success 'pushing non-commit objects should report error' '\n+\ttest_when_finished \"rm -rf dest repo\" &&\n+\tgit init dest &&\n+\tgit init repo &&\n+\n+\t(\n+\t\tcd repo &&\n+\t\ttest_commit --annotate test &&\n+\n+\t\ttagsha=$(git rev-parse test^{tag}) &&\n+\t\ttest_must_fail git push ../dest \"$tagsha:refs/heads/branch\" 2>err &&\n+\t\ttest_grep \"trying to write non-commit object $tagsha to branch ${SQ}refs/heads/branch${SQ}\" err\n+\t)\n+'\n+\n test_done\n\n-- \n2.51.2\n\n"},{"id":"533842","messageId":"xmqq1pjsgn2s.fsf@gitster.g","threadId":"64802","inReplyTo":"20260114-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v1-0-f5f8b173c501@gmail.com","subject":"Re: [PATCH 0/6] refs: provide detailed error messages when using batched update","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-14T16:45:31Z","receivedAt":"2026-01-14T16:45:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak writes:\n\n> The refs namespace uses an error buffer to capture details about failed\n> reference updates. However when we added batched update support to\n> reference transactions, these messages were never propagated, instead\n> only an error code pertaining to the type of failure was propagated.\n>\n> Currently, there are three regions which utilize batched updates:\n>\n> - git update-ref --batch-updates\n> - git fetch\n> - git receive-pack\n>\n> While 'git update-ref --batch-updates' was a newly introduced flag, both\n> 'git fetch' and 'git receive-pack' were pre-existing. Before using\n> batched updates, they provided more detailed error messages to the user,\n> but this changed with the introduction of batched updates. This is a\n> regression in their workings.\n>\n> This patch series fixes this, by passing the detailed error message and\n> utilizing it whenever available. The regression was reported by Elijah\n> Newren [1] and based on the patch submitted by Jeff King [2].\n>\n> [1]: https://lore.kernel.org/all/CABPp-BGL2tJR4dPidQuFcp-X0_VkVTknCY-0Zgo=jHVGv_P=wA@mail.gmail.com/\n> [2]: https://lore.kernel.org/all/20251224081214.GA1879908@coredump.intra.peff.net/\n\nThanks, all. It is very nice to see such a collaboration going ;-)\n\nWill queue.\n\n\n> ---\n> builtin/fetch.c | 9 +++++---\n> builtin/receive-pack.c | 9 ++++++--\n> builtin/update-ref.c | 13 +++++++-----\n> refs.c | 56 ++++++++++++++++++++++++++++++-------------------\n> refs.h | 1 +\n> refs/files-backend.c | 3 ++-\n> refs/packed-backend.c | 9 +++++---\n> refs/refs-internal.h | 4 +++-\n> refs/reftable-backend.c | 3 ++-\n> t/t1400-update-ref.sh | 26 +++++++++++------------\n> t/t5510-fetch.sh | 8 +++----\n> t/t5516-fetch-push.sh | 15 +++++++++++++\n> 12 files changed, 102 insertions(+), 54 deletions(-)\n>\n> Karthik Nayak (6):\n> refs: remove unused header\n> refs: attach rejection details to updates\n> refs: add rejection detail to the callback function\n> update-ref: utilize rejected error details if available\n> fetch: utilize rejected ref error details\n> receive-pack: utilize rejected ref error details\n>\n>\n>\n> base-commit: 8745eae506f700657882b9e32b2aa00f234a6fb6\n> change-id: 20260113-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-17786b20894a\n>\n> Thanks\n> - Karthik\n"},{"id":"533843","messageId":"xmqqwm1kf7gr.fsf@gitster.g","threadId":"64802","inReplyTo":"20260114-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v1-1-f5f8b173c501@gmail.com","subject":"Re: [PATCH 1/6] refs: remove unused header","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-14T17:08:04Z","receivedAt":"2026-01-14T17:08:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak writes:\n\n> Some of the headers in 'refs.c' are no longer required, let's remove\n> them.\n>\n> Signed-off-by: Karthik Nayak \n> ---\n> refs.c | 2 --\n> 1 file changed, 2 deletions(-)\n\nOne thing to note is that The resulting file refs.c still includes\nhook.h and because of that, the removal of run-command.h from here\nhas no effect.\n\n> diff --git a/refs.c b/refs.c\n> index e06e0cb072..965b232a06 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -15,7 +15,6 @@\n> #include \"iterator.h\"\n> #include \"refs.h\"\n> #include \"refs/refs-internal.h\"\n> -#include \"run-command.h\"\n> #include \"hook.h\"\n> #include \"object-name.h\"\n> #include \"odb.h\"\n> @@ -26,7 +25,6 @@\n> #include \"strvec.h\"\n> #include \"repo-settings.h\"\n> #include \"setup.h\"\n> -#include \"sigchain.h\"\n> #include \"date.h\"\n> #include \"commit.h\"\n> #include \"wildmatch.h\"\n"},{"id":"533846","messageId":"xmqqpl7cf6kf.fsf@gitster.g","threadId":"64802","inReplyTo":"20260114-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v1-4-f5f8b173c501@gmail.com","subject":"Re: [PATCH 4/6] update-ref: utilize rejected error details if available","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-14T17:27:28Z","receivedAt":"2026-01-14T17:27:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak writes:\n\n> @@ -573,16 +573,18 @@ static void print_rejected_refs(const char *refname,\n> \t\t\t\tconst char *old_target,\n> \t\t\t\tconst char *new_target,\n> \t\t\t\tenum ref_transaction_error err,\n> -\t\t\t\tconst char *details UNUSED,\n> +\t\t\t\tconst char *details,\n> \t\t\t\tvoid *cb_data UNUSED)\n> {\n> \tstruct strbuf sb = STRBUF_INIT;\n> -\tconst char *reason = ref_transaction_error_msg(err);\n> \n> -\tstrbuf_addf(&sb, \"rejected %s %s %s %s\\n\", refname,\n> -\t\t new_oid ? oid_to_hex(new_oid) : new_target,\n> -\t\t old_oid ? oid_to_hex(old_oid) : old_target,\n> -\t\t reason);\n> +\tif (details)\n> +\t\tstrbuf_addf(&sb, \"%s\\n\", details);\n> +\telse\n> +\t\tstrbuf_addf(&sb, \"rejected %s %s %s %s\\n\", refname,\n> +\t\t\t new_oid ? oid_to_hex(new_oid) : new_target,\n> +\t\t\t old_oid ? oid_to_hex(old_oid) : old_target,\n> +\t\t\t ref_transaction_error_msg(err));\n\nCould \"details\" reported from the lower layer be less detailed than\nwhat we are formulating here, like updating the value of what ref\nfrom what old object to what new object, or what the err code tells\nthe end-user?\n"},{"id":"533848","messageId":"xmqqldi0f6a4.fsf@gitster.g","threadId":"64802","inReplyTo":"20260114-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v1-5-f5f8b173c501@gmail.com","subject":"Re: [PATCH 5/6] fetch: utilize rejected ref error details","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-14T17:33:39Z","receivedAt":"2026-01-14T17:33:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak writes:\n\n> In 0e358de64a (fetch: use batched reference updates, 2025-05-19),\n> git-fetch(1) switched to using batched reference updates. This also\n> introduced a regression wherein instead of providing detailed error\n> messages for failed referenced updates, the users were provided generic\n> error messages based on the error type.\n>\n> Similar to the previous commit, switch to using detailed error messages\n> if present for failed reference updates to fix this regression.\n\nThe same question applkies as the previous step. That is ...\n\n> @@ -1674,9 +1674,11 @@ static void ref_transaction_rejection_handler(const char *refname,\n> \t\t\t\"branches\"), data->remote_name);\n> \t\tdata->conflict_msg_shown = true;\n> \t} else {\n> -\t\tconst char *reason = ref_transaction_error_msg(err);\n> -\n> -\t\terror(_(\"fetching ref %s failed: %s\"), refname, reason);\n> +\t\tif (details)\n> +\t\t\terror(\"%s\", details);\n> +\t\telse\n> +\t\t\terror(_(\"fetching ref %s failed: %s\"),\n> +\t\t\t refname, ref_transaction_error_msg(err));\n\n... would \"details\" always carry enough information to cover\n\"refname\" here, plus what the err code tells us?\n\nI guess ...\n\n> diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\n> index ce1c23684e..c69afb5a60 100755\n> --- a/t/t5510-fetch.sh\n> +++ b/t/t5510-fetch.sh\n> @@ -1516,7 +1516,7 @@ test_expect_success REFFILES 'existing reference lock in repo' '\n> \t\tgit remote add origin ../base &&\n> \t\ttouch refs/heads/foo.lock &&\n> \t\ttest_must_fail git fetch -f origin \"refs/heads/*:refs/heads/*\" 2>err &&\n> -\t\ttest_grep \"error: fetching ref refs/heads/foo failed: reference already exists\" err &&\n> +\t\ttest_grep -e \"error: cannot lock ref ${SQ}refs/heads/foo${SQ}: Unable to create\" -e \"refs/heads/foo.lock${SQ}: File exists.\" err &&\n\n... the error only talks about our local name, and when the command\nis \"git fetch origin refs/heads/foo:refs/remotes/origin/bar\", we\nonly complain about refs/remotes/origin/bar without ever mentioning\nrefs/heads/foo on the remote side, so I think \"details\" has enough\ninformation to replace the existing message here in this case.\n\nThanks.\n"},{"id":"533853","messageId":"20260114174338.GE885771@coredump.intra.peff.net","threadId":"64802","inReplyTo":"20260114-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v1-2-f5f8b173c501@gmail.com","subject":"Re: [PATCH 2/6] refs: attach rejection details to updates","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-01-14T17:43:38Z","receivedAt":"2026-01-14T17:43:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 14, 2026 at 04:40:43PM +0100, Karthik Nayak wrote:\n\n> @@ -1262,6 +1264,8 @@ int ref_transaction_maybe_set_rejected(struct ref_transaction *transaction,\n> \t\t\t transaction->updates[update_idx]->refname, 0);\n> \n> \ttransaction->updates[update_idx]->rejection_err = err;\n> +\tif (details)\n> +\t\ttransaction->updates[update_idx]->rejection_details = xstrdup(details);\n\nI guess this could use xstrdup_or_null(), but probably doesn't matter\nmuch either way. I do wonder if anybody actually passes a NULL value. I\nthink in my hacky patch there were some spots that did, but here you're\nalways setting the \"err\" buf (which is good, as we'll always have\ndetails then).\n\n> @@ -2657,30 +2661,35 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs\n> \t\t\tif (!initial_transaction &&\n> \t\t\t (strset_contains(&conflicting_dirnames, dirname.buf) ||\n> \t\t\t !refs_read_raw_ref(refs, dirname.buf, &oid, &referent,\n> -\t\t\t\t\t\t &type, &ignore_errno))) {\n> +\t\t\t\t\t\t&type, &ignore_errno))) {\n> +\n> +\t\t\t\tstrbuf_addf(err, _(\"'%s' exists; cannot create '%s'\"),\n> +\t\t\t\t\t dirname.buf, refname);\n> +\n> \t\t\t\tif (transaction && ref_transaction_maybe_set_rejected(\n> \t\t\t\t\t transaction, *update_idx,\n> -\t\t\t\t\t REF_TRANSACTION_ERROR_NAME_CONFLICT)) {\n> +\t\t\t\t\t REF_TRANSACTION_ERROR_NAME_CONFLICT, err->buf)) {\n> \t\t\t\t\tstrset_remove(&dirnames, dirname.buf);\n> \t\t\t\t\tstrset_add(&conflicting_dirnames, dirname.buf);\n> -\t\t\t\t\tcontinue;\n> +\t\t\t\t\tstrbuf_reset(err);\n> +\t\t\t\t\tgoto next;\n> \t\t\t\t}\n> \n> -\t\t\t\tstrbuf_addf(err, _(\"'%s' exists; cannot create '%s'\"),\n> -\t\t\t\t\t dirname.buf, refname);\n> \t\t\t\tgoto cleanup;\n> \t\t\t}\n\nOK, so this is a case where we re-ordered the \"err\" handling so that\nit's available for the non-atomic case. Makes sense. We end up\nformatting into err, then copying it via xstrdup(), and then resetting\nthe buffer, which is an extra copy. I think you could probably get\naround that by passing in the strbuf to set_rejected() and using\nstrbuf_detach() to pull the value out. It's probably not worth worrying\nabout optimizing out the copy for an error path like this, but I wonder\nif it would be more ergonomic (the caller does not have to remember to\nstrbuf_reset() then).\n\nI notice that you \"goto next\" now instead of \"continue\". So I was\ncurious what happens in \"next\" now, but...\n\n> +next:;\n> \t}\n\n...the answer is nothing. ;) I guess maybe you were going to\nstrbuf_reset() down here at one point? If the 'next' label remains\nempty, I think I'd prefer to keep these as 'continue'. But maybe you use\nit later in the series. I'll read on.\n\n> [...]\n\nThe rest of the conversions all looked sensible to me. And you fixed my\nmemory leak, which is good. ;)\n\n-Peff\n"},{"id":"533854","messageId":"20260114174458.GF885771@coredump.intra.peff.net","threadId":"64802","inReplyTo":"20260114-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v1-3-f5f8b173c501@gmail.com","subject":"Re: [PATCH 3/6] refs: add rejection detail to the callback function","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-01-14T17:44:58Z","receivedAt":"2026-01-14T17:45:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 14, 2026 at 04:40:44PM +0100, Karthik Nayak wrote:\n\n> The previous commit started storing the rejection details alongside the\n> error code for rejected updates. Pass this along to the callback\n> function `ref_transaction_for_each_rejected_update()`. Currently the\n> field is unused, but will be integrated in the upcoming commits.\n\nSplitting it out like this seems reasonable.\n\n> Co-authored-by: Jeff King \n> Signed-off-by: Karthik Nayak \n\nIn case it matters, you can add my:\n\n Signed-off-by: Jeff King \n\nto this and any other patches which were derived from my earlier\nattempt.\n\n-Peff\n"},{"id":"533857","messageId":"20260114175558.GG885771@coredump.intra.peff.net","threadId":"64802","inReplyTo":"xmqqpl7cf6kf.fsf@gitster.g","subject":"Re: [PATCH 4/6] update-ref: utilize rejected error details if available","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-01-14T17:55:58Z","receivedAt":"2026-01-14T17:55:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 14, 2026 at 09:27:28AM -0800, Junio C Hamano wrote:\n\n> Karthik Nayak writes:\n> \n> > @@ -573,16 +573,18 @@ static void print_rejected_refs(const char *refname,\n> > \t\t\t\tconst char *old_target,\n> > \t\t\t\tconst char *new_target,\n> > \t\t\t\tenum ref_transaction_error err,\n> > -\t\t\t\tconst char *details UNUSED,\n> > +\t\t\t\tconst char *details,\n> > \t\t\t\tvoid *cb_data UNUSED)\n> > {\n> > \tstruct strbuf sb = STRBUF_INIT;\n> > -\tconst char *reason = ref_transaction_error_msg(err);\n> > \n> > -\tstrbuf_addf(&sb, \"rejected %s %s %s %s\\n\", refname,\n> > -\t\t new_oid ? oid_to_hex(new_oid) : new_target,\n> > -\t\t old_oid ? oid_to_hex(old_oid) : old_target,\n> > -\t\t reason);\n> > +\tif (details)\n> > +\t\tstrbuf_addf(&sb, \"%s\\n\", details);\n> > +\telse\n> > +\t\tstrbuf_addf(&sb, \"rejected %s %s %s %s\\n\", refname,\n> > +\t\t\t new_oid ? oid_to_hex(new_oid) : new_target,\n> > +\t\t\t old_oid ? oid_to_hex(old_oid) : old_target,\n> > +\t\t\t ref_transaction_error_msg(err));\n> \n> Could \"details\" reported from the lower layer be less detailed than\n> what we are formulating here, like updating the value of what ref\n> from what old object to what new object, or what the err code tells\n> the end-user?\n\nI wondered that, too, but also: is this supposed to be machine-readable?\nThe \"rejected ...\" output looks like something that could be parsed,\nand it seems to be documented in git-update-ref(1).\n\n Side note: if this is meant to be a stable format, surely there should\n be some coverage in the test suite? There doesn't seem to be.\n\nSo should we just be replacing the ref_transaction_error_msg() part? I\n_think_ the low-level details will usually be more informative there,\nbut not necessarily. So possibly we'd even want to show both, though I\nsuspect just concatenating them would be messy.\n\nPlus the \"details\" one has a lot of redundant information in it (it\nmentions \"refname\", even though it is already on the \"rejected\" line).\n\nIn the short-term, I wonder if we just want:\n\n if (details && *details)\n\terror(\"%s\", details);\n\nThat gets us back to the status quo, where the details are at least\navailable via stderr. And then we can consider how to combine them into\nthe machine-readable format separately.\n\n-Peff\n"},{"id":"533859","messageId":"20260114180040.GH885771@coredump.intra.peff.net","threadId":"64802","inReplyTo":"20260114-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v1-5-f5f8b173c501@gmail.com","subject":"Re: [PATCH 5/6] fetch: utilize rejected ref error details","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-01-14T18:00:40Z","receivedAt":"2026-01-14T18:00:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 14, 2026 at 04:40:46PM +0100, Karthik Nayak wrote:\n\n> @@ -1674,9 +1674,11 @@ static void ref_transaction_rejection_handler(const char *refname,\n> \t\t\t\"branches\"), data->remote_name);\n> \t\tdata->conflict_msg_shown = true;\n> \t} else {\n> -\t\tconst char *reason = ref_transaction_error_msg(err);\n> -\n> -\t\terror(_(\"fetching ref %s failed: %s\"), refname, reason);\n> +\t\tif (details)\n> +\t\t\terror(\"%s\", details);\n> +\t\telse\n> +\t\t\terror(_(\"fetching ref %s failed: %s\"),\n> +\t\t\t refname, ref_transaction_error_msg(err));\n> \t}\n\nOK, so here we're writing to stderr anyway, and now we'll just give the\nmore detailed data. Makes sense (though like Junio, I do wonder if the\nexisting message might provide more details in some cases).\n\nBTW, I think there is still a related fallout for git-fetch. Even with\nyour patch, doing this:\n\n $ git fetch . v1.0.0:refs/heads/foo\n From .\n * [new tag] v1.0.0 -> foo\n error: cannot update ref 'refs/heads/foo': trying to write non-commit object f665776185ad074b236c00751d666da7d1977dbe to branch 'refs/heads/foo'\n\nwill not put anything in the status table. Whereas in v2.50.0 and\nearlier, we get:\n\n $ git.v2.50.0 fetch . v1.0.0:refs/heads/foo\n error: cannot update ref 'refs/heads/foo': trying to write non-commit object f665776185ad074b236c00751d666da7d1977dbe to branch 'refs/heads/foo'\n From .\n ! [new tag] v1.0.0 -> foo (unable to update local ref)\n\nNote the \"!\" and the \"unable to update local ref\" message in the status\ntable.\n\n-Peff\n"},{"id":"533860","messageId":"20260114180306.GI885771@coredump.intra.peff.net","threadId":"64802","inReplyTo":"20260114-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v1-6-f5f8b173c501@gmail.com","subject":"Re: [PATCH 6/6] receive-pack: utilize rejected ref error details","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-01-14T18:03:06Z","receivedAt":"2026-01-14T18:03:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 14, 2026 at 04:40:47PM +0100, Karthik Nayak wrote:\n\n> In 9d2962a7c4 (receive-pack: use batched reference updates, 2025-05-19),\n> git-receive-pack(1) switched to using batched reference updates. This also\n> introduced a regression wherein instead of providing detailed error\n> messages for failed referenced updates, the users were provided generic\n> error messages based on the error type.\n> \n> Similar to the previous commit, switch to using detailed error messages\n> if present for failed reference updates to fix this regression.\n> \n> One downside of this is that the messages can be very verbose, for e.g.\n> in the files backend, when trying to write a non-commit object to a\n> branch, you would see:\n> \n> ! [remote rejected] 3eaec9ccf3a53f168362a6b3fdeb73426fb9813d ->\n> branch (cannot update ref 'refs/heads/branch': trying to write\n> non-commit object 3eaec9ccf3a53f168362a6b3fdeb73426fb9813d to branch\n> 'refs/heads/branch')\n> \n> Here the refname is repeated multiple times due to how error messages\n> are propagated and filled over the code stack. This potentially can be\n> cleaned up in a future commit.\n\nIf we are going to have a \"potentially cleaned up in the future\" state,\nI think I would prefer to see just:\n\n if (details)\n\trp_error(\"%s\", details);\n\nhere. And then it comes over the stderr sideband, but the actual\nstatus-table gets the same non-verbose message. That's what happened\nin v2.50.0 and earlier. Later if we want to try to cram more details\ninto the machine-readable message we can.\n\n-Peff\n"},{"id":"533922","messageId":"CAOLa=ZQfjb1OfHJp6MVkbs=5Wey4Gp6t-jmEQSrojOsp=ge-Jw@mail.gmail.com","threadId":"64802","inReplyTo":"xmqqwm1kf7gr.fsf@gitster.g","subject":"Re: [PATCH 1/6] refs: remove unused header","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-15T09:50:24Z","receivedAt":"2026-01-15T09:50:26Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Junio C Hamano writes:\n\n> Karthik Nayak writes:\n>\n>> Some of the headers in 'refs.c' are no longer required, let's remove\n>> them.\n>>\n>> Signed-off-by: Karthik Nayak \n>> ---\n>> refs.c | 2 --\n>> 1 file changed, 2 deletions(-)\n>\n> One thing to note is that The resulting file refs.c still includes\n> hook.h and because of that, the removal of run-command.h from here\n> has no effect.\n>\n\nGood point, let me modify the commit message to explain this better.\nPerhaps:\n\n-->8--\n\nrefs: drop unnecessary header includes\n\nThe 'sigchain.h' header isn't being used and can be removed.\n\nSimilarly, 'run-command.h' serves no direct purpose here. While it gets\npulled in transitively through 'hook.h', we can still drop the explicit\ninclude for clarity.\n\n>> diff --git a/refs.c b/refs.c\n>> index e06e0cb072..965b232a06 100644\n>> --- a/refs.c\n>> +++ b/refs.c\n>> @@ -15,7 +15,6 @@\n>> #include \"iterator.h\"\n>> #include \"refs.h\"\n>> #include \"refs/refs-internal.h\"\n>> -#include \"run-command.h\"\n>> #include \"hook.h\"\n>> #include \"object-name.h\"\n>> #include \"odb.h\"\n>> @@ -26,7 +25,6 @@\n>> #include \"strvec.h\"\n>> #include \"repo-settings.h\"\n>> #include \"setup.h\"\n>> -#include \"sigchain.h\"\n>> #include \"date.h\"\n>> #include \"commit.h\"\n>> #include \"wildmatch.h\"\n"},{"id":"533923","messageId":"CAOLa=ZSyfkb8oe=ZtkOcsGo9Dk44GZSFiaye3Vw2kDs_XqS8=Q@mail.gmail.com","threadId":"64802","inReplyTo":"20260114174338.GE885771@coredump.intra.peff.net","subject":"Re: [PATCH 2/6] refs: attach rejection details to updates","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-15T10:02:15Z","receivedAt":"2026-01-15T10:02:18Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Jeff King writes:\n\n> On Wed, Jan 14, 2026 at 04:40:43PM +0100, Karthik Nayak wrote:\n>\n>> @@ -1262,6 +1264,8 @@ int ref_transaction_maybe_set_rejected(struct ref_transaction *transaction,\n>> \t\t\t transaction->updates[update_idx]->refname, 0);\n>>\n>> \ttransaction->updates[update_idx]->rejection_err = err;\n>> +\tif (details)\n>> +\t\ttransaction->updates[update_idx]->rejection_details = xstrdup(details);\n>\n> I guess this could use xstrdup_or_null(), but probably doesn't matter\n> much either way. I do wonder if anybody actually passes a NULL value. I\n> think in my hacky patch there were some spots that did, but here you're\n> always setting the \"err\" buf (which is good, as we'll always have\n> details then).\n\nThat's correct, I did ensure that there were no NULLs passed through, we\ncould definitely drop the check. But I was being defensive. I think\n`xstrdup_or_null()` is the better option here.\n\n>> @@ -2657,30 +2661,35 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs\n>> \t\t\tif (!initial_transaction &&\n>> \t\t\t (strset_contains(&conflicting_dirnames, dirname.buf) ||\n>> \t\t\t !refs_read_raw_ref(refs, dirname.buf, &oid, &referent,\n>> -\t\t\t\t\t\t &type, &ignore_errno))) {\n>> +\t\t\t\t\t\t&type, &ignore_errno))) {\n>> +\n>> +\t\t\t\tstrbuf_addf(err, _(\"'%s' exists; cannot create '%s'\"),\n>> +\t\t\t\t\t dirname.buf, refname);\n>> +\n>> \t\t\t\tif (transaction && ref_transaction_maybe_set_rejected(\n>> \t\t\t\t\t transaction, *update_idx,\n>> -\t\t\t\t\t REF_TRANSACTION_ERROR_NAME_CONFLICT)) {\n>> +\t\t\t\t\t REF_TRANSACTION_ERROR_NAME_CONFLICT, err->buf)) {\n>> \t\t\t\t\tstrset_remove(&dirnames, dirname.buf);\n>> \t\t\t\t\tstrset_add(&conflicting_dirnames, dirname.buf);\n>> -\t\t\t\t\tcontinue;\n>> +\t\t\t\t\tstrbuf_reset(err);\n>> +\t\t\t\t\tgoto next;\n>> \t\t\t\t}\n>>\n>> -\t\t\t\tstrbuf_addf(err, _(\"'%s' exists; cannot create '%s'\"),\n>> -\t\t\t\t\t dirname.buf, refname);\n>> \t\t\t\tgoto cleanup;\n>> \t\t\t}\n>\n> OK, so this is a case where we re-ordered the \"err\" handling so that\n> it's available for the non-atomic case. Makes sense. We end up\n> formatting into err, then copying it via xstrdup(), and then resetting\n> the buffer, which is an extra copy. I think you could probably get\n> around that by passing in the strbuf to set_rejected() and using\n> strbuf_detach() to pull the value out. It's probably not worth worrying\n> about optimizing out the copy for an error path like this, but I wonder\n> if it would be more ergonomic (the caller does not have to remember to\n> strbuf_reset() then).\n\nThat's a great idea, let me do that instead.\n\n>\n> I notice that you \"goto next\" now instead of \"continue\". So I was\n> curious what happens in \"next\" now, but...\n>\n>> +next:;\n>> \t}\n>\n> ...the answer is nothing. ;) I guess maybe you were going to\n> strbuf_reset() down here at one point? If the 'next' label remains\n> empty, I think I'd prefer to keep these as 'continue'. But maybe you use\n> it later in the series. I'll read on.\n>\n\nI should have explained this, there are two loops here in play. An outer\nloop going through refnames to check availability for. An inner loop to\nbreakdown the path of each refname to check for path conflicts.\n\nWith continue, we'd skip the inner loop, but would still perform other\nchecks for the refname, this can lead to error details being overridden.\nSo while we could replace s/goto next/continue for the code in the outer\nloop, it would still be needed for the inner loop.\n\n>> [...]\n>\n> The rest of the conversions all looked sensible to me. And you fixed my\n> memory leak, which is good. ;)\n>\n> -Peff\n"},{"id":"533924","messageId":"CAOLa=ZTX620gT+RuQ44AE_f82CibrGzoNv70G8BK7Wvt5HF4Ow@mail.gmail.com","threadId":"64802","inReplyTo":"20260114174458.GF885771@coredump.intra.peff.net","subject":"Re: [PATCH 3/6] refs: add rejection detail to the callback function","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-15T10:10:30Z","receivedAt":"2026-01-15T10:10:33Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Jeff King writes:\n\n> On Wed, Jan 14, 2026 at 04:40:44PM +0100, Karthik Nayak wrote:\n>\n>> The previous commit started storing the rejection details alongside the\n>> error code for rejected updates. Pass this along to the callback\n>> function `ref_transaction_for_each_rejected_update()`. Currently the\n>> field is unused, but will be integrated in the upcoming commits.\n>\n> Splitting it out like this seems reasonable.\n>\n>> Co-authored-by: Jeff King \n>> Signed-off-by: Karthik Nayak \n>\n> In case it matters, you can add my:\n>\n> Signed-off-by: Jeff King \n>\n> to this and any other patches which were derived from my earlier\n> attempt.\n>\n> -Peff\n\nThanks, will add in the next version.\n"},{"id":"533926","messageId":"CAOLa=ZS0i+YXfVHHAax699ME48YG7jXNZ3WOBYryS0hypMZO-A@mail.gmail.com","threadId":"64802","inReplyTo":"xmqqldi0f6a4.fsf@gitster.g","subject":"Re: [PATCH 5/6] fetch: utilize rejected ref error details","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-15T10:54:43Z","receivedAt":"2026-01-15T10:54:45Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Junio C Hamano writes:\n\n> Karthik Nayak writes:\n>\n>> In 0e358de64a (fetch: use batched reference updates, 2025-05-19),\n>> git-fetch(1) switched to using batched reference updates. This also\n>> introduced a regression wherein instead of providing detailed error\n>> messages for failed referenced updates, the users were provided generic\n>> error messages based on the error type.\n>>\n>> Similar to the previous commit, switch to using detailed error messages\n>> if present for failed reference updates to fix this regression.\n>\n> The same question applkies as the previous step. That is ...\n>\n>> @@ -1674,9 +1674,11 @@ static void ref_transaction_rejection_handler(const char *refname,\n>> \t\t\t\"branches\"), data->remote_name);\n>> \t\tdata->conflict_msg_shown = true;\n>> \t} else {\n>> -\t\tconst char *reason = ref_transaction_error_msg(err);\n>> -\n>> -\t\terror(_(\"fetching ref %s failed: %s\"), refname, reason);\n>> +\t\tif (details)\n>> +\t\t\terror(\"%s\", details);\n>> +\t\telse\n>> +\t\t\terror(_(\"fetching ref %s failed: %s\"),\n>> +\t\t\t refname, ref_transaction_error_msg(err));\n>\n> ... would \"details\" always carry enough information to cover\n> \"refname\" here, plus what the err code tells us?\n>\n> I guess ...\n>\n\nIn general, yes. Here's the final detailed error we'd show:\n\ngeneric availability checks (files + reftable backend):\n- '%s' exists; cannot create '%s'\n- cannot process '%s' and '%s' at the same time\n\npacked-backend:\n- cannot update ref '%s': reference already exists\n- cannot update ref '%s': is at %s but expected %s\n- cannot update ref '%s': reference is missing but expected %s\n\nfiles-backend:\n- cannot lock ref '%s': '%s' exists; cannot create '%s'\n- cannot lock ref '%s': cannot process '%s' and '%s' at the same time\n- cannot lock ref '%s': unable to resolve reference '%s'\n- multiple updates for 'HEAD' (including one via its referent '%s')\nare not allowed\n- cannot lock ref '%s': Unable to create '%s.lock': %s.\\n\\n\n Another git process seems to be running in this repository, e.g.\\n an\n editor opened by 'git commit'. Please make sure all processes\\n are\n terminated then try again. If it still fails, a git process\\n may have\n crashed in this repository earlier:\\n remove the file manually to\n continue.\n- cannot lock ref '%s': Unable to create '%s.lock': %s\n- cannot lock ref '%s': dangling symref already exists\n- cannot lock ref '%s': expected symref with target '%s': but is a regular ref\n- cannot lock ref '%s': is at %s but expected %s\n- cannot lock ref '%s': reference already exists\n- cannot lock ref '%s': reference is missing but expected %s\n- cannot lock ref '%s': there is a non-empty directory '%s' blocking\nreference '%s'\n- cannot lock ref '%s': unable to resolve reference '%s'\n- cannot update ref '%s': trying to write non-commit object %s to branch '%s'\n- cannot update ref '%s': trying to write ref '%s' with nonexistent object %s\n- multiple updates for '%s' (including one via symref '%s') are not allowed\n- verifying symref target: '%s': is at %s but expected %s\n- verifying symref target: '%s': reference is missing but expected %s\n\nreftable-backend:\n- cannot lock ref '%s': dangling symref already exists\n- cannot lock ref '%s': expected symref with target '%s': but is a\nregular ref\n- cannot lock ref '%s': is at %s but expected %s\n- cannot lock ref '%s': reference already exists\n- cannot lock ref '%s': reference is missing but expected %s\n- cannot lock ref '%s': unable to resolve reference '%s'\n- multiple updates for '%s' (including one via symref '%s') are not allowed\n- multiple updates for 'HEAD' (including one via its referent '%s')\nare not allowed\n- trying to write non-commit object %s to branch '%s'\n- trying to write ref '%s' with nonexistent object %s\n- verifying symref target: '%s': is at %s but expected %s\n- verifying symref target: '%s': reference is missing but expected %s\n\n\n>> diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\n>> index ce1c23684e..c69afb5a60 100755\n>> --- a/t/t5510-fetch.sh\n>> +++ b/t/t5510-fetch.sh\n>> @@ -1516,7 +1516,7 @@ test_expect_success REFFILES 'existing reference lock in repo' '\n>> \t\tgit remote add origin ../base &&\n>> \t\ttouch refs/heads/foo.lock &&\n>> \t\ttest_must_fail git fetch -f origin \"refs/heads/*:refs/heads/*\" 2>err &&\n>> -\t\ttest_grep \"error: fetching ref refs/heads/foo failed: reference already exists\" err &&\n>> +\t\ttest_grep -e \"error: cannot lock ref ${SQ}refs/heads/foo${SQ}: Unable to create\" -e \"refs/heads/foo.lock${SQ}: File exists.\" err &&\n>\n> ... the error only talks about our local name, and when the command\n> is \"git fetch origin refs/heads/foo:refs/remotes/origin/bar\", we\n> only complain about refs/remotes/origin/bar without ever mentioning\n> refs/heads/foo on the remote side, so I think \"details\" has enough\n> information to replace the existing message here in this case.\n>\n> Thanks.\n>\n\nYup. I do think there is a good cleanup we could potentially do here,\nwith some of the error messages and perhaps following a better pattern\nin general, perhaps a more structured error message. But I didn't want\nto tackle that in this series.\n\nKarthik\n"},{"id":"533942","messageId":"CAOLa=ZQLPB2Tntvimpp2zt=6PiWhJJh_oDCrUk7F8v+pFhyyMA@mail.gmail.com","threadId":"64802","inReplyTo":"20260114175558.GG885771@coredump.intra.peff.net","subject":"Re: [PATCH 4/6] update-ref: utilize rejected error details if available","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-15T11:08:33Z","receivedAt":"2026-01-15T11:08:35Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Jeff King writes:\n\n> On Wed, Jan 14, 2026 at 09:27:28AM -0800, Junio C Hamano wrote:\n>\n>> Karthik Nayak writes:\n>>\n>> > @@ -573,16 +573,18 @@ static void print_rejected_refs(const char *refname,\n>> > \t\t\t\tconst char *old_target,\n>> > \t\t\t\tconst char *new_target,\n>> > \t\t\t\tenum ref_transaction_error err,\n>> > -\t\t\t\tconst char *details UNUSED,\n>> > +\t\t\t\tconst char *details,\n>> > \t\t\t\tvoid *cb_data UNUSED)\n>> > {\n>> > \tstruct strbuf sb = STRBUF_INIT;\n>> > -\tconst char *reason = ref_transaction_error_msg(err);\n>> >\n>> > -\tstrbuf_addf(&sb, \"rejected %s %s %s %s\\n\", refname,\n>> > -\t\t new_oid ? oid_to_hex(new_oid) : new_target,\n>> > -\t\t old_oid ? oid_to_hex(old_oid) : old_target,\n>> > -\t\t reason);\n>> > +\tif (details)\n>> > +\t\tstrbuf_addf(&sb, \"%s\\n\", details);\n>> > +\telse\n>> > +\t\tstrbuf_addf(&sb, \"rejected %s %s %s %s\\n\", refname,\n>> > +\t\t\t new_oid ? oid_to_hex(new_oid) : new_target,\n>> > +\t\t\t old_oid ? oid_to_hex(old_oid) : old_target,\n>> > +\t\t\t ref_transaction_error_msg(err));\n>>\n>> Could \"details\" reported from the lower layer be less detailed than\n>> what we are formulating here, like updating the value of what ref\n>> from what old object to what new object, or what the err code tells\n>> the end-user?\n>\n> I wondered that, too, but also: is this supposed to be machine-readable?\n> The \"rejected ...\" output looks like something that could be parsed,\n> and it seems to be documented in git-update-ref(1).\n>\n> Side note: if this is meant to be a stable format, surely there should\n> be some coverage in the test suite? There doesn't seem to be.\n>\n\nGood catch, the documentation does indeed promise this format, so it\nwouldn't be appropriate to step away from it. Ideally, we should only\nreplace the last field, but that would be a lot of redundant\ninformation.\n\nOverall, we could also drop this patch too, since the flag was\nintroduced with batched updates and we could better justice here once we\ncleanup all other error messages.\n\n> So should we just be replacing the ref_transaction_error_msg() part? I\n> _think_ the low-level details will usually be more informative there,\n> but not necessarily. So possibly we'd even want to show both, though I\n> suspect just concatenating them would be messy.\n>\n> Plus the \"details\" one has a lot of redundant information in it (it\n> mentions \"refname\", even though it is already on the \"rejected\" line).\n\nIndeed, I've noted all possibilities in another response [1], but there\nis some redundancy there and we could do a nice cleanup.\n>\n> In the short-term, I wonder if we just want:\n>\n> if (details && *details)\n> \terror(\"%s\", details);\n>\n> That gets us back to the status quo, where the details are at least\n> available via stderr. And then we can consider how to combine them into\n> the machine-readable format separately.\n>\n\nThat's a good compromise too. I'd say we do this for now and see how we\ncan take it from here.\n\n> -Peff\n\n[1]: CAOLa=ZS0i+YXfVHHAax699ME48YG7jXNZ3WOBYryS0hypMZO-A@mail.gmail.com\n"},{"id":"533962","messageId":"CAOLa=ZQ0ETE+SzRV+M-xzEQFRTjdhSMseHjwnJ314yqPw8BPYA@mail.gmail.com","threadId":"64802","inReplyTo":"20260114180040.GH885771@coredump.intra.peff.net","subject":"Re: [PATCH 5/6] fetch: utilize rejected ref error details","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-15T15:20:50Z","receivedAt":"2026-01-15T15:20:56Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Jeff King writes:\n\n> On Wed, Jan 14, 2026 at 04:40:46PM +0100, Karthik Nayak wrote:\n>\n>> @@ -1674,9 +1674,11 @@ static void ref_transaction_rejection_handler(const char *refname,\n>> \t\t\t\"branches\"), data->remote_name);\n>> \t\tdata->conflict_msg_shown = true;\n>> \t} else {\n>> -\t\tconst char *reason = ref_transaction_error_msg(err);\n>> -\n>> -\t\terror(_(\"fetching ref %s failed: %s\"), refname, reason);\n>> +\t\tif (details)\n>> +\t\t\terror(\"%s\", details);\n>> +\t\telse\n>> +\t\t\terror(_(\"fetching ref %s failed: %s\"),\n>> +\t\t\t refname, ref_transaction_error_msg(err));\n>> \t}\n>\n> OK, so here we're writing to stderr anyway, and now we'll just give the\n> more detailed data. Makes sense (though like Junio, I do wonder if the\n> existing message might provide more details in some cases).\n>\n> BTW, I think there is still a related fallout for git-fetch. Even with\n> your patch, doing this:\n>\n> $ git fetch . v1.0.0:refs/heads/foo\n> From .\n> * [new tag] v1.0.0 -> foo\n> error: cannot update ref 'refs/heads/foo': trying to write non-commit object f665776185ad074b236c00751d666da7d1977dbe to branch 'refs/heads/foo'\n>\n> will not put anything in the status table. Whereas in v2.50.0 and\n> earlier, we get:\n>\n> $ git.v2.50.0 fetch . v1.0.0:refs/heads/foo\n> error: cannot update ref 'refs/heads/foo': trying to write non-commit object f665776185ad074b236c00751d666da7d1977dbe to branch 'refs/heads/foo'\n> From .\n> ! [new tag] v1.0.0 -> foo (unable to update local ref)\n>\n> Note the \"!\" and the \"unable to update local ref\" message in the status\n> table.\n>\n> -Peff\n\nThis one is a bit harder to crack, earlier we were getting reference\nupdate results right as we added individual updates. Now that\ninformation is only received at the end when it is committed, we just\ndon't have that information.\n\nOne way is to delay this output until we commit everything. But we don't\nwant to iterate over refs unnecessarily, so probably store these in a\nlist, and then iterate over them.\n\nI'll try and add a patch for this too. Thanks for reporting.\n"},{"id":"533963","messageId":"CAOLa=ZSCAJ-XPWK6vg3p7TO=3T3y8CD+VY4jqn41X2wbdmoaMg@mail.gmail.com","threadId":"64802","inReplyTo":"20260114180306.GI885771@coredump.intra.peff.net","subject":"Re: [PATCH 6/6] receive-pack: utilize rejected ref error details","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-15T15:21:51Z","receivedAt":"2026-01-15T15:21:57Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Jeff King writes:\n\n> On Wed, Jan 14, 2026 at 04:40:47PM +0100, Karthik Nayak wrote:\n>\n>> In 9d2962a7c4 (receive-pack: use batched reference updates, 2025-05-19),\n>> git-receive-pack(1) switched to using batched reference updates. This also\n>> introduced a regression wherein instead of providing detailed error\n>> messages for failed referenced updates, the users were provided generic\n>> error messages based on the error type.\n>>\n>> Similar to the previous commit, switch to using detailed error messages\n>> if present for failed reference updates to fix this regression.\n>>\n>> One downside of this is that the messages can be very verbose, for e.g.\n>> in the files backend, when trying to write a non-commit object to a\n>> branch, you would see:\n>>\n>> ! [remote rejected] 3eaec9ccf3a53f168362a6b3fdeb73426fb9813d ->\n>> branch (cannot update ref 'refs/heads/branch': trying to write\n>> non-commit object 3eaec9ccf3a53f168362a6b3fdeb73426fb9813d to branch\n>> 'refs/heads/branch')\n>>\n>> Here the refname is repeated multiple times due to how error messages\n>> are propagated and filled over the code stack. This potentially can be\n>> cleaned up in a future commit.\n>\n> If we are going to have a \"potentially cleaned up in the future\" state,\n> I think I would prefer to see just:\n>\n> if (details)\n> \trp_error(\"%s\", details);\n>\n> here. And then it comes over the stderr sideband, but the actual\n> status-table gets the same non-verbose message. That's what happened\n> in v2.50.0 and earlier. Later if we want to try to cram more details\n> into the machine-readable message we can.\n>\n> -Peff\n\nFair enough, I think that would be a better approach for now, will\nchange.\n"},{"id":"533984","messageId":"20260115202929.GC1053259@coredump.intra.peff.net","threadId":"64802","inReplyTo":"CAOLa=ZSyfkb8oe=ZtkOcsGo9Dk44GZSFiaye3Vw2kDs_XqS8=Q@mail.gmail.com","subject":"Re: [PATCH 2/6] refs: attach rejection details to updates","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-01-15T20:29:29Z","receivedAt":"2026-01-15T20:29:30Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jan 15, 2026 at 02:02:15AM -0800, Karthik Nayak wrote:\n\n> >> +\tif (details)\n> >> +\t\ttransaction->updates[update_idx]->rejection_details = xstrdup(details);\n> >\n> > I guess this could use xstrdup_or_null(), but probably doesn't matter\n> > much either way. I do wonder if anybody actually passes a NULL value. I\n> > think in my hacky patch there were some spots that did, but here you're\n> > always setting the \"err\" buf (which is good, as we'll always have\n> > details then).\n> \n> That's correct, I did ensure that there were no NULLs passed through, we\n> could definitely drop the check. But I was being defensive. I think\n> `xstrdup_or_null()` is the better option here.\n\nI don't mind the extra defensiveness here, but I was wondering whether\nthis would also mean that ref_transaction_for_each_rejected_update_fn\ncallbacks could assume that \"details\" is always non-NULL. But maybe it\nis better to be defensive there, too.\n\n> > I notice that you \"goto next\" now instead of \"continue\". So I was\n> > curious what happens in \"next\" now, but...\n> >\n> >> +next:;\n> >> \t}\n> >\n> > ...the answer is nothing. ;) I guess maybe you were going to\n> > strbuf_reset() down here at one point? If the 'next' label remains\n> > empty, I think I'd prefer to keep these as 'continue'. But maybe you use\n> > it later in the series. I'll read on.\n> \n> I should have explained this, there are two loops here in play. An outer\n> loop going through refnames to check availability for. An inner loop to\n> breakdown the path of each refname to check for path conflicts.\n> \n> With continue, we'd skip the inner loop, but would still perform other\n> checks for the refname, this can lead to error details being overridden.\n> So while we could replace s/goto next/continue for the code in the outer\n> loop, it would still be needed for the inner loop.\n\nAh, thanks, I totally missed that it was jumping to the outer loop.\n\nIt's curious that the original did a \"continue\" from that inner loop,\nrather than a \"break\". Once we see that \"refs/heads/foo\" is a conflict\nfor a particular update and mark it as failed, there is no point in\nlooking at \"refs/heads/foo/bar\" at all. So I suspect we were wasting\na tiny bit of processing in this error case before, but never doing the\nwrong thing.\n\nLikewise, if we did \"break\" from the loop, shouldn't we \"continue\" to\nthe next ref immediately? There is no need to do further checks.\n\nYour new goto solves both of those; it's just subtle. So two possible\nsuggestions for making this more clear:\n\n - if we are going to use a label, call it next_ref or something, to\n make it clear we are jumping to the outer loop over the refs.\n\n - switch to the goto as a preparatory patch. It's the right thing even\n before changing the \"err\" handling, and the change will be more\n obvious that way.\n\nThere is another way of writing it, which is to break out of the inner\nloop, and then notice that we did so. Either with an explicit flag, or\nin this case we can do it by checking slash. Like this:\n\ndiff --git a/refs.c b/refs.c\nindex 965b232a06..a3dafdb58b 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2663,7 +2663,7 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs\n \t\t\t\t\t REF_TRANSACTION_ERROR_NAME_CONFLICT)) {\n \t\t\t\t\tstrset_remove(&dirnames, dirname.buf);\n \t\t\t\t\tstrset_add(&conflicting_dirnames, dirname.buf);\n-\t\t\t\t\tcontinue;\n+\t\t\t\t\tbreak;\n \t\t\t\t}\n \n \t\t\t\tstrbuf_addf(err, _(\"'%s' exists; cannot create '%s'\"),\n@@ -2676,7 +2676,7 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs\n \t\t\t\t\t transaction, *update_idx,\n \t\t\t\t\t REF_TRANSACTION_ERROR_NAME_CONFLICT)) {\n \t\t\t\t\tstrset_remove(&dirnames, dirname.buf);\n-\t\t\t\t\tcontinue;\n+\t\t\t\t\tbreak;\n \t\t\t\t}\n \n \t\t\t\tstrbuf_addf(err, _(\"cannot process '%s' and '%s' at the same time\"),\n@@ -2685,6 +2685,13 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs\n \t\t\t}\n \t\t}\n \n+\t\t/*\n+\t\t * We didn't finish our loop over the components, which means\n+\t\t * we hit a conflict. Bail to the next ref now.\n+\t\t */\n+\t\tif (slash)\n+\t\t\tcontinue;\n+\n \t\t/*\n \t\t * We are at the leaf of our refname (e.g., \"refs/foo/bar\").\n \t\t * There is no point in searching for a reference with that\n\n\nThat's more \"structured\" in that we avoid the goto. But I'm not sure it\nis any easier to understand than a \"next_ref\" label. So I'm happy with\neither approach. ;)\n\n-Peff\n"},{"id":"534067","messageId":"CAOLa=ZRbYBJaoFV7eWPsMGhVjqMDj+5-KMcAUPzM6fyXwp3qtg@mail.gmail.com","threadId":"64802","inReplyTo":"20260115202929.GC1053259@coredump.intra.peff.net","subject":"Re: [PATCH 2/6] refs: attach rejection details to updates","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-16T17:56:59Z","receivedAt":"2026-01-16T17:57:01Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Jeff King writes:\n\n> On Thu, Jan 15, 2026 at 02:02:15AM -0800, Karthik Nayak wrote:\n>\n>> >> +\tif (details)\n>> >> +\t\ttransaction->updates[update_idx]->rejection_details = xstrdup(details);\n>> >\n>> > I guess this could use xstrdup_or_null(), but probably doesn't matter\n>> > much either way. I do wonder if anybody actually passes a NULL value. I\n>> > think in my hacky patch there were some spots that did, but here you're\n>> > always setting the \"err\" buf (which is good, as we'll always have\n>> > details then).\n>>\n>> That's correct, I did ensure that there were no NULLs passed through, we\n>> could definitely drop the check. But I was being defensive. I think\n>> `xstrdup_or_null()` is the better option here.\n>\n> I don't mind the extra defensiveness here, but I was wondering whether\n> this would also mean that ref_transaction_for_each_rejected_update_fn\n> callbacks could assume that \"details\" is always non-NULL. But maybe it\n> is better to be defensive there, too.\n>\n\nSince I've moved to passing in the strbuf instead of the 'char *', this\nis now removed!\n\n>> > I notice that you \"goto next\" now instead of \"continue\". So I was\n>> > curious what happens in \"next\" now, but...\n>> >\n>> >> +next:;\n>> >> \t}\n>> >\n>> > ...the answer is nothing. ;) I guess maybe you were going to\n>> > strbuf_reset() down here at one point? If the 'next' label remains\n>> > empty, I think I'd prefer to keep these as 'continue'. But maybe you use\n>> > it later in the series. I'll read on.\n>>\n>> I should have explained this, there are two loops here in play. An outer\n>> loop going through refnames to check availability for. An inner loop to\n>> breakdown the path of each refname to check for path conflicts.\n>>\n>> With continue, we'd skip the inner loop, but would still perform other\n>> checks for the refname, this can lead to error details being overridden.\n>> So while we could replace s/goto next/continue for the code in the outer\n>> loop, it would still be needed for the inner loop.\n>\n> Ah, thanks, I totally missed that it was jumping to the outer loop.\n>\n> It's curious that the original did a \"continue\" from that inner loop,\n> rather than a \"break\". Once we see that \"refs/heads/foo\" is a conflict\n> for a particular update and mark it as failed, there is no point in\n> looking at \"refs/heads/foo/bar\" at all. So I suspect we were wasting\n> a tiny bit of processing in this error case before, but never doing the\n> wrong thing.\n>\n\nWith the previous situation of not resetting strbuf after rejecting an\nupdate, we ended up adding more errors to the same strbuf and since we\nkept rejecting the same reference again and again, this causes the last\nrejection with multiple messages appended to the strbuf to be displayed\nto the user.\n\nThis is moot now, considering we reset the strbuf, but that's how I\nnoticed it.\n\n> Likewise, if we did \"break\" from the loop, shouldn't we \"continue\" to\n> the next ref immediately? There is no need to do further checks.\n>\n> Your new goto solves both of those; it's just subtle. So two possible\n> suggestions for making this more clear:\n>\n\nYes, I agree with it not being explicit.\n\n> - if we are going to use a label, call it next_ref or something, to\n> make it clear we are jumping to the outer loop over the refs.\n>\n\nThis is a good suggestion, makes it much nicer to comprehend.\n\n> - switch to the goto as a preparatory patch. It's the right thing even\n> before changing the \"err\" handling, and the change will be more\n> obvious that way.\n>\n\nThis is a fair point too, I'll do this.\n\n> There is another way of writing it, which is to break out of the inner\n> loop, and then notice that we did so. Either with an explicit flag, or\n> in this case we can do it by checking slash. Like this:\n>\n\nThis is an interesting approach, but it is very a bit harder to read in\nmy opinion since the final logic is collation of 'break' + 'continue if\nslash'.\n\n> diff --git a/refs.c b/refs.c\n> index 965b232a06..a3dafdb58b 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -2663,7 +2663,7 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs\n> \t\t\t\t\t REF_TRANSACTION_ERROR_NAME_CONFLICT)) {\n> \t\t\t\t\tstrset_remove(&dirnames, dirname.buf);\n> \t\t\t\t\tstrset_add(&conflicting_dirnames, dirname.buf);\n> -\t\t\t\t\tcontinue;\n> +\t\t\t\t\tbreak;\n> \t\t\t\t}\n>\n> \t\t\t\tstrbuf_addf(err, _(\"'%s' exists; cannot create '%s'\"),\n> @@ -2676,7 +2676,7 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs\n> \t\t\t\t\t transaction, *update_idx,\n> \t\t\t\t\t REF_TRANSACTION_ERROR_NAME_CONFLICT)) {\n> \t\t\t\t\tstrset_remove(&dirnames, dirname.buf);\n> -\t\t\t\t\tcontinue;\n> +\t\t\t\t\tbreak;\n> \t\t\t\t}\n>\n> \t\t\t\tstrbuf_addf(err, _(\"cannot process '%s' and '%s' at the same time\"),\n> @@ -2685,6 +2685,13 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs\n> \t\t\t}\n> \t\t}\n>\n> +\t\t/*\n> +\t\t * We didn't finish our loop over the components, which means\n> +\t\t * we hit a conflict. Bail to the next ref now.\n> +\t\t */\n> +\t\tif (slash)\n> +\t\t\tcontinue;\n> +\n> \t\t/*\n> \t\t * We are at the leaf of our refname (e.g., \"refs/foo/bar\").\n> \t\t * There is no point in searching for a reference with that\n>\n>\n> That's more \"structured\" in that we avoid the goto. But I'm not sure it\n> is any easier to understand than a \"next_ref\" label. So I'm happy with\n> either approach. ;)\n>\n> -Peff\n\nI think the 'next_ref' approach with a separate commit chalked out for\nit, seems like the best approach. Thanks\n"},{"id":"534625","messageId":"20260125-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v5-0-d58f3a9edf98@gmail.com","threadId":"64802","inReplyTo":"20260114-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v1-0-f5f8b173c501@gmail.com","subject":"[PATCH v5 0/6] refs: provide detailed error messages when using batched update","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-25T22:52:35Z","receivedAt":"2026-01-25T22:52:45Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The refs namespace uses an error buffer to capture details about failed\nreference updates. However when we added batched update support to\nreference transactions, these messages were never propagated, instead\nonly an error code pertaining to the type of failure was propagated.\n\nCurrently, there are three regions which utilize batched updates:\n\n - git update-ref --batch-updates\n - git fetch\n - git receive-pack\n\nWhile 'git update-ref --batch-updates' was a newly introduced flag, both\n'git fetch' and 'git receive-pack' were pre-existing. Before using\nbatched updates, they provided more detailed error messages to the user,\nbut this changed with the introduction of batched updates. This is a\nregression in their workings.\n\nThis patch series fixes this, by passing the detailed error message and\nutilizing it whenever available. The regression was reported by Elijah\nNewren [1] and based on the patch submitted by Jeff King [2].\n\n[1]: https://lore.kernel.org/all/CABPp-BGL2tJR4dPidQuFcp-X0_VkVTknCY-0Zgo=jHVGv_P=wA@mail.gmail.com/\n[2]: https://lore.kernel.org/all/20251224081214.GA1879908@coredump.intra.peff.net/\n\n---\nChanges in v5:\n- In the last commit, drop 'const *' used to indicate immutability of\n fields within the struct. In the project it is more common to use\n 'const *' to indicate ownership. Since the memory of the fields are\n owned by the struct, let's drop the 'const *'.\n- Link to v4: https://patch.msgid.link/20260122-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v4-0-2ddba0832440@gmail.com\n\nChanges in v4:\n- In the last commit, instead of propagating {*list, count}, propagate\n an array with {*list, nr, count} and use ALLOC_GROW. This simplifies\n the variables passed and cleanups the code.\n- 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.com\n\nChanges in v3:\n- Drop the first commit.\n- For the last commit, where we delay 'git fetch' status information,\n delay all information to the end. Also use a list to compliment the\n existing strmap, this ensures that the order is maintained.\n- 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\n\nChanges in v2:\n- Updates to the commit messages to be more descriptive.\n- Instead of passing the char pointer for the error description, pass\n the 'strbuf' itself. This makes the API a lot cleaner to deal with.\n Also avoids having to remember to reset the strbuf after usage.\n- Chalk out a separate commit for using a 'goto next_ref' in\n `refs_verify_refnames_available()`. This makes the intention much\n clearer.\n- For git-update-ref(1), keep the existing implementation as is and only\n output the detailed error message to stderr.\n- For git-receive-pack(1), use 'rp_error()' for detailed error message\n while keeping the current implementation as is.\n- Added a separate patch to handle missing information in git-fetch(1)'s\n status table. This involves delaying updates to the end, where update\n success/failure information is available. I'm not too confident about\n this approach though, we could also drop it from the series and I\n could pick that up independently. This is still 1.19 ± 0.02 times\n faster than non-batched version (v2.50.0) in the files backend.\n- 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\n\n---\n builtin/fetch.c | 255 +++++++++++++++++++++++++++++++++++++-----------\n builtin/receive-pack.c | 7 +-\n builtin/update-ref.c | 7 +-\n refs.c | 46 +++++----\n refs.h | 1 +\n refs/files-backend.c | 5 +-\n refs/packed-backend.c | 12 +--\n refs/refs-internal.h | 4 +-\n refs/reftable-backend.c | 5 +-\n t/t1400-update-ref.sh | 71 ++++++++------\n t/t5510-fetch.sh | 8 +-\n t/t5516-fetch-push.sh | 16 +++\n 12 files changed, 312 insertions(+), 125 deletions(-)\n\nKarthik Nayak (6):\n refs: skip to next ref when current ref is rejected\n refs: add rejection detail to the callback function\n update-ref: utilize rejected error details if available\n fetch: utilize rejected ref error details\n receive-pack: utilize rejected ref error details\n fetch: delay user information post committing of transaction\n\nRange-diff versus v4:\n\n1: 3264b8c3bf = 1: 661265fb86 refs: skip to next ref when current ref is rejected\n2: 3d2af7a15a = 2: 1f0f2b6224 refs: add rejection detail to the callback function\n3: bc7556f4b0 = 3: 8413ca46b5 update-ref: utilize rejected error details if available\n4: d114f13967 = 4: b0c4441c55 fetch: utilize rejected ref error details\n5: 4606c3b991 = 5: 8aa4477f51 receive-pack: utilize rejected ref error details\n6: c75ccc40f3 ! 6: c9698e06bb fetch: delay user information post committing of transaction\n @@ builtin/fetch.c: static void display_ref_update(struct display_state *display_st\n +\tbool failed;\n +\tchar success_code;\n +\tchar fail_code;\n -+\tconst char *summary;\n -+\tconst char *fail_detail;\n -+\tconst char *success_detail;\n -+\tconst char *ref;\n -+\tconst char *remote;\n ++\tchar *summary;\n ++\tchar *fail_detail;\n ++\tchar *success_detail;\n ++\tchar *ref;\n ++\tchar *remote;\n +\tstruct object_id old_oid;\n +\tstruct object_id new_oid;\n +};\n @@ builtin/fetch.c: static void display_ref_update(struct display_state *display_st\n +\n +static void ref_update_display_info_free(struct ref_update_display_info *info)\n +{\n -+\tfree((char *)info->summary);\n -+\tfree((char *)info->success_detail);\n -+\tfree((char *)info->fail_detail);\n -+\tfree((char *)info->remote);\n -+\tfree((char *)info->ref);\n ++\tfree(info->summary);\n ++\tfree(info->success_detail);\n ++\tfree(info->fail_detail);\n ++\tfree(info->remote);\n ++\tfree(info->ref);\n +}\n +\n +static void ref_update_display_info_display(struct ref_update_display_info *info,\n\n\nbase-commit: 8745eae506f700657882b9e32b2aa00f234a6fb6\nchange-id: 20260113-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-17786b20894a\n\nThanks\n- Karthik\n\n"},{"id":"534626","messageId":"20260125-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v5-1-d58f3a9edf98@gmail.com","threadId":"64802","inReplyTo":"20260125-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v5-0-d58f3a9edf98@gmail.com","subject":"[PATCH v5 1/6] refs: skip to next ref when current ref is rejected","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-25T22:52:36Z","receivedAt":"2026-01-25T22:52:46Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"In `refs_verify_refnames_available()` we have two nested loops: the\nouter loop iterates over all references to check, while the inner loop\nchecks for filesystem conflicts for a given ref by breaking down its\npath.\n\nWith batched updates, when we detect a filesystem conflict, we mark the\nupdate as rejected and execute 'continue'. However, this only skips to\nthe next iteration of the inner loop, not the outer loop as intended.\nThis causes the same reference to be repeatedly rejected. Fix this by\nusing a goto statement to skip to the next reference in the outer loop.\n\nSigned-off-by: Karthik Nayak \n---\n refs.c | 44 ++++++++++++++++++++++++++------------------\n refs/files-backend.c | 5 ++---\n refs/packed-backend.c | 12 ++++++------\n refs/refs-internal.h | 4 +++-\n refs/reftable-backend.c | 5 ++---\n 5 files changed, 39 insertions(+), 31 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex e06e0cb072..53919c3d22 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1224,6 +1224,7 @@ void ref_transaction_free(struct ref_transaction *transaction)\n \t\tfree(transaction->updates[i]->committer_info);\n \t\tfree((char *)transaction->updates[i]->new_target);\n \t\tfree((char *)transaction->updates[i]->old_target);\n+\t\tfree((char *)transaction->updates[i]->rejection_details);\n \t\tfree(transaction->updates[i]);\n \t}\n \n@@ -1238,7 +1239,8 @@ void ref_transaction_free(struct ref_transaction *transaction)\n \n int ref_transaction_maybe_set_rejected(struct ref_transaction *transaction,\n \t\t\t\t size_t update_idx,\n-\t\t\t\t enum ref_transaction_error err)\n+\t\t\t\t enum ref_transaction_error err,\n+\t\t\t\t struct strbuf *details)\n {\n \tif (update_idx >= transaction->nr)\n \t\tBUG(\"trying to set rejection on invalid update index\");\n@@ -1264,6 +1266,7 @@ int ref_transaction_maybe_set_rejected(struct ref_transaction *transaction,\n \t\t\t transaction->updates[update_idx]->refname, 0);\n \n \ttransaction->updates[update_idx]->rejection_err = err;\n+\ttransaction->updates[update_idx]->rejection_details = strbuf_detach(details, NULL);\n \tALLOC_GROW(transaction->rejections->update_indices,\n \t\t transaction->rejections->nr + 1,\n \t\t transaction->rejections->alloc);\n@@ -2659,30 +2662,33 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs\n \t\t\tif (!initial_transaction &&\n \t\t\t (strset_contains(&conflicting_dirnames, dirname.buf) ||\n \t\t\t !refs_read_raw_ref(refs, dirname.buf, &oid, &referent,\n-\t\t\t\t\t\t &type, &ignore_errno))) {\n+\t\t\t\t\t\t&type, &ignore_errno))) {\n+\n+\t\t\t\tstrbuf_addf(err, _(\"'%s' exists; cannot create '%s'\"),\n+\t\t\t\t\t dirname.buf, refname);\n+\n \t\t\t\tif (transaction && ref_transaction_maybe_set_rejected(\n \t\t\t\t\t transaction, *update_idx,\n-\t\t\t\t\t REF_TRANSACTION_ERROR_NAME_CONFLICT)) {\n+\t\t\t\t\t REF_TRANSACTION_ERROR_NAME_CONFLICT, err)) {\n \t\t\t\t\tstrset_remove(&dirnames, dirname.buf);\n \t\t\t\t\tstrset_add(&conflicting_dirnames, dirname.buf);\n-\t\t\t\t\tcontinue;\n+\t\t\t\t\tgoto next_ref;\n \t\t\t\t}\n \n-\t\t\t\tstrbuf_addf(err, _(\"'%s' exists; cannot create '%s'\"),\n-\t\t\t\t\t dirname.buf, refname);\n \t\t\t\tgoto cleanup;\n \t\t\t}\n \n \t\t\tif (extras && string_list_has_string(extras, dirname.buf)) {\n+\t\t\t\tstrbuf_addf(err, _(\"cannot process '%s' and '%s' at the same time\"),\n+\t\t\t\t\t refname, dirname.buf);\n+\n \t\t\t\tif (transaction && ref_transaction_maybe_set_rejected(\n \t\t\t\t\t transaction, *update_idx,\n-\t\t\t\t\t REF_TRANSACTION_ERROR_NAME_CONFLICT)) {\n+\t\t\t\t\t REF_TRANSACTION_ERROR_NAME_CONFLICT, err)) {\n \t\t\t\t\tstrset_remove(&dirnames, dirname.buf);\n-\t\t\t\t\tcontinue;\n+\t\t\t\t\tgoto next_ref;\n \t\t\t\t}\n \n-\t\t\t\tstrbuf_addf(err, _(\"cannot process '%s' and '%s' at the same time\"),\n-\t\t\t\t\t refname, dirname.buf);\n \t\t\t\tgoto cleanup;\n \t\t\t}\n \t\t}\n@@ -2712,14 +2718,14 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs\n \t\t\t\tif (skip &&\n \t\t\t\t string_list_has_string(skip, iter->ref.name))\n \t\t\t\t\tcontinue;\n+\t\t\t\tstrbuf_addf(err, _(\"'%s' exists; cannot create '%s'\"),\n+\t\t\t\t\t iter->ref.name, refname);\n \n \t\t\t\tif (transaction && ref_transaction_maybe_set_rejected(\n \t\t\t\t\t transaction, *update_idx,\n-\t\t\t\t\t REF_TRANSACTION_ERROR_NAME_CONFLICT))\n-\t\t\t\t\tcontinue;\n+\t\t\t\t\t REF_TRANSACTION_ERROR_NAME_CONFLICT, err))\n+\t\t\t\t\tgoto next_ref;\n \n-\t\t\t\tstrbuf_addf(err, _(\"'%s' exists; cannot create '%s'\"),\n-\t\t\t\t\t iter->ref.name, refname);\n \t\t\t\tgoto cleanup;\n \t\t\t}\n \n@@ -2729,15 +2735,17 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs\n \n \t\textra_refname = find_descendant_ref(dirname.buf, extras, skip);\n \t\tif (extra_refname) {\n+\t\t\tstrbuf_addf(err, _(\"cannot process '%s' and '%s' at the same time\"),\n+\t\t\t\t refname, extra_refname);\n+\n \t\t\tif (transaction && ref_transaction_maybe_set_rejected(\n \t\t\t\t transaction, *update_idx,\n-\t\t\t\t REF_TRANSACTION_ERROR_NAME_CONFLICT))\n-\t\t\t\tcontinue;\n+\t\t\t\t REF_TRANSACTION_ERROR_NAME_CONFLICT, err))\n+\t\t\t\tgoto next_ref;\n \n-\t\t\tstrbuf_addf(err, _(\"cannot process '%s' and '%s' at the same time\"),\n-\t\t\t\t refname, extra_refname);\n \t\t\tgoto cleanup;\n \t\t}\n+next_ref:;\n \t}\n \n \tret = 0;\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 6f6f76a8d8..6790d8bf53 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -2983,10 +2983,9 @@ static int files_transaction_prepare(struct ref_store *ref_store,\n \t\t\t\t\t head_ref, &refnames_to_check,\n \t\t\t\t\t err);\n \t\tif (ret) {\n-\t\t\tif (ref_transaction_maybe_set_rejected(transaction, i, ret)) {\n-\t\t\t\tstrbuf_reset(err);\n+\t\t\tif (ref_transaction_maybe_set_rejected(transaction, i,\n+\t\t\t\t\t\t\t ret, err)) {\n \t\t\t\tret = 0;\n-\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tgoto cleanup;\ndiff --git a/refs/packed-backend.c b/refs/packed-backend.c\nindex 4ea0c12299..59b3ecb9d6 100644\n--- a/refs/packed-backend.c\n+++ b/refs/packed-backend.c\n@@ -1437,8 +1437,8 @@ static enum ref_transaction_error write_with_updates(struct packed_ref_store *re\n \t\t\t\t\t\t update->refname);\n \t\t\t\t\tret = REF_TRANSACTION_ERROR_CREATE_EXISTS;\n \n-\t\t\t\t\tif (ref_transaction_maybe_set_rejected(transaction, i, ret)) {\n-\t\t\t\t\t\tstrbuf_reset(err);\n+\t\t\t\t\tif (ref_transaction_maybe_set_rejected(transaction, i,\n+\t\t\t\t\t\t\t\t\t ret, err)) {\n \t\t\t\t\t\tret = 0;\n \t\t\t\t\t\tcontinue;\n \t\t\t\t\t}\n@@ -1452,8 +1452,8 @@ static enum ref_transaction_error write_with_updates(struct packed_ref_store *re\n \t\t\t\t\t\t oid_to_hex(&update->old_oid));\n \t\t\t\t\tret = REF_TRANSACTION_ERROR_INCORRECT_OLD_VALUE;\n \n-\t\t\t\t\tif (ref_transaction_maybe_set_rejected(transaction, i, ret)) {\n-\t\t\t\t\t\tstrbuf_reset(err);\n+\t\t\t\t\tif (ref_transaction_maybe_set_rejected(transaction, i,\n+\t\t\t\t\t\t\t\t\t ret, err)) {\n \t\t\t\t\t\tret = 0;\n \t\t\t\t\t\tcontinue;\n \t\t\t\t\t}\n@@ -1496,8 +1496,8 @@ static enum ref_transaction_error write_with_updates(struct packed_ref_store *re\n \t\t\t\t\t oid_to_hex(&update->old_oid));\n \t\t\t\tret = REF_TRANSACTION_ERROR_NONEXISTENT_REF;\n \n-\t\t\t\tif (ref_transaction_maybe_set_rejected(transaction, i, ret)) {\n-\t\t\t\t\tstrbuf_reset(err);\n+\t\t\t\tif (ref_transaction_maybe_set_rejected(transaction, i,\n+\t\t\t\t\t\t\t\t ret, err)) {\n \t\t\t\t\tret = 0;\n \t\t\t\t\tcontinue;\n \t\t\t\t}\ndiff --git a/refs/refs-internal.h b/refs/refs-internal.h\nindex c7d2a6e50b..191a25683f 100644\n--- a/refs/refs-internal.h\n+++ b/refs/refs-internal.h\n@@ -128,6 +128,7 @@ struct ref_update {\n \t * was rejected.\n \t */\n \tenum ref_transaction_error rejection_err;\n+\tconst char *rejection_details;\n \n \t/*\n \t * If this ref_update was split off of a symref update via\n@@ -153,7 +154,8 @@ int refs_read_raw_ref(struct ref_store *ref_store, const char *refname,\n */\n int ref_transaction_maybe_set_rejected(struct ref_transaction *transaction,\n \t\t\t\t size_t update_idx,\n-\t\t\t\t enum ref_transaction_error err);\n+\t\t\t\t enum ref_transaction_error err,\n+\t\t\t\t struct strbuf *details);\n \n /*\n * Add a ref_update with the specified properties to transaction, and\ndiff --git a/refs/reftable-backend.c b/refs/reftable-backend.c\nindex 4319a4eacb..0e2648e36c 100644\n--- a/refs/reftable-backend.c\n+++ b/refs/reftable-backend.c\n@@ -1401,10 +1401,9 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,\n \t\t\t\t\t &refnames_to_check, head_type,\n \t\t\t\t\t &head_referent, &referent, err);\n \t\tif (ret) {\n-\t\t\tif (ref_transaction_maybe_set_rejected(transaction, i, ret)) {\n-\t\t\t\tstrbuf_reset(err);\n+\t\t\tif (ref_transaction_maybe_set_rejected(transaction, i,\n+\t\t\t\t\t\t\t ret, err)) {\n \t\t\t\tret = 0;\n-\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tgoto done;\n\n-- \n2.52.0\n\n"},{"id":"534627","messageId":"20260125-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v5-2-d58f3a9edf98@gmail.com","threadId":"64802","inReplyTo":"20260125-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v5-0-d58f3a9edf98@gmail.com","subject":"[PATCH v5 2/6] refs: add rejection detail to the callback function","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-25T22:52:37Z","receivedAt":"2026-01-25T22:52:47Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The previous commit started storing the rejection details alongside the\nerror code for rejected updates. Pass this along to the callback\nfunction `ref_transaction_for_each_rejected_update()`. Currently the\nfield is unused, but will be integrated in the upcoming commits.\n\nCo-authored-by: Jeff King \nSigned-off-by: Jeff King \nSigned-off-by: Karthik Nayak \n---\n builtin/fetch.c | 1 +\n builtin/receive-pack.c | 1 +\n builtin/update-ref.c | 1 +\n refs.c | 2 +-\n refs.h | 1 +\n 5 files changed, 5 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 288d3772ea..d427adea61 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -1649,6 +1649,7 @@ static void ref_transaction_rejection_handler(const char *refname,\n \t\t\t\t\t const char *old_target UNUSED,\n \t\t\t\t\t const char *new_target UNUSED,\n \t\t\t\t\t enum ref_transaction_error err,\n+\t\t\t\t\t const char *details UNUSED,\n \t\t\t\t\t void *cb_data)\n {\n \tstruct ref_rejection_data *data = cb_data;\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex ef1f77be8c..94d3e73cee 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -1813,6 +1813,7 @@ static void ref_transaction_rejection_handler(const char *refname,\n \t\t\t\t\t const char *old_target UNUSED,\n \t\t\t\t\t const char *new_target UNUSED,\n \t\t\t\t\t enum ref_transaction_error err,\n+\t\t\t\t\t const char *details UNUSED,\n \t\t\t\t\t void *cb_data)\n {\n \tstruct strmap *failed_refs = cb_data;\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 195437e7c6..0046a87c57 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -573,6 +573,7 @@ static void print_rejected_refs(const char *refname,\n \t\t\t\tconst char *old_target,\n \t\t\t\tconst char *new_target,\n \t\t\t\tenum ref_transaction_error err,\n+\t\t\t\tconst char *details UNUSED,\n \t\t\t\tvoid *cb_data UNUSED)\n {\n \tstruct strbuf sb = STRBUF_INIT;\ndiff --git a/refs.c b/refs.c\nindex 53919c3d22..c85c3d2c8b 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2874,7 +2874,7 @@ void ref_transaction_for_each_rejected_update(struct ref_transaction *transactio\n \t\t (update->flags & REF_HAVE_OLD) ? &update->old_oid : NULL,\n \t\t (update->flags & REF_HAVE_NEW) ? &update->new_oid : NULL,\n \t\t update->old_target, update->new_target,\n-\t\t update->rejection_err, cb_data);\n+\t\t update->rejection_err, update->rejection_details, cb_data);\n \t}\n }\n \ndiff --git a/refs.h b/refs.h\nindex d9051bbb04..4fbe3da924 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -975,6 +975,7 @@ typedef void ref_transaction_for_each_rejected_update_fn(const char *refname,\n \t\t\t\t\t\t\t const char *old_target,\n \t\t\t\t\t\t\t const char *new_target,\n \t\t\t\t\t\t\t enum ref_transaction_error err,\n+\t\t\t\t\t\t\t const char *details,\n \t\t\t\t\t\t\t void *cb_data);\n void ref_transaction_for_each_rejected_update(struct ref_transaction *transaction,\n \t\t\t\t\t ref_transaction_for_each_rejected_update_fn cb,\n\n-- \n2.52.0\n\n"},{"id":"534628","messageId":"20260125-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v5-3-d58f3a9edf98@gmail.com","threadId":"64802","inReplyTo":"20260125-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v5-0-d58f3a9edf98@gmail.com","subject":"[PATCH v5 3/6] update-ref: utilize rejected error details if available","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-25T22:52:38Z","receivedAt":"2026-01-25T22:52:48Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"When git-update-ref(1) received the '--update-ref' flag, the error\ndetails generated in the refs namespace wasn't propagated with failed\nupdates. Instead only an error code pertaining to the type of rejection\nwas noted.\n\nThis missed detailed error message which the user can act upon. The\nprevious commits added the required code to propagate these detailed\nerror messages from the refs namespace. Now that additional details are\navailable, let's output this additional details to stderr. This allows\nusers to have additional information over the already present machine\nparsable output.\n\nWhile we're here, improve the existing tests for the machine parsable\noutput by checking for the entire output string and not just the\nrejection reason.\n\nReported-by: Elijah Newren \nCo-authored-by: Jeff King \nSigned-off-by: Karthik Nayak \n---\n builtin/update-ref.c | 8 +++---\n t/t1400-update-ref.sh | 71 ++++++++++++++++++++++++++++++---------------------\n 2 files changed, 47 insertions(+), 32 deletions(-)\n\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 0046a87c57..2d68c40ecb 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -573,16 +573,18 @@ static void print_rejected_refs(const char *refname,\n \t\t\t\tconst char *old_target,\n \t\t\t\tconst char *new_target,\n \t\t\t\tenum ref_transaction_error err,\n-\t\t\t\tconst char *details UNUSED,\n+\t\t\t\tconst char *details,\n \t\t\t\tvoid *cb_data UNUSED)\n {\n \tstruct strbuf sb = STRBUF_INIT;\n-\tconst char *reason = ref_transaction_error_msg(err);\n+\n+\tif (details && *details)\n+\t\terror(\"%s\", details);\n \n \tstrbuf_addf(&sb, \"rejected %s %s %s %s\\n\", refname,\n \t\t new_oid ? oid_to_hex(new_oid) : new_target,\n \t\t old_oid ? oid_to_hex(old_oid) : old_target,\n-\t\t reason);\n+\t\t ref_transaction_error_msg(err));\n \n \tfwrite(sb.buf, sb.len, 1, stdout);\n \tstrbuf_release(&sb);\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex db7f5444da..db6585b8d8 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -2093,14 +2093,15 @@ do\n \n \t\t\tformat_command $type \"update refs/heads/ref1\" \"$old_head\" \"$head\" >stdin &&\n \t\t\tformat_command $type \"update refs/heads/ref2\" \"$(test_oid 001)\" \"$head\" >>stdin &&\n-\t\t\tgit update-ref $type --stdin --batch-updates stdout &&\n+\t\t\tgit update-ref $type --stdin --batch-updates stdout 2>err &&\n \t\t\techo $old_head >expect &&\n \t\t\tgit rev-parse refs/heads/ref1 >actual &&\n \t\t\ttest_cmp expect actual &&\n \t\t\techo $head >expect &&\n \t\t\tgit rev-parse refs/heads/ref2 >actual &&\n \t\t\ttest_cmp expect actual &&\n-\t\t\ttest_grep -q \"invalid new value provided\" stdout\n+\t\t\ttest_grep \"rejected refs/heads/ref2 $(test_oid 001) $head invalid new value provided\" stdout &&\n+\t\t\ttest_grep \"trying to write ref ${SQ}refs/heads/ref2${SQ} with nonexistent object\" err\n \t\t)\n \t'\n \n@@ -2119,14 +2120,15 @@ do\n \n \t\t\tformat_command $type \"update refs/heads/ref1\" \"$old_head\" \"$head\" >stdin &&\n \t\t\tformat_command $type \"update refs/heads/ref2\" \"$head_tree\" \"$head\" >>stdin &&\n-\t\t\tgit update-ref $type --stdin --batch-updates stdout &&\n+\t\t\tgit update-ref $type --stdin --batch-updates stdout 2>err &&\n \t\t\techo $old_head >expect &&\n \t\t\tgit rev-parse refs/heads/ref1 >actual &&\n \t\t\ttest_cmp expect actual &&\n \t\t\techo $head >expect &&\n \t\t\tgit rev-parse refs/heads/ref2 >actual &&\n \t\t\ttest_cmp expect actual &&\n-\t\t\ttest_grep -q \"invalid new value provided\" stdout\n+\t\t\ttest_grep \"rejected refs/heads/ref2 $head_tree $head invalid new value provided\" stdout &&\n+\t\t\ttest_grep \"trying to write non-commit object $head_tree to branch ${SQ}refs/heads/ref2${SQ}\" err\n \t\t)\n \t'\n \n@@ -2143,12 +2145,13 @@ do\n \n \t\t\tformat_command $type \"update refs/heads/ref1\" \"$old_head\" \"$head\" >stdin &&\n \t\t\tformat_command $type \"update refs/heads/ref2\" \"$old_head\" \"$head\" >>stdin &&\n-\t\t\tgit update-ref $type --stdin --batch-updates stdout &&\n+\t\t\tgit update-ref $type --stdin --batch-updates stdout 2>err &&\n \t\t\techo $old_head >expect &&\n \t\t\tgit rev-parse refs/heads/ref1 >actual &&\n \t\t\ttest_cmp expect actual &&\n \t\t\ttest_must_fail git rev-parse refs/heads/ref2 &&\n-\t\t\ttest_grep -q \"reference does not exist\" stdout\n+\t\t\ttest_grep \"rejected refs/heads/ref2 $old_head $head reference does not exist\" stdout &&\n+\t\t\ttest_grep \"cannot lock ref ${SQ}refs/heads/ref2${SQ}: unable to resolve reference ${SQ}refs/heads/ref2${SQ}\" err\n \t\t)\n \t'\n \n@@ -2166,13 +2169,14 @@ do\n \n \t\t\tformat_command $type \"update refs/heads/ref1\" \"$old_head\" \"$head\" >stdin &&\n \t\t\tformat_command $type \"update refs/heads/ref2\" \"$old_head\" \"$head\" >>stdin &&\n-\t\t\tgit update-ref $type --no-deref --stdin --batch-updates stdout &&\n+\t\t\tgit update-ref $type --no-deref --stdin --batch-updates stdout 2>err &&\n \t\t\techo $old_head >expect &&\n \t\t\tgit rev-parse refs/heads/ref1 >actual &&\n \t\t\ttest_cmp expect actual &&\n \t\t\techo $head >expect &&\n \t\t\ttest_must_fail git rev-parse refs/heads/ref2 &&\n-\t\t\ttest_grep -q \"reference does not exist\" stdout\n+\t\t\ttest_grep \"rejected refs/heads/ref2 $old_head $head reference does not exist\" stdout &&\n+\t\t\ttest_grep \"cannot lock ref ${SQ}refs/heads/ref2${SQ}: reference is missing but expected $head\" err\n \t\t)\n \t'\n \n@@ -2190,7 +2194,7 @@ do\n \n \t\t\tformat_command $type \"update refs/heads/ref1\" \"$old_head\" \"$head\" >stdin &&\n \t\t\tformat_command $type \"symref-update refs/heads/ref2\" \"$old_head\" \"ref\" \"refs/heads/nonexistent\" >>stdin &&\n-\t\t\tgit update-ref $type --no-deref --stdin --batch-updates stdout &&\n+\t\t\tgit update-ref $type --no-deref --stdin --batch-updates stdout 2>err &&\n \t\t\techo $old_head >expect &&\n \t\t\tgit rev-parse refs/heads/ref1 >actual &&\n \t\t\ttest_cmp expect actual &&\n@@ -2198,7 +2202,8 @@ do\n \t\t\techo $head >expect &&\n \t\t\tgit rev-parse refs/heads/ref2 >actual &&\n \t\t\ttest_cmp expect actual &&\n-\t\t\ttest_grep -q \"expected symref but found regular ref\" stdout\n+\t\t\ttest_grep \"rejected refs/heads/ref2 $ZERO_OID $ZERO_OID expected symref but found regular ref\" stdout &&\n+\t\t\ttest_grep \"cannot lock ref ${SQ}refs/heads/ref2${SQ}: expected symref with target ${SQ}refs/heads/nonexistent${SQ}: but is a regular ref\" err\n \t\t)\n \t'\n \n@@ -2216,14 +2221,15 @@ do\n \n \t\t\tformat_command $type \"update refs/heads/ref1\" \"$old_head\" \"$head\" >stdin &&\n \t\t\tformat_command $type \"update refs/heads/ref2\" \"$old_head\" \"$Z\" >>stdin &&\n-\t\t\tgit update-ref $type --stdin --batch-updates stdout &&\n+\t\t\tgit update-ref $type --stdin --batch-updates stdout 2>err &&\n \t\t\techo $old_head >expect &&\n \t\t\tgit rev-parse refs/heads/ref1 >actual &&\n \t\t\ttest_cmp expect actual &&\n \t\t\techo $head >expect &&\n \t\t\tgit rev-parse refs/heads/ref2 >actual &&\n \t\t\ttest_cmp expect actual &&\n-\t\t\ttest_grep -q \"reference already exists\" stdout\n+\t\t\ttest_grep \"rejected refs/heads/ref2 $old_head $ZERO_OID reference already exists\" stdout &&\n+\t\t\ttest_grep \"cannot lock ref ${SQ}refs/heads/ref2${SQ}: reference already exists\" err\n \t\t)\n \t'\n \n@@ -2241,14 +2247,15 @@ do\n \n \t\t\tformat_command $type \"update refs/heads/ref1\" \"$old_head\" \"$head\" >stdin &&\n \t\t\tformat_command $type \"update refs/heads/ref2\" \"$head\" \"$old_head\" >>stdin &&\n-\t\t\tgit update-ref $type --stdin --batch-updates stdout &&\n+\t\t\tgit update-ref $type --stdin --batch-updates stdout 2>err &&\n \t\t\techo $old_head >expect &&\n \t\t\tgit rev-parse refs/heads/ref1 >actual &&\n \t\t\ttest_cmp expect actual &&\n \t\t\techo $head >expect &&\n \t\t\tgit rev-parse refs/heads/ref2 >actual &&\n \t\t\ttest_cmp expect actual &&\n-\t\t\ttest_grep -q \"incorrect old value provided\" stdout\n+\t\t\ttest_grep \"rejected refs/heads/ref2 $head $old_head incorrect old value provided\" stdout &&\n+\t\t\ttest_grep \"cannot lock ref ${SQ}refs/heads/ref2${SQ}: is at $head but expected $old_head\" err\n \t\t)\n \t'\n \n@@ -2264,12 +2271,13 @@ do\n \t\t\tgit update-ref refs/heads/ref/foo $head &&\n \n \t\t\tformat_command $type \"update refs/heads/ref/foo\" \"$old_head\" \"$head\" >stdin &&\n-\t\t\tformat_command $type \"update refs/heads/ref\" \"$old_head\" \"\" >>stdin &&\n-\t\t\tgit update-ref $type --stdin --batch-updates stdout &&\n+\t\t\tformat_command $type \"update refs/heads/ref\" \"$old_head\" \"$ZERO_OID\" >>stdin &&\n+\t\t\tgit update-ref $type --stdin --batch-updates stdout 2>err &&\n \t\t\techo $old_head >expect &&\n \t\t\tgit rev-parse refs/heads/ref/foo >actual &&\n \t\t\ttest_cmp expect actual &&\n-\t\t\ttest_grep -q \"refname conflict\" stdout\n+\t\t\ttest_grep \"rejected refs/heads/ref $old_head $ZERO_OID refname conflict\" stdout &&\n+\t\t\ttest_grep \"${SQ}refs/heads/ref/foo${SQ} exists; cannot create ${SQ}refs/heads/ref${SQ}\" err\n \t\t)\n \t'\n \n@@ -2284,13 +2292,14 @@ do\n \t\t\thead=$(git rev-parse HEAD) &&\n \t\t\tgit update-ref refs/heads/ref/foo $head &&\n \n-\t\t\tformat_command $type \"update refs/heads/foo\" \"$old_head\" \"\" >stdin &&\n-\t\t\tformat_command $type \"update refs/heads/ref\" \"$old_head\" \"\" >>stdin &&\n-\t\t\tgit update-ref $type --stdin --batch-updates stdout &&\n+\t\t\tformat_command $type \"update refs/heads/foo\" \"$old_head\" \"$ZERO_OID\" >stdin &&\n+\t\t\tformat_command $type \"update refs/heads/ref\" \"$old_head\" \"$ZERO_OID\" >>stdin &&\n+\t\t\tgit update-ref $type --stdin --batch-updates stdout 2>err &&\n \t\t\techo $old_head >expect &&\n \t\t\tgit rev-parse refs/heads/foo >actual &&\n \t\t\ttest_cmp expect actual &&\n-\t\t\ttest_grep -q \"refname conflict\" stdout\n+\t\t\ttest_grep \"rejected refs/heads/ref $old_head $ZERO_OID refname conflict\" stdout &&\n+\t\t\ttest_grep \"${SQ}refs/heads/ref/foo${SQ} exists; cannot create ${SQ}refs/heads/ref${SQ}\" err\n \t\t)\n \t'\n \n@@ -2309,14 +2318,15 @@ do\n \t\t\t\tformat_command $type \"create refs/heads/ref\" \"$old_head\" &&\n \t\t\t\tformat_command $type \"create refs/heads/Foo\" \"$old_head\"\n \t\t\t} >stdin &&\n-\t\t\tgit update-ref $type --stdin --batch-updates stdout &&\n+\t\t\tgit update-ref $type --stdin --batch-updates stdout 2>err &&\n \n \t\t\techo $head >expect &&\n \t\t\tgit rev-parse refs/heads/foo >actual &&\n \t\t\techo $old_head >expect &&\n \t\t\tgit rev-parse refs/heads/ref >actual &&\n \t\t\ttest_cmp expect actual &&\n-\t\t\ttest_grep -q \"reference conflict due to case-insensitive filesystem\" stdout\n+\t\t\ttest_grep \"rejected refs/heads/Foo $old_head $ZERO_OID reference conflict due to case-insensitive filesystem\" stdout &&\n+\t\t\ttest_grep -e \"cannot lock ref ${SQ}refs/heads/Foo${SQ}: Unable to create\" -e \"Foo.lock\" err\n \t\t)\n \t'\n \n@@ -2357,8 +2367,9 @@ do\n \t\t\tgit symbolic-ref refs/heads/symbolic refs/heads/non-existent &&\n \n \t\t\tformat_command $type \"delete refs/heads/symbolic\" \"$head\" >stdin &&\n-\t\t\tgit update-ref $type --stdin --batch-updates stdout &&\n-\t\t\ttest_grep \"reference does not exist\" stdout\n+\t\t\tgit update-ref $type --stdin --batch-updates stdout 2>err &&\n+\t\t\ttest_grep \"rejected refs/heads/non-existent $ZERO_OID $head reference does not exist\" stdout &&\n+\t\t\ttest_grep \"cannot lock ref ${SQ}refs/heads/symbolic${SQ}: unable to resolve reference ${SQ}refs/heads/non-existent${SQ}\" err\n \t\t)\n \t'\n \n@@ -2373,8 +2384,9 @@ do\n \t\t\thead=$(git rev-parse HEAD) &&\n \n \t\t\tformat_command $type \"delete refs/heads/new-branch\" \"$head\" >stdin &&\n-\t\t\tgit update-ref $type --stdin --batch-updates stdout &&\n-\t\t\ttest_grep \"incorrect old value provided\" stdout\n+\t\t\tgit update-ref $type --stdin --batch-updates stdout 2>err &&\n+\t\t\ttest_grep \"rejected refs/heads/new-branch $ZERO_OID $head incorrect old value provided\" stdout &&\n+\t\t\ttest_grep \"cannot lock ref ${SQ}refs/heads/new-branch${SQ}: is at $(git rev-parse new-branch) but expected $head\" err\n \t\t)\n \t'\n \n@@ -2387,8 +2399,9 @@ do\n \t\t\thead=$(git rev-parse HEAD) &&\n \n \t\t\tformat_command $type \"delete refs/heads/non-existent\" \"$head\" >stdin &&\n-\t\t\tgit update-ref $type --stdin --batch-updates stdout &&\n-\t\t\ttest_grep \"reference does not exist\" stdout\n+\t\t\tgit update-ref $type --stdin --batch-updates stdout 2>err &&\n+\t\t\ttest_grep \"rejected refs/heads/non-existent $ZERO_OID $head reference does not exist\" stdout &&\n+\t\t\ttest_grep \"cannot lock ref ${SQ}refs/heads/non-existent${SQ}: unable to resolve reference ${SQ}refs/heads/non-existent${SQ}\" err\n \t\t)\n \t'\n done\n\n-- \n2.52.0\n\n"},{"id":"534629","messageId":"20260125-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v5-4-d58f3a9edf98@gmail.com","threadId":"64802","inReplyTo":"20260125-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v5-0-d58f3a9edf98@gmail.com","subject":"[PATCH v5 4/6] fetch: utilize rejected ref error details","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-25T22:52:39Z","receivedAt":"2026-01-25T22:52:49Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"In 0e358de64a (fetch: use batched reference updates, 2025-05-19),\ngit-fetch(1) switched to using batched reference updates. This also\nintroduced a regression wherein instead of providing detailed error\nmessages for failed referenced updates, the users were provided generic\nerror messages based on the error type.\n\nSimilar to the previous commit, switch to using detailed error messages\nif present for failed reference updates to fix this regression.\n\nReported-by: Elijah Newren \nCo-authored-by: Jeff King \nSigned-off-by: Karthik Nayak \n---\n builtin/fetch.c | 10 ++++++----\n t/t5510-fetch.sh | 8 ++++----\n 2 files changed, 10 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex d427adea61..49495be0b6 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -1649,7 +1649,7 @@ static void ref_transaction_rejection_handler(const char *refname,\n \t\t\t\t\t const char *old_target UNUSED,\n \t\t\t\t\t const char *new_target UNUSED,\n \t\t\t\t\t enum ref_transaction_error err,\n-\t\t\t\t\t const char *details UNUSED,\n+\t\t\t\t\t const char *details,\n \t\t\t\t\t void *cb_data)\n {\n \tstruct ref_rejection_data *data = cb_data;\n@@ -1674,9 +1674,11 @@ static void ref_transaction_rejection_handler(const char *refname,\n \t\t\t\"branches\"), data->remote_name);\n \t\tdata->conflict_msg_shown = true;\n \t} else {\n-\t\tconst char *reason = ref_transaction_error_msg(err);\n-\n-\t\terror(_(\"fetching ref %s failed: %s\"), refname, reason);\n+\t\tif (details)\n+\t\t\terror(\"%s\", details);\n+\t\telse\n+\t\t\terror(_(\"fetching ref %s failed: %s\"),\n+\t\t\t refname, ref_transaction_error_msg(err));\n \t}\n \n \t*data->retcode = 1;\ndiff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\nindex ce1c23684e..c69afb5a60 100755\n--- a/t/t5510-fetch.sh\n+++ b/t/t5510-fetch.sh\n@@ -1516,7 +1516,7 @@ test_expect_success REFFILES 'existing reference lock in repo' '\n \t\tgit remote add origin ../base &&\n \t\ttouch refs/heads/foo.lock &&\n \t\ttest_must_fail git fetch -f origin \"refs/heads/*:refs/heads/*\" 2>err &&\n-\t\ttest_grep \"error: fetching ref refs/heads/foo failed: reference already exists\" err &&\n+\t\ttest_grep -e \"error: cannot lock ref ${SQ}refs/heads/foo${SQ}: Unable to create\" -e \"refs/heads/foo.lock${SQ}: File exists.\" err &&\n \t\tgit rev-parse refs/heads/main >expect &&\n \t\tgit rev-parse refs/heads/branch >actual &&\n \t\ttest_cmp expect actual\n@@ -1530,7 +1530,7 @@ test_expect_success CASE_INSENSITIVE_FS,REFFILES 'F/D conflict on case insensiti\n \t\tcd case_insensitive &&\n \t\tgit remote add origin -- ../case_sensitive_fd &&\n \t\ttest_must_fail git fetch -f origin \"refs/heads/*:refs/heads/*\" 2>err &&\n-\t\ttest_grep \"failed: refname conflict\" err &&\n+\t\ttest_grep \"cannot process ${SQ}refs/remotes/origin/foo${SQ} and ${SQ}refs/remotes/origin/foo/bar${SQ} at the same time\" err &&\n \t\tgit rev-parse refs/heads/main >expect &&\n \t\tgit rev-parse refs/heads/foo/bar >actual &&\n \t\ttest_cmp expect actual\n@@ -1544,7 +1544,7 @@ test_expect_success CASE_INSENSITIVE_FS,REFFILES 'D/F conflict on case insensiti\n \t\tcd case_insensitive &&\n \t\tgit remote add origin -- ../case_sensitive_df &&\n \t\ttest_must_fail git fetch -f origin \"refs/heads/*:refs/heads/*\" 2>err &&\n-\t\ttest_grep \"failed: refname conflict\" err &&\n+\t\ttest_grep \"cannot lock ref ${SQ}refs/remotes/origin/foo${SQ}: there is a non-empty directory ${SQ}./refs/remotes/origin/foo${SQ} blocking reference ${SQ}refs/remotes/origin/foo${SQ}\" err &&\n \t\tgit rev-parse refs/heads/main >expect &&\n \t\tgit rev-parse refs/heads/Foo/bar >actual &&\n \t\ttest_cmp expect actual\n@@ -1658,7 +1658,7 @@ test_expect_success REFFILES \"FETCH_HEAD is updated even if ref updates fail\" '\n \t\tgit remote add origin ../base &&\n \t\t>refs/heads/foo.lock &&\n \t\ttest_must_fail git fetch -f origin \"refs/heads/*:refs/heads/*\" 2>err &&\n-\t\ttest_grep \"error: fetching ref refs/heads/foo failed: reference already exists\" err &&\n+\t\ttest_grep -e \"error: cannot lock ref ${SQ}refs/heads/foo${SQ}: Unable to create\" -e \"refs/heads/foo.lock${SQ}: File exists.\" err &&\n \t\ttest_grep \"branch ${SQ}branch${SQ} of ../base\" FETCH_HEAD &&\n \t\ttest_grep \"branch ${SQ}foo${SQ} of ../base\" FETCH_HEAD\n \t)\n\n-- \n2.52.0\n\n"},{"id":"534630","messageId":"20260125-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v5-5-d58f3a9edf98@gmail.com","threadId":"64802","inReplyTo":"20260125-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v5-0-d58f3a9edf98@gmail.com","subject":"[PATCH v5 5/6] receive-pack: utilize rejected ref error details","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-25T22:52:40Z","receivedAt":"2026-01-25T22:52:51Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"In 9d2962a7c4 (receive-pack: use batched reference updates, 2025-05-19),\ngit-receive-pack(1) switched to using batched reference updates. This also\nintroduced a regression wherein instead of providing detailed error\nmessages for failed referenced updates, the users were provided generic\nerror messages based on the error type.\n\nNow that the updates also contain detailed error message, propagate\nthose to the client via 'rp_error'. The detailed error messages can be\nvery verbose, for e.g. in the files backend, when trying to write a\nnon-commit object to a branch, you would see:\n\n ! [remote rejected] 3eaec9ccf3a53f168362a6b3fdeb73426fb9813d ->\n branch (cannot update ref 'refs/heads/branch': trying to write\n non-commit object 3eaec9ccf3a53f168362a6b3fdeb73426fb9813d to branch\n 'refs/heads/branch')\n\nHere the refname is repeated multiple times due to how error messages\nare propagated and filled over the code stack. This potentially can be\ncleaned up in a future commit.\n\nReported-by: Elijah Newren \nCo-authored-by: Jeff King \nSigned-off-by: Jeff King \nSigned-off-by: Karthik Nayak \n---\n builtin/receive-pack.c | 8 ++++++--\n t/t5516-fetch-push.sh | 15 +++++++++++++++\n 2 files changed, 21 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 94d3e73cee..70e04b3efb 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -1813,11 +1813,14 @@ static void ref_transaction_rejection_handler(const char *refname,\n \t\t\t\t\t const char *old_target UNUSED,\n \t\t\t\t\t const char *new_target UNUSED,\n \t\t\t\t\t enum ref_transaction_error err,\n-\t\t\t\t\t const char *details UNUSED,\n+\t\t\t\t\t const char *details,\n \t\t\t\t\t void *cb_data)\n {\n \tstruct strmap *failed_refs = cb_data;\n \n+\tif (details)\n+\t\trp_error(\"%s\", details);\n+\n \tstrmap_put(failed_refs, refname, (char *)ref_transaction_error_msg(err));\n }\n \n@@ -1884,6 +1887,7 @@ static void execute_commands_non_atomic(struct command *commands,\n \t\t}\n \n \t\tref_transaction_for_each_rejected_update(transaction,\n+\n \t\t\t\t\t\t\t ref_transaction_rejection_handler,\n \t\t\t\t\t\t\t &failed_refs);\n \n@@ -1895,7 +1899,7 @@ static void execute_commands_non_atomic(struct command *commands,\n \t\t\tif (reported_error)\n \t\t\t\tcmd->error_string = reported_error;\n \t\t\telse if (strmap_contains(&failed_refs, cmd->ref_name))\n-\t\t\t\tcmd->error_string = strmap_get(&failed_refs, cmd->ref_name);\n+\t\t\t\tcmd->error_string = cmd->error_string_owned = xstrdup(strmap_get(&failed_refs, cmd->ref_name));\n \t\t}\n \n \tcleanup:\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 46926e7bbd..45595991c8 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -1882,4 +1882,19 @@ test_expect_success 'push with F/D conflict with deletion and creation' '\n \tgit push testrepo :refs/heads/branch/conflict refs/heads/branch\n '\n \n+test_expect_success 'pushing non-commit objects should report error' '\n+\ttest_when_finished \"rm -rf dest repo\" &&\n+\tgit init dest &&\n+\tgit init repo &&\n+\n+\t(\n+\t\tcd repo &&\n+\t\ttest_commit --annotate test &&\n+\n+\t\ttagsha=$(git rev-parse test^{tag}) &&\n+\t\ttest_must_fail git push ../dest \"$tagsha:refs/heads/branch\" 2>err &&\n+\t\ttest_grep \"trying to write non-commit object $tagsha to branch ${SQ}refs/heads/branch${SQ}\" err\n+\t)\n+'\n+\n test_done\n\n-- \n2.52.0\n\n"},{"id":"534631","messageId":"20260125-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v5-6-d58f3a9edf98@gmail.com","threadId":"64802","inReplyTo":"20260125-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v5-0-d58f3a9edf98@gmail.com","subject":"[PATCH v5 6/6] fetch: delay user information post committing of transaction","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-25T22:52:41Z","receivedAt":"2026-01-25T22:52:52Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"In Git 2.50 and earlier, we would display failure codes and error\nmessage as part of the status display:\n\n $ git fetch . v1.0.0:refs/heads/foo\n error: cannot update ref 'refs/heads/foo': trying to write non-commit object f665776185ad074b236c00751d666da7d1977dbe to branch 'refs/heads/foo'\n From .\n ! [new tag] v1.0.0 -> foo (unable to update local ref)\n\nWith the addition of batched updates, this information is no longer\nshown to the user:\n\n $ git fetch . v1.0.0:refs/heads/foo\n From .\n * [new tag] v1.0.0 -> foo\n error: cannot update ref 'refs/heads/foo': trying to write non-commit object f665776185ad074b236c00751d666da7d1977dbe to branch 'refs/heads/foo'\n\nSince reference updates are batched and processed together at the end,\ninformation around the outcome is not available during individual\nreference parsing.\n\nTo overcome this, collate and delay the output to the end. Introduce\n`ref_update_display_info` which will hold individual update's\ninformation and also whether the update failed or succeeded. This\nfinally allows us to iterate over all such updates and print them to the\nuser.\n\nUsing an dynamic array and strmap does add some overhead to\n'git-fetch(1)', but from benchmarking this seems to be not too bad:\n\n Benchmark 1: fetch: many refs (refformat = files, refcount = 1000, revision = master)\n Time (mean ± σ): 42.6 ms ± 1.2 ms [User: 13.1 ms, System: 29.8 ms]\n Range (min … max): 40.1 ms … 45.8 ms 47 runs\n\n Benchmark 2: fetch: many refs (refformat = files, refcount = 1000, revision = HEAD)\n Time (mean ± σ): 43.1 ms ± 1.2 ms [User: 12.7 ms, System: 30.7 ms]\n Range (min … max): 40.5 ms … 45.8 ms 48 runs\n\n Summary\n fetch: many refs (refformat = files, refcount = 1000, revision = master) ran\n 1.01 ± 0.04 times faster than fetch: many refs (refformat = files, refcount = 1000, revision = HEAD)\n\nAnother approach would be to move the status printing logic to be\nhandled post the transaction being committed. That however would require\nadding an iterator to the ref transaction that tracks both the outcome\n(success/failure) and the original refspec information for each update,\nwhich is more involved infrastructure work compared to the strmap\napproach here.\n\nHelped-by: Phillip Wood \nReported-by: Jeff King \nSigned-off-by: Karthik Nayak \n---\n builtin/fetch.c | 246 +++++++++++++++++++++++++++++++++++++++-----------\n t/t5516-fetch-push.sh | 1 +\n 2 files changed, 193 insertions(+), 54 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 49495be0b6..a3bc7e9380 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -861,12 +861,87 @@ static void display_ref_update(struct display_state *display_state, char code,\n \tfputs(display_state->buf.buf, f);\n }\n \n+struct ref_update_display_info {\n+\tbool failed;\n+\tchar success_code;\n+\tchar fail_code;\n+\tchar *summary;\n+\tchar *fail_detail;\n+\tchar *success_detail;\n+\tchar *ref;\n+\tchar *remote;\n+\tstruct object_id old_oid;\n+\tstruct object_id new_oid;\n+};\n+\n+struct ref_update_display_info_array {\n+\tstruct ref_update_display_info *info;\n+\tsize_t alloc, nr;\n+};\n+\n+static struct ref_update_display_info *ref_update_display_info_append(\n+\t\t\t\t\t struct ref_update_display_info_array *array,\n+\t\t\t\t\t char success_code,\n+\t\t\t\t\t char fail_code,\n+\t\t\t\t\t const char *summary,\n+\t\t\t\t\t const char *success_detail,\n+\t\t\t\t\t const char *fail_detail,\n+\t\t\t\t\t const char *ref,\n+\t\t\t\t\t const char *remote,\n+\t\t\t\t\t const struct object_id *old_oid,\n+\t\t\t\t\t const struct object_id *new_oid)\n+{\n+\tstruct ref_update_display_info *info;\n+\n+\tALLOC_GROW(array->info, array->nr + 1, array->alloc);\n+\tinfo = &array->info[array->nr++];\n+\n+\tinfo->failed = false;\n+\tinfo->success_code = success_code;\n+\tinfo->fail_code = fail_code;\n+\tinfo->summary = xstrdup(summary);\n+\tinfo->success_detail = xstrdup_or_null(success_detail);\n+\tinfo->fail_detail = xstrdup_or_null(fail_detail);\n+\tinfo->remote = xstrdup(remote);\n+\tinfo->ref = xstrdup(ref);\n+\n+\toidcpy(&info->old_oid, old_oid);\n+\toidcpy(&info->new_oid, new_oid);\n+\n+\treturn info;\n+}\n+\n+static void ref_update_display_info_set_failed(struct ref_update_display_info *info)\n+{\n+\tinfo->failed = true;\n+}\n+\n+static void ref_update_display_info_free(struct ref_update_display_info *info)\n+{\n+\tfree(info->summary);\n+\tfree(info->success_detail);\n+\tfree(info->fail_detail);\n+\tfree(info->remote);\n+\tfree(info->ref);\n+}\n+\n+static void ref_update_display_info_display(struct ref_update_display_info *info,\n+\t\t\t\t\t struct display_state *display_state,\n+\t\t\t\t\t int summary_width)\n+{\n+\tdisplay_ref_update(display_state,\n+\t\t\t info->failed ? info->fail_code : info->success_code,\n+\t\t\t info->summary,\n+\t\t\t info->failed ? info->fail_detail : info->success_detail,\n+\t\t\t info->remote, info->ref, &info->old_oid,\n+\t\t\t &info->new_oid, summary_width);\n+}\n+\n static int update_local_ref(struct ref *ref,\n \t\t\t struct ref_transaction *transaction,\n-\t\t\t struct display_state *display_state,\n \t\t\t const struct ref *remote_ref,\n-\t\t\t int summary_width,\n-\t\t\t const struct fetch_config *config)\n+\t\t\t const struct fetch_config *config,\n+\t\t\t struct ref_update_display_info_array *display_array)\n {\n \tstruct commit *current = NULL, *updated;\n \tint fast_forward = 0;\n@@ -877,41 +952,56 @@ static int update_local_ref(struct ref *ref,\n \n \tif (oideq(&ref->old_oid, &ref->new_oid)) {\n \t\tif (verbosity > 0)\n-\t\t\tdisplay_ref_update(display_state, '=', _(\"[up to date]\"), NULL,\n-\t\t\t\t\t remote_ref->name, ref->name,\n-\t\t\t\t\t &ref->old_oid, &ref->new_oid, summary_width);\n+\t\t\tref_update_display_info_append(display_array, '=', '=',\n+\t\t\t\t\t\t _(\"[up to date]\"), NULL,\n+\t\t\t\t\t\t NULL, ref->name,\n+\t\t\t\t\t\t remote_ref->name, &ref->old_oid,\n+\t\t\t\t\t\t &ref->new_oid);\n \t\treturn 0;\n \t}\n \n \tif (!update_head_ok &&\n \t !is_null_oid(&ref->old_oid) &&\n \t branch_checked_out(ref->name)) {\n+\t\tstruct ref_update_display_info *info;\n \t\t/*\n \t\t * If this is the head, and it's not okay to update\n \t\t * the head, and the old value of the head isn't empty...\n \t\t */\n-\t\tdisplay_ref_update(display_state, '!', _(\"[rejected]\"),\n-\t\t\t\t _(\"can't fetch into checked-out branch\"),\n-\t\t\t\t remote_ref->name, ref->name,\n-\t\t\t\t &ref->old_oid, &ref->new_oid, summary_width);\n+\t\tinfo = ref_update_display_info_append(display_array, '!', '!',\n+\t\t\t\t\t\t _(\"[rejected]\"), NULL,\n+\t\t\t\t\t\t _(\"can't fetch into checked-out branch\"),\n+\t\t\t\t\t\t ref->name, remote_ref->name,\n+\t\t\t\t\t\t &ref->old_oid, &ref->new_oid);\n+\t\tref_update_display_info_set_failed(info);\n \t\treturn 1;\n \t}\n \n \tif (!is_null_oid(&ref->old_oid) &&\n \t starts_with(ref->name, \"refs/tags/\")) {\n+\t\tstruct ref_update_display_info *info;\n+\n \t\tif (force || ref->force) {\n \t\t\tint r;\n+\n \t\t\tr = s_update_ref(\"updating tag\", ref, transaction, 0);\n-\t\t\tdisplay_ref_update(display_state, r ? '!' : 't', _(\"[tag update]\"),\n-\t\t\t\t\t r ? _(\"unable to update local ref\") : NULL,\n-\t\t\t\t\t remote_ref->name, ref->name,\n-\t\t\t\t\t &ref->old_oid, &ref->new_oid, summary_width);\n+\n+\t\t\tinfo = ref_update_display_info_append(display_array, 't', '!',\n+\t\t\t\t\t\t\t _(\"[tag update]\"), NULL,\n+\t\t\t\t\t\t\t _(\"unable to update local ref\"),\n+\t\t\t\t\t\t\t ref->name, remote_ref->name,\n+\t\t\t\t\t\t\t &ref->old_oid, &ref->new_oid);\n+\t\t\tif (r)\n+\t\t\t\tref_update_display_info_set_failed(info);\n+\n \t\t\treturn r;\n \t\t} else {\n-\t\t\tdisplay_ref_update(display_state, '!', _(\"[rejected]\"),\n-\t\t\t\t\t _(\"would clobber existing tag\"),\n-\t\t\t\t\t remote_ref->name, ref->name,\n-\t\t\t\t\t &ref->old_oid, &ref->new_oid, summary_width);\n+\t\t\tinfo = ref_update_display_info_append(display_array, '!', '!',\n+\t\t\t\t\t\t\t _(\"[rejected]\"), NULL,\n+\t\t\t\t\t\t\t _(\"would clobber existing tag\"),\n+\t\t\t\t\t\t\t ref->name, remote_ref->name,\n+\t\t\t\t\t\t\t &ref->old_oid, &ref->new_oid);\n+\t\t\tref_update_display_info_set_failed(info);\n \t\t\treturn 1;\n \t\t}\n \t}\n@@ -921,6 +1011,7 @@ static int update_local_ref(struct ref *ref,\n \tupdated = lookup_commit_reference_gently(the_repository,\n \t\t\t\t\t\t &ref->new_oid, 1);\n \tif (!current || !updated) {\n+\t\tstruct ref_update_display_info *info;\n \t\tconst char *msg;\n \t\tconst char *what;\n \t\tint r;\n@@ -941,10 +1032,15 @@ static int update_local_ref(struct ref *ref,\n \t\t}\n \n \t\tr = s_update_ref(msg, ref, transaction, 0);\n-\t\tdisplay_ref_update(display_state, r ? '!' : '*', what,\n-\t\t\t\t r ? _(\"unable to update local ref\") : NULL,\n-\t\t\t\t remote_ref->name, ref->name,\n-\t\t\t\t &ref->old_oid, &ref->new_oid, summary_width);\n+\n+\t\tinfo = ref_update_display_info_append(display_array, '*', '!',\n+\t\t\t\t\t\t what, NULL,\n+\t\t\t\t\t\t _(\"unable to update local ref\"),\n+\t\t\t\t\t\t ref->name, remote_ref->name,\n+\t\t\t\t\t\t &ref->old_oid, &ref->new_oid);\n+\t\tif (r)\n+\t\t\tref_update_display_info_set_failed(info);\n+\n \t\treturn r;\n \t}\n \n@@ -960,6 +1056,7 @@ static int update_local_ref(struct ref *ref,\n \t}\n \n \tif (fast_forward) {\n+\t\tstruct ref_update_display_info *info;\n \t\tstruct strbuf quickref = STRBUF_INIT;\n \t\tint r;\n \n@@ -967,29 +1064,46 @@ static int update_local_ref(struct ref *ref,\n \t\tstrbuf_addstr(&quickref, \"..\");\n \t\tstrbuf_add_unique_abbrev(&quickref, &ref->new_oid, DEFAULT_ABBREV);\n \t\tr = s_update_ref(\"fast-forward\", ref, transaction, 1);\n-\t\tdisplay_ref_update(display_state, r ? '!' : ' ', quickref.buf,\n-\t\t\t\t r ? _(\"unable to update local ref\") : NULL,\n-\t\t\t\t remote_ref->name, ref->name,\n-\t\t\t\t &ref->old_oid, &ref->new_oid, summary_width);\n+\n+\t\tinfo = ref_update_display_info_append(display_array, ' ', '!',\n+\t\t\t\t\t\t quickref.buf, NULL,\n+\t\t\t\t\t\t _(\"unable to update local ref\"),\n+\t\t\t\t\t\t ref->name, remote_ref->name,\n+\t\t\t\t\t\t &ref->old_oid, &ref->new_oid);\n+\t\tif (r)\n+\t\t\tref_update_display_info_set_failed(info);\n+\n \t\tstrbuf_release(&quickref);\n \t\treturn r;\n \t} else if (force || ref->force) {\n+\t\tstruct ref_update_display_info *info;\n \t\tstruct strbuf quickref = STRBUF_INIT;\n \t\tint r;\n+\n \t\tstrbuf_add_unique_abbrev(&quickref, ¤t->object.oid, DEFAULT_ABBREV);\n \t\tstrbuf_addstr(&quickref, \"...\");\n \t\tstrbuf_add_unique_abbrev(&quickref, &ref->new_oid, DEFAULT_ABBREV);\n \t\tr = s_update_ref(\"forced-update\", ref, transaction, 1);\n-\t\tdisplay_ref_update(display_state, r ? '!' : '+', quickref.buf,\n-\t\t\t\t r ? _(\"unable to update local ref\") : _(\"forced update\"),\n-\t\t\t\t remote_ref->name, ref->name,\n-\t\t\t\t &ref->old_oid, &ref->new_oid, summary_width);\n+\n+\t\tinfo = ref_update_display_info_append(display_array, '+', '!',\n+\t\t\t\t\t\t quickref.buf, _(\"forced update\"),\n+\t\t\t\t\t\t _(\"unable to update local ref\"),\n+\t\t\t\t\t\t ref->name, remote_ref->name,\n+\t\t\t\t\t\t &ref->old_oid, &ref->new_oid);\n+\n+\t\tif (r)\n+\t\t\tref_update_display_info_set_failed(info);\n+\n \t\tstrbuf_release(&quickref);\n \t\treturn r;\n \t} else {\n-\t\tdisplay_ref_update(display_state, '!', _(\"[rejected]\"), _(\"non-fast-forward\"),\n-\t\t\t\t remote_ref->name, ref->name,\n-\t\t\t\t &ref->old_oid, &ref->new_oid, summary_width);\n+\t\tstruct ref_update_display_info *info;\n+\t\tinfo = ref_update_display_info_append(display_array, '!', '!',\n+\t\t\t\t\t\t _(\"[rejected]\"), NULL,\n+\t\t\t\t\t\t _(\"non-fast-forward\"),\n+\t\t\t\t\t\t ref->name, remote_ref->name,\n+\t\t\t\t\t\t &ref->old_oid, &ref->new_oid);\n+\t\tref_update_display_info_set_failed(info);\n \t\treturn 1;\n \t}\n }\n@@ -1103,17 +1217,14 @@ static int store_updated_refs(struct display_state *display_state,\n \t\t\t int connectivity_checked,\n \t\t\t struct ref_transaction *transaction, struct ref *ref_map,\n \t\t\t struct fetch_head *fetch_head,\n-\t\t\t const struct fetch_config *config)\n+\t\t\t const struct fetch_config *config,\n+\t\t\t struct ref_update_display_info_array *display_array)\n {\n \tint rc = 0;\n \tstruct strbuf note = STRBUF_INIT;\n \tconst char *what, *kind;\n \tstruct ref *rm;\n \tint want_status;\n-\tint summary_width = 0;\n-\n-\tif (verbosity >= 0)\n-\t\tsummary_width = transport_summary_width(ref_map);\n \n \tif (!connectivity_checked) {\n \t\tstruct check_connected_options opt = CHECK_CONNECTED_INIT;\n@@ -1218,8 +1329,8 @@ static int store_updated_refs(struct display_state *display_state,\n \t\t\t\t\t display_state->url_len);\n \n \t\t\tif (ref) {\n-\t\t\t\trc |= update_local_ref(ref, transaction, display_state,\n-\t\t\t\t\t\t rm, summary_width, config);\n+\t\t\t\trc |= update_local_ref(ref, transaction, rm,\n+\t\t\t\t\t\t config, display_array);\n \t\t\t\tfree(ref);\n \t\t\t} else if (write_fetch_head || dry_run) {\n \t\t\t\t/*\n@@ -1227,12 +1338,12 @@ static int store_updated_refs(struct display_state *display_state,\n \t\t\t\t * would be written to FETCH_HEAD, if --dry-run\n \t\t\t\t * is set).\n \t\t\t\t */\n-\t\t\t\tdisplay_ref_update(display_state, '*',\n-\t\t\t\t\t\t *kind ? kind : \"branch\", NULL,\n-\t\t\t\t\t\t rm->name,\n-\t\t\t\t\t\t \"FETCH_HEAD\",\n-\t\t\t\t\t\t &rm->new_oid, &rm->old_oid,\n-\t\t\t\t\t\t summary_width);\n+\n+\t\t\t\tref_update_display_info_append(display_array, '*', '*',\n+\t\t\t\t\t\t\t *kind ? kind : \"branch\",\n+\t\t\t\t\t\t\t NULL, NULL, \"FETCH_HEAD\",\n+\t\t\t\t\t\t\t rm->name, &rm->new_oid,\n+\t\t\t\t\t\t\t &rm->old_oid);\n \t\t\t}\n \t\t}\n \t}\n@@ -1300,7 +1411,8 @@ static int fetch_and_consume_refs(struct display_state *display_state,\n \t\t\t\t struct ref_transaction *transaction,\n \t\t\t\t struct ref *ref_map,\n \t\t\t\t struct fetch_head *fetch_head,\n-\t\t\t\t const struct fetch_config *config)\n+\t\t\t\t const struct fetch_config *config,\n+\t\t\t\t struct ref_update_display_info_array *display_array)\n {\n \tint connectivity_checked = 1;\n \tint ret;\n@@ -1322,7 +1434,8 @@ static int fetch_and_consume_refs(struct display_state *display_state,\n \n \ttrace2_region_enter(\"fetch\", \"consume_refs\", the_repository);\n \tret = store_updated_refs(display_state, connectivity_checked,\n-\t\t\t\t transaction, ref_map, fetch_head, config);\n+\t\t\t\t transaction, ref_map, fetch_head, config,\n+\t\t\t\t display_array);\n \ttrace2_region_leave(\"fetch\", \"consume_refs\", the_repository);\n \n out:\n@@ -1493,7 +1606,8 @@ static int backfill_tags(struct display_state *display_state,\n \t\t\t struct ref_transaction *transaction,\n \t\t\t struct ref *ref_map,\n \t\t\t struct fetch_head *fetch_head,\n-\t\t\t const struct fetch_config *config)\n+\t\t\t const struct fetch_config *config,\n+\t\t\t struct ref_update_display_info_array *display_array)\n {\n \tint retcode, cannot_reuse;\n \n@@ -1515,7 +1629,7 @@ static int backfill_tags(struct display_state *display_state,\n \ttransport_set_option(transport, TRANS_OPT_DEPTH, \"0\");\n \ttransport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL);\n \tretcode = fetch_and_consume_refs(display_state, transport, transaction, ref_map,\n-\t\t\t\t\t fetch_head, config);\n+\t\t\t\t\t fetch_head, config, display_array);\n \n \tif (gsecondary) {\n \t\ttransport_disconnect(gsecondary);\n@@ -1641,6 +1755,7 @@ struct ref_rejection_data {\n \tbool conflict_msg_shown;\n \tbool case_sensitive_msg_shown;\n \tconst char *remote_name;\n+\tstruct strmap *rejected_refs;\n };\n \n static void ref_transaction_rejection_handler(const char *refname,\n@@ -1681,6 +1796,7 @@ static void ref_transaction_rejection_handler(const char *refname,\n \t\t\t refname, ref_transaction_error_msg(err));\n \t}\n \n+\tstrmap_put(data->rejected_refs, refname, NULL);\n \t*data->retcode = 1;\n }\n \n@@ -1690,6 +1806,7 @@ static void ref_transaction_rejection_handler(const char *refname,\n */\n static int commit_ref_transaction(struct ref_transaction **transaction,\n \t\t\t\t bool is_atomic, const char *remote_name,\n+\t\t\t\t struct strmap *rejected_refs,\n \t\t\t\t struct strbuf *err)\n {\n \tint retcode = ref_transaction_commit(*transaction, err);\n@@ -1701,6 +1818,7 @@ static int commit_ref_transaction(struct ref_transaction **transaction,\n \t\t\t.conflict_msg_shown = 0,\n \t\t\t.remote_name = remote_name,\n \t\t\t.retcode = &retcode,\n+\t\t\t.rejected_refs = rejected_refs,\n \t\t};\n \n \t\tref_transaction_for_each_rejected_update(*transaction,\n@@ -1729,6 +1847,9 @@ static int do_fetch(struct transport *transport,\n \tstruct fetch_head fetch_head = { 0 };\n \tstruct strbuf err = STRBUF_INIT;\n \tint do_set_head = 0;\n+\tstruct ref_update_display_info_array display_array = { 0 };\n+\tstruct strmap rejected_refs = STRMAP_INIT;\n+\tint summary_width = 0;\n \n \tif (tags == TAGS_DEFAULT) {\n \t\tif (transport->remote->fetch_tags == 2)\n@@ -1853,7 +1974,7 @@ static int do_fetch(struct transport *transport,\n \t}\n \n \tif (fetch_and_consume_refs(&display_state, transport, transaction, ref_map,\n-\t\t\t\t &fetch_head, config)) {\n+\t\t\t\t &fetch_head, config, &display_array)) {\n \t\tretcode = 1;\n \t\tgoto cleanup;\n \t}\n@@ -1876,7 +1997,7 @@ static int do_fetch(struct transport *transport,\n \t\t\t * the transaction and don't commit anything.\n \t\t\t */\n \t\t\tif (backfill_tags(&display_state, transport, transaction, tags_ref_map,\n-\t\t\t\t\t &fetch_head, config))\n+\t\t\t\t\t &fetch_head, config, &display_array))\n \t\t\t\tretcode = 1;\n \t\t}\n \n@@ -1886,8 +2007,12 @@ static int do_fetch(struct transport *transport,\n \tif (retcode)\n \t\tgoto cleanup;\n \n+\tif (verbosity >= 0)\n+\t\tsummary_width = transport_summary_width(ref_map);\n+\n \tretcode = commit_ref_transaction(&transaction, atomic_fetch,\n-\t\t\t\t\t transport->remote->name, &err);\n+\t\t\t\t\t transport->remote->name,\n+\t\t\t\t\t &rejected_refs, &err);\n \t/*\n \t * With '--atomic', bail out if the transaction fails. Without '--atomic',\n \t * continue to fetch head and perform other post-fetch operations.\n@@ -1965,7 +2090,17 @@ static int do_fetch(struct transport *transport,\n \t */\n \tif (retcode && !atomic_fetch && transaction)\n \t\tcommit_ref_transaction(&transaction, false,\n-\t\t\t\t transport->remote->name, &err);\n+\t\t\t\t transport->remote->name,\n+\t\t\t\t &rejected_refs, &err);\n+\n+\tfor (size_t i = 0; i < display_array.nr; i++) {\n+\t\tstruct ref_update_display_info *info = &display_array.info[i];\n+\n+\t\tif (!info->failed && strmap_contains(&rejected_refs, info->ref))\n+\t\t\tref_update_display_info_set_failed(info);\n+\t\tref_update_display_info_display(info, &display_state, summary_width);\n+\t\tref_update_display_info_free(info);\n+\t}\n \n \tif (retcode) {\n \t\tif (err.len) {\n@@ -1980,6 +2115,9 @@ static int do_fetch(struct transport *transport,\n \n \tif (transaction)\n \t\tref_transaction_free(transaction);\n+\n+\tfree(display_array.info);\n+\tstrmap_clear(&rejected_refs, 0);\n \tdisplay_state_release(&display_state);\n \tclose_fetch_head(&fetch_head);\n \tstrbuf_release(&err);\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 45595991c8..29e2f17608 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -1893,6 +1893,7 @@ test_expect_success 'pushing non-commit objects should report error' '\n \n \t\ttagsha=$(git rev-parse test^{tag}) &&\n \t\ttest_must_fail git push ../dest \"$tagsha:refs/heads/branch\" 2>err &&\n+\t\ttest_grep \"! \\[remote rejected\\] $tagsha -> branch (invalid new value provided)\" err &&\n \t\ttest_grep \"trying to write non-commit object $tagsha to branch ${SQ}refs/heads/branch${SQ}\" err\n \t)\n '\n\n-- \n2.52.0\n\n"}]}