On Wed, Dec 17, 2014 at 3:26 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Stefan Beller <sbeller@xxxxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html