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

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.