git checkout --orphan skips reflogging

git checkout --orphan skips reflogging

[Dmitry Ivankov]

In short, git checkout --orphan doesn't write
HEAD_sha1 -> 00000
entry to logs/HEAD, while git-comit will write
00000 -> new_orphan_HEAD_sha1
entry. And then reflog backward walk will stop on 000 -> entry and
won't see earlier history.

How to reproduce:
$ git init test && cd test
$ git commit -m A --allow-empty
$ git checkout --orphan topic
$ git commit -m B --allow-empty
$ git log -g --oneline HEAD
some_sha1 HEAD@{0}: commit (initial): B
# oops, where are my old HEADs?
$ cat .git/logs/HEAD
000.. another_sha1 ... commit (initial): A
000.. some_sha1 ... commit (initial): B
# phew, at least I can find them by hand

Isn't  it also a bug in reflog walking that we rely on each old_sha1
being new_sha1 of a previous entry?

Re: git checkout --orphan skips reflogging

[Junio C Hamano]

Dmitry Ivankov <divanorama@gmail.com> writes:

> In short, git checkout --orphan doesn't write
> HEAD_sha1 -> 00000
> entry to logs/HEAD, while git-comit will write
> 00000 -> new_orphan_HEAD_sha1
> entry. And then reflog backward walk will stop on 000 -> entry and
> won't see earlier history.

Funny. From the point of view of the _current_ branch, it sort of makes
sense to stop the traversal at that point, but I agree for HEAD reflog
that records branch switching, the traversal should not stop.

I am not sure if recording 0{40} after --orphan is the right thing to do
either (for that matter, I do not necessarily think running --orphan is a
sane thing to do, but that is a separate issue).

> Isn't  it also a bug in reflog walking that we rely on each old_sha1
> being new_sha1 of a previous entry?

I am not all that familiar with the reflog walking (which is admittedly a
bolted-on hack that injects commits with fake ancestry) code, but I think
it assumes the old sha1 field on the current entry matches the new sha1
field on the previous entry, and we could change it to be a bit more
robust.

The attached patch _may_ (I didn't even compile test it) remove the
dependency on osha1[] and make the code consistently use nsha1[], but I
think stopping at the 0{40} is pretty much fundamental in the revision
walking machinery the reflog walking code is piggy-backing on, and I do
not think this patch would change that.

 reflog-walk.c |   17 ++++++++++++-----
 1 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/reflog-walk.c b/reflog-walk.c
index 5d81d39..261d300 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -211,6 +211,13 @@ int add_reflog_for_walk(struct reflog_walk_info *info,
 	return 0;
 }
 
+static struct reflog_info *peek_reflog_ent(struct commit_reflog *clog)
+{
+	if (clog->recno < 0)
+		return NULL;
+	return &clog->reflogs->items[clog->recno];
+}
+
 void fake_reflog_parent(struct reflog_walk_info *info, struct commit *commit)
 {
 	struct commit_info *commit_info =
@@ -223,20 +230,20 @@ void fake_reflog_parent(struct reflog_walk_info *info, struct commit *commit)
 		return;
 
 	commit_reflog = commit_info->util;
-	if (commit_reflog->recno < 0) {
+	reflog = peek_reflog_ent(commit_reflog);
+	if (!reflog) {
 		commit->parents = NULL;
 		return;
 	}
-
-	reflog = &commit_reflog->reflogs->items[commit_reflog->recno];
 	info->last_commit_reflog = commit_reflog;
 	commit_reflog->recno--;
-	commit_info->commit = (struct commit *)parse_object(reflog->osha1);
-	if (!commit_info->commit) {
+	reflog = peek_reflog_ent(commit_reflog);
+	if (!reflog) {
 		commit->parents = NULL;
 		return;
 	}
 
+	commit_info->commit = (struct commit *)parse_object(reflog->nsha1);
 	commit->parents = xcalloc(sizeof(struct commit_list), 1);
 	commit->parents->item = commit_info->commit;
 }

Re: git checkout --orphan skips reflogging

[Dmitry Ivankov]

On Wed, Sep 14, 2011 at 5:04 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Dmitry Ivankov <divanorama@gmail.com> writes:
>
>> In short, git checkout --orphan doesn't write
>> HEAD_sha1 -> 00000
>> entry to logs/HEAD, while git-comit will write
>> 00000 -> new_orphan_HEAD_sha1
>> entry. And then reflog backward walk will stop on 000 -> entry and
>> won't see earlier history.
>
> Funny. From the point of view of the _current_ branch, it sort of makes
> sense to stop the traversal at that point, but I agree for HEAD reflog
> that records branch switching, the traversal should not stop.
>
> I am not sure if recording 0{40} after --orphan is the right thing to do
> either (for that matter, I do not necessarily think running --orphan is a
> sane thing to do, but that is a separate issue).
In fact, my purpose is more like finding out what branches I were recently
on, most of the time even what branches I've updated recently. So, being
able to see chronologically (by ref modification time) ordered list of refs
may be a better way (than git log -g --decorate).

>> Isn't  it also a bug in reflog walking that we rely on each old_sha1
>> being new_sha1 of a previous entry?
>
> I am not all that familiar with the reflog walking (which is admittedly a
> bolted-on hack that injects commits with fake ancestry) code, but I think
> it assumes the old sha1 field on the current entry matches the new sha1
> field on the previous entry, and we could change it to be a bit more
> robust.
>
> The attached patch _may_ (I didn't even compile test it) remove the
> dependency on osha1[] and make the code consistently use nsha1[], but I
> think stopping at the 0{40} is pretty much fundamental in the revision
> walking machinery the reflog walking code is piggy-backing on, and I do
> not think this patch would change that.
The patch compiles fine, and walks past osha1 == 0{40}. Probably osha1
becomes completely hidden from the walking machinery, and since nsha1 is
never 0{40} (for now) whole history is walked.