Re: [PATCH 07/23] expire_reflog(): use a lock_file for rewriting the reflog file
- From
Jeff King <peff@peff.net>
- Date
- Dec 9, 2014, 18:54 UTC
- Message-ID
- <20141209185433.GB31158@peff.net>
- In-Reply-To
- <xmqqfvcomzib.fsf@gitster.dls.corp.google.com>
On Tue, Dec 09, 2014 at 10:47:24AM -0800, Junio C Hamano wrote:
Show 18 quoted lines
> Michael Haggerty <mhagger@alum.mit.edu> writes: > > > 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. > > Interesting, as I think I saw Peff did something around that area > recently.
Yeah, and I had no idea about the truncated-line race which Michael described at the time. It makes me glad that I took the time to do the more careful thing[1] (fixing the reverse-reflog parser to properly include the newline when present) and not the quick-and-easy thing[2] (teaching show_one_reflog_ent to accept entries without newlines).
What you have queued in jk/for-each-reflog-ent-reverse should be fine, even in light of what Michael said.
-Peff
[1] http://article.gmane.org/gmane.comp.version-control.git/260849
[2] http://article.gmane.org/gmane.comp.version-control.git/260807