Re: [PATCH v3 1/2] rebase: skip branch symref aliases

[Erik Cervin-Edin <erik@cervined.in>]

I ran into a minor regression that I tracked down to v2 of this patch
series. When I was running

    git rebase --interactive --update-refs

I got a comment in my git-rebase-todo for the branch I was rebasing,
even though it's just the current branch in the current worktree:

    # Ref refs/heads/main checked out at '/private/tmp/gittest'

where I'd expect no entry at all, since the branch is updated by the
rebase itself. After applying v3 of the series, the regression went
away. Nevertheless, I thought I ought to share what I found.

On 26/07/24 10:55AM, Phillip Wood wrote:
> > > +        if (head_ref && !strcmp(head_ref, decoration->name)) {
> > > +            free(resolved_ref);
> > >               decoration = decoration->next;
> > >               continue;
> > >           }
> > > +
> > > +        path = branch_checked_out(decoration->name);
> >
> > Then we check to see if the decoration matches HEAD which we used to do
> > above - I'm not clear why we have moved this check.

branch_checked_out() can't tell "checked out in another worktree"
apart from "checked out right here", so `path` is never NULL for the
branch actually being rebased. In v2, the check above was instead:

    if (!path && head_ref && !strcmp(head_ref, decoration->name))
        continue;

which made it a no-op for exactly that branch -- the regression I
observed. v3 drops the `!path` gate and moves `path` below it, which
is why it's fixed.

> > As topic2 is checked out in the worktree where the rebase is running
> > why did this line appear before?

This might be the same symptom from another cause: on master, head_ref
comes from refs_resolve_ref_unsafe(), and as you note its buffer is
overwritten inside branch_checked_out(). So by the time topic2 is
compared, wt-topic's lookup may already have clobbered head_ref,
letting topic2 fall through to the comment. I haven't run the test to
confirm, though.

Thanks,
Erik