Re: [PATCH 12/16] refs: always handle non-normal refs in files backend

[David Turner <dturner@twopensource.com>]

On Wed, 2015-12-23 at 09:02 +0100, Michael Haggerty wrote:
> I very much like the idea of introducing special handling for
> symbolic
> reference updates within a transaction. In fact, I think I would go
> even
> farther:
> 
> Let's take the example of an update to HEAD, which currently points
> at
> refs/heads/master. I think it would *always* be a good idea (i.e.,
> even
> when only the files backend is in use) to split that ref_update into
> two
> ref_updates:
> 
> 1. One to update refs/heads/master and add a reflog entry for that
>    reference.
> 
> 2. One to add a reflog entry for HEAD (i.e. using the new
> REF_LOG_ONLY
>    flag suggested above).

Having now implemented this, I think it's ugly.

While committing a ref_update, we read the ref's old value.  We need
this value for two things: 1. confirming that it matches old_sha1 in
the ref_update and 2. writing to the reflog.  In the case of a symbolic
ref, we need to resolve it in order to get its old value.  But we've
already resolved it during the splitting step (in order to know that it
was a symbolic ref).  So we're duplicating effort.  We can store the
resolved SHA from the split phase (along with the resolved ref type),
but that's just leaking more weird details.  

In addition, this doesn't actually solve the problem that this patch is
intended to solve; I still have to split refs into normal and non
-normal (files-backend).  It's true that this splitting is simplified,
since we don't need to do weird things for the reflogs, but I'm not
sure it's a net win.

I'm going to send just these patches in reply to your mail (I hope),
and see what you think before continuing down this path.