On Thu, Mar 20, 2025 at 12:44:01PM +0100, Karthik Nayak wrote:
> diff --git a/refs.c b/refs.c
> index 3d0b53d56e..b34ec198f5 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -1206,11 +1210,45 @@ void ref_transaction_free(struct ref_transaction *transaction)
> free((char *)transaction->updates[i]->old_target);
> free(transaction->updates[i]);
> }
> +
> + if (transaction->rejections)
> + free(transaction->rejections->update_indices);
> + free(transaction->rejections);
> +
> string_list_clear(&transaction->refnames, 0);
> free(transaction->updates);
> free(transaction);
> }
>
> +int ref_transaction_maybe_set_rejected(struct ref_transaction *transaction,
> + size_t update_idx,
> + enum ref_transaction_error err)
> +{
> + if (update_idx >= transaction->nr)
> + BUG("trying to set rejection on invalid update index");
> +
> + if (!(transaction->flags & REF_TRANSACTION_ALLOW_FAILURE))
> + return 0;
> +
> + if (!transaction->rejections)
> + BUG("transaction not inititalized with failure support");
> +
> + /*
> + * Don't accept generic errors, since these errors are not user
> + * input related.
> + */
> + if (err == REF_TRANSACTION_ERROR_GENERIC)
> + return 0;
> +
> + transaction->updates[update_idx]->rejection_err = err;
> + ALLOC_GROW(transaction->rejections->update_indices,
> + transaction->rejections->nr + 1,
> + transaction->rejections->alloc);
> + transaction->rejections->update_indices[transaction->rejections->nr++] = update_idx;
> +
> + return 1;
> +}
If we had a `struct ref_update_rejection` we could store the update
index and rejection errors in the same location, which might be a bit
easier to reason about.
> diff --git a/refs/packed-backend.c b/refs/packed-backend.c
> index d90bd815a3..7bf57ca948 100644
> --- a/refs/packed-backend.c
> +++ b/refs/packed-backend.c
> @@ -1327,10 +1327,11 @@ static int packed_ref_store_remove_on_disk(struct ref_store *ref_store,
> * remain locked when it is done.
> */
> static enum ref_transaction_error write_with_updates(struct packed_ref_store *refs,
> - struct string_list *updates,
> + struct ref_transaction *transaction,
> struct strbuf *err)
> {
> enum ref_transaction_error ret = REF_TRANSACTION_ERROR_GENERIC;
> + struct string_list *updates = &transaction->refnames;
> struct ref_iterator *iter = NULL;
> size_t i;
> int ok;
> @@ -1411,6 +1412,13 @@ static enum ref_transaction_error write_with_updates(struct packed_ref_store *re
> "reference already exists",
> update->refname);
> ret = REF_TRANSACTION_ERROR_CREATE_EXISTS;
> +
> + if (ref_transaction_maybe_set_rejected(transaction, i, ret)) {
> + strbuf_setlen(err, 0);
Nit: you can use `strbuf_reset()` for this and other instances.
Patrick
