Re: [PATCH 8/8] reflog_expire(): lock symbolic refs themselves, not their referent

On Fri, Feb 13, 2015 at 8:26 AM, Michael Haggerty [off-list ref] wrote:

What is the best way forward?

Switching to holding only "HEAD.lock" while updating "logs/HEAD" is the
right solution, but it would introduce an incompatibility with old
versions of Git and libgit2 (and maybe JGit?) Probably nobody would
notice, but who knows?

Therefore, I propose that we hold *both* "HEAD.lock" *and*
"refs/heads/master.lock" when modifying "logs/HEAD" (even when running
"reflog expire" or "reflog delete"). I think this will be compatible
with old versions of Git and libgit2, and it will also fix some design
problems mentioned above. Plus, it will prevent updates to the two
reflogs from appearing out of order relative to each other.

Yeah, I agree on that. It's ok to lock both for now (it doesn't
really harm the user). But we really need to document it in the
source code why we lock both and that the 2nd lock can be removed
after time has passed, something like:

/*
 * We need to lock both the symbolic ref as well as
 * the dereferenced ref for now because of inconsistencies with
 * older and other implementations of git. Originally only the
 * dereferenced ref lock was held, but discussion($gmane/263555)
 * turned out, we actually want to hold the lock on the symbolic ref.
 * For now hold both locks until all implementation hold the lock on
 * the symbolic ref. (Feb/2015)
 */

Someday, when old clients have been retired, we can optionally switch to
locking *only* "HEAD.lock" when modifying "logs/HEAD".

What do people think?
Michael

[1] https://github.com/libgit2/libgit2/issues/2902

--
Michael Haggerty
mhagger@alum.mit.edu