From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Haggerty Subject: Re: [PATCH v10 25/44] receive-pack.c: use a reference transaction for updating the refs Date: Fri, 23 May 2014 23:02:24 +0200 Message-ID: <537FB760.1080800@alum.mit.edu> References: <1400261852-31303-1-git-send-email-sahlberg@google.com> <1400261852-31303-26-git-send-email-sahlberg@google.com> <537781CA.1010208@alum.mit.edu> <537F51F9.5070600@alum.mit.edu> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: "git@vger.kernel.org" To: Ronnie Sahlberg X-From: git-owner@vger.kernel.org Fri May 23 23:02:33 2014 Return-path: Envelope-to: gcvg-git-2@plane.gmane.org Received: from vger.kernel.org ([209.132.180.67]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1Wnwbo-0002iH-TB for gcvg-git-2@plane.gmane.org; Fri, 23 May 2014 23:02:33 +0200 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751011AbaEWVC2 (ORCPT ); Fri, 23 May 2014 17:02:28 -0400 Received: from alum-mailsec-scanner-4.mit.edu ([18.7.68.15]:60305 "EHLO alum-mailsec-scanner-4.mit.edu" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750923AbaEWVC1 (ORCPT ); Fri, 23 May 2014 17:02:27 -0400 X-AuditID: 1207440f-f79536d000000bcf-41-537fb7628844 Received: from outgoing-alum.mit.edu (OUTGOING-ALUM.MIT.EDU [18.7.68.33]) by alum-mailsec-scanner-4.mit.edu (Symantec Messaging Gateway) with SMTP id A7.EB.03023.267BF735; Fri, 23 May 2014 17:02:26 -0400 (EDT) Received: from [192.168.69.130] (p5DDB0E41.dip0.t-ipconnect.de [93.219.14.65]) (authenticated bits=0) (User authenticated as mhagger@ALUM.MIT.EDU) by outgoing-alum.mit.edu (8.13.8/8.12.4) with ESMTP id s4NL2P1q029343 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT); Fri, 23 May 2014 17:02:26 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Icedove/24.4.0 In-Reply-To: X-Enigmail-Version: 1.6 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprFKsWRmVeSWpSXmKPExsUixO6iqJu0vT7YYMoPA4uuK91MFv8m1Dgw eSzYVOrxeZNcAFMUt01SYklZcGZ6nr5dAnfGpr8H2QreSVRcmbOBrYHxhHAXIyeHhICJRFPH HyYIW0ziwr31bF2MXBxCApcZJc5NP8QO4Zxjklj3bT0zSBWvgLbEs/4+dhCbRUBV4vuKQ2Dd bAK6Eot6moFsDg5RgSCJP2cVIcoFJU7OfMICYosIaErc7D8NVs4soC/x6c8BMFtYIEmi7T/E SCGBY0wSO64YgNicAoESrz5MYQQZKSEgLtHTGARiMguoS6yfJwQxRV5i+9s5zBMYBWchWTYL oWoWkqoFjMyrGOUSc0pzdXMTM3OKU5N1i5MT8/JSi3RN9HIzS/RSU0o3MULCln8HY9d6mUOM AhyMSjy8P/rqg4VYE8uKK3MPMUpyMCmJ8lpvAwrxJeWnVGYkFmfEF5XmpBYfYpTgYFYS4eVd DJTjTUmsrEotyodJSXOwKInzqi9R9xMSSE8sSc1OTS1ILYLJynBwKEnwngIZKliUmp5akZaZ U4KQZuLgBBnOJSVSnJqXklqUWFqSEQ+K3fhiYPSCpHiA9n4GaectLkjMBYpCtJ5i1OU4dedY G5MQS15+XqqUOK8fSJEASFFGaR7cCliSesUoDvSxMO9lkCoeYIKDm/QKaAkT0JIXC2tBlpQk IqSkGhjFdmfuVfctO/wndKHg7fnrMllOHQ17qz/15ze7OS56G1SWNQkvUci6LSwouvI2U/ze 5/byJfdrFH+atq6eo25ulzYpUeV33o0XSi6tz8Pz85/ybnq6poVjpcJV24klR34p Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Archived-At: On 05/23/2014 06:14 PM, Ronnie Sahlberg wrote: > On Fri, May 23, 2014 at 6:49 AM, Michael Haggerty 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/