Re: [PATCH v6 6/7] git-reflog: add create and exists functions

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

On 07/08/2015 02:49 AM, David Turner wrote:
> On Mon, 2015-07-06 at 18:51 +0200, Michael Haggerty wrote:
>> [...]
>> So all in all, I think it is unwise to allow a reflog to be created
>> without its corresponding reference.
>>
>> This, in turn, suggests one or both of the following alternatives:
>>
>> 1. Allow "git reflog create", but only for references that already exist.
> 
> This turns out not to work for git stash, which wants to create a reflog
> for stash creation.
> 
>> 2. If we want to allow a reflog to be created at the same time as the
>> corresponding reference, the reference-creation commands ("git branch",
>> "git tag", "git update-ref", and maybe some others) probably need a new
>> option like "--create-reflog" (and, for symmetry, probably
>> "--no-create-reflog").
> 
> git branch should already autocreate reflogs, since the refs it creates
> are under refs/heads.

`git branch` only autocreates reflogs if core.logAllRefUpdates is on.
That setting happens to be on by default in a non-bare repository but
the user might turn it off. And it is off by default in a bare repository.

In my opinion it would be nice for the user to be able to ask for a
reflog to be created for a branch regardless of how
core.logAllRefUpdates is set. Though I'm not saying that you have to be
the one to implement that functionality :-)

>> At the API level, it might make sense for the ref-transaction functions
>> to get a new "REF_FORCE_CREATE_REFLOG" flag or something.
> 
> Junio was opposed to the converse flag, so I'm going to just add
> manually add code to create reflogs.

Unfortunately I wasn't keeping up with earlier versions of this patch
series and now I can't find the email from Junio that you are referring
to. If the earlier flag had the opposite ("converse"?) sense, like
REF_INHIBIT_CREATE_REFLOG, then I agree that it wouldn't be an improvement.

But I think this functionality *has to* be implemented within ref
transactions for references that are just being created, because

1. The reflog must *not* be created if the reference creation fails for
some reason. For example, the reflog shouldn't be created if the
reference name has a D/F conflict with an existing one in the "refs/foo"
vs. "refs/foo/bar" sense. (This conflict might not be obvious when
creating the reflog file because the other reference might not have its
reflog turned on.) There are other reasons that a reference creation
might fail, and code outside of the refs API can't be expected to know
all possibilities.

2. On the other hand, the reflog for a newly-created reference *should*
reflect the creation of the reference. So it would be awkward to require
the calling code to create the reference and *then* turn on the reflog.

For references that already exist, I see no problem with a command that
turns on the reflog without adding any entries to it. Though if you
implement this, it would be prudent to check that existing
reflog-handling code doesn't fail when confronted with an empty file; I
think empty reflog files are rare now and might not be well-tested.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu