refs: skip to next ref when current ref is rejected

In `refs_verify_refnames_available()` we have two nested loops: the
outer loop iterates over all references to check, while the inner loop
checks for filesystem conflicts for a given ref by breaking down its
path.

With batched updates, when we detect a filesystem conflict, we mark the
update as rejected and execute 'continue'. However, this only skips to
the next iteration of the inner loop, not the outer loop as intended.
This causes the same reference to be repeatedly rejected. Fix this by
using a goto statement to skip to the next reference in the outer loop.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
Karthik Nayak 2026-01-16 22:27:07 +01:00 committed by Junio C Hamano
parent 5971ea2368
commit 3f5f409159
5 changed files with 46 additions and 38 deletions

58
refs.c
View File

@ -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,
struct strbuf *details)
{
if (update_idx >= transaction->nr)
BUG("trying to set rejection on invalid update index");
@ -1262,6 +1264,7 @@ int ref_transaction_maybe_set_rejected(struct ref_transaction *transaction,
transaction->updates[update_idx]->refname, 0);
transaction->updates[update_idx]->rejection_err = err;
transaction->updates[update_idx]->rejection_details = strbuf_detach(details, NULL);
ALLOC_GROW(transaction->rejections->update_indices,
transaction->rejections->nr + 1,
transaction->rejections->alloc);
@ -2657,30 +2660,33 @@ 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))) {
if (transaction && ref_transaction_maybe_set_rejected(
transaction, *update_idx,
REF_TRANSACTION_ERROR_NAME_CONFLICT)) {
strset_remove(&dirnames, dirname.buf);
strset_add(&conflicting_dirnames, dirname.buf);
continue;
}
&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, err)) {
strset_remove(&dirnames, dirname.buf);
strset_add(&conflicting_dirnames, dirname.buf);
goto next_ref;
}
goto cleanup;
}
if (extras && string_list_has_string(extras, dirname.buf)) {
if (transaction && ref_transaction_maybe_set_rejected(
transaction, *update_idx,
REF_TRANSACTION_ERROR_NAME_CONFLICT)) {
strset_remove(&dirnames, dirname.buf);
continue;
}
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, err)) {
strset_remove(&dirnames, dirname.buf);
goto next_ref;
}
goto cleanup;
}
}
@ -2710,14 +2716,14 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs
if (skip &&
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))
goto next_ref;
strbuf_addf(err, _("'%s' exists; cannot create '%s'"),
iter->ref.name, refname);
goto cleanup;
}
@ -2727,15 +2733,17 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs
extra_refname = find_descendant_ref(dirname.buf, extras, skip);
if (extra_refname) {
if (transaction && ref_transaction_maybe_set_rejected(
transaction, *update_idx,
REF_TRANSACTION_ERROR_NAME_CONFLICT))
continue;
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, err))
goto next_ref;
goto cleanup;
}
next_ref:;
}
ret = 0;

View File

@ -2983,10 +2983,9 @@ 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)) {
strbuf_reset(err);
if (ref_transaction_maybe_set_rejected(transaction, i,
ret, err)) {
ret = 0;
continue;
}
goto cleanup;

View File

@ -1437,8 +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)) {
strbuf_reset(err);
if (ref_transaction_maybe_set_rejected(transaction, i,
ret, err)) {
ret = 0;
continue;
}
@ -1452,8 +1452,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)) {
strbuf_reset(err);
if (ref_transaction_maybe_set_rejected(transaction, i,
ret, err)) {
ret = 0;
continue;
}
@ -1496,8 +1496,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)) {
strbuf_reset(err);
if (ref_transaction_maybe_set_rejected(transaction, i,
ret, err)) {
ret = 0;
continue;
}

View File

@ -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,
struct strbuf *details);
/*
* Add a ref_update with the specified properties to transaction, and

View File

@ -1401,10 +1401,9 @@ 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)) {
strbuf_reset(err);
if (ref_transaction_maybe_set_rejected(transaction, i,
ret, err)) {
ret = 0;
continue;
}
goto done;