@matheusbsbs / matheus-bagliano / commits / 31726bb90d

refs: support rejection in batch updates during F/D checks

The `refs_verify_refnames_available()` is used to batch check refnames for F/D conflicts. While this is the more performant alternative than its individual version, it does not provide rejection capabilities on a single update level. For batched updates, this would mean a rejection of the entire transaction whenever one reference has a F/D conflict. Modify the function to call `ref_transaction_maybe_set_rejected()` to check if a single update can be rejected. Since this function is only internally used within 'refs/' and we want to pass in a `struct ref_transaction *` as a variable. We also move and mark `refs_verify_refnames_available()` to 'refs-internal.h' to be an internal function. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Acked-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Karthik Nayak committed Apr 8, 2025 at 10:51 UTC 31726bb90d70236f7afaa345bf45195e2ef62d22

5 files changed +76 -27

refs.c

+34 -3

@@ -2540,6 +2540,7 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs

2540 const struct string_list *refnames,

2541 const struct string_list *extras,

2542 const struct string_list *skip,

2543 + struct ref_transaction *transaction,

2544 unsigned int initial_transaction,

2545 struct strbuf *err)

2546 {

@@ -2547,6 +2548,7 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs

2548 struct strbuf referent = STRBUF_INIT;

2549 struct string_list_item *item;

2550 struct ref_iterator *iter = NULL;

2551 + struct strset conflicting_dirnames;

2552 struct strset dirnames;

2553 int ret = REF_TRANSACTION_ERROR_NAME_CONFLICT;

2554

@@ -2557,9 +2559,11 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs

2559

2560 assert(err);

2561

2562 + strset_init(&conflicting_dirnames);

2563 strset_init(&dirnames);

2564

2565 for_each_string_list_item(item, refnames) {

2566 + const size_t *update_idx = (size_t *)item->util;

2567 const char *refname = item->string;

2568 const char *extra_refname;

2569 struct object_id oid;

@@ -2597,14 +2601,30 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs

2601 continue;

2602

2603 if (!initial_transaction &&

2600 - !refs_read_raw_ref(refs, dirname.buf, &oid, &referent,

2601 - &type, &ignore_errno)) {

2604 + (strset_contains(&conflicting_dirnames, dirname.buf) ||

2605 + !refs_read_raw_ref(refs, dirname.buf, &oid, &referent,

2606 + &type, &ignore_errno))) {

2607 + if (transaction && ref_transaction_maybe_set_rejected(

2608 + transaction, *update_idx,

2609 + REF_TRANSACTION_ERROR_NAME_CONFLICT)) {

2610 + strset_remove(&dirnames, dirname.buf);

2611 + strset_add(&conflicting_dirnames, dirname.buf);

2612 + continue;

2613 + }

2614 +

2615 strbuf_addf(err, _("'%s' exists; cannot create '%s'"),

2616 dirname.buf, refname);

2617 goto cleanup;

2618 }

2619

2620 if (extras && string_list_has_string(extras, dirname.buf)) {

2621 + if (transaction && ref_transaction_maybe_set_rejected(

2622 + transaction, *update_idx,

2623 + REF_TRANSACTION_ERROR_NAME_CONFLICT)) {

2624 + strset_remove(&dirnames, dirname.buf);

2625 + continue;

2626 + }

2627 +

2628 strbuf_addf(err, _("cannot process '%s' and '%s' at the same time"),

2629 refname, dirname.buf);

2630 goto cleanup;

@@ -2637,6 +2657,11 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs

2657 string_list_has_string(skip, iter->refname))

2658 continue;

2659

2660 + if (transaction && ref_transaction_maybe_set_rejected(

2661 + transaction, *update_idx,

2662 + REF_TRANSACTION_ERROR_NAME_CONFLICT))

2663 + continue;

2664 +

2665 strbuf_addf(err, _("'%s' exists; cannot create '%s'"),

2666 iter->refname, refname);

2667 goto cleanup;

@@ -2648,6 +2673,11 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs

2673

2674 extra_refname = find_descendant_ref(dirname.buf, extras, skip);

2675 if (extra_refname) {

2676 + if (transaction && ref_transaction_maybe_set_rejected(

2677 + transaction, *update_idx,

2678 + REF_TRANSACTION_ERROR_NAME_CONFLICT))

2679 + continue;

2680 +

2681 strbuf_addf(err, _("cannot process '%s' and '%s' at the same time"),

2682 refname, extra_refname);

2683 goto cleanup;

@@ -2659,6 +2689,7 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs

2689 cleanup:

2690 strbuf_release(&referent);

2691 strbuf_release(&dirname);

2692 + strset_clear(&conflicting_dirnames);

2693 strset_clear(&dirnames);

2694 ref_iterator_free(iter);

2695 return ret;

@@ -2679,7 +2710,7 @@ enum ref_transaction_error refs_verify_refname_available(

2710 };

2711

2712 return refs_verify_refnames_available(refs, &refnames, extras, skip,

2682 - initial_transaction, err);

2713 + NULL, initial_transaction, err);

2714 }

2715

2716 struct do_for_each_reflog_help {

refs.h

-12

@@ -141,18 +141,6 @@ enum ref_transaction_error refs_verify_refname_available(struct ref_store *refs,

141 unsigned int initial_transaction,

142 struct strbuf *err);

143

144 -/*

145 - * Same as `refs_verify_refname_available()`, but checking for a list of

146 - * refnames instead of only a single item. This is more efficient in the case

147 - * where one needs to check multiple refnames.

148 - */

149 -enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs,

150 - const struct string_list *refnames,

151 - const struct string_list *extras,

152 - const struct string_list *skip,

153 - unsigned int initial_transaction,

154 - struct strbuf *err);

155 -

144 int refs_ref_exists(struct ref_store *refs, const char *refname);

145

146 int should_autocreate_reflog(enum log_refs_config log_all_ref_updates,

refs/files-backend.c

+18 -9

@@ -677,16 +677,18 @@ static void unlock_ref(struct ref_lock *lock)

677 * - Generate informative error messages in the case of failure

678 */

679 static enum ref_transaction_error lock_raw_ref(struct files_ref_store *refs,

680 - const char *refname,

680 + struct ref_update *update,

681 + size_t update_idx,

682 int mustexist,

683 struct string_list *refnames_to_check,

684 const struct string_list *extras,

685 struct ref_lock **lock_p,

686 struct strbuf *referent,

686 - unsigned int *type,

687 struct strbuf *err)

688 {

689 enum ref_transaction_error ret = REF_TRANSACTION_ERROR_GENERIC;

690 + const char *refname = update->refname;

691 + unsigned int *type = &update->type;

692 struct ref_lock *lock;

693 struct strbuf ref_file = STRBUF_INIT;

694 int attempts_remaining = 3;

@@ -785,6 +787,8 @@ retry:

787

788 if (files_read_raw_ref(&refs->base, refname, &lock->old_oid, referent,

789 type, &failure_errno)) {

790 + struct string_list_item *item;

791 +

792 if (failure_errno == ENOENT) {

793 if (mustexist) {

794 /* Garden variety missing reference. */

@@ -864,7 +868,9 @@ retry:

868 * make sure there is no existing packed ref that conflicts

869 * with refname. This check is deferred so that we can batch it.

870 */

867 - string_list_append(refnames_to_check, refname);

871 + item = string_list_append(refnames_to_check, refname);

872 + item->util = xmalloc(sizeof(update_idx));

873 + memcpy(item->util, &update_idx, sizeof(update_idx));

874 }

875

876 ret = 0;

@@ -2547,6 +2553,7 @@ struct files_transaction_backend_data {

2553 */

2554 static enum ref_transaction_error lock_ref_for_update(struct files_ref_store *refs,

2555 struct ref_update *update,

2556 + size_t update_idx,

2557 struct ref_transaction *transaction,

2558 const char *head_ref,

2559 struct string_list *refnames_to_check,

@@ -2575,9 +2582,9 @@ static enum ref_transaction_error lock_ref_for_update(struct files_ref_store *re

2582 if (lock) {

2583 lock->count++;

2584 } else {

2578 - ret = lock_raw_ref(refs, update->refname, mustexist,

2585 + ret = lock_raw_ref(refs, update, update_idx, mustexist,

2586 refnames_to_check, &transaction->refnames,

2580 - &lock, &referent, &update->type, err);

2587 + &lock, &referent, err);

2588 if (ret) {

2589 char *reason;

2590

@@ -2849,7 +2856,7 @@ static int files_transaction_prepare(struct ref_store *ref_store,

2856 for (i = 0; i < transaction->nr; i++) {

2857 struct ref_update *update = transaction->updates[i];

2858

2852 - ret = lock_ref_for_update(refs, update, transaction,

2859 + ret = lock_ref_for_update(refs, update, i, transaction,

2860 head_ref, &refnames_to_check,

2861 err);

2862 if (ret) {

@@ -2905,7 +2912,8 @@ static int files_transaction_prepare(struct ref_store *ref_store,

2912 * So instead, we accept the race for now.

2913 */

2914 if (refs_verify_refnames_available(refs->packed_ref_store, &refnames_to_check,

2908 - &transaction->refnames, NULL, 0, err)) {

2915 + &transaction->refnames, NULL, transaction,

2916 + 0, err)) {

2917 ret = REF_TRANSACTION_ERROR_NAME_CONFLICT;

2918 goto cleanup;

2919 }

@@ -2951,7 +2959,7 @@ static int files_transaction_prepare(struct ref_store *ref_store,

2959

2960 cleanup:

2961 free(head_ref);

2954 - string_list_clear(&refnames_to_check, 0);

2962 + string_list_clear(&refnames_to_check, 1);

2963

2964 if (ret)

2965 files_transaction_cleanup(refs, transaction);

@@ -3097,7 +3105,8 @@ static int files_transaction_finish_initial(struct files_ref_store *refs,

3105 }

3106

3107 if (refs_verify_refnames_available(&refs->base, &refnames_to_check,

3100 - &affected_refnames, NULL, 1, err)) {

3108 + &affected_refnames, NULL, transaction,

3109 + 1, err)) {

3110 packed_refs_unlock(refs->packed_ref_store);

3111 ret = REF_TRANSACTION_ERROR_NAME_CONFLICT;

3112 goto cleanup;

refs/refs-internal.h

+16

@@ -806,4 +806,20 @@ enum ref_transaction_error ref_update_check_old_target(const char *referent,

806 */

807 int ref_update_expects_existing_old_ref(struct ref_update *update);

808

809 +/*

810 + * Same as `refs_verify_refname_available()`, but checking for a list of

811 + * refnames instead of only a single item. This is more efficient in the case

812 + * where one needs to check multiple refnames.

813 + *

814 + * If using batched updates, then individual updates are marked rejected,

815 + * reference backends are then in charge of not committing those updates.

816 + */

817 +enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs,

818 + const struct string_list *refnames,

819 + const struct string_list *extras,

820 + const struct string_list *skip,

821 + struct ref_transaction *transaction,

822 + unsigned int initial_transaction,

823 + struct strbuf *err);

824 +

825 #endif /* REFS_REFS_INTERNAL_H */

refs/reftable-backend.c

+8 -3

@@ -1074,6 +1074,7 @@ static enum ref_transaction_error prepare_single_update(struct reftable_ref_stor

1074 struct ref_transaction *transaction,

1075 struct reftable_backend *be,

1076 struct ref_update *u,

1077 + size_t update_idx,

1078 struct string_list *refnames_to_check,

1079 unsigned int head_type,

1080 struct strbuf *head_referent,

@@ -1149,6 +1150,7 @@ static enum ref_transaction_error prepare_single_update(struct reftable_ref_stor

1150 if (ret < 0)

1151 return REF_TRANSACTION_ERROR_GENERIC;

1152 if (ret > 0 && !ref_update_expects_existing_old_ref(u)) {

1153 + struct string_list_item *item;

1154 /*

1155 * The reference does not exist, and we either have no

1156 * old object ID or expect the reference to not exist.

@@ -1158,7 +1160,9 @@ static enum ref_transaction_error prepare_single_update(struct reftable_ref_stor

1160 * can output a proper error message instead of failing

1161 * at a later point.

1162 */

1161 - string_list_append(refnames_to_check, u->refname);

1163 + item = string_list_append(refnames_to_check, u->refname);

1164 + item->util = xmalloc(sizeof(update_idx));

1165 + memcpy(item->util, &update_idx, sizeof(update_idx));

1166

1167 /*

1168 * There is no need to write the reference deletion

@@ -1368,7 +1372,7 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,

1372

1373 for (i = 0; i < transaction->nr; i++) {

1374 ret = prepare_single_update(refs, tx_data, transaction, be,

1371 - transaction->updates[i],

1375 + transaction->updates[i], i,

1376 &refnames_to_check, head_type,

1377 &head_referent, &referent, err);

1378 if (ret) {

@@ -1384,6 +1388,7 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,

1388

1389 ret = refs_verify_refnames_available(ref_store, &refnames_to_check,

1390 &transaction->refnames, NULL,

1391 + transaction,

1392 transaction->flags & REF_TRANSACTION_FLAG_INITIAL,

1393 err);

1394 if (ret < 0)

@@ -1402,7 +1407,7 @@ done:

1407 }

1408 strbuf_release(&referent);

1409 strbuf_release(&head_referent);

1405 - string_list_clear(&refnames_to_check, 0);

1410 + string_list_clear(&refnames_to_check, 1);

1411

1412 return ret;

1413 }