Patrick Steinhardt <ps@xxxxxx> writes:
> On Tue, Feb 25, 2025 at 10:29:09AM +0100, Karthik Nayak wrote:
>> diff --git a/refs.c b/refs.c
>> index f989a46a5a..243c09c368 100644
>> --- a/refs.c
>> +++ b/refs.c
>> @@ -2726,6 +2736,27 @@ void ref_transaction_for_each_queued_update(struct ref_transaction *transaction,
>> }
>> }
>>
>> +void ref_transaction_for_each_rejected_update(struct ref_transaction *transaction,
>> + ref_transaction_for_each_rejected_update_fn cb,
>> + void *cb_data)
>> +{
>> + if (!(transaction->flags & REF_TRANSACTION_ALLOW_PARTIAL))
>> + return;
>> +
>> + for (size_t i = 0; i < transaction->nr; i++) {
>> + struct ref_update *update = transaction->updates[i];
>> +
>> + if (!update->rejection_err)
>> + continue;
>
> This kind of proves my point that `TRANSACTION_OK` is pointless and
> leads to a mixture of using and not using the enum :)
>
>> diff --git a/refs/files-backend.c b/refs/files-backend.c
>> index 3b0adf8bb2..d0a53c9ace 100644
>> --- a/refs/files-backend.c
>> +++ b/refs/files-backend.c
>> @@ -2851,8 +2851,18 @@ static int files_transaction_prepare(struct ref_store *ref_store,
>> ret = lock_ref_for_update(refs, update, transaction,
>> head_ref, &refnames_to_check,
>> err);
>> - if (ret)
>> + if (ret) {
>> + if (transaction->flags & REF_TRANSACTION_ALLOW_PARTIAL &&
>
> Hm. If the error values were defined as a bitfield we could refactor
> this to not be a flag, but have a `transaction->accepted_rejections`
> instead that allows the caller to ask for only a subset of rejections to
> be accepted.
>
I did consider making them bitfield and even tried playing with that
idea. There are a bunch of side affects of doing that in other
subsystems which check call use the reference API and expect '< 0'
errors.
> I'm not quite sure whether it is a good idea, but the logic to handle
> all of that could be self-contained in `ref_transaction_set_rejected()`:
> if it returned an error code itself, it would swallow any errors in case
> the transaction allows a given error, and bubble up the error again in
> case the error is not allowed. The function could use a rename in that
> case though, e.g. `ref_transaction_maybe_set_rejected()`.
>
> Patrick
I think this is great idea, it does cleanup a bunch of the code. I will
add this in the next version with the rename. Thanks!
Attachment:
signature.asc
Description: PGP signature
