Re: [PATCH v3 00/14] ref-transactions-reflog

From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Nov 18, 2014, 21:16 UTC
Message-ID: <546BB722.5020901@alum.mit.edu>
In-Reply-To: <xmqqr3x0uu81.fsf@gitster.dls.corp.google.com>

On 11/18/2014 09:30 PM, Junio C Hamano wrote:

Show 13 quoted lines

> 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