Re: [PATCH] refs: allow @{n} to work with n-sized reflog
From: Martin Ågren <hidden>
Date: 2021-01-02 22:31:26
On Sat, 2 Jan 2021 at 02:41, Denton Liu [off-list ref] wrote:
But then if you do
$ git reflog expire --expire=now refs/heads/newbranch
$ git commit --allow=empty -m two
$ git show -s newbranch@{1}
you'd be scolded with
fatal: log for 'newbranch' only has 1 entries
While it is true that it has only 1 entry, we have enough
information in that single entry that records the transition between
the state in which the tip of the branch was pointing at commit
'one' to the new commit 'two' built on it, so we should be able to
answer "what object newbranch was pointing at?". But we refuse to
do so.The basic idea seems to make sense to me...
Make @{0} the special case where we use the new side to look up that
entry. Otherwise, look up @{n} using the old side of the (n-1)th entry
of the reflog.quoted hunk ↗ jump to hunk
--- a/refs.c +++ b/refs.c@@ -887,12 +887,16 @@ static int read_ref_at_ent(struct object_id *ooid, struct object_id *noid, const char *message, void *cb_data) { struct read_ref_at_cb *cb = cb_data; + int at_indexed_ent; cb->reccnt++; cb->tz = tz; cb->date = timestamp; - if (timestamp <= cb->at_time || cb->cnt == 0) { + if (cb->cnt > 0) + cb->cnt--; + at_indexed_ent = cb->cnt == 0 && !is_null_oid(ooid); + if (timestamp <= cb->at_time || at_indexed_ent) {
... but I can't really say anything about the implementation.
+test_expect_success '@{1} works with only one reflog entry' '
+ git checkout -B newbranch &&
+ git reflog expire --expire=now refs/heads/newbranch &&
+ git commit --allow-empty -mexpired &&Minor nit: not sure about "expired" -- maybe "first after expiration".
+ git rev-parse --verify newbranch@{1}
+'Should this capture the output and compare it to, e.g., `git rev-parse newbranch^`?
+test_expect_success '@{0} works with empty reflog' '
+ git checkout -B newbranch &&
+ git reflog expire --expire=now refs/heads/newbranch &&
+ git rev-parse --verify newbranch@{0}
+'Same here, but comparing to `git rev-parse newbranch`? Both of these checks seem worthwhile to make sure that we don't just answer *something*, but that we actually get the right answer, as per your "redefinition". Speaking of redefinition, does this warrant an update of the documentation? That's a genuine question -- having browsed git-reflog(1) and gitrevisions(7) a bit, I'm not sure. Martin