Subject: Re: [PATCH 6/9] pseudorefs: create and use pseudoref update and delete functions

From: Junio C Hamano <[email-masked]>
Date: Fri, 24 Jul 2015 13:53:33 -0700

David Turner <[email-masked]> writes:

> On Fri, 2015-07-24 at 12:25 -0700, Junio C Hamano wrote:
>> David Turner <[email-masked]> writes:
>> 
>> > Pseudorefs should not be updated through the ref API, because the ref
>> > API is for real refs.  Instead, use a dedicated pseudoref API.
>> >
>> > This patch changes writes to CHERRY_PICK_HEAD, REVERT_HEAD, and ORIG_HEAD.
>> 
>> This feels somewhat backwards, and it makes me wonder if it is a
>> better approach to teach update_ref() about the naming rules, so
>> that the callers do not have to say the same thing twice "This is
>> not a ref and I am giving all-caps name, by the way I am also not
>> calling update_ref() because that is only for real refs".
>
> Do you mean teach update_ref to call write_pseudoref for pseudorefs and
> call the normal codepath for regular refs?

While reviewing this series, I mostly am viewing them from the point
of view of a user of the ref API.

You may name that "always delegate to filesystem backend" helper
function "write_pseudoref()" or whatever name we (i.e. the ref API
implementors) find easy to remember and understand, and as long as
it is static within refs.c (or refs-be-file.c), a user of the ref
API does not have to know.  He does not have to care (ideally he
shouldn't have to be able to call it directly, but that is merely a
safety against bugs).

> I considered this, but I was worried about ref transactions.

Considering the nature of these FOO_HEADs, I think it is perfectly
fine to declare that handling of them _must_ happen within a single
transaction that does not involve regular refs, or that handling of
them _must_ happen outside transactions.  I do not think any of the
existing creation, update, or deletion of these FOO_HEADs happen
inside any transaction anyway.