From: Elijah Newren <newren@gmail.com>
Date: Tue, 13 Jan 2026 06:00:53 GMT
Subject: Re: [PATCH v10 5/8] replay: support updating detached HEAD
Message-ID: <CABPp-BFvi0xmpP1jcU0qc7c-KhYXgzpCVbXyto3FMSFSSwrKoA@mail.gmail.com>
In-Reply-To: <20260112-b4-pks-history-builtin-v10-5-e3c6aa5b4cec@pks.im>

On Mon, Jan 12, 2026 at 6:17 AM Patrick Steinhardt <ps@pks.im> wrote:
[...]
> +       refs_read_ref_full(get_main_ref_store(repo), "HEAD",
> +                          RESOLVE_REF_NO_RECURSE, NULL, &head_flags);
> +       *detached_head = !(head_flags & REF_ISSYMREF);

Ah, I like this simpler way to determine if we have a detached HEAD.
Out of curiosity, though, is there a need to specify
RESOLVE_REF_NO_RECURSE?  I waffled on that for a bit and just left it
out.

> @@ -312,18 +319,30 @@ int replay_revisions(struct rev_info *revs,
[...]
> +               for (decoration = get_name_decoration(&commit->object);
> +                    decoration;
> +                    decoration = decoration->next)
> +               {
> +                       if (decoration->type != DECORATION_REF_LOCAL &&
> +                           decoration->type != DECORATION_REF_HEAD)
> +                               continue;
> +
> +                       /*
> +                        * We only need to update HEAD separately in case it's
> +                        * detached. If it's not we'd already update the branch
> +                        * it is pointing to.
> +                        */
> +                       if (decoration->type == DECORATION_REF_HEAD && !detached_head)
> +                               continue;
> +
> +                       if (!opts->contained &&
> +                           !strset_contains(update_refs, decoration->name))
> +                               continue;

I like this refactoring to make the conditions clearer; very nice.

> diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh
> index 307101eeb9..c862aa39f3 100755
> --- a/t/t3650-replay-basics.sh
> +++ b/t/t3650-replay-basics.sh
> @@ -249,6 +249,15 @@ test_expect_success 'using replay on bare repo to rebase multiple divergent bran
>         done
>  '
>
> +test_expect_success 'using replay to update detached HEAD' '
> +       current_head=$(git branch --show-current) &&
> +       test_when_finished git switch "$current_head" &&
> +       git switch --detach &&
> +       test_commit something &&
> +       git replay --ref-action=print --onto HEAD~2 --ref-action=print HEAD~..HEAD >updates &&
> +       test_grep "update HEAD " updates
> +'
> +
>  test_expect_success 'merge.directoryRenames=false' '
>         # create a test case that stress-tests the rename caching
>         git switch -c rename-onto &&
>
> --
> 2.52.0.590.g1f87b77810.dirty

Good extra test too.