On Wed, Jun 21, 2017 at 07:04:22PM -0400, Kyle Meyer wrote:
> Junio C Hamano <gitster@xxxxxxxxx> writes:
>
> > "brian m. carlson" <sandals@xxxxxxxxxxxxxxxxxxxx> writes:
> >
> >> I get the following on 2.11.0:
> >>
> >> 2cbfbd5 HEAD@{0}:
> >> 2cbfbd5 HEAD@{1}: checkout: moving from cPanel to master
> >> eaf8db2 HEAD@{2}: checkout: moving from master to cPanel
> >> 2cbfbd5 HEAD@{3}: clone: from https://bmc@xxxxxxxxxxxxxxxxxxxxxxxx/git/bmc/homedir.git
> >>
> >> and this on a recent next:
> >>
> >> 2cbfbd5 HEAD@{0}: Branch: renamed refs/heads/master to refs/heads/new
> >>
> >> For this test, any repo will work; I just picked this one because it had
> >> two branches I could use to generate dummy reflog entries.
> >>
> >> A colleague reported this to me as a bug. I don't see anything in the
> >> release notes about this as a desired behavior change, and it does seem
> >> undesirable to truncate the reflog this way. If this isn't intentional,
> >> I'm happy to work up a patch.
> >
> > I do not think either behaviour is intentional (old one that gives a
> > meaningless empty entry probably is probably not what we want, the
> > new one that truncates is not what we want, either).
>
> Eh, sorry about that. I haven't dug very deeply, but it seems like the
> entry is still in .git/logs/HEAD, while "git reflog" is only showing
> they latest entry. (Maybe because we stop consuming the entries when we
> hit a null sha1 in the old id field?)
Yeah, I think that's it.
I had trouble digging to find the place where this happens, but I
_think_ it's unintentional, and is an artifact of the way the reflog
iteration is bolted onto the regular revision machinery. In
get_revision_1(), we fake the commit parents as if the earlier reflog
entries formed an actual commit history.
There's some magic in fake_reflog_parent() when we hit an entry with a
null sha1 in the old-oid field. But it depends on looking at the new-oid
field of the entry before. And because our entries look like this:
real_sha1 null_sha1 ...
null_sha1 real_sha1 ...
Looking at the prior entry's new-oid field doesn't help at all. And we
end up claiming that there are no parents, and this is the end of the
traversal. The patch below makes it work for me by falling back further
to the previous entry's old oid. This feels like a terrible hack to make
this case work. I'd think we'd actually want some kind of loop to keep
looking. After all, what should:
real_sha1 real_sha1 ...
null_sha1 null_sha1 ...
null_sha1 real_sha1 ...
show? Surely we should not stop traversing when we hit the double
null_sha1. We should either display it in some form, or skip it and
issue a warning that there's a funky entry in your reflog.
This whole fake-parents things does just feel like a gigantic hack,
though. It seems like we should be able to just walk backwards down the
reflog list and show the entries. The revision machinery already
special-cases a bunch of reflog-walk bits; I don't know that adding one
or two more would be the end of the world.
Another approach entirely would be to stop generating two entries in the
reflog, and generate a single one:
real_sha1 real_sha1 ... Rename from "foo" to "bar"
I think we talked about that earlier, but I think the resolution was
just that doing so was hard with the existing ref-update calls.
diff --git a/reflog-walk.c b/reflog-walk.c
index ed99437ad..b7e489ad3 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -259,6 +259,8 @@ void fake_reflog_parent(struct reflog_walk_info *info, struct commit *commit)
/* a root commit, but there are still more entries to show */
reflog = &commit_reflog->reflogs->items[commit_reflog->recno];
logobj = parse_object(&reflog->noid);
+ if (!logobj)
+ logobj = parse_object(&reflog->ooid);
}
if (!logobj || logobj->type != OBJ_COMMIT) {
