Re: [PATCH 00/24] Yet another pre-refs-backend series

[Michael Haggerty <mhagger@alum.mit.edu>]

On 04/07/2016 03:02 PM, David Turner wrote:
> We now have quite a large number of patches before we even get into
> the meat of the pluggable refs backend series.  So it's worth breaking
> those out and getting them in before we get into the main series
> (which Michael Haggerty swants to redesign a bit anyway).
> 
> This set of patches should be applied on top of
> jk/check-repository-format.
> 
> Michael Haggerty has reviewed those of my patches which are in here
> except maybe:
>   refs: on symref reflog expire, lock symref not referrent
> This was the one from later in the series that was straightforward to
> move to before the vtable; the other two were going to be harder to
> move and can wait until after the vtable.

This last patch deserves a little bit of discussion. Currently, when the
reflog of a symref is expired, the pointed-to ref is locked rather than
the symref. This patch changes the code to lock the symref instead.

This is clearly the right thing to do, and I consider this change a bug
fix. However, it introduces an incompatibility. An old version of `git
reflog expire` and a new version wouldn't agree on the locking protocol,
and could potentially try to overwrite the same reflog at the same time.

I think this risk is acceptable nevertheless, because expiring reflogs
is an uncommon operation and unlikely to be done from two processes at
the same time; moreover, the integrity of reflogs is not a matter of
life or death.

A far more likely conflict would be between a reflog expiration and a
symref update (e.g., `git checkout otherbranch`). This use case is
currently *broken* because `git checkout` locks HEAD. It would be fixed
by this patch.

If somebody is really upset about the risk of a race between an old and
new version of `git reflog expire`, the way to increase the safety would
be to lock *both* the symref and the referent while changing the
symref's reflog. I think that would be overkill.

This whole series is

Reviewed-by: Michael Haggerty <mhagger@alum.mit.edu>

David mentioned that I want to redesign the vtable patches somewhat.
Anybody who is curious can look at the work in progress branch on my
GitHub fork [1], branch wip/ref-storage.

Michael

[1] https://github.com/mhagger/git