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

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

On Thu, Dec 04, 2014 at 04:23:31PM -0800, Jonathan Nieder wrote:

quoted

Michael Haggerty wrote:

quoted

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