| Message ID | 6107efeffa3a9e87ed95f4fe592d9b480887c510.1708509190.git.ps@pks.im (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Detect empty or missing reflogs with `ref@{0}` | expand |
On Wed, Feb 21, 2024 at 10:56:40AM +0100, Patrick Steinhardt wrote: > diff --git a/builtin/show-branch.c b/builtin/show-branch.c > index b01ec761d2..8837415031 100644 > --- a/builtin/show-branch.c > +++ b/builtin/show-branch.c > @@ -785,6 +785,8 @@ int cmd_show_branch(int ac, const char **av, const char *prefix) > if (read_ref_at(get_main_ref_store(the_repository), > ref, flags, 0, base + i, &oid, &logmsg, > ×tamp, &tz, NULL)) { > + if (!i) > + die(_("log for %s is empty"), ref); > reflog = i; > break; > } There's another call to read_ref_at() earlier in the function that does not check the return code. I think it is OK, though, because it always passes a timestamp rather than a count, so it never hits the special-case added by 6436a20284. But I think this highlights my confusion with the first patch; most of the time read_ref_at() will print the appropriate message for us and die, but for this one weird case we have to do it ourselves. It makes more sense to me for it to behave consistently for an empty reflog, whether we were looking for branch@{0} or branch@{1}. -Peff
diff --git a/builtin/show-branch.c b/builtin/show-branch.c index b01ec761d2..8837415031 100644 --- a/builtin/show-branch.c +++ b/builtin/show-branch.c @@ -785,6 +785,8 @@ int cmd_show_branch(int ac, const char **av, const char *prefix) if (read_ref_at(get_main_ref_store(the_repository), ref, flags, 0, base + i, &oid, &logmsg, ×tamp, &tz, NULL)) { + if (!i) + die(_("log for %s is empty"), ref); reflog = i; break; } diff --git a/t/t3202-show-branch.sh b/t/t3202-show-branch.sh index 6a98b2df76..e5b74cc55f 100755 --- a/t/t3202-show-branch.sh +++ b/t/t3202-show-branch.sh @@ -264,4 +264,29 @@ test_expect_success 'error descriptions on orphan branch' ' test_branch_op_in_wt -c new-branch ' +test_expect_success 'show-branch --reflog with empty reflog' ' + git checkout -B empty-reflog && + git reflog expire --expire=now refs/heads/empty-reflog && + + cat >expect <<-EOF && + fatal: log for refs/heads/empty-reflog is empty + EOF + test_must_fail git show-branch --reflog=1 empty-reflog 2>err && + test_cmp expect err && + test_must_fail git show-branch --reflog empty-reflog 2>err && + test_cmp expect err +' + +test_expect_success 'show-branch --reflog with missing reflog' ' + git -c core.logAllRefUpdates=false checkout -B missing-reflog && + + cat >expect <<-EOF && + fatal: log for refs/heads/missing-reflog is empty + EOF + test_must_fail git show-branch --reflog=1 missing-reflog 2>err && + test_cmp expect err && + test_must_fail git show-branch --reflog missing-reflog 2>err && + test_cmp expect err +' + test_done
The `--reflog=n` option for git-show-branch(1) allows the user to list the reflog of a specific branch, where `n` specifies how many reflog entries to show. When there are less than `n` entries we handle this gracefully and simply report less entries. There is a special case though when the ref either has no reflog or when its reflog is empty. In this case, we end up printing nothing at all while returning successfully. This is rather confusing as there is no indicator why we didn't print anything. Adapt the behaviour so that we die instead of leaving no user visible traces. This change in behaviour should be fine given that this case used to segfault before the preceding commit. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- builtin/show-branch.c | 2 ++ t/t3202-show-branch.sh | 25 +++++++++++++++++++++++++ 2 files changed, 27 insertions(+)