Re: [PATCHv3 4/6] receive-pack.c: use a single ref_transaction for atomic pushes

[Stefan Beller <sbeller@google.com>]

On Wed, Dec 17, 2014 at 3:26 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> @@ -1086,8 +1100,25 @@ static void execute_commands(struct command *commands,
>>
>>               if (cmd->skip_update)
>>                       continue;
>> -
>> +             if (!use_atomic) {
>> +                     transaction = ref_transaction_begin(&err);
>> +                     if (!transaction) {
>> +                             rp_error("%s", err.buf);
>> +                             strbuf_release(&err);
>> +                             cmd->error_string = "failed to start transaction";
>> +                             return;
>> +                     }
>> +             }
>>               cmd->error_string = update(cmd, si);
>> +             if (!use_atomic)
>> +                     if (ref_transaction_commit(transaction, &err)) {
>> +                             ref_transaction_free(transaction);
>> +                             rp_error("%s", err.buf);
>> +                             strbuf_release(&err);
>> +                             cmd->error_string = "failed to update ref";
>> +                             return;
>> +                     }
>
> Hmm, should the code even attempt to commit if update() returned a
> non NULL, signaling a failure?
>
> Or would we want to do this instead?

This would change the current behavior. In the case of !atomic we want
to consider all commands and not stop early.

So maybe more
if (!cmd->error_string) {
        if (!use_atomic
            && ref_transaction_commit(...)) {
            ...
        }
} else {
        if (use_atomic)
             goto check_atomic_commit;
}

and the  check_atomic_commit label is replacing the loop to check:
-        for (cmd = commands; cmd; cmd = cmd->next)
-                if (cmd->error_string)
-                        break;
+ check_atomic_commit: