Re: [PATCH v10 25/44] receive-pack.c: use a reference transaction for updating the refs

> On 05/23/2014 06:14 PM, Ronnie Sahlberg wrote:
>> On Fri, May 23, 2014 at 6:49 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
>>> [...]
>>> When I combine these two lines of thought, it suggests to me that we
>>> could do a better job of supporting *both* use cases.  What if the
>>> transaction object contained not an err strbuf but a string_list?  If an
>>> error occurs while building up the transaction, a message would be added
>>> to the string list and the function would return an error status.  The
>>> caller can monitor errors while it is building up the transaction and
>>> abort immediately if it wants, or it can ignore the return values and
>>> let the error messages accumulate in the string list.  When the caller
>>> attempts the commit, it would notice that the transaction failed, and at
>>> that time the caller could emit *all* of the accumulated error messages
>>> by reading them out of the string list; e.g.,
>>>
>>>     Error fetching from $REMOTE:   <- this is generated by caller
>>>         $ERR[0]    <- these come from the error string list,
>>>         $ERR[1]       printed with indentation by caller
>>>         $ERR[2]
>>>         $ERR[3]
>>>
>>> This style would have another advantage: we might have some back ends
>>> for which transactions have a high overhead.  Such a back end would
>>> probably choose not to do any checks while the transaction is being
>>> built up, e.g., to avoid a round-trip to a database.  When commit() is
>>> called, it would learn about all of the errors at once.  (1) It would
>>> need a way to return all of the errors to the caller.  (2) It would be
>>> nice for the caller to be able to treat such a back end the same as it
>>> treats a back end that is able to report errors immediately.  It seems
>>> to me that having a way to report multiple errors at the same time would
>>> solve both problems nicely.
>>
>> Inretesting.
>> That would mean changing all functions to take a string_list provided
>> by the caller instead of a strbuf.
>> And then have _update/_create/_delete do actual work instead of
>> bailing out after the first error.
>>
>> Users that want to check for error and log after each call to
>> _update/_create/_delete could do so and
>> just use the last entry added to the string list or otherwise they
>> could just wait until _commit time and if it fails log
>> all the strings.
>
> I still think we should consider storing the err string_list within the
> transaction object; otherwise it's annoying to have to pass that
> parameter around everywhere.  And if there were also a policy that any
> caller that checks and reports any error should report *all* of the
> accumulated errors and abort the transaction, then we don't have to
> worry about error messages being output multiple times or zero times.
>
> It might be nice to have a printf-style helper function like
>
>     ref_transaction_perror(transaction, fmt, ...)
>
> (or maybe ref_transaction_die()) that outputs the accumulated errors
> with msg as a header (like my example output above), to make error
> handling easy and uniform.

>
> Michael
>
> --
> Michael Haggerty
> mhagger@alum.mit.edu
> http://softwareswirl.blogspot.com/