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

On Wed, 2015-07-08 at 15:16 +0200, Michael Haggerty wrote:

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

quoted

On Mon, 2015-07-06 at 18:51 +0200, Michael Haggerty wrote:

quoted

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

quoted

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

quoted

quoted

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.