git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Michael Haggerty <mhagger@alum.mit.edu>
To: Stefan Beller <sbeller@google.com>,
	ronniesahlberg@gmail.com, jrnieder@gmail.com, gitster@pobox.com
Cc: git@vger.kernel.org
Subject: Re: [PATCHv3 00/13] the refs-transactions-reflog series
Date: Thu, 04 Dec 2014 18:10:15 +0100	[thread overview]
Message-ID: <54809577.4080302@alum.mit.edu> (raw)
In-Reply-To: <1417681763-32334-1-git-send-email-sbeller@google.com>

On 12/04/2014 09:29 AM, Stefan Beller wrote:
> This is the whole refs-transactions-reflog series[1],
> which was in discussion for a bit already. It applies to origin/master.

I am still unhappy with the approach of this series, for the reasons
that I explained earlier [1]. In short, I think that the abstraction
level is wrong. In my opinion, consumers of the refs API should barely
even have to *know* about reflogs, let alone implement reflog expiration
themselves.

Of course, reflog expiration *should* be done atomically. But that is
the business of the refs module; callers shouldn't have to do all the
complicated work of building the transaction themselves.

I spent the day working on an alternate proposal, to convert
expire_reflogs() to take callback functions that decide *what* to
expire, but which itself does the work of acquiring locks, iterating
through the reflog entries, rewriting the file, and overwriting the old
file with the new one. The goal is to move this function into refs.c and
make builtin/reflog.c much simpler--it will only have to implement the
callbacks that determine the expiration policy.

I'm almost done with the series but won't be able to finish it until
tomorrow. For those who are impatient, here is my work in progress [2].

Michael

[1]
http://thread.gmane.org/gmane.comp.version-control.git/259712/focus=259770
[2] https://github.com/mhagger/git, branch "reflog-expire-api".

-- 
Michael Haggerty
mhagger@alum.mit.edu

  parent reply	other threads:[~2014-12-04 17:10 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-04  8:29 [PATCHv3 00/13] the refs-transactions-reflog series Stefan Beller
2014-12-04  8:29 ` [PATCH 01/13] refs.c: make ref_transaction_create a wrapper for ref_transaction_update Stefan Beller
2014-12-04  8:29 ` [PATCH 02/13] refs.c: make ref_transaction_delete " Stefan Beller
2014-12-04  8:29 ` [PATCH 03/13] refs.c: add a function to append a reflog entry to a fd Stefan Beller
2014-12-04  8:29 ` [PATCH 04/13] refs.c: rename the transaction functions Stefan Beller
2014-12-04  8:29 ` [PATCH 05/13] refs.c: rename transaction.updates to transaction.ref_updates Stefan Beller
2014-12-04  8:29 ` [PATCH 06/13] refs.c: add a transaction function to truncate or append a reflog entry Stefan Beller
2014-12-04  8:29 ` [PATCH 07/13] reflog.c: use a reflog transaction when writing during expire Stefan Beller
2014-12-04  8:29 ` [PATCH 08/13] refs.c: rename log_ref_setup to create_reflog Stefan Beller
2014-12-04  8:29 ` [PATCH 09/13] refs.c: remove unlock_ref/close_ref/commit_ref from the refs api Stefan Beller
2014-12-04  8:29 ` [PATCH 10/13] refs.c: remove lock_any_ref_for_update Stefan Beller
2014-12-04  8:29 ` [PATCH 11/13] refs.c: don't expose the internal struct ref_lock in the header file Stefan Beller
2014-12-04  8:29 ` [PATCH 12/13] refs.c: use a bit for ref_update have_old Stefan Beller
2014-12-04 16:10   ` Torsten Bögershausen
2014-12-04 17:00   ` Andreas Schwab
2014-12-04  8:29 ` [PATCH 13/13] refs.c: allow deleting refs with a broken sha1 Stefan Beller
2014-12-04 17:10 ` Michael Haggerty [this message]
2014-12-04 17:53   ` [PATCHv3 00/13] the refs-transactions-reflog series Jonathan Nieder
2014-12-04 18:14   ` Jonathan Nieder
2014-12-04 18:32     ` Stefan Beller
2014-12-04 21:13       ` Michael Haggerty
2014-12-04 18:37   ` Junio C Hamano
2014-12-04 18:41 ` Junio C Hamano
2014-12-04 18:49   ` Stefan Beller
2014-12-04 19:27     ` Jonathan Nieder

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=54809577.4080302@alum.mit.edu \
    --to=mhagger@alum.mit.edu \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=ronniesahlberg@gmail.com \
    --cc=sbeller@google.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).