Re: [PATCH 07/23] expire_reflog(): use a lock_file for rewriting the reflog file

From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Dec 8, 2014, 10:07 UTC
Message-ID: <54857871.5090805@alum.mit.edu>
In-Reply-To: <20141205021931.GA29570@google.com>

On 12/05/2014 03:19 AM, Stefan Beller wrote:

Show 22 quoted lines

> On Thu, Dec 04, 2014 at 04:23:31PM -0800, Jonathan Nieder wrote:
>> Michael Haggerty wrote:
>>
>>> We don't actually need the locking functionality, because we already
>>> hold the lock on the reference itself, which is how the reflog file is
>>> locked. But the lock_file code still does some of the bookkeeping for
>>> us and is more careful than the old code here was.
>>
>> As you say, the ref lock takes care of mutual exclusion, so we do not
>> have to be too careful about compatibility with other tools that might
>> not know to lock the reflog.  And this is not tying our hands for a
>> future when I might want to lock logs/refs/heads/topic/1 while
>> logs/refs/heads/topic still exists as part of the implementation of
>> "git mv topic/1 topic".
>>
>> Stefan and I had forgotten about that guarantee when looking at that
>> kind of operation --- thanks for the reminder.
> 
> I did not forget about it, I did not know about that in the first hand.
> We don't seem to have documentation on it?
> 
> So sorry for heading in a direction, which would have been avoidable.

This isn't documented very well. I thought I saw a comment somewhere in the code that stated it explicitly, but I can't find it now. In any case, my understanding of the locking protocol for reflogs is:

    The reflog for "$refname", which is stored at
    "$GIT_DIR/logs/$refname", is locked by holding
    "$GIT_DIR/refs/$refname.lock", *even if the corresponding
    reference is packed*.

This implies that readers, who don't pay attention to locks, have to be prepared for the possibility that the reflog is in the middle of an update and that the last line is incomplete. This is handled by show_one_reflog_ent(), which discards incomplete lines.

This protocol avoids the need to rewrite the reflog from scratch for each reference update.

Given how poorly-documented this point is, I wonder whether other implementations of Git (e.g., libgit2, JGit, Dulwich, ...) got it right.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu