I have changed the transaction functions to die(BUG:) if the user
tries to call _update/_create/_delete on a failed transaction.

On Thu, May 29, 2014 at 10:41 AM, Jonathan Nieder <> wrote:
> Ronnie Sahlberg wrote:
>> On Wed, May 28, 2014 at 4:39 PM, Jonathan Nieder <> wrote:
>>> 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.
> [...]
>>> 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.
> I don't want to push you toward making a change you think is wrong.  I
> certainly don't own the codebase, and there are lots of other people
> (e.g., Michael, Junio, Jeff) to get advice from.  So I guess I should
> try to address this.
> I'm not quite sure what you mean by too restrictive.
>  a. Having API constraints that aren't enforced by the function makes
>     using the API too fussy.
>     I agree with that.  That was something I liked about keeping track
>     of the OPEN/CLOSED state of a transaction, which would let
>     functions like _commit die() if someone is misusing the API so the
>     problem gets detected early.
>  b. Having to check the return value from _update() is too fussy.
>     It certainly seems *possible* to have an API that doesn't require
>     checking the return value, while still avoiding the usability
>     problem I described in the quoted message above.  For example:
>      * _update() returns void and has no strbuf parameter
>      * error handling happens by checking the error from _commit()
>     That would score well on the scale described at
>     An API where checking the return value is optional would be
>     doable, too.  For example:
>      * _update() returns int and has a strbuf parameter
>      * if the strbuf parameter is NULL, the caller is expected to
>        wait for _commit() to check for errors, and a relevant
>        message will be passed back then
>      * if the strbuf parameter is non-NULL, then calling _commit()
>        after an error is an API violation
> I don't understand the comment about no more subtle than most of git.
> Are you talking about the errno action at a distance you found in some
> functions?  I thought we agreed that those were mistakes that accrue
> when people aim for a quick fix without thinking about maintainability
> and something git should have less of.
> Jonathan
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to
More majordomo info at

Reply via email to