On Wed, May 28, 2014 at 4:39 PM, Jonathan Nieder <jrnie...@gmail.com> 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html