git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Michael Haggerty <mhagger@alum.mit.edu>
To: Junio C Hamano <gitster@pobox.com>
Cc: Brad King <brad.king@kitware.com>,
	Johan Herland <johan@herland.net>, Jeff King <peff@peff.net>,
	Vicent Marti <tanoku@gmail.com>,
	git@vger.kernel.org
Subject: Re: [PATCH v2 20/27] update-ref --stdin: Reimplement using reference transactions
Date: Fri, 04 Apr 2014 07:02:48 +0200	[thread overview]
Message-ID: <533E3CF8.5080506@alum.mit.edu> (raw)
In-Reply-To: <xmqq61mqs8wl.fsf@gitster.dls.corp.google.com>

On 04/03/2014 05:57 PM, Junio C Hamano wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
> 
>> I assumed that rolling back a non-consummated transaction in the case of
>> early program death should be the responsibility of the library, not of
>> the caller.  If I'm correct, the caller(s) won't have to be modified
>> when the atexit facility is added, so I don't see a reason to add it
>> before it is needed by a concrete backend.
>>
>> But you suggest that the caller should be involved.
> 
> I didn't say "should".  If the library can automatically rollback
> without being called upon die() anywhere in the system, that is
> better.  The suggestion was because I didn't think you were shooting
> for such a completeness in the library part, and a possible way out
> is for the caller to help.

I was assuming that any ref backends that required rollback-on-fail
would register an atexit handler and a signal handler, similar to how
lock_file rollbacks are done.

I admit that I haven't thought through all the details; for example, are
there restrictions on the things that a signal handler is allowed to do
that would preclude its being able to rollback the types of transactions
that back ends might want to implement?  (Though if so, what hope do we
have that the caller can do better?)

So, if somebody can think of a reason that we would need to involve the
caller in cleanup, please speak up.  Otherwise I think it would be less
error-prone to leave this responsibility with the individual back ends.
 (And if something unexpected comes up, we can make this change later.)

Michael

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

  reply	other threads:[~2014-04-04  5:02 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-03-24 17:56 [PATCH v2 00/27] Clean up update-refs --stdin and implement ref_transaction Michael Haggerty
2014-03-24 17:56 ` [PATCH v2 01/27] t1400: Fix name and expected result of one test Michael Haggerty
2014-03-31 21:30   ` Junio C Hamano
2014-03-31 21:49     ` Michael Haggerty
2014-03-24 17:56 ` [PATCH v2 02/27] t1400: Provide more usual input to the command Michael Haggerty
2014-03-31 21:28   ` Junio C Hamano
2014-03-24 17:56 ` [PATCH v2 03/27] parse_arg(): Really test that argument is properly terminated Michael Haggerty
2014-03-31 21:36   ` Junio C Hamano
2014-03-31 22:11     ` Michael Haggerty
2014-03-24 17:56 ` [PATCH v2 04/27] t1400: Add some more tests involving quoted arguments Michael Haggerty
2014-03-24 17:56 ` [PATCH v2 05/27] refs.h: Rename the action_on_err constants Michael Haggerty
2014-03-24 17:56 ` [PATCH v2 06/27] update_refs(): Fix constness Michael Haggerty
2014-03-31 21:40   ` Junio C Hamano
2014-03-31 22:16     ` Michael Haggerty
2014-03-31 22:38       ` Junio C Hamano
2014-03-24 17:56 ` [PATCH v2 07/27] update-ref --stdin: Read the whole input at once Michael Haggerty
2014-03-24 17:56 ` [PATCH v2 08/27] parse_cmd_verify(): Copy old_sha1 instead of evaluating <oldvalue> twice Michael Haggerty
2014-03-24 17:56 ` [PATCH v2 09/27] update-ref.c: Extract a new function, parse_refname() Michael Haggerty
2014-03-24 17:56 ` [PATCH v2 10/27] update-ref --stdin: Improve error messages for invalid values Michael Haggerty
2014-03-24 17:56 ` [PATCH v2 11/27] update-ref --stdin: Make error messages more consistent Michael Haggerty
2014-03-24 17:56 ` [PATCH v2 12/27] update-ref --stdin: Simplify error messages for missing oldvalues Michael Haggerty
2014-03-24 17:56 ` [PATCH v2 13/27] t1400: Test that stdin -z update treats empty <newvalue> as zeros Michael Haggerty
2014-03-31 21:48   ` Junio C Hamano
2014-03-31 22:20     ` Michael Haggerty
2014-03-24 17:56 ` [PATCH v2 14/27] update-ref.c: Extract a new function, parse_next_sha1() Michael Haggerty
2014-03-26 18:39   ` Brad King
2014-03-24 17:56 ` [PATCH v2 15/27] update-ref --stdin -z: Deprecate interpreting the empty string as zeros Michael Haggerty
2014-03-31 21:49   ` Junio C Hamano
2014-03-24 17:56 ` [PATCH v2 16/27] t1400: Test one mistake at a time Michael Haggerty
2014-03-26 18:39   ` Brad King
2014-03-31 21:50   ` Junio C Hamano
2014-03-31 22:32     ` Michael Haggerty
2014-03-24 17:56 ` [PATCH v2 17/27] update-ref --stdin: Improve the error message for unexpected EOF Michael Haggerty
2014-03-24 17:56 ` [PATCH v2 18/27] update-ref --stdin: Harmonize error messages Michael Haggerty
2014-03-31 21:51   ` Junio C Hamano
2014-03-31 22:37     ` Michael Haggerty
2014-04-01  9:29       ` Michael Haggerty
2014-04-02 16:38         ` Junio C Hamano
2014-03-24 17:56 ` [PATCH v2 19/27] refs: Add a concept of a reference transaction Michael Haggerty
2014-03-26 18:39   ` Brad King
2014-03-26 21:42     ` Michael Haggerty
2014-04-01 19:39   ` Junio C Hamano
2014-04-02  4:57     ` Michael Haggerty
2014-03-24 17:56 ` [PATCH v2 20/27] update-ref --stdin: Reimplement using reference transactions Michael Haggerty
2014-04-01 19:46   ` Junio C Hamano
2014-04-02  5:03     ` Michael Haggerty
2014-04-03 15:57       ` Junio C Hamano
2014-04-04  5:02         ` Michael Haggerty [this message]
2014-03-24 17:56 ` [PATCH v2 21/27] refs: Remove API function update_refs() Michael Haggerty
2014-04-01 19:46   ` Junio C Hamano
2014-03-24 17:56 ` [PATCH v2 22/27] struct ref_update: Rename field "ref_name" to "refname" Michael Haggerty
2014-04-01 19:53   ` Junio C Hamano
2014-04-02  5:11     ` Michael Haggerty
2014-03-24 17:56 ` [PATCH v2 23/27] struct ref_update: Store refname as a FLEX_ARRAY Michael Haggerty
2014-04-01 19:54   ` Junio C Hamano
2014-03-24 17:56 ` [PATCH v2 24/27] ref_transaction_commit(): Introduce temporary variables Michael Haggerty
2014-04-01 19:26   ` Junio C Hamano
2014-03-24 17:56 ` [PATCH v2 25/27] struct ref_update: Add a lock member Michael Haggerty
2014-03-24 17:56 ` [PATCH v2 26/27] struct ref_update: Add type field Michael Haggerty
2014-04-01 20:03   ` Junio C Hamano
2014-04-02 10:13     ` Michael Haggerty
2014-04-02 17:44     ` Junio C Hamano
2014-03-24 17:57 ` [PATCH v2 27/27] ref_transaction_commit(): Work with transaction->updates in place Michael Haggerty
2014-03-26 18:39 ` [PATCH v2 00/27] Clean up update-refs --stdin and implement ref_transaction Brad King
2014-03-26 21:47   ` Michael Haggerty

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=533E3CF8.5080506@alum.mit.edu \
    --to=mhagger@alum.mit.edu \
    --cc=brad.king@kitware.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johan@herland.net \
    --cc=peff@peff.net \
    --cc=tanoku@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).