Re: [PATCH 1/2] object-name: detect and report empty reflogs

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Feb 21, 2024, 16:48 UTC
Message-ID: <CAPig+cQff2LUnsXf+NMuAczN0sVNbPU9fOSFapeMSDBpMYkedQ@mail.gmail.com>
In-Reply-To: <0fac6ebb098c7e8cdc87cb75f2dcffdc4b1ccfaa.1708509190.git.ps@pks.im>

On Wed, Feb 21, 2024 at 5:03 AM Patrick Steinhardt <ps@pks.im> wrote:

Show 20 quoted lines

> [...]
> Start to detect and report empty or missing reflogs in `read_ref_at()`
> and report them to the caller. This results in a change in behaviour
> when asking for `ref@{0}` with an empty or missing reflog because we now
> die instead of returning the object ID of the ref itself. This adapted
> behaviour should lead to less surprises as we now really only report
> object IDs to the caller that actually come from the reflog, thus making
> the user experience a whole lot more consistent.
> [...]
> Reported-by: Yasushi SHOJI <yasushi.shoji@gmail.com>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> diff --git a/t/t1508-at-combinations.sh b/t/t1508-at-combinations.sh
> @@ -110,10 +110,31 @@ test_expect_success '@{1} works with only one reflog entry' '
> +test_expect_success '@{0} fails with empty reflog' '
> +       git checkout -B empty-reflog main &&
> +       git reflog expire --expire=now refs/heads/empty-reflog &&
> +       cat >expect <<-EOF &&
> +       fatal: Needed a single revision
> +       EOF

Typically, we would use <<-\EOF rather than <<-EOF to signal to the reader that no variable interpolation is happening in the here-doc body.

A simpler alternative in this case would, of course, be to use `echo`:

    echo "fatal: Needed a single revision" >expect &&

Show 9 quoted lines

> +       test_must_fail git rev-parse --verify missing-reflog@{0} 2>err &&
> +       test_cmp expect err
> +'
> +
> +test_expect_success '@{0} fails with missing reflog' '
> +       git -c core.logAllRefUpdates=false checkout -B missing-reflog main &&
> +       cat >expect <<-EOF &&
> +       fatal: Needed a single revision
> +       EOF

Ditto.

> +       test_must_fail git rev-parse --verify missing-reflog@{0} 2>err &&
> +       test_cmp expect err
> +'

Probably neither comment is worth a reroll, but if you reroll for some other reason, perhaps consider them.