From: Karthik Nayak Date: Wed, 14 Jan 2026 15:40:43 GMT Subject: [PATCH 2/6] refs: attach rejection details to updates Message-ID: <20260114-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v1-2-f5f8b173c501@gmail.com> In-Reply-To: <20260114-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v1-0-f5f8b173c501@gmail.com> The implementation of batched updates in 23fc8e4f61 (refs: implement batch reference update support, 2025-04-08) added rejection error codes to each reference update. This allowed batching of updates, however while each rejection is linked to a rejection code, the already present user readable error message is simply dropped. Make necessary changes to ensure that the rejection detail is also added to the reference update. In upcoming commits, we'll utilize this field to provide better error message to users, namely in: - git update-ref --batch-updates - git fetch - git receive-pack We move the error message creation right above `ref_transaction_maybe_set_rejected()`, so that the error message is available and also reset the error message if utilized to avoid un-expected concatination. Co-authored-by: Jeff King Signed-off-by: Karthik Nayak --- refs.c | 52 ++++++++++++++++++++++++++++++++----------------- refs/files-backend.c | 3 ++- refs/packed-backend.c | 9 ++++++--- refs/refs-internal.h | 4 +++- refs/reftable-backend.c | 3 ++- 5 files changed, 47 insertions(+), 24 deletions(-) diff --git a/refs.c b/refs.c index 965b232a06..991bd8e6ee 100644 --- a/refs.c +++ b/refs.c @@ -1222,6 +1222,7 @@ void ref_transaction_free(struct ref_transaction *transaction) free(transaction->updates[i]->committer_info); free((char *)transaction->updates[i]->new_target); free((char *)transaction->updates[i]->old_target); + free((char *)transaction->updates[i]->rejection_details); free(transaction->updates[i]); } @@ -1236,7 +1237,8 @@ void ref_transaction_free(struct ref_transaction *transaction) int ref_transaction_maybe_set_rejected(struct ref_transaction *transaction, size_t update_idx, - enum ref_transaction_error err) + enum ref_transaction_error err, + const char *details) { if (update_idx >= transaction->nr) BUG("trying to set rejection on invalid update index"); @@ -1262,6 +1264,8 @@ int ref_transaction_maybe_set_rejected(struct ref_transaction *transaction, transaction->updates[update_idx]->refname, 0); transaction->updates[update_idx]->rejection_err = err; + if (details) + transaction->updates[update_idx]->rejection_details = xstrdup(details); ALLOC_GROW(transaction->rejections->update_indices, transaction->rejections->nr + 1, transaction->rejections->alloc); @@ -2657,30 +2661,35 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs if (!initial_transaction && (strset_contains(&conflicting_dirnames, dirname.buf) || !refs_read_raw_ref(refs, dirname.buf, &oid, &referent, - &type, &ignore_errno))) { + &type, &ignore_errno))) { + + strbuf_addf(err, _("'%s' exists; cannot create '%s'"), + dirname.buf, refname); + if (transaction && ref_transaction_maybe_set_rejected( transaction, *update_idx, - REF_TRANSACTION_ERROR_NAME_CONFLICT)) { + REF_TRANSACTION_ERROR_NAME_CONFLICT, err->buf)) { strset_remove(&dirnames, dirname.buf); strset_add(&conflicting_dirnames, dirname.buf); - continue; + strbuf_reset(err); + goto next; } - strbuf_addf(err, _("'%s' exists; cannot create '%s'"), - dirname.buf, refname); goto cleanup; } if (extras && string_list_has_string(extras, dirname.buf)) { + strbuf_addf(err, _("cannot process '%s' and '%s' at the same time"), + refname, dirname.buf); + if (transaction && ref_transaction_maybe_set_rejected( transaction, *update_idx, - REF_TRANSACTION_ERROR_NAME_CONFLICT)) { + REF_TRANSACTION_ERROR_NAME_CONFLICT, err->buf)) { strset_remove(&dirnames, dirname.buf); - continue; + strbuf_reset(err); + goto next; } - strbuf_addf(err, _("cannot process '%s' and '%s' at the same time"), - refname, dirname.buf); goto cleanup; } } @@ -2711,13 +2720,16 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs string_list_has_string(skip, iter->ref.name)) continue; + strbuf_addf(err, _("'%s' exists; cannot create '%s'"), + iter->ref.name, refname); + if (transaction && ref_transaction_maybe_set_rejected( transaction, *update_idx, - REF_TRANSACTION_ERROR_NAME_CONFLICT)) - continue; + REF_TRANSACTION_ERROR_NAME_CONFLICT, err->buf)) { + strbuf_reset(err); + goto next; + } - strbuf_addf(err, _("'%s' exists; cannot create '%s'"), - iter->ref.name, refname); goto cleanup; } @@ -2727,15 +2739,19 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs extra_refname = find_descendant_ref(dirname.buf, extras, skip); if (extra_refname) { + strbuf_addf(err, _("cannot process '%s' and '%s' at the same time"), + refname, extra_refname); + if (transaction && ref_transaction_maybe_set_rejected( transaction, *update_idx, - REF_TRANSACTION_ERROR_NAME_CONFLICT)) - continue; + REF_TRANSACTION_ERROR_NAME_CONFLICT, err->buf)) { + strbuf_reset(err); + goto next; + } - strbuf_addf(err, _("cannot process '%s' and '%s' at the same time"), - refname, extra_refname); goto cleanup; } +next:; } ret = 0; diff --git a/refs/files-backend.c b/refs/files-backend.c index 6f6f76a8d8..8d22a2e8e3 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -2983,7 +2983,8 @@ static int files_transaction_prepare(struct ref_store *ref_store, head_ref, &refnames_to_check, err); if (ret) { - if (ref_transaction_maybe_set_rejected(transaction, i, ret)) { + if (ref_transaction_maybe_set_rejected(transaction, i, + ret, err->buf)) { strbuf_reset(err); ret = 0; diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 4ea0c12299..535200db01 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1437,7 +1437,8 @@ static enum ref_transaction_error write_with_updates(struct packed_ref_store *re update->refname); ret = REF_TRANSACTION_ERROR_CREATE_EXISTS; - if (ref_transaction_maybe_set_rejected(transaction, i, ret)) { + if (ref_transaction_maybe_set_rejected(transaction, i, + ret, err->buf)) { strbuf_reset(err); ret = 0; continue; @@ -1452,7 +1453,8 @@ static enum ref_transaction_error write_with_updates(struct packed_ref_store *re oid_to_hex(&update->old_oid)); ret = REF_TRANSACTION_ERROR_INCORRECT_OLD_VALUE; - if (ref_transaction_maybe_set_rejected(transaction, i, ret)) { + if (ref_transaction_maybe_set_rejected(transaction, i, + ret, err->buf)) { strbuf_reset(err); ret = 0; continue; @@ -1496,7 +1498,8 @@ static enum ref_transaction_error write_with_updates(struct packed_ref_store *re oid_to_hex(&update->old_oid)); ret = REF_TRANSACTION_ERROR_NONEXISTENT_REF; - if (ref_transaction_maybe_set_rejected(transaction, i, ret)) { + if (ref_transaction_maybe_set_rejected(transaction, i, + ret, err->buf)) { strbuf_reset(err); ret = 0; continue; diff --git a/refs/refs-internal.h b/refs/refs-internal.h index c7d2a6e50b..60d9f015cf 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -128,6 +128,7 @@ struct ref_update { * was rejected. */ enum ref_transaction_error rejection_err; + const char *rejection_details; /* * If this ref_update was split off of a symref update via @@ -153,7 +154,8 @@ int refs_read_raw_ref(struct ref_store *ref_store, const char *refname, */ int ref_transaction_maybe_set_rejected(struct ref_transaction *transaction, size_t update_idx, - enum ref_transaction_error err); + enum ref_transaction_error err, + const char *details); /* * Add a ref_update with the specified properties to transaction, and diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index 4319a4eacb..a9c9ceebf3 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -1401,7 +1401,8 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store, &refnames_to_check, head_type, &head_referent, &referent, err); if (ret) { - if (ref_transaction_maybe_set_rejected(transaction, i, ret)) { + if (ref_transaction_maybe_set_rejected(transaction, i, + ret, err->buf)) { strbuf_reset(err); ret = 0; -- 2.51.2