All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Haggerty <mhagger@alum.mit.edu>
To: Junio C Hamano <gitster@pobox.com>
Cc: Ronnie Sahlberg <sahlberg@google.com>,
	Stefan Beller <sbeller@google.com>,
	"git@vger.kernel.org" <git@vger.kernel.org>
Subject: Re: [PATCH v3 00/14] ref-transactions-reflog
Date: Tue, 18 Nov 2014 22:16:18 +0100	[thread overview]
Message-ID: <546BB722.5020901@alum.mit.edu> (raw)
In-Reply-To: <xmqqr3x0uu81.fsf@gitster.dls.corp.google.com>

On 11/18/2014 09:30 PM, Junio C Hamano wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
> 
>> I'm still not convinced. For me, "reflog_expire()" is an unusual outlier
>> operation, much like "git gc" or "git pack-refs" or "git fsck". None of
>> these are part of the beautiful Git data model; they are messy
>> maintenance operations. Forcing reference transactions to be general
>> enough to allow reflog expiration to be implemented *outside* the refs
>> API sacrificies their simplicity for lots of infrastructure that will
>> probably only be used to implement this single operation. Better to
>> implement reflog expiration *inside* the refs API.
> 
> Sorry, but I lost track---which one is inside and which one is
> outside?

By "inside" I mean the code that would be within the reference-handling
library if we had such a thing; i.e., implemented in refs.c. By
"outside" I mean in the code that calls the library; in this case the
"outside" code would live in builtin/reflog.c.

In other words, I'd prefer the "outside" code in builtin/reflog.c to
look vaguely like

    expire_reflogs_for_me_please(refname,
                                 should_expire_cb, cbdata, flags)

rather than

    transaction = ...
    for_each_reflog_entry {
        if should_expire()
            adjust neighbor reflog entries if necessary (actually,
                   they're transaction entries so we would have to
                   preprocess them before putting them in the
                   transaction)
        else
            add reflog entry to transaction
    }
    ref_transaction_commit()

and instead handle as much of the iteration, bookkeeping, and rewriting
as possible inside expire_reflogs_for_me_please().

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu

  reply	other threads:[~2014-11-18 21:16 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-18  1:35 [PATCH v3 00/14] ref-transactions-reflog Stefan Beller
2014-11-18  1:35 ` [PATCH v3 01/14] refs.c: make ref_transaction_create a wrapper for ref_transaction_update Stefan Beller
2014-11-18  1:35 ` [PATCH v3 02/14] refs.c: make ref_transaction_delete " Stefan Beller
2014-11-18  1:35 ` [PATCH v3 03/14] refs.c: rename the transaction functions Stefan Beller
2014-11-18  1:35 ` [PATCH v3 04/14] refs.c: add a function to append a reflog entry to a fd Stefan Beller
2014-11-18  1:35 ` [PATCH v3 05/14] refs.c: add a new update_type field to ref_update Stefan Beller
2014-11-18  1:35 ` [PATCH v3 06/14] refs.c: add a transaction function to append a reflog entry Stefan Beller
2014-11-18  1:35 ` [PATCH v3 07/14] refs.c: add a flag to allow reflog updates to truncate the log Stefan Beller
2014-11-18  1:35 ` [PATCH v3 08/14] refs.c: only write reflog update if msg is non-NULL Stefan Beller
2014-11-18  1:35 ` [PATCH v3 09/14] refs.c: allow multiple reflog updates during a single transaction Stefan Beller
2014-11-18  1:35 ` [PATCH v3 10/14] reflog.c: use a reflog transaction when writing during expire Stefan Beller
2014-11-18  1:35 ` [PATCH v3 11/14] refs.c: rename log_ref_setup to create_reflog Stefan Beller
2014-11-18  1:35 ` [PATCH v3 12/14] refs.c: Remove unlock_ref/close_ref/commit_ref from the refs api Stefan Beller
2014-11-18  1:35 ` [PATCH v3 13/14] refs.c: remove lock_any_ref_for_update Stefan Beller
2014-11-18  1:35 ` [PATCH v3 14/14] refs.c: allow deleting refs with a broken sha1 Stefan Beller
2014-11-18 11:26 ` [PATCH v3 00/14] ref-transactions-reflog Michael Haggerty
2014-11-18 18:36   ` Ronnie Sahlberg
2014-11-18 19:46     ` Michael Haggerty
2014-11-18 20:30       ` Junio C Hamano
2014-11-18 21:16         ` Michael Haggerty [this message]
2014-11-18 21:28           ` Junio C Hamano
2014-11-19 23:22             ` Stefan Beller
2014-11-20  3:24               ` Jonathan Nieder
2014-11-20 17:34                 ` Junio C Hamano
2014-11-20 10:56               ` Michael Haggerty
2014-11-20 18:17                 ` Jonathan Nieder
2014-11-27  5:34                   ` [PATCH 0/4] Using transactions for the reflog Stefan Beller
2014-11-27  5:34                     ` [PATCH 1/4] refs.c: rename the transaction functions Stefan Beller
2014-11-27  5:34                     ` [PATCH 2/4] refs.c: add a new update_type field to ref_update Stefan Beller
2014-11-27  5:34                     ` [PATCH 3/4] refs.c: add a transaction function to append a reflog entry Stefan Beller
2014-11-27  5:34                     ` [PATCH 4/4] reflog.c: use a reflog transaction when writing during expire Stefan Beller
  -- strict thread matches above, loose matches on Subject: below --
2014-06-18 17:08 [PATCH v3 00/14] ref-transactions-reflog Ronnie Sahlberg

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=546BB722.5020901@alum.mit.edu \
    --to=mhagger@alum.mit.edu \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=sahlberg@google.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.