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

From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Jul 8, 2015, 13:16 UTC
Message-ID: <559D22A1.9030100@alum.mit.edu>
In-Reply-To: <1436316577.5521.25.camel@twopensource.com>

On 07/08/2015 02:49 AM, David Turner wrote:

Show 20 quoted lines

> 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 :-)

Show 5 quoted lines

>> 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