On Wed, May 28, 2014 at 4:39 PM, Jonathan Nieder <jrnieder@xxxxxxxxx> wrote:
> 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.
In this case "fatal:" can not happen.
This is no more subtle than most of the git core.
I have changed this function to explicitly abort on _update failing
but I think this is making the api too restrictive.
>
> 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
--
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