On Wed, Dec 17, 2014 at 3:26 PM, Junio C Hamano <[email protected]> wrote:
> Stefan Beller <[email protected]> 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:
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to