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>
Karthik Nayak committed
Jan 25, 2026 at 23:52 UTC
b52a28b03ec99f2cfe4ef921b0d47250c665b0c6
5 files changed
+39
-31
refs.c
+26
-18
@@ -1224,6 +1224,7 @@ void ref_transaction_free(struct ref_transaction *transaction)
1224
free(transaction->updates[i]->committer_info);
1225
free((char *)transaction->updates[i]->new_target);
1226
free((char *)transaction->updates[i]->old_target);
1227
+ free((char *)transaction->updates[i]->rejection_details);
1228
free(transaction->updates[i]);
1229
}
1230
@@ -1238,7 +1239,8 @@ void ref_transaction_free(struct ref_transaction *transaction)
1239
1240
int ref_transaction_maybe_set_rejected(struct ref_transaction *transaction,
1241
size_t update_idx,
1241
- enum ref_transaction_error err)
1242
+ enum ref_transaction_error err,
1243
+ struct strbuf *details)
1244
{
1245
if (update_idx >= transaction->nr)
1246
BUG("trying to set rejection on invalid update index");
@@ -1264,6 +1266,7 @@ int ref_transaction_maybe_set_rejected(struct ref_transaction *transaction,
1266
transaction->updates[update_idx]->refname, 0);
1267
1268
transaction->updates[update_idx]->rejection_err = err;
1269
+ transaction->updates[update_idx]->rejection_details = strbuf_detach(details, NULL);
1270
ALLOC_GROW(transaction->rejections->update_indices,
1271
transaction->rejections->nr + 1,
1272
transaction->rejections->alloc);
@@ -2659,30 +2662,33 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs
2662
if (!initial_transaction &&
2663
(strset_contains(&conflicting_dirnames, dirname.buf) ||
2664
!refs_read_raw_ref(refs, dirname.buf, &oid, &referent,
2662
- &type, &ignore_errno))) {
2665
+ &type, &ignore_errno))) {
2666
+
2667
+ strbuf_addf(err, _("'%s' exists; cannot create '%s'"),
2668
+ dirname.buf, refname);
2669
+
2670
if (transaction && ref_transaction_maybe_set_rejected(
2671
transaction, *update_idx,
2665
- REF_TRANSACTION_ERROR_NAME_CONFLICT)) {
2672
+ REF_TRANSACTION_ERROR_NAME_CONFLICT, err)) {
2673
strset_remove(&dirnames, dirname.buf);
2674
strset_add(&conflicting_dirnames, dirname.buf);
2668
- continue;
2675
+ goto next_ref;
2676
}
2677
2671
- strbuf_addf(err, _("'%s' exists; cannot create '%s'"),
2672
- dirname.buf, refname);
2678
goto cleanup;
2679
}
2680
2681
if (extras && string_list_has_string(extras, dirname.buf)) {
2682
+ strbuf_addf(err, _("cannot process '%s' and '%s' at the same time"),
2683
+ refname, dirname.buf);
2684
+
2685
if (transaction && ref_transaction_maybe_set_rejected(
2686
transaction, *update_idx,
2679
- REF_TRANSACTION_ERROR_NAME_CONFLICT)) {
2687
+ REF_TRANSACTION_ERROR_NAME_CONFLICT, err)) {
2688
strset_remove(&dirnames, dirname.buf);
2681
- continue;
2689
+ goto next_ref;
2690
}
2691
2684
- strbuf_addf(err, _("cannot process '%s' and '%s' at the same time"),
2685
- refname, dirname.buf);
2692
goto cleanup;
2693
}
2694
}
@@ -2712,14 +2718,14 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs
2718
if (skip &&
2719
string_list_has_string(skip, iter->ref.name))
2720
continue;
2721
+ strbuf_addf(err, _("'%s' exists; cannot create '%s'"),
2722
+ iter->ref.name, refname);
2723
2724
if (transaction && ref_transaction_maybe_set_rejected(
2725
transaction, *update_idx,
2718
- REF_TRANSACTION_ERROR_NAME_CONFLICT))
2719
- continue;
2726
+ REF_TRANSACTION_ERROR_NAME_CONFLICT, err))
2727
+ goto next_ref;
2728
2721
- strbuf_addf(err, _("'%s' exists; cannot create '%s'"),
2722
- iter->ref.name, refname);
2729
goto cleanup;
2730
}
2731
@@ -2729,15 +2735,17 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs
2735
2736
extra_refname = find_descendant_ref(dirname.buf, extras, skip);
2737
if (extra_refname) {
2738
+ strbuf_addf(err, _("cannot process '%s' and '%s' at the same time"),
2739
+ refname, extra_refname);
2740
+
2741
if (transaction && ref_transaction_maybe_set_rejected(
2742
transaction, *update_idx,
2734
- REF_TRANSACTION_ERROR_NAME_CONFLICT))
2735
- continue;
2743
+ REF_TRANSACTION_ERROR_NAME_CONFLICT, err))
2744
+ goto next_ref;
2745
2737
- strbuf_addf(err, _("cannot process '%s' and '%s' at the same time"),
2738
- refname, extra_refname);
2746
goto cleanup;
2747
}
2748
+next_ref:;
2749
}
2750
2751
ret = 0;
refs/files-backend.c
+2
-3
@@ -2983,10 +2983,9 @@ static int files_transaction_prepare(struct ref_store *ref_store,
2983
head_ref, &refnames_to_check,
2984
err);
2985
if (ret) {
2986
- if (ref_transaction_maybe_set_rejected(transaction, i, ret)) {
2987
- strbuf_reset(err);
2986
+ if (ref_transaction_maybe_set_rejected(transaction, i,
2987
+ ret, err)) {
2988
ret = 0;
2989
-
2989
continue;
2990
}
2991
goto cleanup;
refs/packed-backend.c
+6
-6
@@ -1437,8 +1437,8 @@ static enum ref_transaction_error write_with_updates(struct packed_ref_store *re
1437
update->refname);
1438
ret = REF_TRANSACTION_ERROR_CREATE_EXISTS;
1439
1440
- if (ref_transaction_maybe_set_rejected(transaction, i, ret)) {
1441
- strbuf_reset(err);
1440
+ if (ref_transaction_maybe_set_rejected(transaction, i,
1441
+ ret, err)) {
1442
ret = 0;
1443
continue;
1444
}
@@ -1452,8 +1452,8 @@ static enum ref_transaction_error write_with_updates(struct packed_ref_store *re
1452
oid_to_hex(&update->old_oid));
1453
ret = REF_TRANSACTION_ERROR_INCORRECT_OLD_VALUE;
1454
1455
- if (ref_transaction_maybe_set_rejected(transaction, i, ret)) {
1456
- strbuf_reset(err);
1455
+ if (ref_transaction_maybe_set_rejected(transaction, i,
1456
+ ret, err)) {
1457
ret = 0;
1458
continue;
1459
}
@@ -1496,8 +1496,8 @@ static enum ref_transaction_error write_with_updates(struct packed_ref_store *re
1496
oid_to_hex(&update->old_oid));
1497
ret = REF_TRANSACTION_ERROR_NONEXISTENT_REF;
1498
1499
- if (ref_transaction_maybe_set_rejected(transaction, i, ret)) {
1500
- strbuf_reset(err);
1499
+ if (ref_transaction_maybe_set_rejected(transaction, i,
1500
+ ret, err)) {
1501
ret = 0;
1502
continue;
1503
}
refs/refs-internal.h
+3
-1
@@ -128,6 +128,7 @@ struct ref_update {
128
* was rejected.
129
*/
130
enum ref_transaction_error rejection_err;
131
+ const char *rejection_details;
132
133
/*
134
* 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,
154
*/
155
int ref_transaction_maybe_set_rejected(struct ref_transaction *transaction,
156
size_t update_idx,
156
- enum ref_transaction_error err);
157
+ enum ref_transaction_error err,
158
+ struct strbuf *details);
159
160
/*
161
* Add a ref_update with the specified properties to transaction, and
refs/reftable-backend.c
+2
-3
@@ -1401,10 +1401,9 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,
1401
&refnames_to_check, head_type,
1402
&head_referent, &referent, err);
1403
if (ret) {
1404
- if (ref_transaction_maybe_set_rejected(transaction, i, ret)) {
1405
- strbuf_reset(err);
1404
+ if (ref_transaction_maybe_set_rejected(transaction, i,
1405
+ ret, err)) {
1406
ret = 0;
1407
-
1407
continue;
1408
}
1409
goto done;