Re: [PATCH v11 25/41] fast-import.c: use a ref transaction when dumping tags

> Ronnie Sahlberg wrote:
>
>> I rely on the fact that if the transaction has failed then it is safe
>> to call ref_transaction_commit since it is guaranteed to return an
>> error too.
>
> Yes, I am saying that behavior for ref_transaction_commit is weird.
>
> Usually when ref_transaction_commit is called I can do
>
>         struct strbuf err = STRBUF_INIT;
>         if (ref_transaction_commit(..., &err))
>                 die("%s", err.buf);
>
> and I know that since ref_transaction_commit has returned a nonzero
> result, err.buf is populated with a sensible message that will
> describe what went wrong.
>
> That's true even if there's a bug elsewhere in code I didn't write
> (e.g., someone forgot to check the return value from
> ref_transaction_update).
>
> But the guarantee you are describing removes that property.  It
> creates a case where ref_transaction_commit can return nonzero without
> updating err.  So I get the following message:
>
>         fatal:
>
> I don't think that's a good outcome.

>
> Sure, if I am well acquainted with the API, I can make sure to use the
> same strbuf for all transaction API calls.  But that would result in
> strange behavior, too: if multiple _update calls fail, then I get
> concatenated messages.
>
> Okay, I can make sure to do at most one failing _update, before
> calling _commit and printing the error.  But at that point, what is
> the advantage over normal exception handling, where the error gets
> handled at the _update call site?
>
> Jonathan