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

[David Turner <dturner@twopensource.com>]

On Wed, 2015-07-08 at 15:16 +0200, Michael Haggerty wrote:
> 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.

Oh, right.  Well, we can always add that later, if anyone needs it.

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

Yep, makes sense.

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

I think empty reflog files are fine; I recall a few tests creating them
(and git stash did so as well).

But I also don't need that command (I think), so I won't implement it
right now.